Skip to content
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

Existence of @NonNull #230

Closed
kevinb9n opened this issue Jan 31, 2022 · 13 comments
Closed

Existence of @NonNull #230

kevinb9n opened this issue Jan 31, 2022 · 13 comments
Labels
design An issue that is resolved by making a decision, about whether and how something should work. nullness For issues specific to nullness analysis.
Milestone

Comments

@kevinb9n
Copy link
Collaborator

kevinb9n commented Jan 31, 2022

This issue

This issue is about the yes/no decision to add a @NonNull type-use annotation to the artifact.

How these issues are organized

Significant use cases whose expected solution requires this annotation are filed separately:

  • Marked as blocked by this issue, and listed here.
  • A "yes" here should be motivated only by "yes" to one or more of those.
  • It then also would reduce the cost of "yes" decisions to more of those.
  • Discussion of disadvantages stemming from the annotation's mere existence belongs here.
  • We can list other minor advantages here that don't seem to constitute proper use cases.

Compatibility note

  • We think it is possible to postpone this to post-1.0 but still add it compatibly later. But if we're missing something -- like if adding it forces small shifts in other decisions we'd committed -- it could be necessary to make a final yes/no decision now.
  • (Second-order compatibility.) If jspecify 1.0 lacks this annotation, we also think that a released library that adopts it would probably not be put in any awkward position by its later addition. That is, we don't see a reason they would feel pressure to make incompatible changes to their own API's annotations as a result.

Use cases

Known use cases for which at least one plausible solution would require this annotation:

Other small advantages

  • User expectation -- particularly as they will type @NonNull and their IDE may grab one from wherever (still happens with other spellings anyway though)

Disadvantages

  • If some of the use cases motivate this, it may drag us into having to support the rest of them too, due to user expectations.
  • Its mere existence gives users the impression of much more central importance than it really has. (We get some requests for @NonNull citing use cases that don't actually need it.)
  • Its mere existence somewhat undercuts the clean conceptual model we want most to promote: String is intrinsically non-nullable.
  • We have to deal with @Nullable @NonNull (this is extremely minor)

Design

Naming it is #44, currently closed favoring @NonNull

History

#4

@kevinb9n
Copy link
Collaborator Author

kevinb9n commented Feb 1, 2022

The disadvantages so far look kinda slight.

Disadvantages

  • If some of the use cases motivate this, it may drag us into having to support the rest of them too, due to user expectations.

Honestly those all look pretty good to do, tho. If not the "non-null projection" one, that's still okay: to decide against it means we'd have figured that the awkwardness of having to deal with meaningless @NonNull T usages is worth suffering.

  • Its mere existence gives users the impression of much more central importance than it really has. (We get some requests for @NonNull citing use cases that don't actually need it.)
  • Its mere existence somewhat undercuts the clean conceptual model we want most to promote: String is intrinsically non-nullable.

So we just have to deal with that in documentation. People who didn't encounter that information will think and do wrong things, but probably not too harmfully. But I can see alluding to its "notallthatness" in its javadoc summary sentence. Basically this problem seems manageable to me right now.

  • We have to deal with @Nullable @NonNull (this is extremely minor)

(that is extremely minor)

@cpovirk
Copy link
Collaborator

cpovirk commented Feb 2, 2022

I suspect that Kotlin is more or less going to "force" us to add this eventually, as kevinb9n says above.

I still think that it would be good for me to lay out in more detail specifically the ways I've seen people confused by @NonNull inside Google, so I can do that at some point, especially if this decision appears to be need to be made imminently at some point.

(It sounds like we now have an offer from Artem to share some similar experiences. Yay!)

One thing that I'd like to someday consider is whether we could pick a different name so as to better discourage people from using it unless they really need it. But it may be hard to find a good name. The name is already filed as a separate issue, but I want to flag it here as something to consider as a way to fight the misuse that I worry about.

@kevinb9n
Copy link
Collaborator Author

kevinb9n commented Feb 3, 2022

@artempyanykh If you could share experiences about how this annotation has distracted/confused your users.

@artempyanykh
Copy link
Collaborator

Sure, a bit about our experience.

  1. When it comes to the tooling support, we do support @NotNull and probably will support it for years to come simply because it's out there (incl. android SDK; incl. variations like @RecentlyNotNull) and we need to work with the existing code.
  2. When it comes to usage of @NotNull internally we actually have a linter that prohibits developers from using the annotation. There are several reasons for this:
    a. People pay much more attention to @NotNull than they should. They start using it when it's not needed, which is distracting and breaks the uniformity of the codebase.
    b. This problem is exacerbated by the influx of new devs. With no enforcement things would get hairy very quickly.
    c. We have found that people are usually OK to migrate to @NullMarked on a class-by-class basis. Very rarely we've seen devs annotating a single type.

However, I wouldn't go as far to say that @NotNull is not needed. Perhaps, as we get more support for nullness-aware generics, we'll see more use-cases where people would use it. If so, the linter itself may be adjusted to ban all usages, except those when it's used for not-null projections.

@kevinb9n
Copy link
Collaborator Author

kevinb9n commented Feb 3, 2022

Note: I've edited the top comment for clarity considerably; the changes don't seem to me to render any following comments confusing, so I haven't switched to "keep both original and updated text separated by a line" mode. You can of course view all the past versions if necessary.

How best to use editing here is an in-progress experiment.

@kevinb9n
Copy link
Collaborator Author

Just a note that if we create an 0.3 milestone I would expect to keep this one 1.0.

@cpovirk
Copy link
Collaborator

cpovirk commented Mar 3, 2022

I'm increasingly convinced that Kotlin's <T & Any> forces us to include @NonNull. If anyone other than me has hoped that we could keep it out, do speak up: I can share how I've become at least somewhat less worried about it. Or I may hear something from you that convinces me that I should resume pushing back :) Or I can sit things out and let you take up the baton.

Given that, I think I would actually argue in favor of putting @NonNull out there sooner rather than later: That gives tools more time to support it. And perhaps more importantly, it give us more opportunity to see how users react to it, increasing the chances that our big 1.0 release can present the role of @NonNull to users in a way that sticks with them.

@kevinb9n
Copy link
Collaborator Author

kevinb9n commented Mar 3, 2022 via email

@cpovirk
Copy link
Collaborator

cpovirk commented Mar 3, 2022

I am pretty much always in favor of "kevinb writes user documentation" :)

(and the 4 lines of boilerplate code that goes along with the docs :))

