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

Whether to put nullness annotations in their own subpackage #160

Closed
cpovirk opened this issue Jan 13, 2021 · 9 comments
Closed

Whether to put nullness annotations in their own subpackage #160

cpovirk opened this issue Jan 13, 2021 · 9 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.

Comments

@cpovirk
Copy link
Collaborator

cpovirk commented Jan 13, 2021

Previously discussed in #1 (here, here, here -- apologies if I missed relevant posts while skimming). Our tentative decision (and the state of the annotation sources that exist in this repo) is to put the annotations directly in org.jspecify.annotations. That may well be fine. We just want to consciously decide one way or the other :)

[update: We are now leaning toward eliminating the annotations component (contrary to #1) and putting nullness annotations in org.jspecify.nullness.]

@cpovirk
Copy link
Collaborator Author

cpovirk commented Feb 3, 2021

Assumption: "We'll probably never have more than 25 annotations." (Here are some possible future annotations. Note also the possibility of meta-annotations (#101, #109 (@NegatedBy), #136) -- though we could consider putting only those in a separate package if we wanted.)

Advantages of org.jspecify.annotations.Nullable, etc.:

  • The name is shorter -- not that most users are typing imports by hand most of the time, but it happens. I could even imagine that some users will be stuck using the fully qualified name directly on types on occasion, given the need to work with tools that don't recognize our specific @Nullable.
  • We don't have to debate whether the right name for the subpackage is "nullness" or "nullability" or something else -- and then debate again the next time we create a subpackage :)
  • If we end up adding "one-off" annotations that don't fit neatly into a group, we don't need to figure out whether to put them in org.jspecify.annotations (which may make them look "too important" or at least make users fail to notice our more important subpackages), in org.jspecify.annotations.misc (eww), or in a bunch of org.jspecify.annotations.thatoneparticularpurpose packages with only one class each.

Advantages of org.jspecify.annotations.nullness.Nullable, etc.:

  • If we do end up with a bunch of annotations, it's easy to pick out the ones in any given category. Really, this could be useful even if there are relatively few total annotations, especially if the names of related annotations don't always sort near each other alphabetically and/or if we sometimes define enums for our annotations to use, as Javadoc lists enums separately from annotations. (But we'll probably avoid enums.) Possible example:
    • @MayIgnoreResult
    • @MustClose // unrelated to the other two
    • @MustUseResult
  • We can put nullness-specific documentation into the package-info file. That said, I'm not sure how often normal users read package-info files, especially from IDEs. So maybe there isn't much advantage to a package-info file over any other web site.
  • If we ever had a reason to split our annotations across multiple artifacts, separate packages would allow us to do that without causing split-packages problems for JPMS, etc. However, it seems somewhat unlikely that we'd need such a split for mere annotations. (But again, even if we choose to put most annotations directly in org.jspecify.annotations, we can always make an exception for some particular group of annotations that needs to be a separate artifact.)
  • [added after the switch to org.jspecify.nullness: hints that more domains other than nullness are to come]

@netdpb
Copy link
Collaborator

netdpb commented Feb 3, 2021

I was slightly in favor of subpackages from a purely abstract organizational perspective and also for the discoverability and documentation of related annotations.

However, pure abstractions like that are not very important to users. And I don't think a segregated list of related annotations is sufficient documentation, so I would also expect one entry point for documentation of each group of related annotations, and that document is where I would expect users to look, not the package directory file list. (I would expect each related annotation to link to that entry-point document.) So that mitigates that concern.

The concern about one-off annotations rings true to me, although I'm not so bothered by one-file packages.

Maybe related, if we introduce general-purpose annotations (including but not limited to meta-annotations) that might apply to several different groups, we'd be in the same boat: either put them at the root, elevating their importance, or put them in a org.jspecify.annotations.general package, which has the same problems as misc.

So over all I'm happy to leave everything in one package.

@kevin1e100
Copy link
Collaborator

kevin1e100 commented Feb 4, 2021 via email

@mernst
Copy link
Contributor

mernst commented Feb 4, 2021

I am convinced by the arguments for a separate package for each type system (plus one for the common elements if needed). The most compelling arguments (to me) are that it is a logical organization that puts similar things together and separates dissimilar things, and that it is more future-proof.

I am not compelled by the arguments for lumping everything in to a single package.

  • Nullness annotations are no more special than any other variety -- I am aware of many projects that use multiple pluggable type systems, but not nullness.
  • I assumed the "I think we'll probably never have more than 25 annotations" was a joke or sarcastic, referencing Thomas Watson's "I think there is a world market for about five computers" or Bill Gates's "640K [of RAM ought to be enough for anybody." In any event, the right number of annotations in a package is subjective and does not help us make principled decisions, so I think we should discount it.

The idea of dropping the "annotations" intermediate package is fine with me, and makes sense if JSpecify is nothing more than annotations.

@Stephan202
Copy link
Collaborator

Some other thoughts that came up during the meeting just now:

  1. Whether to include .annotations seems orthogonal to a decision on whether to have subpackages. Thus, placing all annotations into org.jspecify could also be an option.
  2. While perhaps not a large concern when it comes to annotations, (excessive) use of subpackages could lead one to (want to) introduce cycles between packages. While Java does allow this, this is considered a bad practice. (For example, Guava disallows this.)
  3. The JDK itself has many packages containing a lot of classes (see e.g. java.util). It'd be interesting to learn how that design decision came about.

@cpovirk cpovirk changed the title Whether to put nullness annotations in org.jspecify.annotations or a subpackage Whether to put nullness annotations in their own subpackage Feb 26, 2021
@cpovirk
Copy link
Collaborator Author

cpovirk commented Feb 26, 2021

As noted in a couple other places, we seem to be converging on:

org.jspecify.nullness

Things to think about before finalizing that:

"nullness"

Everyone seems to feel that "nullness" is the right name for that component of the package. I note that this matches the Checker Framework's package, and I didn't immediately see any precedent for other terms like "nullability" (unless you count this jOOQ class). And we seem to be pretty happy with "nullness" over "nullability" in general -- particularly for actual annotation names, like @NullnessSpecified (potentially; the exact name is up in the air, and I'm bringing this up here only to note that I haven't heard a push for @NullabilitySpecified).

module and artifact names

We'll also have a JPMS module and Maven artifact to name. Currently, they're set to org.jspecify.annotations (partially discussed in #1) and org.jspecify:annotations (#108). We can decide on those well after we figure out the package name. However, it is important to know if anyone feels that a package of org.jspecify.nullness forces our decision to be (a) to use the ones we have now or (b) to switch to something like org.jspecify and org.jspecify:jspecify. I think I personally can live with either of those sets. But if we find that some people can live with only one set and other people can live with only the other, than we should see if it's easier to find common ground with a different package name, and we should do that now.

Thanks for all the discussion on this question, both from people who are fine with whatever and from people who have expressed preferences for the various options.

@cpovirk
Copy link
Collaborator Author

cpovirk commented Feb 26, 2021

Also RE: module and artifact names:

When writing the above, I implicitly assumed that all our annotations -- including hypothetical annotations unrelated to nullness (and thus in different packages) -- would live in a single JPMS module and a single Maven artifact. Now, I don't think that org.jspecify.nullness forces that choice: While we should have a 1:1 mapping between JPMS modules and Maven artifacts (because the rule is "one module per jar"), we could still choose to have put some annotations in one module+artifact and other annotations in another.

But if we were to consider a split, I would be nervous. In particular, if we want our annotations to be adopted nearly universally, we would benefit from making our dependency as low-cost as possible. Ideally, that means one jar. Otherwise, we risk making even "nullness-only" users depend on both a nullness jar and a "meta" jar that it depends on. (They can sometimes get away without the "meta" dependency, but we'd at least be forcing people to consider tradeoffs -- plus some JSpecify-specific tradeoffs around aliases.) And of course we might like those people to also consider @Immutable, @MustUseResult, etc. someday, and that's easier if they're getting them in the artifact they've already chosen to depend on. (A single artifact may give us some responsibility to not stuff 500+ annotations into it, but I don't think anyone envisions our doing so :))

(I would be in favor of separate modules/artifacts/jars if we ever need to take on additional dependencies for some of our annotations (not that I think we're likely to even need that). But I think that kind of split is compatible with everything I've said above: Most users would depend on org.jspecify:jspecify (or org.jspecify:annotations or whatever we call it). Users who do want org.jspecify:android or whatever could naturally see it as an "add on" (and so it would make sense for the main artifact to have the generic name). And users who don't want it will appreciate that it's separate (because it ensures that they don't need to add a dependency on Android). I see this as kind of split as very different from a preemptive split of artifacts into "nullness," "meta," "threading," "must-close-use-result-etc.," and so forth.)

@kevin1e100
Copy link
Collaborator

kevin1e100 commented Mar 2, 2021 via email

@cpovirk
Copy link
Collaborator Author

cpovirk commented Mar 12, 2021

Working decision is "yes": org.jspecify.nullness.

@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
@kevinb9n kevinb9n added the nullness For issues specific to nullness analysis. 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

6 participants