From e247f9f9d953a977b71c68f6d6637346ea531b69 Mon Sep 17 00:00:00 2001 From: Gerd Aschemann Date: Wed, 7 Jan 2026 18:40:14 +0100 Subject: [PATCH 1/5] Unify source tracking with SourceHandlingContext (#11612) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace boolean flags (hasMain, hasTest, etc.) with flexible set-based tracking for all language/scope combinations. - Rename ResourceHandlingContext to SourceHandlingContext - Add duplicate detection with WARNING for enabled sources - Add hasSources() method for checking language/scope combinations - Rename 'src' variable to 'sourceRoot' for clarity 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- .../maven/project/DefaultProjectBuilder.java | 72 ++--- ...ontext.java => SourceHandlingContext.java} | 105 ++++++- .../maven/project/ProjectBuilderTest.java | 267 +++++++++++++++++- .../duplicate-enabled-sources/pom.xml | 64 +++++ .../project-builder/mixed-sources/pom.xml | 38 +++ .../multiple-directories-same-module/pom.xml | 51 ++++ .../sources-mixed-modules/pom.xml | 47 +++ 7 files changed, 588 insertions(+), 56 deletions(-) rename impl/maven-core/src/main/java/org/apache/maven/project/{ResourceHandlingContext.java => SourceHandlingContext.java} (65%) create mode 100644 impl/maven-core/src/test/projects/project-builder/duplicate-enabled-sources/pom.xml create mode 100644 impl/maven-core/src/test/projects/project-builder/mixed-sources/pom.xml create mode 100644 impl/maven-core/src/test/projects/project-builder/multiple-directories-same-module/pom.xml create mode 100644 impl/maven-core/src/test/projects/project-builder/sources-mixed-modules/pom.xml diff --git a/impl/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java b/impl/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java index 99289fc7aff6..825e23fbb418 100644 --- a/impl/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java +++ b/impl/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java @@ -665,65 +665,51 @@ private void initProject(MavenProject project, ModelBuilderResult result) { return build.getDirectory(); } }; - boolean hasScript = false; - boolean hasMain = false; - boolean hasTest = false; - boolean hasMainResources = false; - boolean hasTestResources = false; + // Extract modules from sources to detect modular projects + Set modules = extractModules(sources); + boolean isModularProject = !modules.isEmpty(); + + logger.trace( + "Module detection for project {}: found {} module(s) {} - modular project: {}.", + project.getId(), + modules.size(), + modules, + isModularProject); + + // Create source handling context for unified tracking of all lang/scope combinations + SourceHandlingContext sourceContext = + new SourceHandlingContext(project, baseDir, modules, isModularProject, result); + + // Process all sources, tracking enabled ones and detecting duplicates for (var source : sources) { - var src = DefaultSourceRoot.fromModel(session, baseDir, outputDirectory, source); - project.addSourceRoot(src); - Language language = src.language(); - if (Language.JAVA_FAMILY.equals(language)) { - ProjectScope scope = src.scope(); - if (ProjectScope.MAIN.equals(scope)) { - hasMain = true; - } else { - hasTest |= ProjectScope.TEST.equals(scope); - } - } else if (Language.RESOURCES.equals(language)) { - ProjectScope scope = src.scope(); - if (ProjectScope.MAIN.equals(scope)) { - hasMainResources = true; - } else if (ProjectScope.TEST.equals(scope)) { - hasTestResources = true; - } - } else { - hasScript |= Language.SCRIPT.equals(language); + var sourceRoot = DefaultSourceRoot.fromModel(session, baseDir, outputDirectory, source); + // Track enabled sources for duplicate detection and hasSources() queries + // Only add source if it's not a duplicate enabled source (first enabled wins) + if (sourceContext.shouldAddSource(sourceRoot)) { + project.addSourceRoot(sourceRoot); } } + /* * `sourceDirectory`, `testSourceDirectory` and `scriptSourceDirectory` - * are ignored if the POM file contains at least one element + * are ignored if the POM file contains at least one enabled element * for the corresponding scope and language. This rule exists because * Maven provides default values for those elements which may conflict * with user's configuration. */ - if (!hasScript) { + if (!sourceContext.hasSources(Language.SCRIPT, ProjectScope.MAIN)) { project.addScriptSourceRoot(build.getScriptSourceDirectory()); } - if (!hasMain) { + if (!sourceContext.hasSources(Language.JAVA_FAMILY, ProjectScope.MAIN)) { project.addCompileSourceRoot(build.getSourceDirectory()); } - if (!hasTest) { + if (!sourceContext.hasSources(Language.JAVA_FAMILY, ProjectScope.TEST)) { project.addTestCompileSourceRoot(build.getTestSourceDirectory()); } - // Extract modules from sources to detect modular projects - Set modules = extractModules(sources); - boolean isModularProject = !modules.isEmpty(); - - logger.trace( - "Module detection for project {}: found {} module(s) {} - modular project: {}.", - project.getId(), - modules.size(), - modules, - isModularProject); - // Handle main and test resources - ResourceHandlingContext resourceContext = - new ResourceHandlingContext(project, baseDir, modules, isModularProject, result); - resourceContext.handleResourceConfiguration(ProjectScope.MAIN, hasMainResources); - resourceContext.handleResourceConfiguration(ProjectScope.TEST, hasTestResources); + // Handle main and test resources using unified source handling + sourceContext.handleResourceConfiguration(ProjectScope.MAIN); + sourceContext.handleResourceConfiguration(ProjectScope.TEST); } project.setActiveProfiles( diff --git a/impl/maven-core/src/main/java/org/apache/maven/project/ResourceHandlingContext.java b/impl/maven-core/src/main/java/org/apache/maven/project/SourceHandlingContext.java similarity index 65% rename from impl/maven-core/src/main/java/org/apache/maven/project/ResourceHandlingContext.java rename to impl/maven-core/src/main/java/org/apache/maven/project/SourceHandlingContext.java index 48fc9e7e03c9..1029a8298912 100644 --- a/impl/maven-core/src/main/java/org/apache/maven/project/ResourceHandlingContext.java +++ b/impl/maven-core/src/main/java/org/apache/maven/project/SourceHandlingContext.java @@ -19,34 +19,55 @@ package org.apache.maven.project; import java.nio.file.Path; +import java.util.HashSet; import java.util.List; import java.util.Set; import org.apache.maven.api.Language; import org.apache.maven.api.ProjectScope; +import org.apache.maven.api.SourceRoot; import org.apache.maven.api.model.Resource; import org.apache.maven.api.services.BuilderProblem.Severity; import org.apache.maven.api.services.ModelBuilderResult; import org.apache.maven.api.services.ModelProblem.Version; import org.apache.maven.impl.DefaultSourceRoot; +import org.apache.maven.impl.model.DefaultModelProblem; import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** - * Handles resource configuration for Maven projects. - * Groups parameters shared between main and test resource handling. + * Handles source configuration for Maven projects with unified tracking for all language/scope combinations. + *

+ * This class replaces the previous approach of hardcoded boolean flags (hasMain, hasTest, etc.) + * with a flexible set-based tracking mechanism that works for any language and scope combination. + *

+ * Key features: + *

    + *
  • Tracks declared sources using {@code (language, scope, module, directory)} identity
    • + *
  • Only tracks enabled sources - disabled sources are effectively no-ops
    • + *
  • Detects duplicate enabled sources and emits warnings
    • + *
  • Provides {@link #hasSources(Language, ProjectScope)} to check if sources exist for a combination
    • + *
+ * + * @since 4.0.0 */ -class ResourceHandlingContext { +class SourceHandlingContext { - private static final Logger LOGGER = LoggerFactory.getLogger(ResourceHandlingContext.class); + private static final Logger LOGGER = LoggerFactory.getLogger(SourceHandlingContext.class); + + /** + * Identity key for source tracking. Two sources with the same key are considered duplicates. + */ + record SourceKey(Language language, ProjectScope scope, String module, Path directory) {} private final MavenProject project; private final Path baseDir; private final Set modules; private final boolean modularProject; private final ModelBuilderResult result; + private final Set declaredSources = new HashSet<>(); - ResourceHandlingContext( + SourceHandlingContext( MavenProject project, Path baseDir, Set modules, @@ -59,6 +80,73 @@ class ResourceHandlingContext { this.result = result; } + /** + * Determines if a source root should be added to the project and tracks it for duplicate detection. + *

+ * Rules: + *

    + *
  • Disabled sources are always added (they're filtered by {@code getEnabledSourceRoots()})
    • + *
  • First enabled source for an identity is added and tracked
    • + *
  • Subsequent enabled sources with same identity trigger a WARNING and are NOT added
    • + *
+ * + * @param sourceRoot the source root to evaluate + * @return true if the source should be added to the project, false if it's a duplicate enabled source + */ + boolean shouldAddSource(SourceRoot sourceRoot) { + if (!sourceRoot.enabled()) { + // Disabled sources are always added - they're filtered out by getEnabledSourceRoots() + LOGGER.debug( + "Adding disabled source (will be filtered by getEnabledSourceRoots): lang={}, scope={}, module={}, dir={}", + sourceRoot.language(), + sourceRoot.scope(), + sourceRoot.module().orElse(null), + sourceRoot.directory()); + return true; + } + + SourceKey key = new SourceKey( + sourceRoot.language(), sourceRoot.scope(), sourceRoot.module().orElse(null), sourceRoot.directory()); + + if (declaredSources.contains(key)) { + String message = String.format( + "Duplicate enabled source detected: lang=%s, scope=%s, module=%s, directory=%s. " + + "First enabled source wins, this duplicate is ignored.", + key.language(), key.scope(), key.module() != null ? key.module() : "(none)", key.directory()); + LOGGER.warn(message); + result.getProblemCollector() + .reportProblem(new DefaultModelProblem( + message, + Severity.WARNING, + Version.V41, + project.getModel().getDelegate(), + -1, + -1, + null)); + return false; // Don't add duplicate enabled source + } + + declaredSources.add(key); + LOGGER.debug( + "Adding and tracking enabled source: lang={}, scope={}, module={}, dir={}", + key.language(), + key.scope(), + key.module(), + key.directory()); + return true; // Add first enabled source with this identity + } + + /** + * Checks if any enabled sources have been declared for the given language and scope combination. + * + * @param language the language to check (e.g., {@link Language#JAVA_FAMILY}, {@link Language#RESOURCES}) + * @param scope the scope to check (e.g., {@link ProjectScope#MAIN}, {@link ProjectScope#TEST}) + * @return true if at least one enabled source exists for this combination + */ + boolean hasSources(Language language, ProjectScope scope) { + return declaredSources.stream().anyMatch(key -> language.equals(key.language()) && scope.equals(key.scope())); + } + /** * Handles resource configuration for a given scope (main or test). * This method applies the resource priority rules: @@ -68,9 +156,10 @@ class ResourceHandlingContext { * * * @param scope the project scope (MAIN or TEST) - * @param hasResourcesInSources whether resources are configured via {@code } */ - void handleResourceConfiguration(ProjectScope scope, boolean hasResourcesInSources) { + void handleResourceConfiguration(ProjectScope scope) { + boolean hasResourcesInSources = hasSources(Language.RESOURCES, scope); + List resources = scope == ProjectScope.MAIN ? project.getBuild().getDelegate().getResources() : project.getBuild().getDelegate().getTestResources(); @@ -105,7 +194,7 @@ void handleResourceConfiguration(ProjectScope scope, boolean hasResourcesInSourc + "Use " + sourcesConfig + " in for custom resource paths."; LOGGER.warn(message); result.getProblemCollector() - .reportProblem(new org.apache.maven.impl.model.DefaultModelProblem( + .reportProblem(new DefaultModelProblem( message, Severity.WARNING, Version.V41, diff --git a/impl/maven-core/src/test/java/org/apache/maven/project/ProjectBuilderTest.java b/impl/maven-core/src/test/java/org/apache/maven/project/ProjectBuilderTest.java index a8825bc5245b..3e71b73dff82 100644 --- a/impl/maven-core/src/test/java/org/apache/maven/project/ProjectBuilderTest.java +++ b/impl/maven-core/src/test/java/org/apache/maven/project/ProjectBuilderTest.java @@ -379,6 +379,10 @@ void testLocationTrackingResolution() throws Exception { /** * Tests that a project with multiple modules defined in sources is detected as modular, * and module-aware resource roots are injected for each module. + *

+ * Acceptance Criterion: AC2 (unified source tracking for all lang/scope combinations) + * + * @see Issue #11612 */ @Test void testModularSourcesInjectResourceRoots() throws Exception { @@ -389,7 +393,7 @@ void testModularSourcesInjectResourceRoots() throws Exception { // Get all resource source roots for main scope List mainResourceRoots = project.getEnabledSourceRoots(ProjectScope.MAIN, Language.RESOURCES) - .collect(Collectors.toList()); + .toList(); // Should have resource roots for both modules Set modules = mainResourceRoots.stream() @@ -404,7 +408,7 @@ void testModularSourcesInjectResourceRoots() throws Exception { // Get all resource source roots for test scope List testResourceRoots = project.getEnabledSourceRoots(ProjectScope.TEST, Language.RESOURCES) - .collect(Collectors.toList()); + .toList(); // Should have test resource roots for both modules Set testModules = testResourceRoots.stream() @@ -421,10 +425,14 @@ void testModularSourcesInjectResourceRoots() throws Exception { /** * Tests that when modular sources are configured alongside explicit legacy resources, * the legacy resources are ignored and a warning is issued. - * + *

* This verifies the behavior described in the design: * - Modular projects with explicit legacy {@code } configuration should issue a warning * - The modular resource roots are injected instead of using the legacy configuration + *

+ * Acceptance Criterion: AC2 (unified source tracking for all lang/scope combinations) + * + * @see Issue #11612 */ @Test void testModularSourcesWithExplicitResourcesIssuesWarning() throws Exception { @@ -444,7 +452,7 @@ void testModularSourcesWithExplicitResourcesIssuesWarning() throws Exception { List warnings = result.getProblems().stream() .filter(p -> p.getSeverity() == ModelProblem.Severity.WARNING) .filter(p -> p.getMessage().contains("Legacy") && p.getMessage().contains("ignored")) - .collect(Collectors.toList()); + .toList(); assertEquals(2, warnings.size(), "Should have 2 warnings (one for resources, one for testResources)"); assertTrue( @@ -456,7 +464,7 @@ void testModularSourcesWithExplicitResourcesIssuesWarning() throws Exception { // Verify modular resources are still injected correctly List mainResourceRoots = project.getEnabledSourceRoots(ProjectScope.MAIN, Language.RESOURCES) - .collect(Collectors.toList()); + .toList(); assertEquals(2, mainResourceRoots.size(), "Should have 2 modular resource roots (one per module)"); @@ -470,4 +478,253 @@ 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 mixed source configuration where: + * - Modular sources are defined for main Java (should override sourceDirectory) + * - Classic testSourceDirectory is used (should be preserved since no modular test sources) + *

+ * This verifies: + * - sourceDirectory is ignored when modular main sources exist + * - testSourceDirectory is used when no modular test sources are defined + *

+ * Acceptance Criterion: AC1 (boolean flags eliminated - uses hasSources() for main/test detection) + * + * @see Issue #11612 + */ + @Test + void testMixedSourcesModularMainClassicTest() throws Exception { + File pom = getProject("mixed-sources"); + + MavenSession session = createMavenSession(pom); + MavenProject project = session.getCurrentProject(); + + // Get main Java source roots - should have modular sources, not classic sourceDirectory + List 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 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().contains("src/classic/main/java")); + assertTrue(!hasClassicMainSource, "Classic sourceDirectory should be ignored"); + + // Get test Java source roots - should use classic testSourceDirectory since no modular test sources + List testJavaRoots = project.getEnabledSourceRoots(ProjectScope.TEST, Language.JAVA_FAMILY) + .toList(); + + // Should have 1 test source (from classic testSourceDirectory) + assertEquals(1, testJavaRoots.size(), "Should have 1 test Java source root (classic)"); + + // The test source should be the classic one (no module) + SourceRoot testRoot = testJavaRoots.get(0); + assertTrue(testRoot.module().isEmpty(), "Classic test source should not have a module"); + assertTrue( + testRoot.directory().toString().contains("src/classic/test/java"), + "Should use classic testSourceDirectory"); + } + + /** + * Tests mixed modular/non-modular sources within the same {@code } element. + *

+ * This verifies: + * - Both modular sources (with module attribute) are added + * - Non-modular sources (without module attribute) are added + * - sourceDirectory is ignored because {@code } exists + *

+ * Acceptance Criterion: AC1 (boolean flags eliminated - uses hasSources() for source detection) + * + * @see Issue #11612 + */ + @Test + void testSourcesMixedModulesWithinSources() throws Exception { + File pom = getProject("sources-mixed-modules"); + + MavenSession session = createMavenSession(pom); + MavenProject project = session.getCurrentProject(); + + // Get main Java source roots + List mainJavaRoots = project.getEnabledSourceRoots(ProjectScope.MAIN, Language.JAVA_FAMILY) + .toList(); + + // Should have 2 main sources: 1 modular (moduleA) + 1 non-modular + assertEquals(2, mainJavaRoots.size(), "Should have 2 main Java source roots (1 modular + 1 non-modular)"); + + // Count modular vs non-modular sources + long modularCount = + mainJavaRoots.stream().filter(sr -> sr.module().isPresent()).count(); + long nonModularCount = + mainJavaRoots.stream().filter(sr -> sr.module().isEmpty()).count(); + + assertEquals(1, modularCount, "Should have 1 modular main source"); + assertEquals(1, nonModularCount, "Should have 1 non-modular main source"); + + // Verify the modular source is moduleA + Set mainModules = mainJavaRoots.stream() + .map(SourceRoot::module) + .filter(opt -> opt.isPresent()) + .map(opt -> opt.get()) + .collect(Collectors.toSet()); + + assertTrue(mainModules.contains("org.foo.moduleA"), "Should have modular source for moduleA"); + + // Verify sourceDirectory is NOT used (should be ignored because has main java) + boolean hasIgnoredSource = + mainJavaRoots.stream().anyMatch(sr -> sr.directory().toString().contains("src/should-be-ignored")); + assertTrue(!hasIgnoredSource, "sourceDirectory should be ignored when has main java"); + + // Get test Java source roots - should have modular test source for moduleA + List testJavaRoots = project.getEnabledSourceRoots(ProjectScope.TEST, Language.JAVA_FAMILY) + .toList(); + + assertEquals(1, testJavaRoots.size(), "Should have 1 test Java source root"); + assertTrue(testJavaRoots.get(0).module().isPresent(), "Test source should be modular"); + assertEquals("org.foo.moduleA", testJavaRoots.get(0).module().get(), "Test source should be for moduleA"); + } + + /** + * Tests that multiple source directories for the same (lang, scope, module) combination + * are allowed and all are added as source roots. + *

+ * 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. + *

+ * Acceptance Criterion: AC2 (unified source tracking - multiple directories per module supported) + * + * @see Issue #11612 + */ + @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 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().contains("src/com.example.app/main/java")); + boolean hasGeneratedDir = mainJavaRoots.stream() + .anyMatch(sr -> sr.directory().toString().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 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. + *

+ * 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 + *

+ * Verifies: + * - First enabled source wins, subsequent duplicates trigger WARNING + * - Disabled sources don't count as duplicates + * - Different modules are unaffected + *

+ * 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 Issue #11612 + */ + @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 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 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() + && "com.example.dup".equals(sr.module().get())); + assertTrue(hasDupModule, "Should have source root for com.example.dup module"); + + // Get test Java source roots + List testJavaRoots = project.getEnabledSourceRoots(ProjectScope.TEST, Language.JAVA_FAMILY) + .toList(); + + // Test scope has 1 source for com.example.dup (first wins) + assertEquals(1, testJavaRoots.size(), "Should have 1 test Java source root"); + + // Verify it's for the dup module + assertTrue( + testJavaRoots.get(0).module().isPresent() + && "com.example.dup" + .equals(testJavaRoots.get(0).module().get()), + "Test source root should be for com.example.dup module"); + } } diff --git a/impl/maven-core/src/test/projects/project-builder/duplicate-enabled-sources/pom.xml b/impl/maven-core/src/test/projects/project-builder/duplicate-enabled-sources/pom.xml new file mode 100644 index 000000000000..42d48ddcdce0 --- /dev/null +++ b/impl/maven-core/src/test/projects/project-builder/duplicate-enabled-sources/pom.xml @@ -0,0 +1,64 @@ + + + + 4.1.0 + + org.apache.maven.tests + duplicate-enabled-sources-test + 1.0-SNAPSHOT + jar + + + + + + main + java + com.example.dup + true + + + + main + java + com.example.dup + true + + + + main + java + com.example.dup + false + + + + main + java + com.example.other + + + + test + java + com.example.dup + true + + + test + java + com.example.dup + true + + + + diff --git a/impl/maven-core/src/test/projects/project-builder/mixed-sources/pom.xml b/impl/maven-core/src/test/projects/project-builder/mixed-sources/pom.xml new file mode 100644 index 000000000000..ac2553aab4cd --- /dev/null +++ b/impl/maven-core/src/test/projects/project-builder/mixed-sources/pom.xml @@ -0,0 +1,38 @@ + + + + 4.1.0 + + org.apache.maven.tests + mixed-sources-test + 1.0-SNAPSHOT + jar + + + + src/classic/main/java + + src/classic/test/java + + + + + main + java + org.foo.moduleA + + + main + java + org.foo.moduleB + + + + + diff --git a/impl/maven-core/src/test/projects/project-builder/multiple-directories-same-module/pom.xml b/impl/maven-core/src/test/projects/project-builder/multiple-directories-same-module/pom.xml new file mode 100644 index 000000000000..a1128eaa567b --- /dev/null +++ b/impl/maven-core/src/test/projects/project-builder/multiple-directories-same-module/pom.xml @@ -0,0 +1,51 @@ + + + + 4.1.0 + + org.apache.maven.tests + multiple-directories-same-module-test + 1.0-SNAPSHOT + jar + + + + + + main + java + com.example.app + + + + main + java + com.example.app + target/generated-sources/com.example.app/java + + + + test + java + com.example.app + + + + test + java + com.example.app + target/generated-test-sources/com.example.app/java + + + + diff --git a/impl/maven-core/src/test/projects/project-builder/sources-mixed-modules/pom.xml b/impl/maven-core/src/test/projects/project-builder/sources-mixed-modules/pom.xml new file mode 100644 index 000000000000..ee27556b2ee7 --- /dev/null +++ b/impl/maven-core/src/test/projects/project-builder/sources-mixed-modules/pom.xml @@ -0,0 +1,47 @@ + + + + 4.1.0 + + org.apache.maven.tests + sources-mixed-modules-test + 1.0-SNAPSHOT + jar + + + + src/should-be-ignored/java + + + + + main + java + org.foo.moduleA + + + + main + java + + + + + test + java + org.foo.moduleA + + + + From 9098924d7c52f1c1846155017b1096c4f7ab7be9 Mon Sep 17 00:00:00 2001 From: Gerd Aschemann Date: Wed, 7 Jan 2026 18:42:32 +0100 Subject: [PATCH 2/5] Replace Collectors.toList() with .toList() in DefaultProjectBuilder MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use Java 17+ Stream.toList() instead of Collectors.toList(). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- .../org/apache/maven/project/DefaultProjectBuilder.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/impl/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java b/impl/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java index 825e23fbb418..5f7af9f2098e 100644 --- a/impl/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java +++ b/impl/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java @@ -525,7 +525,7 @@ List doBuild(List pomFiles, boolean recursive) { return pomFiles.stream() .map(pomFile -> build(pomFile, recursive)) .flatMap(List::stream) - .collect(Collectors.toList()); + .toList(); } finally { Thread.currentThread().setContextClassLoader(oldContextClassLoader); } @@ -571,7 +571,7 @@ private List build(File pomFile, boolean recursive) { project.setCollectedProjects(results(r) .filter(cr -> cr != r && cr.getEffectiveModel() != null) .map(cr -> projectIndex.get(cr.getEffectiveModel().getId())) - .collect(Collectors.toList())); + .toList()); DependencyResolutionResult resolutionResult = null; if (request.isResolveDependencies()) { @@ -1022,7 +1022,7 @@ private DependencyResolutionResult resolveDependencies(MavenProject project) { } private List getProfileIds(List profiles) { - return profiles.stream().map(Profile::getId).collect(Collectors.toList()); + return profiles.stream().map(Profile::getId).toList(); } private static ModelSource createStubModelSource(Artifact artifact) { From 8c91a97f90e4474f3e3bc2361afe3408399860b1 Mon Sep 17 00:00:00 2001 From: Gerd Aschemann Date: Thu, 8 Jan 2026 15:39:59 +0100 Subject: [PATCH 3/5] Fix Windows path separator issue in ProjectBuilderTest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Normalize path separators to forward slashes before comparing directory paths in tests. On Windows, Path.toString() returns backslashes, causing contains() checks with forward slashes to fail. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- .../apache/maven/project/ProjectBuilderTest.java | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/impl/maven-core/src/test/java/org/apache/maven/project/ProjectBuilderTest.java b/impl/maven-core/src/test/java/org/apache/maven/project/ProjectBuilderTest.java index 3e71b73dff82..4d6a03a0bcce 100644 --- a/impl/maven-core/src/test/java/org/apache/maven/project/ProjectBuilderTest.java +++ b/impl/maven-core/src/test/java/org/apache/maven/project/ProjectBuilderTest.java @@ -517,8 +517,8 @@ void testMixedSourcesModularMainClassicTest() throws Exception { 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().contains("src/classic/main/java")); + boolean hasClassicMainSource = mainJavaRoots.stream() + .anyMatch(sr -> sr.directory().toString().replace('\\', '/').contains("src/classic/main/java")); assertTrue(!hasClassicMainSource, "Classic sourceDirectory should be ignored"); // Get test Java source roots - should use classic testSourceDirectory since no modular test sources @@ -532,7 +532,7 @@ void testMixedSourcesModularMainClassicTest() throws Exception { SourceRoot testRoot = testJavaRoots.get(0); assertTrue(testRoot.module().isEmpty(), "Classic test source should not have a module"); assertTrue( - testRoot.directory().toString().contains("src/classic/test/java"), + testRoot.directory().toString().replace('\\', '/').contains("src/classic/test/java"), "Should use classic testSourceDirectory"); } @@ -628,9 +628,11 @@ void testMultipleDirectoriesSameModule() throws Exception { // One should be implicit directory, one should be generated-sources boolean hasImplicitDir = mainJavaRoots.stream() - .anyMatch(sr -> sr.directory().toString().contains("src/com.example.app/main/java")); - boolean hasGeneratedDir = mainJavaRoots.stream() - .anyMatch(sr -> sr.directory().toString().contains("target/generated-sources/com.example.app/java")); + .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"); From 19fd98d2040ef7bbff4ea94c0c881e3133ff4291 Mon Sep 17 00:00:00 2001 From: Gerd Aschemann Date: Tue, 13 Jan 2026 21:20:57 +0100 Subject: [PATCH 4/5] Add AC6 and AC7 validation for modular sources Implement validation rules for modular source handling (#11612): - AC6: ERROR when mixing modular (with module) and classic (without module) sources within - AC7: WARNING when legacy / are used in modular projects (both explicit config and filesystem existence) Changes: - Add validateNoMixedModularAndClassicSources() to SourceHandlingContext - Add warnIfExplicitLegacyDirectory() to DefaultProjectBuilder - Update tests to verify AC6 and AC7 behavior - Update test project comments to reflect correct behavior Co-Authored-By: Claude Opus 4.5 --- .../maven/project/DefaultProjectBuilder.java | 78 +++++++++++- .../maven/project/SourceHandlingContext.java | 52 +++++++- .../maven/project/ProjectBuilderTest.java | 120 +++++++++--------- .../project-builder/mixed-sources/pom.xml | 15 ++- .../sources-mixed-modules/pom.xml | 20 +-- 5 files changed, 203 insertions(+), 82 deletions(-) diff --git a/impl/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java b/impl/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java index 5f7af9f2098e..0783d9deabd8 100644 --- a/impl/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java +++ b/impl/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java @@ -27,6 +27,7 @@ import java.io.IOException; import java.io.InputStream; import java.nio.charset.StandardCharsets; +import java.nio.file.Files; import java.nio.file.Path; import java.util.AbstractMap; import java.util.ArrayList; @@ -696,17 +697,41 @@ private void initProject(MavenProject project, ModelBuilderResult result) { * for the corresponding scope and language. This rule exists because * Maven provides default values for those elements which may conflict * with user's configuration. + * + * Additionally, for 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. */ if (!sourceContext.hasSources(Language.SCRIPT, ProjectScope.MAIN)) { project.addScriptSourceRoot(build.getScriptSourceDirectory()); } - if (!sourceContext.hasSources(Language.JAVA_FAMILY, ProjectScope.MAIN)) { - project.addCompileSourceRoot(build.getSourceDirectory()); - } - if (!sourceContext.hasSources(Language.JAVA_FAMILY, ProjectScope.TEST)) { - project.addTestCompileSourceRoot(build.getTestSourceDirectory()); + if (isModularProject) { + // Modular projects: unconditionally ignore legacy directories, warn if explicitly set + warnIfExplicitLegacyDirectory( + build.getSourceDirectory(), + baseDir.resolve("src/main/java"), + "", + project.getId(), + result); + warnIfExplicitLegacyDirectory( + build.getTestSourceDirectory(), + baseDir.resolve("src/test/java"), + "", + project.getId(), + result); + } else { + // Classic projects: use legacy directories if no sources defined in + if (!sourceContext.hasSources(Language.JAVA_FAMILY, ProjectScope.MAIN)) { + project.addCompileSourceRoot(build.getSourceDirectory()); + } + if (!sourceContext.hasSources(Language.JAVA_FAMILY, ProjectScope.TEST)) { + project.addTestCompileSourceRoot(build.getTestSourceDirectory()); + } } + // Validate that modular and classic sources are not mixed within + sourceContext.validateNoMixedModularAndClassicSources(); + // Handle main and test resources using unified source handling sourceContext.handleResourceConfiguration(ProjectScope.MAIN); sourceContext.handleResourceConfiguration(ProjectScope.TEST); @@ -880,6 +905,49 @@ private void initProject(MavenProject project, ModelBuilderResult result) { project.setRemoteArtifactRepositories(remoteRepositories); } + /** + * Warns about legacy directory usage in a modular project. Two cases are handled: + *

    + *
  • Case 1: The default legacy directory exists on the filesystem (e.g., src/main/java exists)
    • + *
  • Case 2: An explicit legacy directory is configured that differs from the default
    • + *
+ * Legacy directories are unconditionally ignored in modular projects because it is not clear + * how to dispatch their content between different modules. + */ + private void warnIfExplicitLegacyDirectory( + String configuredDir, + Path defaultDir, + String elementName, + String projectId, + ModelBuilderResult result) { + if (configuredDir != null) { + Path configuredPath = Path.of(configuredDir).toAbsolutePath().normalize(); + Path defaultPath = defaultDir.toAbsolutePath().normalize(); + if (!configuredPath.equals(defaultPath)) { + // Case 2: Explicit configuration differs from default - always warn + String message = String.format( + "Legacy %s is ignored in modular project %s. " + + "In modular projects, source directories must be defined via " + + "with a module element for each module.", + elementName, projectId); + logger.warn(message); + result.getProblemCollector() + .reportProblem(new org.apache.maven.impl.model.DefaultModelProblem( + message, Severity.WARNING, Version.V41, null, -1, -1, null)); + } else if (Files.isDirectory(defaultPath)) { + // Case 1: Default configuration, but the default directory exists on filesystem + String message = String.format( + "Legacy %s '%s' exists but is ignored in modular project %s. " + + "In modular projects, source directories must be defined via .", + elementName, defaultPath, projectId); + logger.warn(message); + result.getProblemCollector() + .reportProblem(new org.apache.maven.impl.model.DefaultModelProblem( + message, Severity.WARNING, Version.V41, null, -1, -1, null)); + } + } + } + private void initParent(MavenProject project, ModelBuilderResult result) { Model parentModel = result.getParentModel(); diff --git a/impl/maven-core/src/main/java/org/apache/maven/project/SourceHandlingContext.java b/impl/maven-core/src/main/java/org/apache/maven/project/SourceHandlingContext.java index 1029a8298912..e7691eb86bcf 100644 --- a/impl/maven-core/src/main/java/org/apache/maven/project/SourceHandlingContext.java +++ b/impl/maven-core/src/main/java/org/apache/maven/project/SourceHandlingContext.java @@ -65,7 +65,7 @@ record SourceKey(Language language, ProjectScope scope, String module, Path dire private final Set modules; private final boolean modularProject; private final ModelBuilderResult result; - private final Set declaredSources = new HashSet<>(); + private final Set declaredSources; SourceHandlingContext( MavenProject project, @@ -78,6 +78,8 @@ record SourceKey(Language language, ProjectScope scope, String module, Path dire this.modules = modules; this.modularProject = modularProject; this.result = result; + // Each module typically has main, test, main resources, test resources = 4 sources + this.declaredSources = new HashSet<>(4 * modules.size()); } /** @@ -96,7 +98,7 @@ record SourceKey(Language language, ProjectScope scope, String module, Path dire boolean shouldAddSource(SourceRoot sourceRoot) { if (!sourceRoot.enabled()) { // Disabled sources are always added - they're filtered out by getEnabledSourceRoots() - LOGGER.debug( + LOGGER.trace( "Adding disabled source (will be filtered by getEnabledSourceRoots): lang={}, scope={}, module={}, dir={}", sourceRoot.language(), sourceRoot.scope(), @@ -105,8 +107,10 @@ boolean shouldAddSource(SourceRoot sourceRoot) { return true; } + // Normalize path for consistent duplicate detection (handles symlinks, relative paths) + Path normalizedDir = sourceRoot.directory().toAbsolutePath().normalize(); SourceKey key = new SourceKey( - sourceRoot.language(), sourceRoot.scope(), sourceRoot.module().orElse(null), sourceRoot.directory()); + sourceRoot.language(), sourceRoot.scope(), sourceRoot.module().orElse(null), normalizedDir); if (declaredSources.contains(key)) { String message = String.format( @@ -147,6 +151,48 @@ boolean hasSources(Language language, ProjectScope scope) { return declaredSources.stream().anyMatch(key -> language.equals(key.language()) && scope.equals(key.scope())); } + /** + * Validates that a project does not mix modular and classic (non-modular) sources. + *

+ * A project must be either fully modular (all sources have a module) or fully classic + * (no sources have a module). Mixing modular and non-modular sources within the same + * project is not supported because the compiler plugin cannot handle such configurations. + *

+ * This validation checks each (language, scope) combination and reports an ERROR if + * both modular and non-modular sources are found. + */ + void validateNoMixedModularAndClassicSources() { + for (ProjectScope scope : List.of(ProjectScope.MAIN, ProjectScope.TEST)) { + for (Language language : List.of(Language.JAVA_FAMILY, Language.RESOURCES)) { + boolean hasModular = declaredSources.stream() + .anyMatch(key -> + language.equals(key.language()) && scope.equals(key.scope()) && key.module() != null); + boolean hasClassic = declaredSources.stream() + .anyMatch(key -> + language.equals(key.language()) && scope.equals(key.scope()) && key.module() == null); + + if (hasModular && hasClassic) { + String message = String.format( + "Mixed modular and classic sources detected for lang=%s, scope=%s. " + + "A project must be either fully modular (all sources have a module) " + + "or fully classic (no sources have a module). " + + "The compiler plugin cannot handle mixed configurations.", + language.id(), scope.id()); + LOGGER.error(message); + result.getProblemCollector() + .reportProblem(new DefaultModelProblem( + message, + Severity.ERROR, + Version.V41, + project.getModel().getDelegate(), + -1, + -1, + null)); + } + } + } + } + /** * Handles resource configuration for a given scope (main or test). * This method applies the resource priority rules: diff --git a/impl/maven-core/src/test/java/org/apache/maven/project/ProjectBuilderTest.java b/impl/maven-core/src/test/java/org/apache/maven/project/ProjectBuilderTest.java index 4d6a03a0bcce..8c25ccc88ad8 100644 --- a/impl/maven-core/src/test/java/org/apache/maven/project/ProjectBuilderTest.java +++ b/impl/maven-core/src/test/java/org/apache/maven/project/ProjectBuilderTest.java @@ -480,15 +480,21 @@ void testModularSourcesWithExplicitResourcesIssuesWarning() throws Exception { } /** - * Tests mixed source configuration where: - * - Modular sources are defined for main Java (should override sourceDirectory) - * - Classic testSourceDirectory is used (should be preserved since no modular test sources) + * Tests that legacy sourceDirectory and testSourceDirectory are ignored in modular projects. + *

+ * 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). *

* This verifies: - * - sourceDirectory is ignored when modular main sources exist - * - testSourceDirectory is used when no modular test sources are defined + * - WARNINGs are emitted for explicitly set legacy directories in modular projects + * - sourceDirectory and testSourceDirectory are both ignored + * - Only modular sources from {@code } are used *

- * Acceptance Criterion: AC1 (boolean flags eliminated - uses hasSources() for main/test detection) + * Acceptance Criteria: + * - AC1 (boolean flags eliminated - uses hasSources() for main/test detection) + * - AC7 (legacy directories warning - {@code } and {@code } + * are unconditionally ignored with a WARNING in modular projects) * * @see Issue #11612 */ @@ -496,8 +502,30 @@ void testModularSourcesWithExplicitResourcesIssuesWarning() throws Exception { void testMixedSourcesModularMainClassicTest() throws Exception { File pom = getProject("mixed-sources"); - MavenSession session = createMavenSession(pom); - MavenProject project = session.getCurrentProject(); + 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 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("")), + "Should warn about ignored "); + assertTrue( + warnings.stream().anyMatch(w -> w.getMessage().contains("")), + "Should warn about ignored "); // Get main Java source roots - should have modular sources, not classic sourceDirectory List mainJavaRoots = project.getEnabledSourceRoots(ProjectScope.MAIN, Language.JAVA_FAMILY) @@ -521,30 +549,27 @@ void testMixedSourcesModularMainClassicTest() throws Exception { .anyMatch(sr -> sr.directory().toString().replace('\\', '/').contains("src/classic/main/java")); assertTrue(!hasClassicMainSource, "Classic sourceDirectory should be ignored"); - // Get test Java source roots - should use classic testSourceDirectory since no modular test sources + // Test sources should NOT be added (legacy testSourceDirectory is ignored in modular projects) List testJavaRoots = project.getEnabledSourceRoots(ProjectScope.TEST, Language.JAVA_FAMILY) .toList(); - - // Should have 1 test source (from classic testSourceDirectory) - assertEquals(1, testJavaRoots.size(), "Should have 1 test Java source root (classic)"); - - // The test source should be the classic one (no module) - SourceRoot testRoot = testJavaRoots.get(0); - assertTrue(testRoot.module().isEmpty(), "Classic test source should not have a module"); - assertTrue( - testRoot.directory().toString().replace('\\', '/').contains("src/classic/test/java"), - "Should use classic testSourceDirectory"); + assertEquals(0, testJavaRoots.size(), "Should have no test Java sources (legacy is ignored)"); } /** - * Tests mixed modular/non-modular sources within the same {@code } element. + * Tests that mixing modular and non-modular sources within {@code } is not allowed. + *

+ * 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. *

* This verifies: - * - Both modular sources (with module attribute) are added - * - Non-modular sources (without module attribute) are added + * - An ERROR is reported when both modular and non-modular sources exist in {@code } * - sourceDirectory is ignored because {@code } exists *

- * Acceptance Criterion: AC1 (boolean flags eliminated - uses hasSources() for source detection) + * Acceptance Criteria: + * - AC1 (boolean flags eliminated - uses hasSources() for source detection) + * - AC6 (mixed sources error - mixing modular and classic sources within {@code } + * triggers an ERROR) * * @see Issue #11612 */ @@ -552,46 +577,23 @@ void testMixedSourcesModularMainClassicTest() throws Exception { void testSourcesMixedModulesWithinSources() throws Exception { File pom = getProject("sources-mixed-modules"); - MavenSession session = createMavenSession(pom); - MavenProject project = session.getCurrentProject(); - - // Get main Java source roots - List mainJavaRoots = project.getEnabledSourceRoots(ProjectScope.MAIN, Language.JAVA_FAMILY) - .toList(); - - // Should have 2 main sources: 1 modular (moduleA) + 1 non-modular - assertEquals(2, mainJavaRoots.size(), "Should have 2 main Java source roots (1 modular + 1 non-modular)"); - - // Count modular vs non-modular sources - long modularCount = - mainJavaRoots.stream().filter(sr -> sr.module().isPresent()).count(); - long nonModularCount = - mainJavaRoots.stream().filter(sr -> sr.module().isEmpty()).count(); - - assertEquals(1, modularCount, "Should have 1 modular main source"); - assertEquals(1, nonModularCount, "Should have 1 non-modular main source"); - - // Verify the modular source is moduleA - Set mainModules = mainJavaRoots.stream() - .map(SourceRoot::module) - .filter(opt -> opt.isPresent()) - .map(opt -> opt.get()) - .collect(Collectors.toSet()); - - assertTrue(mainModules.contains("org.foo.moduleA"), "Should have modular source for moduleA"); + MavenSession mavenSession = createMavenSession(null); + ProjectBuildingRequest configuration = new DefaultProjectBuildingRequest(); + configuration.setRepositorySession(mavenSession.getRepositorySession()); - // Verify sourceDirectory is NOT used (should be ignored because has main java) - boolean hasIgnoredSource = - mainJavaRoots.stream().anyMatch(sr -> sr.directory().toString().contains("src/should-be-ignored")); - assertTrue(!hasIgnoredSource, "sourceDirectory should be ignored when has main java"); + ProjectBuildingResult result = getContainer() + .lookup(org.apache.maven.project.ProjectBuilder.class) + .build(pom, configuration); - // Get test Java source roots - should have modular test source for moduleA - List testJavaRoots = project.getEnabledSourceRoots(ProjectScope.TEST, Language.JAVA_FAMILY) + // Verify an ERROR is reported for mixing modular and non-modular sources + List errors = result.getProblems().stream() + .filter(p -> p.getSeverity() == ModelProblem.Severity.ERROR) + .filter(p -> p.getMessage().contains("Mixed modular and classic sources")) .toList(); - assertEquals(1, testJavaRoots.size(), "Should have 1 test Java source root"); - assertTrue(testJavaRoots.get(0).module().isPresent(), "Test source should be modular"); - assertEquals("org.foo.moduleA", testJavaRoots.get(0).module().get(), "Test source should be for moduleA"); + 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"); } /** diff --git a/impl/maven-core/src/test/projects/project-builder/mixed-sources/pom.xml b/impl/maven-core/src/test/projects/project-builder/mixed-sources/pom.xml index ac2553aab4cd..caa10d988502 100644 --- a/impl/maven-core/src/test/projects/project-builder/mixed-sources/pom.xml +++ b/impl/maven-core/src/test/projects/project-builder/mixed-sources/pom.xml @@ -1,8 +1,11 @@ jar - + src/classic/main/java - + src/classic/test/java @@ -32,7 +35,7 @@ java org.foo.moduleB - + diff --git a/impl/maven-core/src/test/projects/project-builder/sources-mixed-modules/pom.xml b/impl/maven-core/src/test/projects/project-builder/sources-mixed-modules/pom.xml index ee27556b2ee7..0c658483c091 100644 --- a/impl/maven-core/src/test/projects/project-builder/sources-mixed-modules/pom.xml +++ b/impl/maven-core/src/test/projects/project-builder/sources-mixed-modules/pom.xml @@ -1,13 +1,15 @@ java org.foo.moduleA - + main java - + From 08fa756ce2972618e099cb0689678e02a121adb0 Mon Sep 17 00:00:00 2001 From: Gerd Aschemann Date: Sun, 1 Feb 2026 17:26:03 +0100 Subject: [PATCH 5/5] Apply code review suggestions from PR #11632 - Use flatMap(Optional::stream) instead of filter/map - Use File.separatorChar instead of hardcoded backslash - Simplify module checks with orElse(null) - Make getProfileIds() static Co-Authored-By: Claude Opus 4.5 --- .../maven/project/DefaultProjectBuilder.java | 2 +- .../maven/project/ProjectBuilderTest.java | 46 +++++++++---------- 2 files changed, 22 insertions(+), 26 deletions(-) diff --git a/impl/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java b/impl/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java index 0783d9deabd8..9de2ca1348ff 100644 --- a/impl/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java +++ b/impl/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java @@ -1089,7 +1089,7 @@ private DependencyResolutionResult resolveDependencies(MavenProject project) { } } - private List getProfileIds(List profiles) { + private static List getProfileIds(List profiles) { return profiles.stream().map(Profile::getId).toList(); } diff --git a/impl/maven-core/src/test/java/org/apache/maven/project/ProjectBuilderTest.java b/impl/maven-core/src/test/java/org/apache/maven/project/ProjectBuilderTest.java index 8c25ccc88ad8..5da89a0f58b9 100644 --- a/impl/maven-core/src/test/java/org/apache/maven/project/ProjectBuilderTest.java +++ b/impl/maven-core/src/test/java/org/apache/maven/project/ProjectBuilderTest.java @@ -25,6 +25,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.Optional; import java.util.Properties; import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; @@ -398,8 +399,7 @@ void testModularSourcesInjectResourceRoots() throws Exception { // Should have resource roots for both modules Set modules = mainResourceRoots.stream() .map(SourceRoot::module) - .filter(opt -> opt.isPresent()) - .map(opt -> opt.get()) + .flatMap(Optional::stream) .collect(Collectors.toSet()); assertEquals(2, modules.size(), "Should have resource roots for 2 modules"); @@ -413,8 +413,7 @@ void testModularSourcesInjectResourceRoots() throws Exception { // Should have test resource roots for both modules Set testModules = testResourceRoots.stream() .map(SourceRoot::module) - .filter(opt -> opt.isPresent()) - .map(opt -> opt.get()) + .flatMap(Optional::stream) .collect(Collectors.toSet()); assertEquals(2, testModules.size(), "Should have test resource roots for 2 modules"); @@ -470,8 +469,7 @@ void testModularSourcesWithExplicitResourcesIssuesWarning() throws Exception { Set mainModules = mainResourceRoots.stream() .map(SourceRoot::module) - .filter(opt -> opt.isPresent()) - .map(opt -> opt.get()) + .flatMap(Optional::stream) .collect(Collectors.toSet()); assertEquals(2, mainModules.size(), "Should have resource roots for 2 modules"); @@ -536,8 +534,7 @@ void testMixedSourcesModularMainClassicTest() throws Exception { Set mainModules = mainJavaRoots.stream() .map(SourceRoot::module) - .filter(opt -> opt.isPresent()) - .map(opt -> opt.get()) + .flatMap(Optional::stream) .collect(Collectors.toSet()); assertEquals(2, mainModules.size(), "Should have main sources for 2 modules"); @@ -545,8 +542,10 @@ void testMixedSourcesModularMainClassicTest() throws Exception { 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")); + boolean hasClassicMainSource = mainJavaRoots.stream().anyMatch(sr -> sr.directory() + .toString() + .replace(File.separatorChar, '/') + .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) @@ -623,17 +622,18 @@ void testMultipleDirectoriesSameModule() throws Exception { // Both should be for the same module long moduleCount = mainJavaRoots.stream() - .filter(sr -> sr.module().isPresent() - && "com.example.app".equals(sr.module().get())) + .filter(sr -> "com.example.app".equals(sr.module().orElse(null))) .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 hasImplicitDir = mainJavaRoots.stream().anyMatch(sr -> sr.directory() + .toString() + .replace(File.separatorChar, '/') + .contains("src/com.example.app/main/java")); boolean hasGeneratedDir = mainJavaRoots.stream().anyMatch(sr -> sr.directory() .toString() - .replace('\\', '/') + .replace(File.separatorChar, '/') .contains("target/generated-sources/com.example.app/java")); assertTrue(hasImplicitDir, "Should have implicit source directory for module"); @@ -648,8 +648,7 @@ void testMultipleDirectoriesSameModule() throws Exception { // 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())) + .filter(sr -> "com.example.app".equals(sr.module().orElse(null))) .count(); assertEquals(2, testModuleCount, "Both test sources should be for com.example.app module"); } @@ -707,14 +706,12 @@ void testDuplicateEnabledSources() throws Exception { // Verify com.example.other module is present boolean hasOtherModule = mainJavaRoots.stream() - .anyMatch(sr -> sr.module().isPresent() - && "com.example.other".equals(sr.module().get())); + .anyMatch(sr -> "com.example.other".equals(sr.module().orElse(null))); 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() - && "com.example.dup".equals(sr.module().get())); + .anyMatch(sr -> "com.example.dup".equals(sr.module().orElse(null))); assertTrue(hasDupModule, "Should have source root for com.example.dup module"); // Get test Java source roots @@ -725,10 +722,9 @@ void testDuplicateEnabledSources() throws Exception { assertEquals(1, testJavaRoots.size(), "Should have 1 test Java source root"); // Verify it's for the dup module - assertTrue( - testJavaRoots.get(0).module().isPresent() - && "com.example.dup" - .equals(testJavaRoots.get(0).module().get()), + assertEquals( + "com.example.dup", + testJavaRoots.get(0).module().orElse(null), "Test source root should be for com.example.dup module"); } }