desruisseaux commented on code in PR #11632:
URL: https://github.com/apache/maven/pull/11632#discussion_r2751403493
##########
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('\\', '/')
Review Comment:
I suggest `replace(File.separatorChar, '/')`.
--
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]