Skip to content

Commit 63d43d5

Browse files
aschemanclaude
andcommitted
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 <sources> - AC7: WARNING when legacy <sourceDirectory>/<testSourceDirectory> 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 <noreply@anthropic.com>
1 parent 1c067a0 commit 63d43d5

5 files changed

Lines changed: 203 additions & 82 deletions

File tree

impl/maven-core/src/main/java/org/apache/maven/project/DefaultProjectBuilder.java

Lines changed: 73 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import java.io.IOException;
2828
import java.io.InputStream;
2929
import java.nio.charset.StandardCharsets;
30+
import java.nio.file.Files;
3031
import java.nio.file.Path;
3132
import java.util.AbstractMap;
3233
import java.util.ArrayList;
@@ -696,17 +697,41 @@ private void initProject(MavenProject project, ModelBuilderResult result) {
696697
* for the corresponding scope and language. This rule exists because
697698
* Maven provides default values for those elements which may conflict
698699
* with user's configuration.
700+
*
701+
* Additionally, for modular projects, legacy directories are unconditionally
702+
* ignored because it is not clear how to dispatch their content between
703+
* different modules. A warning is emitted if these properties are explicitly set.
699704
*/
700705
if (!sourceContext.hasSources(Language.SCRIPT, ProjectScope.MAIN)) {
701706
project.addScriptSourceRoot(build.getScriptSourceDirectory());
702707
}
703-
if (!sourceContext.hasSources(Language.JAVA_FAMILY, ProjectScope.MAIN)) {
704-
project.addCompileSourceRoot(build.getSourceDirectory());
705-
}
706-
if (!sourceContext.hasSources(Language.JAVA_FAMILY, ProjectScope.TEST)) {
707-
project.addTestCompileSourceRoot(build.getTestSourceDirectory());
708+
if (isModularProject) {
709+
// Modular projects: unconditionally ignore legacy directories, warn if explicitly set
710+
warnIfExplicitLegacyDirectory(
711+
build.getSourceDirectory(),
712+
baseDir.resolve("src/main/java"),
713+
"<sourceDirectory>",
714+
project.getId(),
715+
result);
716+
warnIfExplicitLegacyDirectory(
717+
build.getTestSourceDirectory(),
718+
baseDir.resolve("src/test/java"),
719+
"<testSourceDirectory>",
720+
project.getId(),
721+
result);
722+
} else {
723+
// Classic projects: use legacy directories if no sources defined in <sources>
724+
if (!sourceContext.hasSources(Language.JAVA_FAMILY, ProjectScope.MAIN)) {
725+
project.addCompileSourceRoot(build.getSourceDirectory());
726+
}
727+
if (!sourceContext.hasSources(Language.JAVA_FAMILY, ProjectScope.TEST)) {
728+
project.addTestCompileSourceRoot(build.getTestSourceDirectory());
729+
}
708730
}
709731

732+
// Validate that modular and classic sources are not mixed within <sources>
733+
sourceContext.validateNoMixedModularAndClassicSources();
734+
710735
// Handle main and test resources using unified source handling
711736
sourceContext.handleResourceConfiguration(ProjectScope.MAIN);
712737
sourceContext.handleResourceConfiguration(ProjectScope.TEST);
@@ -880,6 +905,49 @@ private void initProject(MavenProject project, ModelBuilderResult result) {
880905
project.setRemoteArtifactRepositories(remoteRepositories);
881906
}
882907

908+
/**
909+
* Warns about legacy directory usage in a modular project. Two cases are handled:
910+
* <ul>
911+
* <li>Case 1: The default legacy directory exists on the filesystem (e.g., src/main/java exists)</li>
912+
* <li>Case 2: An explicit legacy directory is configured that differs from the default</li>
913+
* </ul>
914+
* Legacy directories are unconditionally ignored in modular projects because it is not clear
915+
* how to dispatch their content between different modules.
916+
*/
917+
private void warnIfExplicitLegacyDirectory(
918+
String configuredDir,
919+
Path defaultDir,
920+
String elementName,
921+
String projectId,
922+
ModelBuilderResult result) {
923+
if (configuredDir != null) {
924+
Path configuredPath = Path.of(configuredDir).toAbsolutePath().normalize();
925+
Path defaultPath = defaultDir.toAbsolutePath().normalize();
926+
if (!configuredPath.equals(defaultPath)) {
927+
// Case 2: Explicit configuration differs from default - always warn
928+
String message = String.format(
929+
"Legacy %s is ignored in modular project %s. "
930+
+ "In modular projects, source directories must be defined via <sources> "
931+
+ "with a module attribute for each module.",
932+
elementName, projectId);
933+
logger.warn(message);
934+
result.getProblemCollector()
935+
.reportProblem(new org.apache.maven.impl.model.DefaultModelProblem(
936+
message, Severity.WARNING, Version.V41, null, -1, -1, null));
937+
} else if (Files.isDirectory(defaultPath)) {
938+
// Case 1: Default configuration, but the default directory exists on filesystem
939+
String message = String.format(
940+
"Legacy %s '%s' exists but is ignored in modular project %s. "
941+
+ "In modular projects, source directories must be defined via <sources>.",
942+
elementName, defaultPath, projectId);
943+
logger.warn(message);
944+
result.getProblemCollector()
945+
.reportProblem(new org.apache.maven.impl.model.DefaultModelProblem(
946+
message, Severity.WARNING, Version.V41, null, -1, -1, null));
947+
}
948+
}
949+
}
950+
883951
private void initParent(MavenProject project, ModelBuilderResult result) {
884952
Model parentModel = result.getParentModel();
885953

impl/maven-core/src/main/java/org/apache/maven/project/SourceHandlingContext.java

Lines changed: 49 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ record SourceKey(Language language, ProjectScope scope, String module, Path dire
6565
private final Set<String> modules;
6666
private final boolean modularProject;
6767
private final ModelBuilderResult result;
68-
private final Set<SourceKey> declaredSources = new HashSet<>();
68+
private final Set<SourceKey> declaredSources;
6969

7070
SourceHandlingContext(
7171
MavenProject project,
@@ -78,6 +78,8 @@ record SourceKey(Language language, ProjectScope scope, String module, Path dire
7878
this.modules = modules;
7979
this.modularProject = modularProject;
8080
this.result = result;
81+
// Each module typically has main, test, main resources, test resources = 4 sources
82+
this.declaredSources = new HashSet<>(4 * modules.size());
8183
}
8284

8385
/**
@@ -96,7 +98,7 @@ record SourceKey(Language language, ProjectScope scope, String module, Path dire
9698
boolean shouldAddSource(SourceRoot sourceRoot) {
9799
if (!sourceRoot.enabled()) {
98100
// Disabled sources are always added - they're filtered out by getEnabledSourceRoots()
99-
LOGGER.debug(
101+
LOGGER.trace(
100102
"Adding disabled source (will be filtered by getEnabledSourceRoots): lang={}, scope={}, module={}, dir={}",
101103
sourceRoot.language(),
102104
sourceRoot.scope(),
@@ -105,8 +107,10 @@ boolean shouldAddSource(SourceRoot sourceRoot) {
105107
return true;
106108
}
107109

110+
// Normalize path for consistent duplicate detection (handles symlinks, relative paths)
111+
Path normalizedDir = sourceRoot.directory().toAbsolutePath().normalize();
108112
SourceKey key = new SourceKey(
109-
sourceRoot.language(), sourceRoot.scope(), sourceRoot.module().orElse(null), sourceRoot.directory());
113+
sourceRoot.language(), sourceRoot.scope(), sourceRoot.module().orElse(null), normalizedDir);
110114

111115
if (declaredSources.contains(key)) {
112116
String message = String.format(
@@ -147,6 +151,48 @@ boolean hasSources(Language language, ProjectScope scope) {
147151
return declaredSources.stream().anyMatch(key -> language.equals(key.language()) && scope.equals(key.scope()));
148152
}
149153

154+
/**
155+
* Validates that a project does not mix modular and classic (non-modular) sources.
156+
* <p>
157+
* A project must be either fully modular (all sources have a module) or fully classic
158+
* (no sources have a module). Mixing modular and non-modular sources within the same
159+
* project is not supported because the compiler plugin cannot handle such configurations.
160+
* <p>
161+
* This validation checks each (language, scope) combination and reports an ERROR if
162+
* both modular and non-modular sources are found.
163+
*/
164+
void validateNoMixedModularAndClassicSources() {
165+
for (ProjectScope scope : List.of(ProjectScope.MAIN, ProjectScope.TEST)) {
166+
for (Language language : List.of(Language.JAVA_FAMILY, Language.RESOURCES)) {
167+
boolean hasModular = declaredSources.stream()
168+
.anyMatch(key ->
169+
language.equals(key.language()) && scope.equals(key.scope()) && key.module() != null);
170+
boolean hasClassic = declaredSources.stream()
171+
.anyMatch(key ->
172+
language.equals(key.language()) && scope.equals(key.scope()) && key.module() == null);
173+
174+
if (hasModular && hasClassic) {
175+
String message = String.format(
176+
"Mixed modular and classic sources detected for lang=%s, scope=%s. "
177+
+ "A project must be either fully modular (all sources have a module) "
178+
+ "or fully classic (no sources have a module). "
179+
+ "The compiler plugin cannot handle mixed configurations.",
180+
language.id(), scope.id());
181+
LOGGER.error(message);
182+
result.getProblemCollector()
183+
.reportProblem(new DefaultModelProblem(
184+
message,
185+
Severity.ERROR,
186+
Version.V41,
187+
project.getModel().getDelegate(),
188+
-1,
189+
-1,
190+
null));
191+
}
192+
}
193+
}
194+
}
195+
150196
/**
151197
* Handles resource configuration for a given scope (main or test).
152198
* This method applies the resource priority rules:

impl/maven-core/src/test/java/org/apache/maven/project/ProjectBuilderTest.java

Lines changed: 61 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -480,24 +480,52 @@ void testModularSourcesWithExplicitResourcesIssuesWarning() throws Exception {
480480
}
481481

482482
/**
483-
* Tests mixed source configuration where:
484-
* - Modular sources are defined for main Java (should override sourceDirectory)
485-
* - Classic testSourceDirectory is used (should be preserved since no modular test sources)
483+
* Tests that legacy sourceDirectory and testSourceDirectory are ignored in modular projects.
484+
* <p>
485+
* In modular projects, legacy directories are unconditionally ignored because it is not clear
486+
* how to dispatch their content between different modules. A warning is emitted if these
487+
* properties are explicitly set (differ from Super POM defaults).
486488
* <p>
487489
* This verifies:
488-
* - sourceDirectory is ignored when modular main sources exist
489-
* - testSourceDirectory is used when no modular test sources are defined
490+
* - WARNINGs are emitted for explicitly set legacy directories in modular projects
491+
* - sourceDirectory and testSourceDirectory are both ignored
492+
* - Only modular sources from {@code <sources>} are used
490493
* <p>
491-
* Acceptance Criterion: AC1 (boolean flags eliminated - uses hasSources() for main/test detection)
494+
* Acceptance Criteria:
495+
* - AC1 (boolean flags eliminated - uses hasSources() for main/test detection)
496+
* - AC7 (legacy directories warning - {@code <sourceDirectory>} and {@code <testSourceDirectory>}
497+
* are unconditionally ignored with a WARNING in modular projects)
492498
*
493499
* @see <a href="https://github.com/apache/maven/issues/11612">Issue #11612</a>
494500
*/
495501
@Test
496502
void testMixedSourcesModularMainClassicTest() throws Exception {
497503
File pom = getProject("mixed-sources");
498504

499-
MavenSession session = createMavenSession(pom);
500-
MavenProject project = session.getCurrentProject();
505+
MavenSession mavenSession = createMavenSession(null);
506+
ProjectBuildingRequest configuration = new DefaultProjectBuildingRequest();
507+
configuration.setRepositorySession(mavenSession.getRepositorySession());
508+
509+
ProjectBuildingResult result = getContainer()
510+
.lookup(org.apache.maven.project.ProjectBuilder.class)
511+
.build(pom, configuration);
512+
513+
MavenProject project = result.getProject();
514+
515+
// Verify WARNINGs are emitted for explicitly set legacy directories
516+
List<ModelProblem> warnings = result.getProblems().stream()
517+
.filter(p -> p.getSeverity() == ModelProblem.Severity.WARNING)
518+
.filter(p -> p.getMessage().contains("Legacy") && p.getMessage().contains("ignored in modular project"))
519+
.toList();
520+
521+
// Should have 2 warnings: one for sourceDirectory, one for testSourceDirectory
522+
assertEquals(2, warnings.size(), "Should have 2 warnings for ignored legacy directories");
523+
assertTrue(
524+
warnings.stream().anyMatch(w -> w.getMessage().contains("<sourceDirectory>")),
525+
"Should warn about ignored <sourceDirectory>");
526+
assertTrue(
527+
warnings.stream().anyMatch(w -> w.getMessage().contains("<testSourceDirectory>")),
528+
"Should warn about ignored <testSourceDirectory>");
501529

502530
// Get main Java source roots - should have modular sources, not classic sourceDirectory
503531
List<SourceRoot> mainJavaRoots = project.getEnabledSourceRoots(ProjectScope.MAIN, Language.JAVA_FAMILY)
@@ -521,77 +549,51 @@ void testMixedSourcesModularMainClassicTest() throws Exception {
521549
.anyMatch(sr -> sr.directory().toString().replace('\\', '/').contains("src/classic/main/java"));
522550
assertTrue(!hasClassicMainSource, "Classic sourceDirectory should be ignored");
523551

524-
// Get test Java source roots - should use classic testSourceDirectory since no modular test sources
552+
// Test sources should NOT be added (legacy testSourceDirectory is ignored in modular projects)
525553
List<SourceRoot> testJavaRoots = project.getEnabledSourceRoots(ProjectScope.TEST, Language.JAVA_FAMILY)
526554
.toList();
527-
528-
// Should have 1 test source (from classic testSourceDirectory)
529-
assertEquals(1, testJavaRoots.size(), "Should have 1 test Java source root (classic)");
530-
531-
// The test source should be the classic one (no module)
532-
SourceRoot testRoot = testJavaRoots.get(0);
533-
assertTrue(testRoot.module().isEmpty(), "Classic test source should not have a module");
534-
assertTrue(
535-
testRoot.directory().toString().replace('\\', '/').contains("src/classic/test/java"),
536-
"Should use classic testSourceDirectory");
555+
assertEquals(0, testJavaRoots.size(), "Should have no test Java sources (legacy is ignored)");
537556
}
538557

539558
/**
540-
* Tests mixed modular/non-modular sources within the same {@code <sources>} element.
559+
* Tests that mixing modular and non-modular sources within {@code <sources>} is not allowed.
560+
* <p>
561+
* A project must be either fully modular (all sources have a module) or fully classic
562+
* (no sources have a module). Mixing them within the same project is not supported
563+
* because the compiler plugin cannot handle such configurations.
541564
* <p>
542565
* This verifies:
543-
* - Both modular sources (with module attribute) are added
544-
* - Non-modular sources (without module attribute) are added
566+
* - An ERROR is reported when both modular and non-modular sources exist in {@code <sources>}
545567
* - sourceDirectory is ignored because {@code <source scope="main" lang="java">} exists
546568
* <p>
547-
* Acceptance Criterion: AC1 (boolean flags eliminated - uses hasSources() for source detection)
569+
* Acceptance Criteria:
570+
* - AC1 (boolean flags eliminated - uses hasSources() for source detection)
571+
* - AC6 (mixed sources error - mixing modular and classic sources within {@code <sources>}
572+
* triggers an ERROR)
548573
*
549574
* @see <a href="https://github.com/apache/maven/issues/11612">Issue #11612</a>
550575
*/
551576
@Test
552577
void testSourcesMixedModulesWithinSources() throws Exception {
553578
File pom = getProject("sources-mixed-modules");
554579

555-
MavenSession session = createMavenSession(pom);
556-
MavenProject project = session.getCurrentProject();
557-
558-
// Get main Java source roots
559-
List<SourceRoot> mainJavaRoots = project.getEnabledSourceRoots(ProjectScope.MAIN, Language.JAVA_FAMILY)
560-
.toList();
561-
562-
// Should have 2 main sources: 1 modular (moduleA) + 1 non-modular
563-
assertEquals(2, mainJavaRoots.size(), "Should have 2 main Java source roots (1 modular + 1 non-modular)");
564-
565-
// Count modular vs non-modular sources
566-
long modularCount =
567-
mainJavaRoots.stream().filter(sr -> sr.module().isPresent()).count();
568-
long nonModularCount =
569-
mainJavaRoots.stream().filter(sr -> sr.module().isEmpty()).count();
570-
571-
assertEquals(1, modularCount, "Should have 1 modular main source");
572-
assertEquals(1, nonModularCount, "Should have 1 non-modular main source");
573-
574-
// Verify the modular source is moduleA
575-
Set<String> mainModules = mainJavaRoots.stream()
576-
.map(SourceRoot::module)
577-
.filter(opt -> opt.isPresent())
578-
.map(opt -> opt.get())
579-
.collect(Collectors.toSet());
580-
581-
assertTrue(mainModules.contains("org.foo.moduleA"), "Should have modular source for moduleA");
580+
MavenSession mavenSession = createMavenSession(null);
581+
ProjectBuildingRequest configuration = new DefaultProjectBuildingRequest();
582+
configuration.setRepositorySession(mavenSession.getRepositorySession());
582583

583-
// Verify sourceDirectory is NOT used (should be ignored because <sources> has main java)
584-
boolean hasIgnoredSource =
585-
mainJavaRoots.stream().anyMatch(sr -> sr.directory().toString().contains("src/should-be-ignored"));
586-
assertTrue(!hasIgnoredSource, "sourceDirectory should be ignored when <sources> has main java");
584+
ProjectBuildingResult result = getContainer()
585+
.lookup(org.apache.maven.project.ProjectBuilder.class)
586+
.build(pom, configuration);
587587

588-
// Get test Java source roots - should have modular test source for moduleA
589-
List<SourceRoot> testJavaRoots = project.getEnabledSourceRoots(ProjectScope.TEST, Language.JAVA_FAMILY)
588+
// Verify an ERROR is reported for mixing modular and non-modular sources
589+
List<ModelProblem> errors = result.getProblems().stream()
590+
.filter(p -> p.getSeverity() == ModelProblem.Severity.ERROR)
591+
.filter(p -> p.getMessage().contains("Mixed modular and classic sources"))
590592
.toList();
591593

592-
assertEquals(1, testJavaRoots.size(), "Should have 1 test Java source root");
593-
assertTrue(testJavaRoots.get(0).module().isPresent(), "Test source should be modular");
594-
assertEquals("org.foo.moduleA", testJavaRoots.get(0).module().get(), "Test source should be for moduleA");
594+
assertEquals(1, errors.size(), "Should have 1 error for mixed modular/classic configuration");
595+
assertTrue(errors.get(0).getMessage().contains("lang=java"), "Error should mention java language");
596+
assertTrue(errors.get(0).getMessage().contains("scope=main"), "Error should mention main scope");
595597
}
596598

597599
/**

0 commit comments

Comments
 (0)