desruisseaux commented on code in PR #11632:
URL: https://github.com/apache/maven/pull/11632#discussion_r2751410425


##########
impl/maven-core/src/test/java/org/apache/maven/project/ProjectBuilderTest.java:
##########
@@ -470,4 +478,257 @@ void 
testModularSourcesWithExplicitResourcesIssuesWarning() throws Exception {
         assertTrue(mainModules.contains("org.foo.moduleA"), "Should have 
resource root for moduleA");
         assertTrue(mainModules.contains("org.foo.moduleB"), "Should have 
resource root for moduleB");
     }
+
+    /**
+     * Tests that legacy sourceDirectory and testSourceDirectory are ignored 
in modular projects.
+     * <p>
+     * In modular projects, legacy directories are unconditionally ignored 
because it is not clear
+     * how to dispatch their content between different modules. A warning is 
emitted if these
+     * properties are explicitly set (differ from Super POM defaults).
+     * <p>
+     * This verifies:
+     * - WARNINGs are emitted for explicitly set legacy directories in modular 
projects
+     * - sourceDirectory and testSourceDirectory are both ignored
+     * - Only modular sources from {@code <sources>} are used
+     * <p>
+     * Acceptance Criteria:
+     * - AC1 (boolean flags eliminated - uses hasSources() for main/test 
detection)
+     * - AC7 (legacy directories warning - {@code <sourceDirectory>} and 
{@code <testSourceDirectory>}
+     *   are unconditionally ignored with a WARNING in modular projects)
+     *
+     * @see <a href="https://github.com/apache/maven/issues/11612";>Issue 
#11612</a>
+     */
+    @Test
+    void testMixedSourcesModularMainClassicTest() throws Exception {
+        File pom = getProject("mixed-sources");
+
+        MavenSession mavenSession = createMavenSession(null);
+        ProjectBuildingRequest configuration = new 
DefaultProjectBuildingRequest();
+        
configuration.setRepositorySession(mavenSession.getRepositorySession());
+
+        ProjectBuildingResult result = getContainer()
+                .lookup(org.apache.maven.project.ProjectBuilder.class)
+                .build(pom, configuration);
+
+        MavenProject project = result.getProject();
+
+        // Verify WARNINGs are emitted for explicitly set legacy directories
+        List<ModelProblem> warnings = result.getProblems().stream()
+                .filter(p -> p.getSeverity() == ModelProblem.Severity.WARNING)
+                .filter(p -> p.getMessage().contains("Legacy") && 
p.getMessage().contains("ignored in modular project"))
+                .toList();
+
+        // Should have 2 warnings: one for sourceDirectory, one for 
testSourceDirectory
+        assertEquals(2, warnings.size(), "Should have 2 warnings for ignored 
legacy directories");
+        assertTrue(
+                warnings.stream().anyMatch(w -> 
w.getMessage().contains("<sourceDirectory>")),
+                "Should warn about ignored <sourceDirectory>");
+        assertTrue(
+                warnings.stream().anyMatch(w -> 
w.getMessage().contains("<testSourceDirectory>")),
+                "Should warn about ignored <testSourceDirectory>");
+
+        // Get main Java source roots - should have modular sources, not 
classic sourceDirectory
+        List<SourceRoot> mainJavaRoots = 
project.getEnabledSourceRoots(ProjectScope.MAIN, Language.JAVA_FAMILY)
+                .toList();
+
+        // Should have 2 modular main Java sources (moduleA and moduleB)
+        assertEquals(2, mainJavaRoots.size(), "Should have 2 modular main Java 
source roots");
+
+        Set<String> mainModules = mainJavaRoots.stream()
+                .map(SourceRoot::module)
+                .filter(opt -> opt.isPresent())
+                .map(opt -> opt.get())
+                .collect(Collectors.toSet());
+
+        assertEquals(2, mainModules.size(), "Should have main sources for 2 
modules");
+        assertTrue(mainModules.contains("org.foo.moduleA"), "Should have main 
source for moduleA");
+        assertTrue(mainModules.contains("org.foo.moduleB"), "Should have main 
source for moduleB");
+
+        // Verify the classic sourceDirectory is NOT used (should be ignored)
+        boolean hasClassicMainSource = mainJavaRoots.stream()
+                .anyMatch(sr -> sr.directory().toString().replace('\\', 
'/').contains("src/classic/main/java"));
+        assertTrue(!hasClassicMainSource, "Classic sourceDirectory should be 
ignored");
+
+        // Test sources should NOT be added (legacy testSourceDirectory is 
ignored in modular projects)
+        List<SourceRoot> testJavaRoots = 
project.getEnabledSourceRoots(ProjectScope.TEST, Language.JAVA_FAMILY)
+                .toList();
+        assertEquals(0, testJavaRoots.size(), "Should have no test Java 
sources (legacy is ignored)");
+    }
+
+    /**
+     * Tests that mixing modular and non-modular sources within {@code 
<sources>} is not allowed.
+     * <p>
+     * A project must be either fully modular (all sources have a module) or 
fully classic
+     * (no sources have a module). Mixing them within the same project is not 
supported
+     * because the compiler plugin cannot handle such configurations.
+     * <p>
+     * This verifies:
+     * - An ERROR is reported when both modular and non-modular sources exist 
in {@code <sources>}
+     * - sourceDirectory is ignored because {@code <source scope="main" 
lang="java">} exists
+     * <p>
+     * Acceptance Criteria:
+     * - AC1 (boolean flags eliminated - uses hasSources() for source 
detection)
+     * - AC6 (mixed sources error - mixing modular and classic sources within 
{@code <sources>}
+     *   triggers an ERROR)
+     *
+     * @see <a href="https://github.com/apache/maven/issues/11612";>Issue 
#11612</a>
+     */
+    @Test
+    void testSourcesMixedModulesWithinSources() throws Exception {
+        File pom = getProject("sources-mixed-modules");
+
+        MavenSession mavenSession = createMavenSession(null);
+        ProjectBuildingRequest configuration = new 
DefaultProjectBuildingRequest();
+        
configuration.setRepositorySession(mavenSession.getRepositorySession());
+
+        ProjectBuildingResult result = getContainer()
+                .lookup(org.apache.maven.project.ProjectBuilder.class)
+                .build(pom, configuration);
+
+        // Verify an ERROR is reported for mixing modular and non-modular 
sources
+        List<ModelProblem> errors = result.getProblems().stream()
+                .filter(p -> p.getSeverity() == ModelProblem.Severity.ERROR)
+                .filter(p -> p.getMessage().contains("Mixed modular and 
classic sources"))
+                .toList();
+
+        assertEquals(1, errors.size(), "Should have 1 error for mixed 
modular/classic configuration");
+        assertTrue(errors.get(0).getMessage().contains("lang=java"), "Error 
should mention java language");
+        assertTrue(errors.get(0).getMessage().contains("scope=main"), "Error 
should mention main scope");
+    }
+
+    /**
+     * Tests that multiple source directories for the same (lang, scope, 
module) combination
+     * are allowed and all are added as source roots.
+     * <p>
+     * This is a valid use case for Phase 2: users may have generated sources 
alongside regular sources,
+     * both belonging to the same module. Different directories = different 
identities = not duplicates.
+     * <p>
+     * Acceptance Criterion: AC2 (unified source tracking - multiple 
directories per module supported)
+     *
+     * @see <a href="https://github.com/apache/maven/issues/11612";>Issue 
#11612</a>
+     */
+    @Test
+    void testMultipleDirectoriesSameModule() throws Exception {
+        File pom = getProject("multiple-directories-same-module");
+
+        MavenSession session = createMavenSession(pom);
+        MavenProject project = session.getCurrentProject();
+
+        // Get main Java source roots
+        List<SourceRoot> mainJavaRoots = 
project.getEnabledSourceRoots(ProjectScope.MAIN, Language.JAVA_FAMILY)
+                .toList();
+
+        // Should have 2 main sources: both for com.example.app but different 
directories
+        assertEquals(2, mainJavaRoots.size(), "Should have 2 main Java source 
roots for same module");
+
+        // Both should be for the same module
+        long moduleCount = mainJavaRoots.stream()
+                .filter(sr -> sr.module().isPresent()

Review Comment:
   Can be simplified as below (removing the `isPresent()` check, replaced by 
the fact that `String.equals(Object)` is null-safe):
   
   ```java
                   .filter(sr -> 
"com.example.app".equals(sr.module().orElse(null)))
   ```
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to