arturobernalg commented on code in PR #2209:
URL: https://github.com/apache/maven-resolver/pull/2209#discussion_r4313458321
##########
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultArtifactResolver.java:
##########
@@ -307,6 +307,19 @@ private List<ArtifactResult> resolve(
artifact = artifact.setVersion(versionResult.getVersion());
+ if (artifact.getVersion() == null ||
artifact.getVersion().isEmpty()) {
Review Comment:
Good catch on the whitespace-only case, added `trim()` for that.
I kept the `== null` guard, though: `Artifact` is a public interface
third-party code can implement directly without going through
`AbstractArtifact`/`DefaultArtifact`, so `getVersion()` returning `null` isn't
impossible from the type system's point of view, even though it can't happen
with this module's own classes. Removing the check would trade a clear
`ArtifactNotFoundException` for an uncaught `NullPointerException` in that
case, which is a regression for the exact problem this PR is fixing. Applied as
`artifact.getVersion() == null || artifact.getVersion().trim().isEmpty()`, with
a new test for the whitespace-only case (fails without the `trim()`, passes
with it).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
