jspecify: Ability to correctly annotate without immediately breaking callers (e.g. `@Migrating`)

Motivation

There is a huge body of existing Java code that only sparingly uses annotations, e.g. some usage of @Nullable. How can we make it as easy as possible to transition such code to Java with codeanalysis-annotations, where tools enforce correct usage.

Let’s take an un-annotated Java file:

class Demo {
  Object m(Object p) ...
}

This class can be used from a context that enforces, e.g., null safety.

@NotNullByDefault
class User {
  void use(Demo d) {
    d.m(null).toString();
  }
}

The code analysis can now either make optimistic or conservative assumptions about the signature of Demo or introduce platform types to check correct usage. Let us assume that User produces no compile-time errors.

We now want to convert Demo to also be null safe:

@NotNullByDefault
class Demo {
  @Nullable Object m(Object p) ...
}

The code analysis can now ensure that the implementation of Demo is safe. However, now the usage in User will produce two errors:

  • passing null to m, which now defaults to @NotNull
  • dereferencing the @Nullable return value of m

Because Demo is now fully annotated (considering the `@NotNullByDefault and explicit annotations) no optimistic defaults would be assumed or no platform types would be generated.

If there are many uses of an API, we need a way to help with this migration.

Proposal

We introduce a new declaration annotation tentatively named @UnderMigration, which is applicable to declarations of fields, methods, type parameters, types, and packages.

In our example, we could annotate Demo as:

@UnderMigration(since = "2019-01-30)
@NotNullByDefault
class Demo {
  @Nullable Object m(Object p) ...
}

This marks Demo as being newly annotated and gives code analysis tools the additional information that the new signatures might produce many warnings. For example, a tool could decide to initially continue to use platform types, then issue warnings instead of errors for such APIs, and then finally treat the API as final.

The new @UnderMigration annotation separates the semantic information of all codeanalysis-annotations from the migration issues. The API is annotated with the semantically correct annotations and one annotation can be used for migration instead of mixing this concern with every check.

The annotation would look roughly like:

@interface UnderMigration {
  // yyyy-mm-dd since when the API is under migration.
  String since();

  // Checkers that are under migration.
  // Empty default signifies all checkers.
  // Strings identify checkers, in the same format as used for @SuppressWarnings
  String[] checkers() default {};
}

The since attribute conveys the date when migration began. Tool users can use this information to decide how to use the signatures.

The checkers attribute conveys which checkers are under migration. For example, some API might have been converted for null safety, but not yet for @CheckReturnValue. The strings follow the same format as what @SuppressWarnings would use to suppress warnings from a particular checker.

Applicability to methods, classes, and packages allows nested specification of APIs, e.g. this whole package hasn’t been transitioned for null-safety and this class additionally hasn’t been transitioned for CRV.

Discussion

How should migration status be designated?

The proposal above includes a single since date that allows users to decide how to handle signatures. This has the advantage that the API doesn’t need to be changed and users are free to decide on severity.

Alternatives considered:

  • Use some enum constants to mark how far along the API is. E.g. initially something would be @UnderMigration(status = MigrationStatus.ALPHA), then change to @UnderMigration(status = MigrationStatus.BETA), before becoming @UnderMigration(status = MigrationStatus.FINAL), which is equivalent to having no annotation. Compared to the since date this has the disadvantage that the API needs to change to use different migration statuses.

  • Similarly, use an enum that describes severity e.g. Usage.INFO, Usage.WARN, and Usage.ERROR. This has the disadvantage that it prescribes tool behavior, which we don’t want to prescribe in the API.

Instead use attributes on the analysis annotations

Instead of marking an API as @UnderMigration the type qualifiers could convey migration status, e.g. as @Nullable(since = "2019-01-28"). This has several disadvantages:

  • we need to add the same attributes to all code analysis annotations, leading to duplication and maintenance efforts
  • type use annotations should generally be short
  • it seems error prone to require every parameter/return type to specify different migration levels

Instead, the checkers attribute on @UnderMigration gives us the possibility to mark API as under migration for a particular checker.

Instead use three possible qualifiers

Alternatively, we could give developers the option to specify the “third option” explicitly, e.g. by using @LikelyNonNull or some such. Tools could then choose to interpret these as platform types or handle them optimistically or conservatively. This has several advantages:

  • we need a migration option for every code analysis check
  • we mix migration and specification issues
  • it seems hard when to choose the third option relative to the other options.

Separating migration issues into @UnderMigration gives us one concept that applies across all code analysis checkers.

Why allow use on type parameter declarations?

This allows to also migrate the annotations on a type parameter bound:

class C<@UnderMigration(since = "2019-01-10") T extends @Nullable Data> {}

Changing the upper bound of a type parameter has an impact on possible instantiations and wildcard uses. The alternative would require marking at least the whole class as under migration.

Usage in the JDK or Android sources

The @UnderMigration annotations should be usable instead of using @RecentlyNullable/@RecentlyNonNull. An API would be annotated with a migration date and tools can decide when they want to start enforcing the semantics of these annotations correctly.

About this issue

  • Original URL
  • State: open
  • Created 5 years ago
  • Comments: 18 (1 by maintainers)

Most upvoted comments

We made it post-1.0 due to having such low chatter. Then, chattter!

Your doc reminded me of my primary concern: that I really don’t want anything to block anyone from annotating an API correctly. That makes this issue feel essential to me.

I’m going to provisionally 1.0 it based on the new energy around it, and because it’s okay if things move back and forth a few times.