null-safety annotations on non-migrated packages

[jonasfj]

Reproduction steps:

# Extract package:retry version 3.0.1
$ curl -L https://storage.googleapis.com/pub-packages/packages/retry-3.0.1.tar.gz | tar -xz
$ pub get

# Get dartdoc from git
$ mkdir /source/dartdoc
$ git clone https://github.com/dart-lang/dartdoc.git /source/dartdoc
$ cd /source/dartdoc; pub get; cd -;

# Generate docs
$ dart /source/dartdoc/bin/dartdoc.dart 

Notice that the pubspec.yaml for package:retry has:

environment:
  sdk: ">=2.0.0 <3.0.0"

And .dart_tool/package_config.json specifies language version 2.0 for package:retry:

{
  "configVersion": 2,
  "packages": [
    /* various other dependencies, mostly test and it's transitive dependencies, omitted here */
    {
      "name": "retry",
      "rootUri": "../",
      "packageUri": "lib/",
      "languageVersion": "2.0" // <-- language version 2.0, hence, no library in package:retry supports null-safety
    }
  ],
  "generated": "2020-10-21T10:44:27.912220Z",
  "generator": "pub",
  "generatorVersion": "2.10.0"
}

---

Unexpected result:

While it is correct that the retry function cannot return null, the package is not migrated to null-safety, so should we really be adding any sort of null-safety tag here?

[jonasfj]

Note. it's is correct that individual libraries can opt-out of null-safety, and thus, it can be argued that it's valuable to annotate whether a function or class supports null-safety.

But for most packages that are migrated to null-safety, all libraries will be migrated to null-safety.

Edit: I'm not entirely sure what we should be doing here. Just that adding "null-safety" to libraries in packages that do not have language-version >= 2.12.0 is probably wrong.

[jcollins-g]

Agree that this is probably wrong. Not sure what's happening here but will try to look into it tomorrow; if it is widespread (i.e. we're now marking EVERYTHING like this for some reason) should probably elevate to P1.

[jcollins-g]

I know that the analyzer still knows the truth that these aren't null-safe packages, however now we are sprinkling null safety tags all over the place.

[mit-mit]

@jcollins-g do we ever want this tag at the member level? Elsewhere, e.g. on pub.dev, we're going to treat null safety as a package-level property (either the whole package has it, or it doesn't)

[jcollins-g]

@mit-mit I don't have a strong opinion on it either way. There's an argument to be made that when browsing the corpus of API docs and crossing package boundaries, it might be nice to know when you've stumbled into (or out of) a null safe package, and a member tag is one way to do that.

[mit-mit]

We could put it here?

[jonasfj]

There's an argument to be made that when browsing the corpus of API docs and crossing package boundaries, it might be nice to know when you've stumbled into (or out of) a null safe package, and a member tag is one way to do that.

Is it commonly possible to stumble out of a null-safe package into something that isn't null-safe?

On pub.dev the analysis in pana will only classify a package as supports null-safety, if:

• Language version is >=2.12,
• No library in the package opts-out of null-safety using a language version marker (// @dart=2.5),
• All dependencies (excluding dev_dependencies) in the current pub upgrade resolution supports null-safety (following this recursive definition).

Hence, if you start reading documentation for a package that supports null-safety, it's unlikely that you accidentally stumble into a package/library that isn't null-safe.

But you can go the other way, if you start from a package that isn't null-safe. And if you end up on a documentation page by finding it in a google search I suppose it's also nice to know. So a library-level label could be nice.

[jcollins-g]

This turns out to be a hard problem to solve due to dart-lang/sdk#43903 which I understand to also be a hard problem to solve due to the experiment flag and how it is used for non-nullable. This issue may "go away" when the flag becomes default. My inclination is to allow the tags to be wrong for a short period, assuming the flag becomes default-on in the analyzer soon, as working around this will take significant effort. Will discuss with the analyzer team Monday, most of them are out today.