Skip to content
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

docs: annotate all Option factory methods with their Nullability bounds #1775

Merged
merged 1 commit into from Nov 17, 2022

Conversation

BenWhitehead
Copy link
Collaborator

Update all option factory methods to explicitly state their Nullability bounds. All methods which take non-primitive parameters are @nonnull, and will be published in the javadocs.

Add new tests to explicitly verify null argument validation.

All factory methods now check their args for nullness, instead of depending on implicit checking.

Update all option factory methods to explicitly state their Nullability bounds. All methods which take non-primitive parameters are @nonnull, and will be published in the javadocs.

Add new tests to explicitly verify null argument validation.

All factory methods now check their args for nullness, instead of depending on implicit checking.
@BenWhitehead BenWhitehead requested a review from a team as a code owner November 16, 2022 23:27
@product-auto-label product-auto-label bot added size: l Pull request size is large. api: storage Issues related to the googleapis/java-storage API. labels Nov 16, 2022
@@ -142,7 +143,7 @@ private BlobTargetOption(ObjectTargetOpt opt) {

/** Returns an option for specifying blob's predefined ACL configuration. */
@TransportCompatibility({Transport.HTTP, Transport.GRPC})
public static BlobTargetOption predefinedAcl(Storage.PredefinedAcl acl) {
public static BlobTargetOption predefinedAcl(Storage.@NonNull PredefinedAcl acl) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can this not look like
public static BlobTargetOption predefinedAcl(@NonNull Storage.PredefinedAcl acl)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The @NonNull is applicable to a type, not the method parameter. So it needs to immediately precede the type, in this case PredefinedAcl which happens to be nested in Storage.

If the annotation is moved before Storage it will result in a compile error:

.../google-cloud-storage/src/main/java/com/google/cloud/storage/Bucket.java:146:66
java: scoping construct cannot be annotated with type-use annotation: @org.checkerframework.checker.nullness.qual.NonNull

@BenWhitehead BenWhitehead merged commit 3b8d137 into main Nov 17, 2022
@BenWhitehead BenWhitehead deleted the opts/filter-out-null-values branch November 17, 2022 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: storage Issues related to the googleapis/java-storage API. size: l Pull request size is large.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants