desruisseaux commented on code in PR #11632:
URL: https://github.com/apache/maven/pull/11632#discussion_r2751395796
##########
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()
+ && "com.example.app".equals(sr.module().get()))
+ .count();
+ assertEquals(2, moduleCount, "Both main sources should be for
com.example.app module");
+
+ // One should be implicit directory, one should be generated-sources
+ boolean hasImplicitDir = mainJavaRoots.stream()
+ .anyMatch(sr -> sr.directory().toString().replace('\\',
'/').contains("src/com.example.app/main/java"));
+ boolean hasGeneratedDir = mainJavaRoots.stream().anyMatch(sr ->
sr.directory()
+ .toString()
+ .replace('\\', '/')
+ .contains("target/generated-sources/com.example.app/java"));
+
+ assertTrue(hasImplicitDir, "Should have implicit source directory for
module");
+ assertTrue(hasGeneratedDir, "Should have generated-sources directory
for module");
+
+ // Get test Java source roots
+ List<SourceRoot> testJavaRoots =
project.getEnabledSourceRoots(ProjectScope.TEST, Language.JAVA_FAMILY)
+ .toList();
+
+ // Should have 2 test sources: both for com.example.app
+ assertEquals(2, testJavaRoots.size(), "Should have 2 test Java source
roots for same module");
+
+ // Both test sources should be for the same module
+ long testModuleCount = testJavaRoots.stream()
+ .filter(sr -> sr.module().isPresent()
+ && "com.example.app".equals(sr.module().get()))
+ .count();
+ assertEquals(2, testModuleCount, "Both test sources should be for
com.example.app module");
+ }
+
+ /**
+ * Tests duplicate handling with enabled discriminator.
+ * <p>
+ * Test scenario:
+ * - Same (lang, scope, module, directory) with enabled=true appearing
twice → triggers WARNING
+ * - Same identity with enabled=false → should be filtered out (disabled
sources are no-ops)
+ * - Different modules should be added normally
+ * <p>
+ * Verifies:
+ * - First enabled source wins, subsequent duplicates trigger WARNING
+ * - Disabled sources don't count as duplicates
+ * - Different modules are unaffected
+ * <p>
+ * Acceptance Criteria:
+ * - AC3 (duplicate detection - duplicates trigger WARNING)
+ * - AC4 (first enabled wins - duplicates are skipped)
+ * - AC5 (disabled sources unchanged - still added but filtered by
getEnabledSourceRoots)
+ *
+ * @see <a href="https://github.com/apache/maven/issues/11612";>Issue
#11612</a>
+ */
+ @Test
+ void testDuplicateEnabledSources() throws Exception {
+ File pom = getProject("duplicate-enabled-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 issued for duplicate enabled sources
+ List<ModelProblem> duplicateWarnings = result.getProblems().stream()
+ .filter(p -> p.getSeverity() == ModelProblem.Severity.WARNING)
+ .filter(p -> p.getMessage().contains("Duplicate enabled
source"))
+ .toList();
+
+ // We have 2 duplicate pairs: main scope and test scope for
com.example.dup
+ assertEquals(2, duplicateWarnings.size(), "Should have 2 duplicate
warnings (main and test scope)");
+
+ // Get main Java source roots
+ List<SourceRoot> mainJavaRoots =
project.getEnabledSourceRoots(ProjectScope.MAIN, Language.JAVA_FAMILY)
+ .toList();
+
+ // Should have 2 main sources: 1 for com.example.dup (first wins) + 1
for com.example.other
+ // Note: MavenProject.addSourceRoot still adds all sources, but
tracking only counts first enabled
+ assertEquals(2, mainJavaRoots.size(), "Should have 2 main Java source
roots");
+
+ // Verify com.example.other module is present
+ boolean hasOtherModule = mainJavaRoots.stream()
+ .anyMatch(sr -> sr.module().isPresent()
+ && "com.example.other".equals(sr.module().get()));
+ assertTrue(hasOtherModule, "Should have source root for
com.example.other module");
+
+ // Verify com.example.dup module is present (first enabled wins)
+ boolean hasDupModule = mainJavaRoots.stream()
+ .anyMatch(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
.anyMatch(sr ->
"com.example.dup".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]
