-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Improve error message for unresolved expressions #11615
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -174,7 +174,8 @@ public DependencyResolverResult collect(@Nonnull DependencyResolverRequest reque | |||||
| session.getNode(result.getRoot(), request.getVerbose()), | ||||||
| 0); | ||||||
| } catch (DependencyCollectionException e) { | ||||||
| throw new DependencyResolverException("Unable to collect dependencies", e); | ||||||
| String enhancedMessage = enhanceCollectionError(e, collectRequest); | ||||||
| throw new DependencyResolverException(enhancedMessage, e); | ||||||
| } | ||||||
| } finally { | ||||||
| RequestTraceHelper.exit(trace); | ||||||
|
|
@@ -274,4 +275,61 @@ public DependencyResolverResult resolve(DependencyResolverRequest request) | |||||
| private static DependencyResolverException cannotReadModuleInfo(final Path path, final IOException cause) { | ||||||
| return new DependencyResolverException("Cannot read module information of " + path, cause); | ||||||
| } | ||||||
|
|
||||||
| private static boolean containsUnresolvedExpression(String value) { | ||||||
| return value != null && value.contains("${") && value.contains("}"); | ||||||
| } | ||||||
|
|
||||||
| private static String enhanceCollectionError(DependencyCollectionException e, CollectRequest request) { | ||||||
| if (e.getMessage() != null && e.getMessage().contains("Invalid Collect Request")) { | ||||||
| StringBuilder enhanced = new StringBuilder(); | ||||||
| enhanced.append("Failed to collect dependencies"); | ||||||
|
|
||||||
| org.eclipse.aether.graph.Dependency root = request.getRoot(); | ||||||
| if (root != null && root.getArtifact() != null) { | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
could be extracted as dedicated branch/concern to provide cohesion over coupling.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ternary operators are harder to read
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||||
| org.eclipse.aether.artifact.Artifact artifact = root.getArtifact(); | ||||||
| String groupId = artifact.getGroupId(); | ||||||
| String artifactId = artifact.getArtifactId(); | ||||||
| String version = artifact.getVersion(); | ||||||
|
|
||||||
| if (containsUnresolvedExpression(groupId) | ||||||
| || containsUnresolvedExpression(artifactId) | ||||||
| || containsUnresolvedExpression(version)) { | ||||||
| enhanced.append(" due to unresolved expression(s) in dependency: ") | ||||||
| .append(groupId) | ||||||
| .append(":") | ||||||
| .append(artifactId) | ||||||
| .append(":") | ||||||
| .append(version) | ||||||
| .append(".\n") | ||||||
| .append("Please check that all properties are defined in your POM or settings.xml."); | ||||||
| return enhanced.toString(); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| for (org.eclipse.aether.graph.Dependency dep : request.getDependencies()) { | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could be extracted as dedicated branch/concern to provide cohesion over coupling.
Suggested change
|
||||||
| if (dep != null && dep.getArtifact() != null) { | ||||||
| org.eclipse.aether.artifact.Artifact artifact = dep.getArtifact(); | ||||||
| String groupId = artifact.getGroupId(); | ||||||
| String artifactId = artifact.getArtifactId(); | ||||||
| String version = artifact.getVersion(); | ||||||
|
|
||||||
| if (containsUnresolvedExpression(groupId) | ||||||
| || containsUnresolvedExpression(artifactId) | ||||||
| || containsUnresolvedExpression(version)) { | ||||||
| enhanced.append(" due to unresolved expression(s) in dependency: ") | ||||||
| .append(groupId) | ||||||
| .append(":") | ||||||
| .append(artifactId) | ||||||
| .append(":") | ||||||
| .append(version) | ||||||
| .append(".\n") | ||||||
| .append("Please check that all properties are defined in your POM or settings.xml."); | ||||||
| return enhanced.toString(); | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
| return e.getMessage(); | ||||||
| } | ||||||
| } | ||||||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is fine as is