Require JSpecify annotations on new Public API and Experimental API classes#3892
Require JSpecify annotations on new Public API and Experimental API classes#3892
Conversation
Test Results 324 files 324 suites 3m 26s ⏱️ Results for commit c36c533. ♻️ This comment has been updated with latest results. |
| "graphql.ErrorClassification", | ||
| "graphql.ErrorType", | ||
| "graphql.ExceptionWhileDataFetching", | ||
| "graphql.ExecutionInput", |
There was a problem hiding this comment.
Can remove ExecutionInput
|
Todo - some classes have since been updated to include nullability annotations like GraphQL, Execution Input, remove agent To do - add a rule to double check when a class does have nullability annotations added, we want to progressively make this list smaller |
| class JSpecifyAnnotationsCheck extends Specification { | ||
|
|
||
| private static final Set<String> JSPECIFY_EXEMPTION_LIST = [ | ||
| "graphql.AssertException", |
There was a problem hiding this comment.
Now updated to remove a bunch of classes we've since annotated
| } | ||
| } | ||
|
|
||
| def "exempted classes should not be annotated with @NullMarked or @NullUnmarked"() { |
There was a problem hiding this comment.
Now also allows @NullUnmarked, which we usually put on builders
andimarek
left a comment
andimarek
left a comment
There was a problem hiding this comment.
Approve, but this needs to be updated after 4075 and 3899 are merged
2 participants
Resolves #3877
This PR adds an ArchUnit test to require new Public API and Experimental API classes have the
@NullMarkedannotation on the class. See the documentation: https://jspecify.dev/docs/user-guide/#nullmarkedI put a PR up so it's easier to have a discussion, I'm open to ideas.
I originally tried to allow list all classes, but because we're starting off from almost zero, we have another 1,200 classes to go. That's a very, very long allow list! I thought instead we can start with public & experimental API classes, which is a mere ~350 classes. This would add the most value for users of the library.
I had thought about using Git to collect a list of new classes, but I don't want to spin up a new process every time we do a test run.
Sample test output
https://github.com/graphql-java/graphql-java/actions/runs/14279382170/job/40027003549?pr=3892
Update, now with a new test to make sure we keep the exemption list up to date!