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;
}
