Remove unreachable null check from BuildClasspathMojo - #1666
Open
wilx wants to merge 1 commit into
Open
Conversation
wilx
marked this pull request as ready for review
August 1, 2026 16:54
elharo
requested changes
Aug 2, 2026
| @@ -198,7 +199,10 @@ protected void doExecute() throws MojoExecutionException { | |||
|
|
|||
| Set<Artifact> artifacts = getResolvedDependencies(true); | |||
Contributor
There was a problem hiding this comment.
can this method return null? what's it's contract? and if it can return null, should it be changed to return an empty set instead?
Author
There was a problem hiding this comment.
Not in the current Maven, I think. But I did not study all the versions.
Contributor
There was a problem hiding this comment.
Is the method marked nullable or non-nullable?
Contributor
There was a problem hiding this comment.
I don't see an issue here. If the method doesn;t return null, it doesn't return null.
Author
There was a problem hiding this comment.
I have removed the check and reworded the commit.
Rely on getResolvedDependencies(boolean) to return a non-null set while retaining the existing empty-set handling. Fixes apache#1648.
wilx
force-pushed
the
issue-1648-build-classpath-null
branch
from
August 2, 2026 12:38
409ed09 to
a94714a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
getResolvedDependencies(true)supplies a non-nullSet; projects with no resolved dependencies receive an empty set. Remove the misleadingartifacts == nullcondition fromBuildClasspathMojowhile retaining the existing empty-set logging and empty classpath output.This aligns the Mojo with the dependency resolver's effective contract. Because the null case cannot be produced through the supported Maven dependency-resolution path, no defensive conversion or null-specific test is needed.
Fixes #1648.
Following this checklist to help us incorporate your
contribution quickly and easily:
Note that commits might be squashed by a maintainer on merge.
This may not always be possible but is a best-practice.
This is not a behavioral change: the null branch is unreachable, and the existing empty-set behavior is unchanged.
mvn verifyto make sure basic checks pass.A more thorough check will be performed on your pull request automatically.
mvn -Prun-its verify).If your pull request is about ~20 lines of code you don't need to sign an
Individual Contributor License Agreement if you are unsure
please ask on the developers list.
To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.