jspecify
Proposal: allow for an optional "reason" parameter for `@NullUnmarked`
We are working on @NullUnmarked support in NullAway, and @lazaroclapp and I think it could be useful for the @NullUnmarked annotation to have an optional String reason parameter, to give extra context around the reason for using the annotation. The reason could be, e.g., a description of what code behavior cannot yet be verified by a checker, or the URL of a task / issue that gives further details or is tracking removal of the annotation (as part of a gradual migration). We have seen this type of functionality to be useful in the past. E.g., NullAway supports auto-fixing warnings by inserting a @SuppressWarnings annotation, with an additional comment string to go along with the suppression:
https://github.com/uber/NullAway/blob/cba66678d2c2a9b677ab943c755bf1b75f3eda7f/nullaway/src/main/java/com/uber/nullaway/Config.java#L241-L247
We could use comment strings with @NullUnmarked also. But, having an annotation parameter will make things easier to parse, and will ensure that when a @NullUnmarked annotation is removed, out-of-date explanation comments aren't left behind.
We have had a similar minor annoyance when we try to remove a @SuppressWarnings("nullness") and find that we've left an associated comment behind... :) Now that I think about it, maybe we should get in the habit of...
@SuppressWarnings(
// comment goes here
"nullness")
...which would lend itself naturally to removing the comment along with the suppression.
Anyway, apparently we talked about this in a meeting at some point, and the result was https://github.com/jspecify/jspecify/issues/149. That issue mentions some downsides that I'd forgotten. Have a look, and see what you think.
Fine to leave the old issue closed and this one open.
I'll give my current "no" arguments.
I'd advocate for a general principle that only specified, actionable information should be conveyed by our annotations. If we don't define a meaning for it, that's plausible for tools to make use of in some way, then I think we'd rather tools didn't depend on that info at all. While a tool could always parse a comment, sure, it just doesn't seem appropriate to make the data so trivially available to it as this, all tied up with a bow.
A second, probably much smaller good reason for putting info into the annotation might be if that data is relatively structured; an annotation can impose that structure, while a "we parse your comment" policy can only ask you to please obey.
EDIT: a third reason, much smaller yet, might be if you actually want the information to show up in javadoc. This case seems like much more of an implementation comment.
WDYT?
About the "pro" arguments, of course comments not getting removed with the code they belong with is a much more general problem that calls for more general strategies.
@cpovirk @kevinb9n thanks for the discussion and for pointing to #149 (I tried to find one with some searches but failed)! I think I'm convinced that not having a baked-in annotation parameter may be best, to avoid amorphous and ambiguous elements in the spec, and to avoid the Javadoc rendering of what will usually be implementation details. I do think this type of feature may be commonly used by tools, but there are alternatives like code comments that don't work not as well but are adequate.
I'll let @lazaroclapp chime in if he has any further thoughts.
To add, I think there's no pressure on "yes": this would still be a compatible change after 1.0 (I assume!), since the attribute would certainly have a default value of "".
(EDIT: related issue: #6)