Skip to content

Commit c1b431b

Browse files
aschemanclaude
andcommitted
Fail on legacy config in modular projects
In modular projects, legacy directories and resources that would be silently ignored now trigger an ERROR instead of WARNING: - Explicit <sourceDirectory>/<testSourceDirectory> differing from defaults - Default src/main/java or src/test/java existing on filesystem - Explicit <resources>/<testResources> differing from Super POM defaults This prevents silent loss of user-configured sources/resources. AC8 supersedes AC7 which originally used WARNING. Fixes #11701 See #11701 (comment) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent fa36e7a commit c1b431b

File tree

3 files changed

+91
-70
lines changed

3 files changed

+91
-70
lines changed

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

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -693,27 +693,27 @@ private void initProject(MavenProject project, ModelBuilderResult result) {
693693

694694
/*
695695
* `sourceDirectory`, `testSourceDirectory` and `scriptSourceDirectory`
696-
* are ignored if the POM file contains at least one enabled <source> element
696+
* are not used if the POM file contains at least one enabled <source> element
697697
* for the corresponding scope and language. This rule exists because
698698
* Maven provides default values for those elements which may conflict
699699
* with user's configuration.
700700
*
701701
* Additionally, for modular projects, legacy directories are unconditionally
702-
* ignored because it is not clear how to dispatch their content between
702+
* rejected because it is not clear how to dispatch their content between
703703
* different modules. A warning is emitted if these properties are explicitly set.
704704
*/
705705
if (!sourceContext.hasSources(Language.SCRIPT, ProjectScope.MAIN)) {
706706
project.addScriptSourceRoot(build.getScriptSourceDirectory());
707707
}
708708
if (isModularProject) {
709-
// Modular projects: unconditionally ignore legacy directories, warn if explicitly set
710-
warnIfExplicitLegacyDirectory(
709+
// Modular projects: legacy directories conflict with modular sources
710+
failIfLegacyDirectoryPresent(
711711
build.getSourceDirectory(),
712712
baseDir.resolve("src/main/java"),
713713
"<sourceDirectory>",
714714
project.getId(),
715715
result);
716-
warnIfExplicitLegacyDirectory(
716+
failIfLegacyDirectoryPresent(
717717
build.getTestSourceDirectory(),
718718
baseDir.resolve("src/test/java"),
719719
"<testSourceDirectory>",
@@ -906,15 +906,17 @@ private void initProject(MavenProject project, ModelBuilderResult result) {
906906
}
907907

908908
/**
909-
* Warns about legacy directory usage in a modular project. Two cases are handled:
909+
* Fails the build if a legacy directory is present in a modular project.
910+
* <p>
911+
* "Present" means either:
910912
* <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+
* <li><b>Configuration presence</b>: an explicit configuration differs from the default</li>
914+
* <li><b>Physical presence</b>: the default directory exists on the filesystem</li>
913915
* </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+
* In both cases, the legacy directory conflicts with modular sources and must not be used.
917+
* Failing the build forces the user to resolve the conflict explicitly.
916918
*/
917-
private void warnIfExplicitLegacyDirectory(
919+
private void failIfLegacyDirectoryPresent(
918920
String configuredDir,
919921
Path defaultDir,
920922
String elementName,
@@ -924,26 +926,26 @@ private void warnIfExplicitLegacyDirectory(
924926
Path configuredPath = Path.of(configuredDir).toAbsolutePath().normalize();
925927
Path defaultPath = defaultDir.toAbsolutePath().normalize();
926928
if (!configuredPath.equals(defaultPath)) {
927-
// Case 2: Explicit configuration differs from default - always warn
929+
// Configuration presence: explicit config differs from default
928930
String message = String.format(
929-
"Legacy %s is ignored in modular project %s. "
931+
"Legacy %s must not be used in modular project %s."
930932
+ "In modular projects, source directories must be defined via <sources> "
931933
+ "with a module element for each module.",
932934
elementName, projectId);
933-
logger.warn(message);
935+
logger.error(message);
934936
result.getProblemCollector()
935937
.reportProblem(new org.apache.maven.impl.model.DefaultModelProblem(
936-
message, Severity.WARNING, Version.V41, null, -1, -1, null));
938+
message, Severity.ERROR, Version.V41, null, -1, -1, null));
937939
} else if (Files.isDirectory(defaultPath)) {
938-
// Case 1: Default configuration, but the default directory exists on filesystem
940+
// Physical presence: default directory exists on filesystem
939941
String message = String.format(
940-
"Legacy %s '%s' exists but is ignored in modular project %s. "
942+
"Legacy %s '%s' exists but must not be used in modular project %s. "
941943
+ "In modular projects, source directories must be defined via <sources>.",
942944
elementName, defaultPath, projectId);
943-
logger.warn(message);
945+
logger.error(message);
944946
result.getProblemCollector()
945947
.reportProblem(new org.apache.maven.impl.model.DefaultModelProblem(
946-
message, Severity.WARNING, Version.V41, null, -1, -1, null));
948+
message, Severity.ERROR, Version.V41, null, -1, -1, null));
947949
}
948950
}
949951
}

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

Lines changed: 30 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -221,11 +221,19 @@ void handleResourceConfiguration(ProjectScope scope) {
221221
if (hasResourcesInSources) {
222222
// Modular project with resources configured via <sources> - already added above
223223
if (hasExplicitLegacyResources(resources, scopeId)) {
224-
LOGGER.warn(
225-
"Legacy {} element is ignored because {} resources are configured via {} in <sources>.",
226-
legacyElement,
227-
scopeId,
228-
sourcesConfig);
224+
String message = String.format(
225+
"Legacy %s element must not be used because %s resources are configured via %s in <sources>.",
226+
legacyElement, scopeId, sourcesConfig);
227+
LOGGER.error(message);
228+
result.getProblemCollector()
229+
.reportProblem(new DefaultModelProblem(
230+
message,
231+
Severity.ERROR,
232+
Version.V41,
233+
project.getModel().getDelegate(),
234+
-1,
235+
-1,
236+
null));
229237
} else {
230238
LOGGER.debug(
231239
"{} resources configured via <sources> element, ignoring legacy {} element.",
@@ -236,13 +244,13 @@ void handleResourceConfiguration(ProjectScope scope) {
236244
// Modular project without resources in <sources> - inject module-aware defaults
237245
if (hasExplicitLegacyResources(resources, scopeId)) {
238246
String message = "Legacy " + legacyElement
239-
+ " element is ignored because modular sources are configured. "
247+
+ " element must not be used because modular sources are configured. "
240248
+ "Use " + sourcesConfig + " in <sources> for custom resource paths.";
241-
LOGGER.warn(message);
249+
LOGGER.error(message);
242250
result.getProblemCollector()
243251
.reportProblem(new DefaultModelProblem(
244252
message,
245-
Severity.WARNING,
253+
Severity.ERROR,
246254
Version.V41,
247255
project.getModel().getDelegate(),
248256
-1,
@@ -265,11 +273,19 @@ void handleResourceConfiguration(ProjectScope scope) {
265273
if (hasResourcesInSources) {
266274
// Resources configured via <sources> - already added above
267275
if (hasExplicitLegacyResources(resources, scopeId)) {
268-
LOGGER.warn(
269-
"Legacy {} element is ignored because {} resources are configured via {} in <sources>.",
270-
legacyElement,
271-
scopeId,
272-
sourcesConfig);
276+
String message = String.format(
277+
"Legacy %s element must not be used because %s resources are configured via %s in <sources>.",
278+
legacyElement, scopeId, sourcesConfig);
279+
LOGGER.error(message);
280+
result.getProblemCollector()
281+
.reportProblem(new DefaultModelProblem(
282+
message,
283+
Severity.ERROR,
284+
Version.V41,
285+
project.getModel().getDelegate(),
286+
-1,
287+
-1,
288+
null));
273289
} else {
274290
LOGGER.debug(
275291
"{} resources configured via <sources> element, ignoring legacy {} element.",
@@ -319,7 +335,7 @@ private DefaultSourceRoot createModularResourceRoot(String module, ProjectScope
319335
*
320336
* @param resources list of resources to check
321337
* @param scope scope (main or test)
322-
* @return true if explicit legacy resources are present that would be ignored
338+
* @return true if explicit legacy resources are present that conflict with modular sources
323339
*/
324340
private boolean hasExplicitLegacyResources(List<Resource> resources, String scope) {
325341
if (resources.isEmpty()) {

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

Lines changed: 40 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -422,19 +422,21 @@ void testModularSourcesInjectResourceRoots() throws Exception {
422422
}
423423

424424
/**
425-
* Tests that when modular sources are configured alongside explicit legacy resources,
426-
* the legacy resources are ignored and a warning is issued.
425+
* Tests that when modular sources are configured alongside explicit legacy resources, an error is raised.
427426
* <p>
428427
* This verifies the behavior described in the design:
429-
* - Modular projects with explicit legacy {@code <resources>} configuration should issue a warning
428+
* - Modular projects with explicit legacy {@code <resources>} configuration should raise an error
430429
* - The modular resource roots are injected instead of using the legacy configuration
431430
* <p>
432-
* Acceptance Criterion: AC2 (unified source tracking for all lang/scope combinations)
431+
* Acceptance Criteria:
432+
* - AC2 (unified source tracking for all lang/scope combinations)
433+
* - AC8 (legacy directories error - supersedes AC7 which originally used WARNING)
433434
*
434435
* @see <a href="https://github.com/apache/maven/issues/11612">Issue #11612</a>
436+
* @see <a href="https://github.com/apache/maven/issues/11701#issuecomment-3858462609">AC8 definition</a>
435437
*/
436438
@Test
437-
void testModularSourcesWithExplicitResourcesIssuesWarning() throws Exception {
439+
void testModularSourcesWithExplicitResourcesIssuesError() throws Exception {
438440
File pom = getProject("modular-sources-with-explicit-resources");
439441

440442
MavenSession mavenSession = createMavenSession(null);
@@ -447,19 +449,19 @@ void testModularSourcesWithExplicitResourcesIssuesWarning() throws Exception {
447449

448450
MavenProject project = result.getProject();
449451

450-
// Verify warnings are issued for ignored legacy resources
451-
List<ModelProblem> warnings = result.getProblems().stream()
452-
.filter(p -> p.getSeverity() == ModelProblem.Severity.WARNING)
453-
.filter(p -> p.getMessage().contains("Legacy") && p.getMessage().contains("ignored"))
452+
// Verify errors are raised for conflicting legacy resources (AC8)
453+
List<ModelProblem> errors = result.getProblems().stream()
454+
.filter(p -> p.getSeverity() == ModelProblem.Severity.ERROR)
455+
.filter(p -> p.getMessage().contains("Legacy") && p.getMessage().contains("must not be used"))
454456
.toList();
455457

456-
assertEquals(2, warnings.size(), "Should have 2 warnings (one for resources, one for testResources)");
458+
assertEquals(2, errors.size(), "Should have 2 errors (one for resources, one for testResources)");
457459
assertTrue(
458-
warnings.stream().anyMatch(w -> w.getMessage().contains("<resources>")),
459-
"Should warn about ignored <resources>");
460+
errors.stream().anyMatch(e -> e.getMessage().contains("<resources>")),
461+
"Should error about conflicting <resources>");
460462
assertTrue(
461-
warnings.stream().anyMatch(w -> w.getMessage().contains("<testResources>")),
462-
"Should warn about ignored <testResources>");
463+
errors.stream().anyMatch(e -> e.getMessage().contains("<testResources>")),
464+
"Should error about conflicting <testResources>");
463465

464466
// Verify modular resources are still injected correctly
465467
List<SourceRoot> mainResourceRoots = project.getEnabledSourceRoots(ProjectScope.MAIN, Language.RESOURCES)
@@ -478,23 +480,23 @@ void testModularSourcesWithExplicitResourcesIssuesWarning() throws Exception {
478480
}
479481

480482
/**
481-
* Tests that legacy sourceDirectory and testSourceDirectory are ignored in modular projects.
483+
* Tests that legacy sourceDirectory and testSourceDirectory raise an error in modular projects.
482484
* <p>
483-
* In modular projects, legacy directories are unconditionally ignored because it is not clear
484-
* how to dispatch their content between different modules. A warning is emitted if these
485-
* properties are explicitly set (differ from Super POM defaults).
485+
* In modular projects, legacy directories must not occur because it is not clear
486+
* how to dispatch their content between different modules. An error is raised if these
487+
* properties are explicitly set (and differ from Super POM defaults).
486488
* <p>
487489
* This verifies:
488-
* - WARNINGs are emitted for explicitly set legacy directories in modular projects
489-
* - sourceDirectory and testSourceDirectory are both ignored
490+
* - ERRORs are raised for explicitly set legacy directories in modular projects
490491
* - Only modular sources from {@code <sources>} are used
491492
* <p>
492493
* Acceptance Criteria:
493494
* - AC1 (boolean flags eliminated - uses hasSources() for main/test detection)
494-
* - AC7 (legacy directories warning - {@code <sourceDirectory>} and {@code <testSourceDirectory>}
495-
* are unconditionally ignored with a WARNING in modular projects)
495+
* - AC8 (legacy directories error - supersedes AC7 which originally used WARNING;
496+
* {@code <sourceDirectory>} and {@code <testSourceDirectory>} are raising an ERROR in modular projects)
496497
*
497498
* @see <a href="https://github.com/apache/maven/issues/11612">Issue #11612</a>
499+
* @see <a href="https://github.com/apache/maven/issues/11701#issuecomment-3858462609">AC8 definition</a>
498500
*/
499501
@Test
500502
void testMixedSourcesModularMainClassicTest() throws Exception {
@@ -510,20 +512,21 @@ void testMixedSourcesModularMainClassicTest() throws Exception {
510512

511513
MavenProject project = result.getProject();
512514

513-
// Verify WARNINGs are emitted for explicitly set legacy directories
514-
List<ModelProblem> warnings = result.getProblems().stream()
515-
.filter(p -> p.getSeverity() == ModelProblem.Severity.WARNING)
516-
.filter(p -> p.getMessage().contains("Legacy") && p.getMessage().contains("ignored in modular project"))
515+
// Verify ERRORs are raised for explicitly set legacy directories (AC8)
516+
List<ModelProblem> errors = result.getProblems().stream()
517+
.filter(p -> p.getSeverity() == ModelProblem.Severity.ERROR)
518+
.filter(p -> p.getMessage().contains("Legacy")
519+
&& p.getMessage().contains("must not be used in modular project"))
517520
.toList();
518521

519-
// Should have 2 warnings: one for sourceDirectory, one for testSourceDirectory
520-
assertEquals(2, warnings.size(), "Should have 2 warnings for ignored legacy directories");
522+
// Should have 2 errors: one for sourceDirectory, one for testSourceDirectory
523+
assertEquals(2, errors.size(), "Should have 2 errors for conflicting legacy directories");
521524
assertTrue(
522-
warnings.stream().anyMatch(w -> w.getMessage().contains("<sourceDirectory>")),
523-
"Should warn about ignored <sourceDirectory>");
525+
errors.stream().anyMatch(e -> e.getMessage().contains("<sourceDirectory>")),
526+
"Should error about conflicting <sourceDirectory>");
524527
assertTrue(
525-
warnings.stream().anyMatch(w -> w.getMessage().contains("<testSourceDirectory>")),
526-
"Should warn about ignored <testSourceDirectory>");
528+
errors.stream().anyMatch(e -> e.getMessage().contains("<testSourceDirectory>")),
529+
"Should error about conflicting <testSourceDirectory>");
527530

528531
// Get main Java source roots - should have modular sources, not classic sourceDirectory
529532
List<SourceRoot> mainJavaRoots = project.getEnabledSourceRoots(ProjectScope.MAIN, Language.JAVA_FAMILY)
@@ -541,17 +544,17 @@ void testMixedSourcesModularMainClassicTest() throws Exception {
541544
assertTrue(mainModules.contains("org.foo.moduleA"), "Should have main source for moduleA");
542545
assertTrue(mainModules.contains("org.foo.moduleB"), "Should have main source for moduleB");
543546

544-
// Verify the classic sourceDirectory is NOT used (should be ignored)
547+
// Verify the classic sourceDirectory is NOT used (rejected in modular projects)
545548
boolean hasClassicMainSource = mainJavaRoots.stream().anyMatch(sr -> sr.directory()
546549
.toString()
547550
.replace(File.separatorChar, '/')
548551
.contains("src/classic/main/java"));
549-
assertTrue(!hasClassicMainSource, "Classic sourceDirectory should be ignored");
552+
assertTrue(!hasClassicMainSource, "Classic sourceDirectory must not be used");
550553

551-
// Test sources should NOT be added (legacy testSourceDirectory is ignored in modular projects)
554+
// Test sources should NOT be added (legacy testSourceDirectory is rejected in modular projects)
552555
List<SourceRoot> testJavaRoots = project.getEnabledSourceRoots(ProjectScope.TEST, Language.JAVA_FAMILY)
553556
.toList();
554-
assertEquals(0, testJavaRoots.size(), "Should have no test Java sources (legacy is ignored)");
557+
assertEquals(0, testJavaRoots.size(), "Should have no test Java sources (legacy is rejected)");
555558
}
556559

557560
/**
@@ -563,7 +566,7 @@ void testMixedSourcesModularMainClassicTest() throws Exception {
563566
* <p>
564567
* This verifies:
565568
* - An ERROR is reported when both modular and non-modular sources exist in {@code <sources>}
566-
* - sourceDirectory is ignored because {@code <source scope="main" lang="java">} exists
569+
* - sourceDirectory is not used because {@code <source scope="main" lang="java">} exists
567570
* <p>
568571
* Acceptance Criteria:
569572
* - AC1 (boolean flags eliminated - uses hasSources() for source detection)

0 commit comments

Comments
 (0)