feat: add NullMarked annotation to classes#13517
Conversation
nnicolee
added
the
do not merge
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request integrates JSpecify nullness annotations (NullMarked and Nullable) across several GAPIC class composers to support better null-safety. The feedback highlights a few areas for improvement: an unused constant JSPECIFY_ANNOTATIONS_PACKAGE was introduced in CommonStrings.java, fully qualified class names were used in AbstractServiceStubSettingsClassComposer.java instead of imports (which is inconsistent with other files), and the import ordering in ResourceNameHelperClassComposer.java violates the Google Java Style Guide.
| public static final String JSPECIFY_ANNOTATIONS_PACKAGE = "org.jspecify.annotations"; | ||
| } |
There was a problem hiding this comment.
The constant JSPECIFY_ANNOTATIONS_PACKAGE is defined but not used. If this is a public API constant that cannot be removed for backward compatibility, it should be marked as obsolete (e.g., with @ObsoleteApi) instead of being removed. Otherwise, if it is safe to remove, please delete it to avoid dead code.
References
- Public API constants that cannot be removed for backward compatibility should be marked as obsolete (e.g., with
@ObsoleteApi) instead of being removed.
| org.jspecify.annotations.NullMarked.class, | ||
| org.jspecify.annotations.Nullable.class); |
There was a problem hiding this comment.
For consistency with the other composer classes in this pull request, consider importing org.jspecify.annotations.NullMarked and org.jspecify.annotations.Nullable at the top of the file and using their simple names here instead of fully qualified names.
| import org.jspecify.annotations.NullMarked; | ||
| import org.jspecify.annotations.Nullable; | ||
| import com.google.api.generator.engine.ast.AnnotationNode; |
There was a problem hiding this comment.
According to the Google Java Style Guide (Section 4.5.2), imports should be ordered alphabetically. The org.jspecify imports should be placed after the com.google imports.
References
- Google Java Style Guide Section 4.5.2: Ordering of imports. Imports are ordered alphabetically. (link)
nnicolee
force-pushed
the
feat/jspecify-nullmarked
branch
from
ef5eb49 to
dae9b95
Compare
nnicolee
force-pushed
the
feat/jspecify-nullmarked
branch
from
dae9b95 to
b1aab81
Compare
|
|
1 participant
This PR updates the gapic-generator-java to add the JSpecify @NullMarked annotation to all generated class declarations. Adding @NullMarked at the class level defines the class scope as null-safe, establishing that all unannotated types in method signatures (parameters and return types) are nonnull able by default. This is the first phase in onboarding the generated client libraries to compile-time safety validation, see design doc for more details: go/sdk:java-jspecify-null-annotations-gapic
Classes Annotated:
Implementation Changes:
Verification/Testing:
Next Steps: