Add JSpecify nullability annotations #175
Merged
Conversation
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
This change adds nullability annotations ([JSpecify](https://jspecify.dev)) to `packageurl-java` and modifies somes behavior. Specifically: - It marks with `@Nullable` all public methods that can return `null`. - It marks with `@Nullable` all method parameters that can have a `null` value **without** throwing any exception. - It modifies the behavior of the remaining parameters: passing `null` to those parameters will result in an NPE, instead of `MalformedPackageURLException`. This is at least a behavior change (minor version bump), but it _might_ be a breaking change.
Collaborator
|
@ppkarwasz this is likely one of the few types of dependencies I'm okay with for |
ppkarwasz
commented
Mar 12, 2025
Comment on lines
288
to
+291
| * @return all qualifiers set in this builder, or an empty map if none are set | ||
| */ | ||
| public Map<String, String> getQualifiers() { | ||
| if (qualifiers == null) { | ||
| return null; | ||
| } | ||
| return Collections.unmodifiableMap(qualifiers); | ||
| return qualifiers != null ? Collections.unmodifiableMap(qualifiers) : Collections.emptyMap(); |
Contributor
Author
There was a problem hiding this comment.
The current Javadoc suggests that this method returns an empty map instead of null, so I changed the implementation…
Comment on lines
+245
to
+250
| * | ||
| * @return all the qualifiers, or an empty map if none are set | ||
| * @since 1.0.0 | ||
| */ | ||
| public Map<String, String> getQualifiers() { | ||
| return (qualifiers != null) ? Collections.unmodifiableMap(qualifiers) : null; | ||
| return qualifiers != null ? Collections.unmodifiableMap(qualifiers) : Collections.emptyMap(); |
Contributor
Author
There was a problem hiding this comment.
Since one of the tests checks if all PackageURL getters return the same as the PackageURLBuilder getters, I introduced a behavioral change here too.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This change adds nullability annotations (JSpecify) to
packageurl-javaand modifies somes behavior. Specifically:@Nullableall public methods that can returnnull.@Nullableall method parameters that can safely be passednullwithout throwing any exception.nullto those parameters will result in an NPE, instead ofMalformedPackageURLException. This is at least a behavioral change (minor version bump), but it might be considered a breaking change.