-
Notifications
You must be signed in to change notification settings - Fork 3
Add some missing @Nullable annotations.
#129
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -991,7 +991,7 @@ void initializeFocusTraversalKeys() { | |
| * Constructs a name for this component. Called by {@code getName} | ||
| * when the name is {@code null}. | ||
| */ | ||
| @Nullable String constructComponentName() { | ||
| String constructComponentName() { | ||
| return null; // For strict compliance with prior platform versions, a Component | ||
| // that doesn't set its name should return null from | ||
| // getName() | ||
|
|
@@ -1003,7 +1003,7 @@ void initializeFocusTraversalKeys() { | |
| * @see #setName | ||
| * @since 1.1 | ||
| */ | ||
| public String getName() { | ||
| public @Nullable String getName() { | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I could see also adding
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would seem consistent to make the
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, thanks, I hadn't paid attention to the |
||
| if (name == null && !nameExplicitlySet) { | ||
| synchronized(getObjectLock()) { | ||
| if (name == null && !nameExplicitlySet) | ||
|
|
@@ -1020,7 +1020,7 @@ public String getName() { | |
| * @see #getName | ||
| * @since 1.1 | ||
| */ | ||
| public void setName(String name) { | ||
| public void setName(@Nullable String name) { | ||
| String oldName; | ||
| synchronized(getObjectLock()) { | ||
| oldName = this.name; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really ugly, as you note. It breaks behavioral subtyping, contradicting the contract from java.lang.Object#toString.
jspecify/jspecify#111 would say that this annotation should cause a distinct warning.
But as this is what the code is clearly doing, the annotation matches the code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep :(
One note for anyone in the future who might be interested: It's not really fair of me to call this annotation "missing" in the way that the others are: This class is not
@NullMarked, so JSpecify would say that the return type is unspecified. (Of course, I could hardly criticize a tool for still treating it as non-null, given everything you've said abouttoString().) Thus, the@Nullableannotation here is "missing" only in the sense that all unannotated code is "missing" tons of annotations. That is in contrast to the other edits I'm making, which are in null-marked classes, since those are changing types from non-null to nullable.