You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by cs...@apache.org on 2025/04/16 09:43:56 UTC
(maven) branch master updated: [MNG-8676] Improve model builder error messages (#2257)
This is an automated email from the ASF dual-hosted git repository.
cstamas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/maven.git
The following commit(s) were added to refs/heads/master by this push:
new 3ba948913b [MNG-8676] Improve model builder error messages (#2257)
3ba948913b is described below
commit 3ba948913b5db84d90e3647b77eadc4c91dee767
Author: Tamas Cservenak <ta...@cservenak.net>
AuthorDate: Wed Apr 16 11:43:51 2025 +0200
[MNG-8676] Improve model builder error messages (#2257)
Only the one that is easy to be mixed up with GAV: DepMgtKey is GAT and not GAV. The rest (XML, PK/GA, REPOID, DIR, GAV) ones are not quite mixable, and string "A:B:C" is usually always assumed is GAV.
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.
This PR now lessens the "A:B:C" string overload a bit, and also makes crystal clear that T is not V. The message is precise, but a bit longer. In turn, is not misleading at all now.
---
https://issues.apache.org/jira/browse/MNG-8676
---
.../apache/maven/project/ProjectBuilderTest.java | 5 +-
.../maven/impl/model/DefaultModelValidator.java | 22 ++++-
.../impl/model/DefaultModelValidatorTest.java | 96 ++++++++++++----------
.../maven/it/MavenITmng3220ImportScopeTest.java | 3 +-
4 files changed, 79 insertions(+), 47 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 9cdd456ad4..541ac8063c 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 is missing")));
+ contains(
+ projectBuildingResultWithProblemMessage(
+ "'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 b6c4586b11..126de225d4 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,29 @@ public static SourceHint xmlNodeInputLocation(XmlNode xmlNode) {
}
public static SourceHint gav(String gav) {
- return new SourceHint(gav, null); // "GAV"
+ return new SourceHint(gav, null); // GAV
}
public static SourceHint dependencyManagementKey(Dependency dependency) {
- return new SourceHint(dependency.getManagementKey(), null); // DMK
+ String hint;
+ if (dependency.getClassifier() == null
+ || dependency.getClassifier().trim().isEmpty()) {
+ hint = String.format(
+ "groupId=%s, artifactId=%s, type=%s",
+ nvl(dependency.getGroupId()), nvl(dependency.getArtifactId()), nvl(dependency.getType()));
+ } else {
+ hint = String.format(
+ "groupId=%s, artifactId=%s, classifier=%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 dcd6fa99cc..06947221f3 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 groupId:null: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 null:artifactId: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 groupId:artifactId:jar is missing"));
+ assertTrue(
+ result.getErrors()
+ .get(0)
+ .contains(
+ "'dependencies.dependency.version' for groupId='groupId', artifactId='artifactId', type='jar' is missing"));
}
@Test
@@ -229,9 +235,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='groupId', artifactId=, type='jar' is missing"));
}
@Test
@@ -240,9 +248,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 groupId=, artifactId='artifactId', type='jar' is missing"));
}
@Test
@@ -327,11 +337,11 @@ void testBadPluginDependencyScope() throws Exception {
assertViolations(result, 0, 3, 0);
- assertTrue(result.getErrors().get(0).contains("test:d"));
+ assertTrue(result.getErrors().get(0).contains("groupId='test', artifactId='d'"));
- assertTrue(result.getErrors().get(1).contains("test:e"));
+ assertTrue(result.getErrors().get(1).contains("groupId='test', artifactId='e'"));
- assertTrue(result.getErrors().get(2).contains("test:f"));
+ assertTrue(result.getErrors().get(2).contains("groupId='test', artifactId='f'"));
}
@Test
@@ -340,9 +350,9 @@ void testBadDependencyScope() throws Exception {
assertViolations(result, 0, 0, 2);
- assertTrue(result.getWarnings().get(0).contains("test:f"));
+ assertTrue(result.getWarnings().get(0).contains("groupId='test', artifactId='f'"));
- assertTrue(result.getWarnings().get(1).contains("test:g"));
+ assertTrue(result.getWarnings().get(1).contains("groupId='test', artifactId='g'"));
}
@Test
@@ -351,7 +361,7 @@ void testBadDependencyManagementScope() throws Exception {
assertViolations(result, 0, 0, 1);
- assertContains(result.getWarnings().get(0), "test:g");
+ assertContains(result.getWarnings().get(0), "groupId='test', artifactId='g'");
}
@Test
@@ -361,10 +371,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 groupId='test', artifactId='b', type='jar' 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 groupId='test', artifactId='c', type='jar' must not contain any of these characters");
}
@Test
@@ -441,13 +452,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 groupId='test', artifactId='a', type='jar' 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 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 test:b:jar declares usage of deprecated 'system' scope");
+ "'dependencies.dependency.scope' for groupId='test', artifactId='b', type='jar' declares usage of deprecated 'system' scope");
}
@Test
@@ -501,7 +512,7 @@ void testMissingPluginDependencyGroupId() throws Exception {
assertViolations(result, 0, 1, 0);
- assertTrue(result.getErrors().get(0).contains(":a:"));
+ assertTrue(result.getErrors().get(0).contains("groupId=, artifactId='a',"));
}
@Test
@@ -510,7 +521,7 @@ void testMissingPluginDependencyArtifactId() throws Exception {
assertViolations(result, 0, 1, 0);
- assertTrue(result.getErrors().get(0).contains("test:"));
+ assertTrue(result.getErrors().get(0).contains("groupId='test', artifactId=,"));
}
@Test
@@ -519,7 +530,7 @@ void testMissingPluginDependencyVersion() throws Exception {
assertViolations(result, 0, 1, 0);
- assertTrue(result.getErrors().get(0).contains("test:a"));
+ assertTrue(result.getErrors().get(0).contains("groupId='test', artifactId='a',"));
}
@Test
@@ -528,7 +539,7 @@ void testBadPluginDependencyVersion() throws Exception {
assertViolations(result, 0, 1, 0);
- assertTrue(result.getErrors().get(0).contains("test:b"));
+ assertTrue(result.getErrors().get(0).contains("groupId='test', artifactId='b'"));
}
@Test
@@ -576,10 +587,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 groupId='gid', artifactId='aid', type='jar'");
assertContains(
result.getWarnings().get(1),
- "'dependencies.dependency.exclusions.exclusion.artifactId' for gid:aid:jar");
+ "'dependencies.dependency.exclusions.exclusion.artifactId' for groupId='gid', artifactId='aid', type='jar'");
// MNG-3832: Aether (part of M3+) supports wildcard expressions for exclusions
@@ -596,10 +608,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 groupId='gid', artifactId='aid', type='jar' is missing");
assertContains(
result.getWarnings().get(1),
- "'dependencies.dependency.exclusions.exclusion.artifactId' for gid:aid:jar is missing");
+ "'dependencies.dependency.exclusions.exclusion.artifactId' for groupId='gid', artifactId='aid', type='jar' is missing");
}
@Test
@@ -610,7 +622,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 groupId='test', artifactId='a', type='jar' must be 'pom'");
}
@Test
@@ -621,7 +633,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 groupId='test', artifactId='a', classifier='cls', type='pom' must be empty");
}
@Test
@@ -632,16 +644,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 groupId='test', artifactId='a', type='jar' 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 groupId='test', artifactId='a', type='jar' 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 groupId='test', artifactId='b', type='jar' 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 groupId='test', artifactId='b', type='jar' should not point at files within the project directory");
}
@Test
@@ -707,10 +719,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 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 test:b: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 77c2b89a0d..f662ff577d 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 junit:junit:jar is missing")) {
+ || line.contains(
+ "\'dependencies.dependency.version\' for groupId='junit', artifactId='junit', type='jar' is missing")) {
found = true;
break;
}