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

final package names/locations [decision: all in org.jspecify.annotations (flat)] #260

Closed
kevinb9n opened this issue Aug 9, 2022 · 15 comments
Labels
design An issue that is resolved by making a decision, about whether and how something should work.
Milestone

Comments

@kevinb9n
Copy link
Collaborator

kevinb9n commented Aug 9, 2022

edit: This issue will now be decided in this document

Sincere apologies to the public who cannot access that at this time. Feel free to append your arguments here and we will factor them in.


Currently the four nullness annotations are in org.jspecify.nullness. Where does @Implies go?

  • org.jspecify
  • org.jspecify.nullness
  • org.jspecify.meta
  • org.jspecify.shared
  • org.jspecify.implies
  • org.jspecify.annotations and move nullness in there too
  • other
@kevinb9n
Copy link
Collaborator Author

kevinb9n commented Aug 9, 2022

I feel we will create constant headaches for ourselves by trying to aggressively categorize all our annotations, and having org.jspecify.nullness (as we "working-decided" in #160) is putting us on that path. Could we stomach a move to org.jspecify.annotations?

@cpovirk
Copy link
Collaborator

cpovirk commented Aug 11, 2022

Thanks for opening this. The thought had crossed by mind during the review for #256, but it immediately left.

The question of the best package structure is still one that I don't feel I have a firm opinion on. (History is available in #160, as linked above, as well as #1, which is linked from #160. See also this thread and these meeting notes.)

The main claim I would make on that front is that it's at least not obviously a terrible user experience for us to have any of the following scenarios:

  • all the annotations in one package
  • all the annotations in subpackages (nullness, meta, resultuse(???), ...)—though I do see how this could get tricky to name
  • a mix of annotations in subpackages (nullness, meta, ...) and annotations in the "everything else" superpackage (or misc-like subpackage)

So, to your second post: While a 1.0 with org.jspecify.nullness would suggest a path of further subpackages for categorization, I don't see it as committing us to that. (I'm not sure you're claiming it's committing us to that, either. But maybe you'd be more concerned about subsequently "changing course" than I would?) Further, even supposing that we were to use subpackage categorization throughout the project's life, I think the resulting headaches would be mostly ours (rather than users'), headaches that are possibly years down the road. So I think org.jspecify.nullness is (if I may be so bold)... "non-disastrous" :)

@cpovirk
Copy link
Collaborator

cpovirk commented Aug 11, 2022

Then: If it's reasonably possible to stomach an org.jspecify.nullness, my firmer opinion is that we should stick with it—not because it's best but because it's what we have right now. And the biggest reason for that is tool support for users who try out JSpecify.

If we want users to try our annotations, then we don't want to tell them that they have to wait potentially multiple months for a Kotlin, IntelliJ, or Checker Framework release—worse, longer if their company/project can't update to that version right away.

Beyond that, a change in package name is likely to prompt tool owners to set back their timetables. Not that Kotlin (for example) was likely to flip on checking of JSpecify annotations by default just yet, anyway. But the longer that Kotlin has been issuing warnings for JSpecify annotations under the final package name, the sooner the Kotlin developers are likely to be willing to issue errors by default—knowing, again, that some of their users will be using older versions of Kotlin for a while, not to mention that some will be using versions of Guava, etc. with no JSpecify annotations for a while, so the Kotlin developers may need to wait a while to get feedback from such users.

Coincidentally(?), just today there was an update on the IntelliJ bug tracker that said:

We had JSpecify support hidden under the key because JSpecify was experimental, and annotations names, package, and semantics were changing pretty often. If it's stabilized now, we may probably make them enabled by default.

Now, maybe we don't want them to enable it by default today, and we could let them know that :) But if we change the package name again, then we have to consider how long the IntelliJ developers will want things to be stable before they're willing to attempt finalization again. Maybe the release of 1.0 is good enough for them, but we should check whether they have any other reservations beyond what was suggested by that post. And whatever the IntelliJ and Kotlin developers think, we may hear reservations from other tool authors or others in the wider community.

But back to users who try out JSpecify: To be fair, I'm not sure how much we can expect users to try our annotations soon. But if they do, then I'd want Kotlin support to work, since Kotlin is a big part of our value proposition for some users. And if this is a chance for any number of users to tell tool authors "Because you haven't implemented full JSpecify semantics yet, I'm having <such and such a problem>," I think that such feedback would be worth a lot more than: "Hey, could you support JSpecify annotations? Oh, I just need to wait for the next release for the package change? OK, everything will probably work fine then."

And one thing that I do think has at least some chance to actually be disastrous is if users can't see how tools respond to our annotations. I worry that that would block the feedback loop we want to create: We want users to try annotations and tools so that tool authors respond by offering more support so that more users respond by trying them out, etc. I'm more concerned about making sure that that happens than I am about picking the best package name.

@kevinb9n
Copy link
Collaborator Author

I appreciate (in both senses) what you're saying.

My main concern about all that is... a meta-concern. We/Google have generally been making "working decisions" left and right, usually without having the benefit of real substantive arguments about this pain vs. that pain coming from users yet (for the most part), and usually without even much active involvement from our partners. And we've justified that by promising that they're only working decisions and that (a) we will still have some process for explicitly making final decisions, and (b) when we do, we won't treat "because that's how we're already doing it" as a valid argument (maybe as the very last tiebreaker, I guess). Without promising (a)/(b) then the burden of getting the working decisions made would have felt too high, to me.

So I worry that calling ourselves painted-into-a-corner on even one little issue over here might kind of portray those promises as being generally fraudulent.

But then: I worry.

@kevinb9n kevinb9n added this to the 1.0 milestone Aug 15, 2022
@kevinb9n
Copy link
Collaborator Author

As part of this we should also look at where the proposed (#200) @MayIgnoreResult and @MustUseResult annotations would go.

If there is a nullness package, then I would have to favor putting everything else in a parallel misc package. There would be very few annotation "domains" that I can see meriting a package of their own (maybe state for @Immutable and such). The rest we really don't want to have to categorize.

@cpovirk
Copy link
Collaborator

cpovirk commented Aug 17, 2022

What's the latest sense of how many annotations #200 will result in? If it turned out to be the two you mentioned plus two separate annotations for flipping the default must-use-result-ness on and off, that would bring us to the same number as we currently have nullness annotations. Would they have as good a case for a package as nullness -- or as bad a case, if you prefer? :) Or maybe we can expect less than four, or maybe we can predict that nullness is more likely to grow beyond four than #200 is?

(If the mere name misc sounds like a wart, we could think about using the top-level org.jspecify instead.)

It sounds like you're advocating against having a separate package for @Implies and other such things (#101, #136, @NegationOf)? I could see a case for putting them in a separate package (even if there turned out to be only one or two) just to hide them from the annotations that are more clearly of interest to average users, but I don't feel that strongly.

@kevinb9n
Copy link
Collaborator Author

kevinb9n commented Aug 17, 2022

New high-level summary:

In the meeting we decided that rethinking this layout should be "on the table", not ruled out for being definitely painful. (We could still discover more pain and pull back from the change ideally before 0.3.)

We also established that we expect to always have a single released artifact for all of our annotations. (Even if things go very well for the next 5 years I would be surprised if we had even 20 of them, and we're not allowing even so much as a plain interface or enum.)

The main organizational alternatives I'm aware of:

  • One subpackage of $common_parent for every "domain", even if only one annotation.
  • Adopt criteria for when a domain merits its own subpackage of $common_parent, which presumably nullness and meta/shared (name TBD) would meet, but a single-annotation domain would not.
    • variant: The rest go in a parallel .misc package (name TBD)
    • variant: The rest go in $common_parent
  • One flat package for everything (which I'll call $common_parent here for consistency).

Then, there is the question of what $common_parent should be, with the obvious options being org.jspecify or org.jspecify.annotations. (I believe we all feel fine about the org. prefix. I do, even if our canonical web site is stuck at jspecify.dev.)

@kevinb9n
Copy link
Collaborator Author

kevinb9n commented Aug 22, 2022

Some advantages of one flat package:

  • Some users will import it manually and they would have an easy time remembering where to find everything.
  • We don't have to be delayed by categorization decisions with every new annotation we add (e.g. we don't know how ambiguous some of the "should these go together or not" decisions might prove to be).

Some disadvantages:

  • We can't expect the annotations to group naturally (even @NonNull could end up split from the @Null* group). Nor should we go to heroic lengths in naming to try to make them do so. (I see this as just par for the course; the javadoc package index is where things can be nicely grouped.)
  • It makes the "meta" annotations more visible than they perhaps should be. (So, we could discuss a slight modification to put only those into a subpackage.)

Some advantages of $common_parent being org.jspecify.annotation(s?) even if all annotations are in parallel subpackages:

  • I think it is at least good style to have each released artifact "own" its own disjoint package subtree. Modules enforce a rule that "code under package X is found in artifact Y", but I think you don't really want a long list of X's for the same Y, while other very parallel-looking X's belong to Z. See Wrapper libraries (release and/or support and/or evangelize) #220 for examples of other artifacts that would also want to come in under org.jspecify (like in org.jspecify.lib or org.jspecify.runtime).

Some disadvantages:

  • More typing if importing manually.
  • Have to decide annotation vs annotations and users won't all remember it.

@cpovirk
Copy link
Collaborator

cpovirk commented Aug 22, 2022

Simplicity is a good thing, and it's nice to make our annotations easy to remember and type in blogs, presentation slides, Twitter, etc., where there's no autocomplete available. Granted, I'd probably make that an argument for going all the way to org.jspecify, which is shorter and sidesteps the "Is it annotation or annotations?" question. (I'd expect the pluralization to trip up more users than overlapping subpackages, but maybe I'd be surprised.)

(I would also be somewhat surprised if the delay from categorizing future annotations added up to the delay that would be spent discussing and recategorizing/uncategorizing the package of the annotations we currently have. But I think we agree that we can accept a delay (whichever option we consider to be the delay-ier one) if we can convince ourselves that that package layout offers a better overall experience for users.)

@netdpb
Copy link
Collaborator

netdpb commented Aug 22, 2022

A slight advantage to having a subpackage per domain: each subpackage can have Javadoc in its package-info.java file.

@cpovirk
Copy link
Collaborator

cpovirk commented Aug 22, 2022

Should one of us be collecting notes from here, #160, and #1 into a doc? :)

@kevinb9n
Copy link
Collaborator Author

sigh, yes, I guess that is what I should do.

@kevinb9n kevinb9n modified the milestones: 1.0, 0.3 Sep 20, 2022
@kevinb9n
Copy link
Collaborator Author

This issue will now be decided in this document

Sincere apologies to the public who cannot access that at this time. Feel free to append your arguments here and we will factor them in.

@kevinb9n kevinb9n changed the title package locations, e.g. @Implies final package names/locations Nov 10, 2022
@kevinb9n
Copy link
Collaborator Author

After many rounds of inviting comment I'm soon to decide between org.jspecify.Nullable and org.jspecify.annotations.Nullable. The doc has the deets.

I'm assuming that we will not need to make a release that contains both old and new locations, because at the very worst, someone in transition could always just use both 0.2 and 0.3 jars at once. (!) Is that even right?

@kevinb9n
Copy link
Collaborator Author

TL;DR all annotations will be in one package, org.jspecify.annotations.

There's a six-page document going through all the twists and turns that led us there; for now it's not publicly accessible, but if anyone is interested enough to ask I can share it. A summary could be made, too, just not sure it's worth the time right now.

@kevinb9n kevinb9n changed the title final package names/locations final package names/locations [decision: all in org.jspecify.annotations (flat)] Nov 15, 2022
@kevinb9n kevinb9n added the design An issue that is resolved by making a decision, about whether and how something should work. label Nov 30, 2022
Stephan202 added a commit to PicnicSupermarket/google-java-format that referenced this issue Dec 15, 2022
So that its `@Nullable` annotation is treated as a type-use annotation
and formatted accordingly. See jspecify/jspecify#260.
copybara-service bot pushed a commit to google/google-java-format that referenced this issue Dec 15, 2022
So that its `@Nullable` annotation is treated as a type-use annotation and formatted accordingly. See jspecify/jspecify#260.

Fixes #869

FUTURE_COPYBARA_INTEGRATE_REVIEW=#869 from PicnicSupermarket:improvement/support-new-jpecify-package-name 7a0dc20
PiperOrigin-RevId: 495699887
copybara-service bot pushed a commit to google/google-java-format that referenced this issue Dec 15, 2022
So that its `@Nullable` annotation is treated as a type-use annotation and formatted accordingly. See jspecify/jspecify#260.

Fixes #869

COPYBARA_INTEGRATE_REVIEW=#869 from PicnicSupermarket:improvement/support-new-jpecify-package-name 7a0dc20
PiperOrigin-RevId: 495701635
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.
Projects
None yet
Development

No branches or pull requests

3 participants