From 1fb817830f79ac85e6ee7250bc522effceda7083 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Sun, 13 Apr 2025 20:57:33 +0200 Subject: [PATCH 1/7] [MNG-8676] Improve model builder error messages Only the two that are easy to be mixed up: GAV and GAT related, as the rest (XML, PK/GA, REPOID, DIR) ones are not quite mixable. The problem with these two are that they look pretty much same (both a "A:B:C", colon segmented string of 3 or 4), but in one case C is dependency type, in other is version. --- https://issues.apache.org/jira/browse/MNG-8676 --- .../org/apache/maven/impl/model/DefaultModelValidator.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelValidator.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelValidator.java index b6c4586b114a..f4f374f276ae 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelValidator.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelValidator.java @@ -2241,11 +2241,11 @@ public static SourceHint xmlNodeInputLocation(XmlNode xmlNode) { } public static SourceHint gav(String gav) { - return new SourceHint(gav, null); // "GAV" + return new SourceHint(gav, "G:A:V"); // GAV } public static SourceHint dependencyManagementKey(Dependency dependency) { - return new SourceHint(dependency.getManagementKey(), null); // DMK + return new SourceHint(dependency.getManagementKey(), "G:A:T"); // DMK } public static SourceHint pluginKey(Plugin plugin) { From cf8efc4186293c6c006937832214e13e8df21ef8 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Sun, 13 Apr 2025 21:17:00 +0200 Subject: [PATCH 2/7] Fix test --- .../maven/project/ProjectBuilderTest.java | 2 +- .../impl/model/DefaultModelValidator.java | 4 +- .../impl/model/DefaultModelValidatorTest.java | 53 ++++++++++--------- 3 files changed, 32 insertions(+), 27 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 9cdd456ad4bc..780ce8ecdd5b 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 @@ -101,7 +101,7 @@ void testVersionlessManagedDependency() throws Exception { assertThat( e.getResults(), contains(projectBuildingResultWithProblemMessage( - "'dependencies.dependency.version' for org.apache.maven.its:a:jar is missing"))); + "'dependencies.dependency.version' for org.apache.maven.its:a:jar (GAT) is missing"))); assertThat(e.getResults(), contains(projectBuildingResultWithLocation(5, 9))); } diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelValidator.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelValidator.java index f4f374f276ae..bddd3109b987 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelValidator.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelValidator.java @@ -2241,11 +2241,11 @@ public static SourceHint xmlNodeInputLocation(XmlNode xmlNode) { } public static SourceHint gav(String gav) { - return new SourceHint(gav, "G:A:V"); // GAV + return new SourceHint(gav, null); // GAV } public static SourceHint dependencyManagementKey(Dependency dependency) { - return new SourceHint(dependency.getManagementKey(), "G:A:T"); // DMK + return new SourceHint(dependency.getManagementKey(), "GAT"); // DMK } public static SourceHint pluginKey(Plugin plugin) { diff --git a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelValidatorTest.java b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelValidatorTest.java index dcd6fa99cc9b..9184e19d12c3 100644 --- a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelValidatorTest.java +++ b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelValidatorTest.java @@ -198,7 +198,7 @@ void testMissingDependencyArtifactId() throws Exception { assertTrue(result.getErrors() .get(0) - .contains("'dependencies.dependency.artifactId' for groupId:null:jar is missing")); + .contains("'dependencies.dependency.artifactId' for groupId:null:jar (GAT) is missing")); } @Test @@ -209,7 +209,7 @@ void testMissingDependencyGroupId() throws Exception { assertTrue(result.getErrors() .get(0) - .contains("'dependencies.dependency.groupId' for null:artifactId:jar is missing")); + .contains("'dependencies.dependency.groupId' for null:artifactId:jar (GAT) is missing")); } @Test @@ -220,7 +220,7 @@ void testMissingDependencyVersion() throws Exception { assertTrue(result.getErrors() .get(0) - .contains("'dependencies.dependency.version' for groupId:artifactId:jar is missing")); + .contains("'dependencies.dependency.version' for groupId:artifactId:jar (GAT) is missing")); } @Test @@ -229,9 +229,11 @@ void testMissingDependencyManagementArtifactId() throws Exception { assertViolations(result, 0, 1, 0); - assertTrue(result.getErrors() - .get(0) - .contains("'dependencyManagement.dependencies.dependency.artifactId' for groupId:null:jar is missing")); + assertTrue( + result.getErrors() + .get(0) + .contains( + "'dependencyManagement.dependencies.dependency.artifactId' for groupId:null:jar (GAT) is missing")); } @Test @@ -240,9 +242,11 @@ void testMissingDependencyManagementGroupId() throws Exception { assertViolations(result, 0, 1, 0); - assertTrue(result.getErrors() - .get(0) - .contains("'dependencyManagement.dependencies.dependency.groupId' for null:artifactId:jar is missing")); + assertTrue( + result.getErrors() + .get(0) + .contains( + "'dependencyManagement.dependencies.dependency.groupId' for null:artifactId:jar (GAT) is missing")); } @Test @@ -361,10 +365,11 @@ void testBadDependencyVersion() throws Exception { assertViolations(result, 0, 2, 0); assertContains( - result.getErrors().get(0), "'dependencies.dependency.version' for test:b:jar must be a valid version"); + result.getErrors().get(0), + "'dependencies.dependency.version' for test:b:jar (GAT) must be a valid version"); assertContains( result.getErrors().get(1), - "'dependencies.dependency.version' for test:c:jar must not contain any of these characters"); + "'dependencies.dependency.version' for test:c:jar (GAT) must not contain any of these characters"); } @Test @@ -441,13 +446,13 @@ void testHardCodedSystemPath() throws Exception { assertContains( result.getWarnings().get(0), - "'dependencies.dependency.scope' for test:a:jar declares usage of deprecated 'system' scope"); + "'dependencies.dependency.scope' for test:a:jar (GAT) declares usage of deprecated 'system' scope"); assertContains( result.getWarnings().get(1), - "'dependencies.dependency.systemPath' for test:a:jar should use a variable instead of a hard-coded path"); + "'dependencies.dependency.systemPath' for test:a:jar (GAT) should use a variable instead of a hard-coded path"); assertContains( result.getWarnings().get(2), - "'dependencies.dependency.scope' for test:b:jar declares usage of deprecated 'system' scope"); + "'dependencies.dependency.scope' for test:b:jar (GAT) declares usage of deprecated 'system' scope"); } @Test @@ -596,10 +601,10 @@ void testMissingDependencyExclusionId() throws Exception { assertContains( result.getWarnings().get(0), - "'dependencies.dependency.exclusions.exclusion.groupId' for gid:aid:jar is missing"); + "'dependencies.dependency.exclusions.exclusion.groupId' for gid:aid:jar (GAT) is missing"); assertContains( result.getWarnings().get(1), - "'dependencies.dependency.exclusions.exclusion.artifactId' for gid:aid:jar is missing"); + "'dependencies.dependency.exclusions.exclusion.artifactId' for gid:aid:jar (GAT) is missing"); } @Test @@ -610,7 +615,7 @@ void testBadImportScopeType() throws Exception { assertContains( result.getWarnings().get(0), - "'dependencyManagement.dependencies.dependency.type' for test:a:jar must be 'pom'"); + "'dependencyManagement.dependencies.dependency.type' for test:a:jar (GAT) must be 'pom'"); } @Test @@ -621,7 +626,7 @@ void testBadImportScopeClassifier() throws Exception { assertContains( result.getErrors().get(0), - "'dependencyManagement.dependencies.dependency.classifier' for test:a:pom:cls must be empty"); + "'dependencyManagement.dependencies.dependency.classifier' for test:a:pom:cls (GAT) must be empty"); } @Test @@ -632,16 +637,16 @@ void testSystemPathRefersToProjectBasedir() throws Exception { assertContains( result.getWarnings().get(0), - "'dependencies.dependency.scope' for test:a:jar declares usage of deprecated 'system' scope"); + "'dependencies.dependency.scope' for test:a:jar (GAT) declares usage of deprecated 'system' scope"); assertContains( result.getWarnings().get(1), - "'dependencies.dependency.systemPath' for test:a:jar should not point at files within the project directory"); + "'dependencies.dependency.systemPath' for test:a:jar (GAT) should not point at files within the project directory"); assertContains( result.getWarnings().get(2), - "'dependencies.dependency.scope' for test:b:jar declares usage of deprecated 'system' scope"); + "'dependencies.dependency.scope' for test:b:jar (GAT) declares usage of deprecated 'system' scope"); assertContains( result.getWarnings().get(3), - "'dependencies.dependency.systemPath' for test:b:jar should not point at files within the project directory"); + "'dependencies.dependency.systemPath' for test:b:jar (GAT) should not point at files within the project directory"); } @Test @@ -707,10 +712,10 @@ void testDeprecatedDependencyMetaversionsLatestAndRelease() throws Exception { assertContains( result.getWarnings().get(0), - "'dependencies.dependency.version' for test:a:jar is either LATEST or RELEASE (both of them are being deprecated)"); + "'dependencies.dependency.version' for test:a:jar (GAT) is either LATEST or RELEASE (both of them are being deprecated)"); assertContains( result.getWarnings().get(1), - "'dependencies.dependency.version' for test:b:jar is either LATEST or RELEASE (both of them are being deprecated)"); + "'dependencies.dependency.version' for test:b:jar (GAT) is either LATEST or RELEASE (both of them are being deprecated)"); } @Test From 3190ae334763fa3db7f60ac4189b71586dc18634 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Sun, 13 Apr 2025 21:37:01 +0200 Subject: [PATCH 3/7] Fix IT --- .../java/org/apache/maven/it/MavenITmng3220ImportScopeTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng3220ImportScopeTest.java b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng3220ImportScopeTest.java index 77c2b89a0d28..d3807cd05220 100644 --- a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng3220ImportScopeTest.java +++ b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng3220ImportScopeTest.java @@ -81,7 +81,7 @@ public void testitMNG3220b() throws Exception { boolean found = false; for (String line : lines) { if (line.contains("\'dependencies.dependency.version\' is missing for junit:junit") - || line.contains("\'dependencies.dependency.version\' for junit:junit:jar is missing")) { + || line.contains("\'dependencies.dependency.version\' for junit:junit:jar (GAT) is missing")) { found = true; break; } From 9cfc1747ad31b0aab6b89e7b1fa3c3738bd5e1c3 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Mon, 14 Apr 2025 20:34:47 +0200 Subject: [PATCH 4/7] Just change how we emit DMK to users --- .../maven/project/ProjectBuilderTest.java | 5 +- .../impl/model/DefaultModelValidator.java | 20 +++++- .../impl/model/DefaultModelValidatorTest.java | 65 ++++++++++--------- 3 files changed, 55 insertions(+), 35 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 780ce8ecdd5b..e45811188edb 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 @@ -100,8 +100,9 @@ void testVersionlessManagedDependency() throws Exception { .build(pomFile, configuration)); assertThat( e.getResults(), - contains(projectBuildingResultWithProblemMessage( - "'dependencies.dependency.version' for org.apache.maven.its:a:jar (GAT) is missing"))); + contains( + projectBuildingResultWithProblemMessage( + "'dependencies.dependency.version' for g='org.apache.maven.its', a='a', type='jar' is missing"))); assertThat(e.getResults(), contains(projectBuildingResultWithLocation(5, 9))); } diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelValidator.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelValidator.java index bddd3109b987..86bb2d25497c 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelValidator.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelValidator.java @@ -2245,7 +2245,25 @@ public static SourceHint gav(String gav) { } public static SourceHint dependencyManagementKey(Dependency dependency) { - return new SourceHint(dependency.getManagementKey(), "GAT"); // DMK + String hint; + if (dependency.getClassifier() == null + || dependency.getClassifier().trim().isEmpty()) { + hint = String.format( + "g=%s, a=%s, type=%s", + nvl(dependency.getGroupId()), nvl(dependency.getArtifactId()), nvl(dependency.getType())); + } else { + hint = String.format( + "g=%s, a=%s, c=%s, type=%s", + nvl(dependency.getGroupId()), + nvl(dependency.getArtifactId()), + nvl(dependency.getClassifier()), + nvl(dependency.getType())); + } + return new SourceHint(hint, null); // DMK + } + + private static String nvl(String value) { + return value == null ? "" : "'" + value + "'"; } public static SourceHint pluginKey(Plugin plugin) { diff --git a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelValidatorTest.java b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelValidatorTest.java index 9184e19d12c3..9791878f7ffb 100644 --- a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelValidatorTest.java +++ b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelValidatorTest.java @@ -198,7 +198,7 @@ void testMissingDependencyArtifactId() throws Exception { assertTrue(result.getErrors() .get(0) - .contains("'dependencies.dependency.artifactId' for groupId:null:jar (GAT) is missing")); + .contains("'dependencies.dependency.artifactId' for g='groupId', a=, type='jar' is missing")); } @Test @@ -209,7 +209,7 @@ void testMissingDependencyGroupId() throws Exception { assertTrue(result.getErrors() .get(0) - .contains("'dependencies.dependency.groupId' for null:artifactId:jar (GAT) is missing")); + .contains("'dependencies.dependency.groupId' for g=, a='artifactId', type='jar' is missing")); } @Test @@ -220,7 +220,7 @@ void testMissingDependencyVersion() throws Exception { assertTrue(result.getErrors() .get(0) - .contains("'dependencies.dependency.version' for groupId:artifactId:jar (GAT) is missing")); + .contains("'dependencies.dependency.version' for g='groupId', a='artifactId', type='jar' is missing")); } @Test @@ -233,7 +233,7 @@ void testMissingDependencyManagementArtifactId() throws Exception { result.getErrors() .get(0) .contains( - "'dependencyManagement.dependencies.dependency.artifactId' for groupId:null:jar (GAT) is missing")); + "'dependencyManagement.dependencies.dependency.artifactId' for g='groupId', a=, type='jar' is missing")); } @Test @@ -246,7 +246,7 @@ void testMissingDependencyManagementGroupId() throws Exception { result.getErrors() .get(0) .contains( - "'dependencyManagement.dependencies.dependency.groupId' for null:artifactId:jar (GAT) is missing")); + "'dependencyManagement.dependencies.dependency.groupId' for g=, a='artifactId', type='jar' is missing")); } @Test @@ -331,11 +331,11 @@ void testBadPluginDependencyScope() throws Exception { assertViolations(result, 0, 3, 0); - assertTrue(result.getErrors().get(0).contains("test:d")); + assertTrue(result.getErrors().get(0).contains("g='test', a='d'")); - assertTrue(result.getErrors().get(1).contains("test:e")); + assertTrue(result.getErrors().get(1).contains("g='test', a='e'")); - assertTrue(result.getErrors().get(2).contains("test:f")); + assertTrue(result.getErrors().get(2).contains("g='test', a='f'")); } @Test @@ -344,9 +344,9 @@ void testBadDependencyScope() throws Exception { assertViolations(result, 0, 0, 2); - assertTrue(result.getWarnings().get(0).contains("test:f")); + assertTrue(result.getWarnings().get(0).contains("g='test', a='f'")); - assertTrue(result.getWarnings().get(1).contains("test:g")); + assertTrue(result.getWarnings().get(1).contains("g='test', a='g'")); } @Test @@ -355,7 +355,7 @@ void testBadDependencyManagementScope() throws Exception { assertViolations(result, 0, 0, 1); - assertContains(result.getWarnings().get(0), "test:g"); + assertContains(result.getWarnings().get(0), "g='test', a='g'"); } @Test @@ -366,10 +366,10 @@ void testBadDependencyVersion() throws Exception { assertContains( result.getErrors().get(0), - "'dependencies.dependency.version' for test:b:jar (GAT) must be a valid version"); + "'dependencies.dependency.version' for g='test', a='b', type='jar' must be a valid version"); assertContains( result.getErrors().get(1), - "'dependencies.dependency.version' for test:c:jar (GAT) must not contain any of these characters"); + "'dependencies.dependency.version' for g='test', a='c', type='jar' must not contain any of these characters"); } @Test @@ -446,13 +446,13 @@ void testHardCodedSystemPath() throws Exception { assertContains( result.getWarnings().get(0), - "'dependencies.dependency.scope' for test:a:jar (GAT) declares usage of deprecated 'system' scope"); + "'dependencies.dependency.scope' for g='test', a='a', type='jar' declares usage of deprecated 'system' scope"); assertContains( result.getWarnings().get(1), - "'dependencies.dependency.systemPath' for test:a:jar (GAT) should use a variable instead of a hard-coded path"); + "'dependencies.dependency.systemPath' for g='test', a='a', type='jar' should use a variable instead of a hard-coded path"); assertContains( result.getWarnings().get(2), - "'dependencies.dependency.scope' for test:b:jar (GAT) declares usage of deprecated 'system' scope"); + "'dependencies.dependency.scope' for g='test', a='b', type='jar' declares usage of deprecated 'system' scope"); } @Test @@ -506,7 +506,7 @@ void testMissingPluginDependencyGroupId() throws Exception { assertViolations(result, 0, 1, 0); - assertTrue(result.getErrors().get(0).contains(":a:")); + assertTrue(result.getErrors().get(0).contains("g=, a='a',")); } @Test @@ -515,7 +515,7 @@ void testMissingPluginDependencyArtifactId() throws Exception { assertViolations(result, 0, 1, 0); - assertTrue(result.getErrors().get(0).contains("test:")); + assertTrue(result.getErrors().get(0).contains("g='test', a=,")); } @Test @@ -524,7 +524,7 @@ void testMissingPluginDependencyVersion() throws Exception { assertViolations(result, 0, 1, 0); - assertTrue(result.getErrors().get(0).contains("test:a")); + assertTrue(result.getErrors().get(0).contains("g='test', a='a',")); } @Test @@ -533,7 +533,7 @@ void testBadPluginDependencyVersion() throws Exception { assertViolations(result, 0, 1, 0); - assertTrue(result.getErrors().get(0).contains("test:b")); + assertTrue(result.getErrors().get(0).contains("g='test', a='b'")); } @Test @@ -581,10 +581,11 @@ void testBadDependencyExclusionId() throws Exception { assertViolations(result, 0, 0, 2); assertContains( - result.getWarnings().get(0), "'dependencies.dependency.exclusions.exclusion.groupId' for gid:aid:jar"); + result.getWarnings().get(0), + "'dependencies.dependency.exclusions.exclusion.groupId' for g='gid', a='aid', type='jar'"); assertContains( result.getWarnings().get(1), - "'dependencies.dependency.exclusions.exclusion.artifactId' for gid:aid:jar"); + "'dependencies.dependency.exclusions.exclusion.artifactId' for g='gid', a='aid', type='jar'"); // MNG-3832: Aether (part of M3+) supports wildcard expressions for exclusions @@ -601,10 +602,10 @@ void testMissingDependencyExclusionId() throws Exception { assertContains( result.getWarnings().get(0), - "'dependencies.dependency.exclusions.exclusion.groupId' for gid:aid:jar (GAT) is missing"); + "'dependencies.dependency.exclusions.exclusion.groupId' for g='gid', a='aid', type='jar' is missing"); assertContains( result.getWarnings().get(1), - "'dependencies.dependency.exclusions.exclusion.artifactId' for gid:aid:jar (GAT) is missing"); + "'dependencies.dependency.exclusions.exclusion.artifactId' for g='gid', a='aid', type='jar' is missing"); } @Test @@ -615,7 +616,7 @@ void testBadImportScopeType() throws Exception { assertContains( result.getWarnings().get(0), - "'dependencyManagement.dependencies.dependency.type' for test:a:jar (GAT) must be 'pom'"); + "'dependencyManagement.dependencies.dependency.type' for g='test', a='a', type='jar' must be 'pom'"); } @Test @@ -626,7 +627,7 @@ void testBadImportScopeClassifier() throws Exception { assertContains( result.getErrors().get(0), - "'dependencyManagement.dependencies.dependency.classifier' for test:a:pom:cls (GAT) must be empty"); + "'dependencyManagement.dependencies.dependency.classifier' for g='test', a='a', c='cls', type='pom' must be empty"); } @Test @@ -637,16 +638,16 @@ void testSystemPathRefersToProjectBasedir() throws Exception { assertContains( result.getWarnings().get(0), - "'dependencies.dependency.scope' for test:a:jar (GAT) declares usage of deprecated 'system' scope"); + "'dependencies.dependency.scope' for g='test', a='a', type='jar' declares usage of deprecated 'system' scope"); assertContains( result.getWarnings().get(1), - "'dependencies.dependency.systemPath' for test:a:jar (GAT) should not point at files within the project directory"); + "'dependencies.dependency.systemPath' for g='test', a='a', type='jar' should not point at files within the project directory"); assertContains( result.getWarnings().get(2), - "'dependencies.dependency.scope' for test:b:jar (GAT) declares usage of deprecated 'system' scope"); + "'dependencies.dependency.scope' for g='test', a='b', type='jar' declares usage of deprecated 'system' scope"); assertContains( result.getWarnings().get(3), - "'dependencies.dependency.systemPath' for test:b:jar (GAT) should not point at files within the project directory"); + "'dependencies.dependency.systemPath' for g='test', a='b', type='jar' should not point at files within the project directory"); } @Test @@ -712,10 +713,10 @@ void testDeprecatedDependencyMetaversionsLatestAndRelease() throws Exception { assertContains( result.getWarnings().get(0), - "'dependencies.dependency.version' for test:a:jar (GAT) is either LATEST or RELEASE (both of them are being deprecated)"); + "'dependencies.dependency.version' for g='test', a='a', type='jar' is either LATEST or RELEASE (both of them are being deprecated)"); assertContains( result.getWarnings().get(1), - "'dependencies.dependency.version' for test:b:jar (GAT) is either LATEST or RELEASE (both of them are being deprecated)"); + "'dependencies.dependency.version' for g='test', a='b', type='jar' is either LATEST or RELEASE (both of them are being deprecated)"); } @Test From 87af2d5c1488ccf36620d2ce2fe96dc9e20f8d1c Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Mon, 14 Apr 2025 20:37:27 +0200 Subject: [PATCH 5/7] Fix IT as well --- .../java/org/apache/maven/it/MavenITmng3220ImportScopeTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng3220ImportScopeTest.java b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng3220ImportScopeTest.java index d3807cd05220..1f52acf42c52 100644 --- a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng3220ImportScopeTest.java +++ b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng3220ImportScopeTest.java @@ -81,7 +81,7 @@ public void testitMNG3220b() throws Exception { boolean found = false; for (String line : lines) { if (line.contains("\'dependencies.dependency.version\' is missing for junit:junit") - || line.contains("\'dependencies.dependency.version\' for junit:junit:jar (GAT) is missing")) { + || line.contains("\'dependencies.dependency.version\' for g='junit', a='junit', type='jar' is missing")) { found = true; break; } From e3c60391265d229bfbf023ecc8ab48101ed7a360 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Mon, 14 Apr 2025 20:48:30 +0200 Subject: [PATCH 6/7] Do not user abbreviations, use model field name (XML tag name) --- .../maven/project/ProjectBuilderTest.java | 2 +- .../impl/model/DefaultModelValidator.java | 4 +- .../impl/model/DefaultModelValidatorTest.java | 82 ++++++++++--------- .../it/MavenITmng3220ImportScopeTest.java | 2 +- 4 files changed, 48 insertions(+), 42 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 e45811188edb..541ac8063c60 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 @@ -102,7 +102,7 @@ void testVersionlessManagedDependency() throws Exception { e.getResults(), contains( projectBuildingResultWithProblemMessage( - "'dependencies.dependency.version' for g='org.apache.maven.its', a='a', type='jar' is missing"))); + "'dependencies.dependency.version' for groupId='org.apache.maven.its', artifactId='a', type='jar' is missing"))); assertThat(e.getResults(), contains(projectBuildingResultWithLocation(5, 9))); } diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelValidator.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelValidator.java index 86bb2d25497c..126de225d4ea 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelValidator.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelValidator.java @@ -2249,11 +2249,11 @@ public static SourceHint dependencyManagementKey(Dependency dependency) { if (dependency.getClassifier() == null || dependency.getClassifier().trim().isEmpty()) { hint = String.format( - "g=%s, a=%s, type=%s", + "groupId=%s, artifactId=%s, type=%s", nvl(dependency.getGroupId()), nvl(dependency.getArtifactId()), nvl(dependency.getType())); } else { hint = String.format( - "g=%s, a=%s, c=%s, type=%s", + "groupId=%s, artifactId=%s, classifier=%s, type=%s", nvl(dependency.getGroupId()), nvl(dependency.getArtifactId()), nvl(dependency.getClassifier()), diff --git a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelValidatorTest.java b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelValidatorTest.java index 9791878f7ffb..06947221f3b5 100644 --- a/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelValidatorTest.java +++ b/impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelValidatorTest.java @@ -196,9 +196,11 @@ void testMissingDependencyArtifactId() throws Exception { assertViolations(result, 0, 1, 0); - assertTrue(result.getErrors() - .get(0) - .contains("'dependencies.dependency.artifactId' for g='groupId', a=, type='jar' is missing")); + assertTrue( + result.getErrors() + .get(0) + .contains( + "'dependencies.dependency.artifactId' for groupId='groupId', artifactId=, type='jar' is missing")); } @Test @@ -207,9 +209,11 @@ void testMissingDependencyGroupId() throws Exception { assertViolations(result, 0, 1, 0); - assertTrue(result.getErrors() - .get(0) - .contains("'dependencies.dependency.groupId' for g=, a='artifactId', type='jar' is missing")); + assertTrue( + result.getErrors() + .get(0) + .contains( + "'dependencies.dependency.groupId' for groupId=, artifactId='artifactId', type='jar' is missing")); } @Test @@ -218,9 +222,11 @@ void testMissingDependencyVersion() throws Exception { assertViolations(result, 0, 1, 0); - assertTrue(result.getErrors() - .get(0) - .contains("'dependencies.dependency.version' for g='groupId', a='artifactId', type='jar' is missing")); + assertTrue( + result.getErrors() + .get(0) + .contains( + "'dependencies.dependency.version' for groupId='groupId', artifactId='artifactId', type='jar' is missing")); } @Test @@ -233,7 +239,7 @@ void testMissingDependencyManagementArtifactId() throws Exception { result.getErrors() .get(0) .contains( - "'dependencyManagement.dependencies.dependency.artifactId' for g='groupId', a=, type='jar' is missing")); + "'dependencyManagement.dependencies.dependency.artifactId' for groupId='groupId', artifactId=, type='jar' is missing")); } @Test @@ -246,7 +252,7 @@ void testMissingDependencyManagementGroupId() throws Exception { result.getErrors() .get(0) .contains( - "'dependencyManagement.dependencies.dependency.groupId' for g=, a='artifactId', type='jar' is missing")); + "'dependencyManagement.dependencies.dependency.groupId' for groupId=, artifactId='artifactId', type='jar' is missing")); } @Test @@ -331,11 +337,11 @@ void testBadPluginDependencyScope() throws Exception { assertViolations(result, 0, 3, 0); - assertTrue(result.getErrors().get(0).contains("g='test', a='d'")); + assertTrue(result.getErrors().get(0).contains("groupId='test', artifactId='d'")); - assertTrue(result.getErrors().get(1).contains("g='test', a='e'")); + assertTrue(result.getErrors().get(1).contains("groupId='test', artifactId='e'")); - assertTrue(result.getErrors().get(2).contains("g='test', a='f'")); + assertTrue(result.getErrors().get(2).contains("groupId='test', artifactId='f'")); } @Test @@ -344,9 +350,9 @@ void testBadDependencyScope() throws Exception { assertViolations(result, 0, 0, 2); - assertTrue(result.getWarnings().get(0).contains("g='test', a='f'")); + assertTrue(result.getWarnings().get(0).contains("groupId='test', artifactId='f'")); - assertTrue(result.getWarnings().get(1).contains("g='test', a='g'")); + assertTrue(result.getWarnings().get(1).contains("groupId='test', artifactId='g'")); } @Test @@ -355,7 +361,7 @@ void testBadDependencyManagementScope() throws Exception { assertViolations(result, 0, 0, 1); - assertContains(result.getWarnings().get(0), "g='test', a='g'"); + assertContains(result.getWarnings().get(0), "groupId='test', artifactId='g'"); } @Test @@ -366,10 +372,10 @@ void testBadDependencyVersion() throws Exception { assertContains( result.getErrors().get(0), - "'dependencies.dependency.version' for g='test', a='b', type='jar' must be a valid version"); + "'dependencies.dependency.version' for groupId='test', artifactId='b', type='jar' must be a valid version"); assertContains( result.getErrors().get(1), - "'dependencies.dependency.version' for g='test', a='c', type='jar' must not contain any of these characters"); + "'dependencies.dependency.version' for groupId='test', artifactId='c', type='jar' must not contain any of these characters"); } @Test @@ -446,13 +452,13 @@ void testHardCodedSystemPath() throws Exception { assertContains( result.getWarnings().get(0), - "'dependencies.dependency.scope' for g='test', a='a', type='jar' declares usage of deprecated 'system' scope"); + "'dependencies.dependency.scope' for groupId='test', artifactId='a', type='jar' declares usage of deprecated 'system' scope"); assertContains( result.getWarnings().get(1), - "'dependencies.dependency.systemPath' for g='test', a='a', type='jar' should use a variable instead of a hard-coded path"); + "'dependencies.dependency.systemPath' for groupId='test', artifactId='a', type='jar' should use a variable instead of a hard-coded path"); assertContains( result.getWarnings().get(2), - "'dependencies.dependency.scope' for g='test', a='b', type='jar' declares usage of deprecated 'system' scope"); + "'dependencies.dependency.scope' for groupId='test', artifactId='b', type='jar' declares usage of deprecated 'system' scope"); } @Test @@ -506,7 +512,7 @@ void testMissingPluginDependencyGroupId() throws Exception { assertViolations(result, 0, 1, 0); - assertTrue(result.getErrors().get(0).contains("g=, a='a',")); + assertTrue(result.getErrors().get(0).contains("groupId=, artifactId='a',")); } @Test @@ -515,7 +521,7 @@ void testMissingPluginDependencyArtifactId() throws Exception { assertViolations(result, 0, 1, 0); - assertTrue(result.getErrors().get(0).contains("g='test', a=,")); + assertTrue(result.getErrors().get(0).contains("groupId='test', artifactId=,")); } @Test @@ -524,7 +530,7 @@ void testMissingPluginDependencyVersion() throws Exception { assertViolations(result, 0, 1, 0); - assertTrue(result.getErrors().get(0).contains("g='test', a='a',")); + assertTrue(result.getErrors().get(0).contains("groupId='test', artifactId='a',")); } @Test @@ -533,7 +539,7 @@ void testBadPluginDependencyVersion() throws Exception { assertViolations(result, 0, 1, 0); - assertTrue(result.getErrors().get(0).contains("g='test', a='b'")); + assertTrue(result.getErrors().get(0).contains("groupId='test', artifactId='b'")); } @Test @@ -582,10 +588,10 @@ void testBadDependencyExclusionId() throws Exception { assertContains( result.getWarnings().get(0), - "'dependencies.dependency.exclusions.exclusion.groupId' for g='gid', a='aid', type='jar'"); + "'dependencies.dependency.exclusions.exclusion.groupId' for groupId='gid', artifactId='aid', type='jar'"); assertContains( result.getWarnings().get(1), - "'dependencies.dependency.exclusions.exclusion.artifactId' for g='gid', a='aid', type='jar'"); + "'dependencies.dependency.exclusions.exclusion.artifactId' for groupId='gid', artifactId='aid', type='jar'"); // MNG-3832: Aether (part of M3+) supports wildcard expressions for exclusions @@ -602,10 +608,10 @@ void testMissingDependencyExclusionId() throws Exception { assertContains( result.getWarnings().get(0), - "'dependencies.dependency.exclusions.exclusion.groupId' for g='gid', a='aid', type='jar' is missing"); + "'dependencies.dependency.exclusions.exclusion.groupId' for groupId='gid', artifactId='aid', type='jar' is missing"); assertContains( result.getWarnings().get(1), - "'dependencies.dependency.exclusions.exclusion.artifactId' for g='gid', a='aid', type='jar' is missing"); + "'dependencies.dependency.exclusions.exclusion.artifactId' for groupId='gid', artifactId='aid', type='jar' is missing"); } @Test @@ -616,7 +622,7 @@ void testBadImportScopeType() throws Exception { assertContains( result.getWarnings().get(0), - "'dependencyManagement.dependencies.dependency.type' for g='test', a='a', type='jar' must be 'pom'"); + "'dependencyManagement.dependencies.dependency.type' for groupId='test', artifactId='a', type='jar' must be 'pom'"); } @Test @@ -627,7 +633,7 @@ void testBadImportScopeClassifier() throws Exception { assertContains( result.getErrors().get(0), - "'dependencyManagement.dependencies.dependency.classifier' for g='test', a='a', c='cls', type='pom' must be empty"); + "'dependencyManagement.dependencies.dependency.classifier' for groupId='test', artifactId='a', classifier='cls', type='pom' must be empty"); } @Test @@ -638,16 +644,16 @@ void testSystemPathRefersToProjectBasedir() throws Exception { assertContains( result.getWarnings().get(0), - "'dependencies.dependency.scope' for g='test', a='a', type='jar' declares usage of deprecated 'system' scope"); + "'dependencies.dependency.scope' for groupId='test', artifactId='a', type='jar' declares usage of deprecated 'system' scope"); assertContains( result.getWarnings().get(1), - "'dependencies.dependency.systemPath' for g='test', a='a', type='jar' should not point at files within the project directory"); + "'dependencies.dependency.systemPath' for groupId='test', artifactId='a', type='jar' should not point at files within the project directory"); assertContains( result.getWarnings().get(2), - "'dependencies.dependency.scope' for g='test', a='b', type='jar' declares usage of deprecated 'system' scope"); + "'dependencies.dependency.scope' for groupId='test', artifactId='b', type='jar' declares usage of deprecated 'system' scope"); assertContains( result.getWarnings().get(3), - "'dependencies.dependency.systemPath' for g='test', a='b', type='jar' should not point at files within the project directory"); + "'dependencies.dependency.systemPath' for groupId='test', artifactId='b', type='jar' should not point at files within the project directory"); } @Test @@ -713,10 +719,10 @@ void testDeprecatedDependencyMetaversionsLatestAndRelease() throws Exception { assertContains( result.getWarnings().get(0), - "'dependencies.dependency.version' for g='test', a='a', type='jar' is either LATEST or RELEASE (both of them are being deprecated)"); + "'dependencies.dependency.version' for groupId='test', artifactId='a', type='jar' is either LATEST or RELEASE (both of them are being deprecated)"); assertContains( result.getWarnings().get(1), - "'dependencies.dependency.version' for g='test', a='b', type='jar' is either LATEST or RELEASE (both of them are being deprecated)"); + "'dependencies.dependency.version' for groupId='test', artifactId='b', type='jar' is either LATEST or RELEASE (both of them are being deprecated)"); } @Test diff --git a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng3220ImportScopeTest.java b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng3220ImportScopeTest.java index 1f52acf42c52..7ab177117ab6 100644 --- a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng3220ImportScopeTest.java +++ b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng3220ImportScopeTest.java @@ -81,7 +81,7 @@ public void testitMNG3220b() throws Exception { boolean found = false; for (String line : lines) { if (line.contains("\'dependencies.dependency.version\' is missing for junit:junit") - || line.contains("\'dependencies.dependency.version\' for g='junit', a='junit', type='jar' is missing")) { + || line.contains("\'dependencies.dependency.version\' for groupId='junit', artifactId='junit', type='jar' is missing")) { found = true; break; } From 6215fd566e1e66e28ab2817c3534f3f2b4766f4a Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Mon, 14 Apr 2025 20:49:27 +0200 Subject: [PATCH 7/7] Reformat --- .../org/apache/maven/it/MavenITmng3220ImportScopeTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng3220ImportScopeTest.java b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng3220ImportScopeTest.java index 7ab177117ab6..f662ff577dcc 100644 --- a/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng3220ImportScopeTest.java +++ b/its/core-it-suite/src/test/java/org/apache/maven/it/MavenITmng3220ImportScopeTest.java @@ -81,7 +81,8 @@ public void testitMNG3220b() throws Exception { boolean found = false; for (String line : lines) { if (line.contains("\'dependencies.dependency.version\' is missing for junit:junit") - || line.contains("\'dependencies.dependency.version\' for groupId='junit', artifactId='junit', type='jar' is missing")) { + || line.contains( + "\'dependencies.dependency.version\' for groupId='junit', artifactId='junit', type='jar' is missing")) { found = true; break; }