@kevinb9n
Copy link
Collaborator Author

kevinb9n commented Apr 2, 2022

I think we don't have a significant argument against this, and we need to at least put it in 0.3 to see how it goes.

@kevinb9n kevinb9n changed the title Existence of @NonNull Existence of @NonNull [working decision: yes] May 16, 2022
@kevinb9n
Copy link
Collaborator Author

Working decision: yes, we will have @NonNull in 0.3 and it will work in the 2-3 ways expected.

@kevinb9n kevinb9n closed this as completed Jun 8, 2022
netdpb added a commit that referenced this issue Aug 8, 2022
* Add `METHOD` and `CONSTRUCTOR` targets to `@NullMarked` (#43).
* Update basic Javadoc for `@NullMarked` and `@Nullable`.
* Add `@NullUnmarked` (#127).
* Add `@NonNull` (#230).
* Add `@Implies` (#35) (and its containing annotation `@Implies.Container`).
@kevinb9n kevinb9n added design An issue that is resolved by making a decision, about whether and how something should work. nullness For issues specific to nullness analysis. labels Nov 30, 2022
@kevinb9n kevinb9n modified the milestones: 1.0, 0.3 Dec 2, 2022
@netdpb
Copy link
Collaborator

netdpb commented Apr 9, 2024

Current decision: Yes.

Argument for changing: None.

Timing: This must be decided before version 1.0 of the jar.

Proposal for 1.0: Finalize the current decision. If you agree, please add a thumbs-up emoji (👍) to this comment. If you disagree, please add a thumbs-down emoji (👎) to this comment and briefly explain your disagreement. Please only add a thumbs-down if you feel you can make a strong case why this decision will be materially worse for users or tool providers than an alternative.

Results: This proposal received six 👍s and no other votes. It is finalized. I'll edit the title to reflect that.

@netdpb netdpb added the needs-decision-before-jar-1.0 Issues needing a finalized decision for the 1.0 jar release label Apr 9, 2024
@netdpb
Copy link
Collaborator

netdpb commented May 17, 2024

Decision: Yes, @NonNull exists.

@netdpb netdpb changed the title Existence of @NonNull [working decision: yes] Existence of @NonNull May 17, 2024
@netdpb netdpb removed the needs-decision-before-jar-1.0 Issues needing a finalized decision for the 1.0 jar release label May 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design An issue that is resolved by making a decision, about whether and how something should work. nullness For issues specific to nullness analysis.
Projects
None yet
Development

No branches or pull requests

4 participants