WIP Even more JSpecify annotations #4098
Conversation
Test Results 326 files 326 suites 3m 31s ⏱️ Results for commit afdd408. |
| if (enumValueDefinition == null) { | ||
| assertShouldNeverHappen(i18nMsg(locale, "Enum.badName", name, input.toString())); | ||
| }; | ||
| } |
There was a problem hiding this comment.
Take that semi colon!!! Nah I just did what the IDE told me to do, because I was looking for NullAway errors
| public GraphQLCodeRegistry getCodeRegistry() { | ||
| public @Nullable GraphQLCodeRegistry getCodeRegistry() { | ||
| return codeRegistry; | ||
| } |
There was a problem hiding this comment.
I thought it was strange too, if I recall correctly it's because we have an unusual constructor where it's allowed to be null. I was going to ask if there's a historical reason for that case. I'll check when I'm at a computer next!
There was a problem hiding this comment.
Check out the first constructor in this class where the code registry is set to null
this.codeRegistry = null;
In the same constructor there is also an assertion that it's not null in the builder, which seems unusual
assertNotNull(builder.codeRegistry, () -> "codeRegistry can't be null");
I guess the first constructor is a "partial" one, but because the object is created, we have to annotate the code registry as nullable
There was a problem hiding this comment.
Perhaps that first constructor was acting more like a builder. We could also change the constructor to avoid this nullable annotation
|
All the changes here are contained in #4105, continued on that PR |
Trying out IntelliJ's Infer Nullity tool to speed things up
I gave https://github.com/ucr-riple/NullAwayAnnotator a try but I couldn't get it to work. I'm sure I am missing something, but anyway IntelliJ's tool works pretty well and doesn't require extra installation