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

Name(s) of the annotation(s) expressing "in the nullness club" or "not" #87

Closed
kevinb9n opened this issue Dec 11, 2019 · 36 comments
Closed
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.

Comments

@kevinb9n
Copy link
Collaborator

kevinb9n commented Dec 11, 2019

We used to talk about three defaulting annotations to parallel the three kinds of (non-parametric) nullness: @DefaultNonNull, @DefaultNullable, and @DefaultNullnessUnspecified.

Currently we want to drop @DefaultNullable [#41], and I believe it's a very good idea to at least limit the extra-file context readers need to just one 'bit' of knowledge.

We could keep the other two annotations as the way to control that bit, but here's another possibility: a single annotation like @NullnessAnnotated that means "these files are opted in to our nice beautiful new world... where everything in APIs is presumed non-nullable except unbounded wildcards and type variable usages", or something like that.

If something at a smaller scope needs to negate that, that can be @NullnessAnnotated(false).

@kevinb9n kevinb9n added the nullness For issues specific to nullness analysis. label Dec 12, 2019
@wmdietlGC
Copy link
Collaborator

Regarding naming of @DefaultNullnessUnspecified: I sometimes trip over whether this is supposed to mean the default is @NullnessUnspecified or the default nullness is unspecified.
Using @DefaultIsNonNull and @DefaultIsNullnessUnspecified would make that clearer, although it's more verbose.

I do like the single "in the club" annotation.
In the CF we use @AnnotatedFor for this purpose.
The logic we would want for CAA is like always using -AuseDefaultsForUncheckedCode=source and requiring an explicit @AnnotatedFor("nullness") to opt in a file. Using @AnnotatedFor("") could opt smaller scopes out again.

Maybe this is a revival of #5, this time for a general "in the club" annotation that takes a string array as value.

@kevinb9n
Copy link
Collaborator Author

kevinb9n commented Jan 9, 2020

(We can reopen #5 if we want to revive that possibility, but for purposes of this issue we're only talking about a nullness-specific annotation.)

@kevinb9n kevinb9n changed the title One single defaulting annotation, expressing a binary choice of in-the-club or not One single defaulting annotation, expressing a binary choice of in-the-nullness-club or not Jan 9, 2020
@cpovirk
Copy link
Collaborator

cpovirk commented Jan 13, 2020

which mostly means everything is non-null by default although maybe with some exceptions like [#83] / [#12]

We've said this before, but, for the purpose of having this all in one place: Another exception is type-variable usages: T get(int index) doesn't mean @NonNull T get(int index). (Probably most people will expect this, anyway, so it may be too natural to even call an "exception," except to the very mechanical process of pasting @NonNull onto every type in the file.)

Interestingly, in the context of @DefaultNullnessUnspecified, T get(int index) very well could mean @NullnessUnspecified T get(int index). I think this only strengthens the case for naming the defaulting annotation in a way that suggests "in the club" rather than "the default is x."

(And yes, I still cling to the hope that, if we keep @NonNull, I can convince us to name it something weirder, like "@DefaultNullness" -- and it would be weird for the "in the club" annotation to be named "@DefaultDefaultNullness" :))

@kevinb9n
Copy link
Collaborator Author

kevinb9n commented Jan 15, 2020

@NullnessAnnotated
@NullAnnotated
@NullSpecified
@SpecifiedNullness
@AnnotatedForNullness
@ApplyNullnessDefaults

(Imagine each of these followed by (false), as well.)

@cpovirk
Copy link
Collaborator

cpovirk commented Jan 15, 2020

Even though a general-purpose defaulting annotation is out of scope for now, do we want to pick a name with the goal of using "similar" names for other future features? (I know we have a doc with some possible annotation names for future features, but I think it's currently Google-internal.)

(One interesting question we'd raised is whether some annotations, like @MustUseResult (aka @CheckReturnValue) would be their own default annotations. This seems to work OK with @CheckReturnValue at the moment. I wonder how many other features besides nullness will even need defaulting annotations -- and, if they do, how many will be their own defaulting annotations?)

(Another name I'd tossed around yesterday is @ApplyNullnessDefaults.)

@kevinb9n
Copy link
Collaborator Author

I looked through all our candidate annotations and the only ones I found were @MustUseResult, @MustClose, and @ExportParameterNames. They're all boolean attributes that methods/constructors do or don't have, and therefore they seem to work naturally at any method-or-wider scope to mean "every method under this scope". Nullness is a little more complex than that.

@kevin1e100
Copy link
Collaborator

@NullSpecified reads like the antonym of @NullnessUnspecified, but it's not, and this arguably applies to other names starting with @Null. Point being that I think it's important to convey in the name that this is a "scope" or "default", as opposed annotations that affect a single type use.

I may be missing something but @DefaultNonNull does kind of seem to fit the bill? What I like about that is that it gives a (maybe imperfect) intuition about what the default might be. Maybe another option sort of along those lines would be

@[Default]NullExplicit

or some variant thereof.

@kevinb9n
Copy link
Collaborator Author

The meaning of this annotation will be something like "within this scope, all potentially-nullable type usages in API elements are presumed non-nullable except unbounded wildcards, type variable usages, and whatever the third case we decide we need to exclude is." I'm a little concerned that "default non-null" gives the reader a false sense that it's much simpler than that.

So for example, in @NullAnnotated class Foo<T extends List<? extends @NonNull Object>>, the @NonNull doesn't as obviously look like a redundancy to be cleaned up (which it, of course, isn't).

@kevin1e100
Copy link
Collaborator

@NonNull is redundant in this example though, at least based on how wildcards with explicit bounds are handled in the proposal ATM. Every explicitly written type (excepting type variables) is implicitly annotated @NonNull the way it's set up right now.

I do understand the idea of picking a "neutral" name, which then unties our hands a bit in terms of what we can do without being confusing. But I feel that's backwards: how about we figure out what the default should do first, and then find a name that fits that behavior, whatever it may be?

As another general point, tangentially related to that idea, I'm not sure a boolean attribute is a good idea. The negation of a given behavior is not going to be very obviously defined, plus the problem that seems to come up often with that is that it turns out there's a third choice down the line. So instead, I propose we stick with creating separately named annotations for each different defaulting we want. I believe that's what was also done with @CheckReturnValue, for instance, with @CanIgnoreReturnValue. But, I'd support the idea of initially only having one of these, understanding that the absence of that annotation implicitly creates a different default.

@cpovirk
Copy link
Collaborator

cpovirk commented Jan 23, 2020

Bringing in some new-ish thoughts from other places:

  • Suppose that we provide @Nullable, @NonNull, and @NullAnnotated. As @kevinb9n says, this helps make @NullAnnotated class Foo<T extends List<? extends @NonNull Object>> look less redundant. However, it also makes @NullAnnotated class Foo<T extends @NonNull Object> { @NonNull T foo(@NonNull Object) look less redundant, even though that's very redundant. It's good to avoid suggesting that @NonNull is always the default (since that wouldn't be true -- or at least might be misleading, depending on how you look at it), but it would be nice to still suggest that it's usually the default.

  • @NullAnnotated and similar names might not suggest to users that there is defaulting happening even in general. Like, if I annotate one type somewhere in my file, does that mean I should apply @NullAnnotated to the file? The concept that we're going for is arguably more like @FullyNullAnnotated.

  • An interpretation that I've been pushing this week is that the "in the club" annotation really means "Foo means Foo" -- as opposed to how it works in Java today, which is "Foo means either Foo or Foo?, but you don't know which." The underlying model for the interpretation isn't that we just automatically paste @NonNull onto (almost) every type usage but rather that we shift the meaning of Foo to mean what we want, which is something more like what it means in Kotlin.

For all of these reasons, I like @kevin1e100 's @DefaultNullExplicit or some variant thereof. (I think I'm also still OK with @ApplyNullnessDefaults. And I'd imagine we could find other good names, too.)

@cpovirk
Copy link
Collaborator

cpovirk commented Jan 24, 2020

(...although I have lately been saying some negative things about describing what we're doing as "defaulting," so maybe I should rethink my preferences...)

@cpovirk
Copy link
Collaborator

cpovirk commented Feb 3, 2020

Another exception is type-variable usages: T get(int index) doesn't mean @NonNull T get(int index). (Probably most people will expect this, anyway, so it may be too natural to even call an "exception," except to the very mechanical process of pasting @NonNull onto every type in the file.)

@kevin1e100 pointed out that this exception is a little stranger than I'd realized:

We already knew that we could spin this as (hopefully) the only "real" exception: Even though we default <?> to a nullable bound, we can say that that's "because there is no bound," not "because the bound is @Nullable Object."

The new weird part is: If you do want @NonNull T in the sense of "project to non-null" (#91, #89, #4), then you have to write the string "@NonNull," even in a @DefaultNonNull context. This is different than the <?> case because, in the latter case, you can write <? extends Object> to set a non-null bound -- all without writing the string "@NonNull."

This isn't strictly "problematic," only a little weird. And it does improve if we pick a name other than "@DefaultNonNull" -- though I wouldn't say it goes away completely because, if nothing else, it would be the sole reason to use @NonNull in code that is fully "in the club."

@kevin1e100
Copy link
Collaborator

The new weird part is: If you do want @NonNull T in the sense of "project to non-null" (#91, #89, #4), then you have to write the string "@NonNull," even in a @DefaultNonNull context.

Sure, but, T's bound has to allow nullable (or nullness-unspecified) type arguments for it to matter, so there would be an annotation there (unless we go the other way in issue #12, in which case I think we understand that there'll be non-redundant @NonNulls needed on type parameter bounds sometimes also.) Conversely, if T's bound is @NonNull then @NonNull T is redundant (at least inside @DefaultNonNull, see issue #89). It's true though that "projecting" would constitute a non-redundant use of @NonNull (or @NonNull(overwriteTypeArgument=true) or @SomeOtherAnnotation) inside @DefaultNonNull, but indeed the only one I have seen.

Wildcards, in particular, never need it:

@DefaultNonNull
class Foo<T extends List<? extends Anything>> {...}

Introduces a non-null wildcard as written (as things stand anyway); ? extends @NonNull Anything is redundant. I'm repeating that to make sure we're on the same page on that, since some of the comments here seem to--maybe unintentionally--suggest otherwise. As mentioned, List<? extends Object> is a bit awkward, but that's kind of a different problem (potentially addressed by the speculated <@NonNull ?>, see #83, which however then would constitute another non-redundant @NonNull use).

@cpovirk
Copy link
Collaborator

cpovirk commented Feb 5, 2020

I think we are on the same page. Definitely we agree that ? extends @NonNull Anything can be written as ? extends Anything. (I was trying to say that here but may well have said things to accidentally suggest the contrary.)

I did want to ask about this:

Sure, but, T's bound has to allow nullable (or nullness-unspecified) type arguments for it to matter, so there would be an annotation there

I think you're saying that, if we have...

@DefaultNonNull
class Foo<T extends @Nullable Object> {
  @NonNull T bar();
}

...then @NonNull T won't necessarily look redundant to the reader because it's somewhat obvious that it's there to overrule the potentially nullable type argument. So when you say "there would be an annotation there," you're referring to the @Nullable on Object? Is that right?

If so, I agree that that helps clarify things to the reader. It might still be nice for readers of @DefaultNonNull code to theoretically not even have to be aware of @NonNull -- not that they wouldn't actually know about it but that they could put it out of mind entirely, run s/@NonNull //g, etc. :)

Again, though, you make a good point.

@kevin1e100
Copy link
Collaborator

kevin1e100 commented Feb 6, 2020

when you say "there would be an annotation there," you're referring to the @Nullable on Object? Is that right?

Yep, thanks for clarifying. To add to that, my (unproven) sense is that users would likely have an intuition that annotating a type variable is "special", so I suspect few users would find @NonNull T "redundant-looking" anyway (plus, as mentioned, there would be an indicator that it's not redundant at least).

It might still be nice for readers of @DefaultNonNull code to theoretically not even have to be aware of @NonNull

I understand that sentiment but I kind of took away from your last comments that if non-null "projections" of type variables are in the picture then that "ideal" isn't really achievable. Did I miss something? Conversely, without projections, it does seem possible to elide every @NonNull inside @DefaultNonNull (assuming we decide a bunch of things, including #12, in a particular way).

@cpovirk
Copy link
Collaborator

cpovirk commented Feb 6, 2020

Right, if we support non-null projections, then we have to give up on the ideal. And if we don't, then the ideal is (possibly) achievable. I'm filing this away as an extremely minor reason to avoid projections.

@cpovirk
Copy link
Collaborator

cpovirk commented Apr 13, 2020

A small bonus of using a single annotation for both cases: We can make it invalid Java for a declaration to be annotated @InTheClub(true) @InTheClub(false). In contrast, we can't make it invalid Java for a declaration to be annotated @InTheClub @NotInTheClub (though our checkers can refuse to compile sources in which that appears [#101]). This helps slightly with #100. (But we may still have similar issues with the type annotations: @Nullable @NonNull, @NullnessUnspecified @Nullable, etc.)

@kevinb9n
Copy link
Collaborator Author

That is a good illustration of why the single annotation appeals to me so much in cases like this as well as the future @MustUseReturnValue(false).

There are technical workarounds, but it's more elegant to not need those.

@kevinb9n
Copy link
Collaborator Author

kevinb9n commented May 8, 2020

For better or worse, I generalized this debate and stuck it into #109.

This issue should only be about agreeing that nullness-defaulting really is just binary (two choices), and then we should be bound by the decision made in #109 from there. (Edit: oh right, and the NAMING of this annotation.)

(It might have been better to repurpose this issue for that, maybe, sorry.)

@cpovirk
Copy link
Collaborator

cpovirk commented May 8, 2020

It looks like we at least tentatively agreed on the binary-ness of nullness-defaulting in #41. So maybe this current issue can be solely about naming, and we can reopen #41 if needed?

@cpovirk cpovirk changed the title One single defaulting annotation, expressing a binary choice of in-the-nullness-club or not Name(s) of the annotation(s) expressing "in the nullness club" or "not" May 8, 2020
@cpovirk
Copy link
Collaborator

cpovirk commented May 8, 2020

We're going to have an entry in the glossary for "code that is in the scope of such an annotation," so we might hope that that term can be close to the name of the annotation itself.

Maybe this is a slight vote against "nullness-annotated," since it requires that we distinguish between "a type with a nullness annotation on it" and "a type in a nullness-annotated context." (And I think that all 4 combinations of those 2 binary states are possible?)

But we have explicitly postponed this decision to future quarters, so we don't need to figure this out now.

@cpovirk
Copy link
Collaborator

cpovirk commented May 8, 2020

But naturally, in reading this, I got to thinking... :)

I like including kevin1e100' s concept of "explicit" in the name -- or possibly alternative words related to that, like null-"aware?" [edit for searchability: @NullAware]

I said up-thread that, to me, "null annotated" doesn't completely suggest "fully null annotated." Also, sometimes you don't need to annotate anything at all (because all types are non-null), in which case you're really saying "I intentionally did not annotate any types in this file." (Also, in rare cases, you need to do something, but it's not to annotate but rather to switch from ? to ? extends Object.)

@kevinb9n
Copy link
Collaborator Author

kevinb9n commented May 11, 2020

For use in prose I kinda really like "null-aware context". It really should be "nullness-aware context", but the cheat seems okay and makes the phrase much handier.

The notion is: sure, the same nulls are flying around at runtime either way, but this way the developer is aware of them, the tools are aware of them, the code is aware of them. (Well, kind of. If you went and annotated everything @nullable indiscriminately then you'd not be very aware.) I wonder if "null-aware" might sound odd to users of "NullAway".

@lazaroclapp
Copy link
Collaborator

lazaroclapp commented Jul 21, 2020

I am tentatively not a fan of using @NullAware for this, as the name is a bit close to NullAway*, and I can foresee developers making mistakes with having to e.g. "Annotate your packages as @NullAware in order for them to be picked up by NullAway". To me, @DefaultNonNull was also generally clearer.

cc: @msridhar

  • Anecdotally, I've even seen people make that mistake even in a world where nothing actually called "NullAware" exists...

@cpovirk
Copy link
Collaborator

cpovirk commented Jul 21, 2020

I think @kevinb9n had raised the possibility of that confusion to me, and it's been perpetually on my to-do list to ask you about it. Thanks for preemptively raising it!

Would you feel better about @NullnessAware, at least in that respect? Of course, it's also longer :\ Plus, we definitely will want to explore the question of whether "nullness-aware" or "defaulting to non-null" aligns with the mental model that users have -- and/or with the mental model that we want them to have.

@kevinb9n
Copy link
Collaborator Author

I have so far been really liking the "null-aware" terminology at least in prose. Though, I can't square the idea that a single bit of nullness is either specified or not with the idea that a broader swath of code is either aware or not. What accounts for the difference between the two unrelated terms? I'm not sure.

@cpovirk
Copy link
Collaborator

cpovirk commented Jul 21, 2020

Hmm, good point. It's... aware that it's doesn't know? That maybe makes some sense, but it's hardly a natural way to put it.

That is perhaps a reason to prefer terminology like @NullExplicit. Did you (or anyone else, aside from @kevin1e100, who presumably still likes it) have an opinion on that?

@cpovirk
Copy link
Collaborator

cpovirk commented Jul 22, 2020

@DefaultNonNull may at least hint that the default is otherwise @Nullable. But in fact it's closer to @NullnessUnspecified. (That confusion is likely to arise no matter what, but maybe @DefaultNonNull encourages it?)

@kevin1e100
Copy link
Collaborator

I'm guessing this is alluding to the following derivation someone might make: "since the apparent opposite of @NonNull is @Nullable, the absence of @DefaultNonNull must mean that @Nullable is the default". I'll admit I find that to be an unlikely line of thinking in this case (because absence doesn't imply opposite in my mind). Moreover, regardless of what name we choose for annotating "in the club" code, I find myself believing that most people will intuit that in the absence of an explicit default annotation, the legacy rules they've been used to in Java for decades will apply.

(I suppose the last sentence is a point in favor of expressing "out of the club" with @InTheClub(false), since people may more easily interpret that as going back to the default-default than a separate @OutOfTheClub, depending on the concrete name.)

@kevin1e100
Copy link
Collaborator

kevin1e100 commented Jul 22, 2020

Apologies, I did mean to say that the "default-default must be @Nullable" derivation appears possible for someone to make. I don't find it likely but the point is that intuition can be different for different people.

@cpovirk
Copy link
Collaborator

cpovirk commented Jul 22, 2020

I do think that "default is @Nullable" is an assumption that some people will make. My impression is that some people conceptualize the legacy Java rules as "everything is @Nullable." And certainly many systems (e.g., Checker Framework) have a binary view of nullness.

One of our challenges may be to emphasize to users that there is a third option, "unspecified nullness." This is of course what Kotlin has done with platform types.

In a small way, I think that the name @DefaultNonNull works against that: I wouldn't say that Kotlin is different from Java because Kotlin "defaults to non-null"; I would say that Kotlin is different because it "includes nullness in the type system at all."

(And sorry, I didn't intend anything I wrote as an argument for @InTheClub(false). If anyone wants to read more on that, see #109.)

@cpovirk
Copy link
Collaborator

cpovirk commented Jul 22, 2020

And sorry, I missed your followup.

It may depend in large part on how familiar the users are with Kotlin.

@kevinb9n
Copy link
Collaborator Author

This is not exactly a data point, but when I was playing at drafting a user guide chapter, I draft-titled the document "Writing null-aware code with jspecify", and the chapter, "Interacting with null-unaware code". So, to me, the adjective seemed to "work".

@kevinb9n
Copy link
Collaborator Author

In fact, I might like centering on "awareness" enough to even consider dropping the "unspecified" terminology for it completely. Potentially a single @NullUnaware could serve as both the cascading-default annotation and the (if needed) type annotation?

@cpovirk
Copy link
Collaborator

cpovirk commented Aug 27, 2020

I think that a single annotation that plays double duty is likely to be confusing:

@NullAware
interface Foo {
  @NullUnaware String bar(String p);
}

Is p specified to be non-nullable, or is its nullness unspecified? It depends on whether @NullUnaware is treated as a method annotation or a type annotation in this case.

@cpovirk
Copy link
Collaborator

cpovirk commented Mar 12, 2021

Working decision: @NullMarked.

edit: relevant docs:

@kevinb9n kevinb9n added the design An issue that is resolved by making a decision, about whether and how something should work. label Mar 13, 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

5 participants