dartdoc should follow package allow-list on nnbd experiment

[jcollins-g]

Updated:

Dartdoc can treat the packages on the allow-list as having the experiment flag enabled by default.

• A package that otherwise appears to be migrated via package versioning but no other indications should be treated as legacy, and crash if NNBD is used.
• A package that is apparently migrated via pubspec version constraints and has an analysis_options.yaml flag indicating the NNBD experiment is enabled should be treated as a migrated package, regardless of the allow list contents.
• A package that is apparently migrated via pubspec version constraints and is in the package allow-list should be treated as a migrated package, regardless of the contents of analysis_options.yaml.
• Because dartdoc can not control experiment options in the analyzer in a package-specific way, documenting a set of packages at the same time with mixed modes will not be allowed. Most usages of dartdoc will not be impacted, but this may have implications for how flutter docs are generated. TBD: determine impact and either remove this restriction (very hard), change flutter doc generation to build sub-packages individually, or force all packages flutter depends on to be migrated.. -- This should not be an issue with how the analyzer will be working with the flags.

A package documented as "legacy" will show no star * or nullable ? type information. A package documented as NNBD will show nullable type information per #2210.

prior to offline discussion with @jakemac53 :

For the tech preview, it seems the right thing to do for dartdoc is to enable display of nullability (#2210) for packages in the allow-list as defined here. For any package in this list, dartdoc should assume both that it is migrated, and should be documented with its strong-mode restrictions.

A migrated package that does not appear on this list will be documented with its weak-mode interface.

[jakemac53]

Strong or weak mode is not defined at the package level at all though? Unless we are mixing terms, afaik the weak mode (*) types are never supposed to be shown to a user, which implies to me that all opted in packages should be documented as if strong mode was enabled.

[jcollins-g]

@jakemac53 Perhaps I'm not understanding the point of the package allow-list. Should packages not on this list but are migrated show the legacy interface, the migrated interface, or ...

[jcollins-g]

@jakemac53 updated the issue summary, let me know what you think.

[jakemac53]

SGTM

Does dartdoc still ship as a package as well as with the sdk? If so then you will need to be careful about your sdk constraints, or you will want to read the allow list from the current users sdk. The mechanics of the allow list is still up for debate but I am pushing for an actual file shipped with the SDK.

[jcollins-g]

@jakemac53 Yes, dartdoc's primary usages are via pub and that's how I recommend it is used. We'll use the user's current SDK to determine which list to use.

I'm actually slightly in favor of using a pub package for the allow list instead of a SDK file, but either way would work.

[jakemac53]

If we used a pub package it would have to be pinned to the SDK, so imo we might as well ship it with the SDK.

[jcollins-g]

The direction I'm going with this is to rely more on user flags rather than the package allow-list, after a conversation with Kevin. This is because whether we document as NNBD or not is more of an output choice since dartdoc is not going to crash.

So, if the package allow list, analysis options, or internally hard-coded versions (for the SDK) allow for NNBD, dartdoc will document NNBD if and only if the experiment flag is passed. If not, it will document the legacy interface.

[jakemac53]

I would suggest instead doing it based on whether the package is opted in or not. Otherwise the pre-releases of the packages in the allow list (that flutter depends on, and that require NNBD) will be documented without their NNBD interfaces on pub.dev?

Similarly, somebody could be documenting an opted out library, and only passing --enable-experiment because some dependency requires it.

So, I don't think the presence of that flag is actually a good indicator. What matters is whether or not the package has opted in.

[jcollins-g]

If you document an opted-out library, it will always document as legacy.

The only change is that we won't automatically document as NNBD without the experiment flag.

[jakemac53]

The only change is that we won't automatically document as NNBD without the experiment flag.

So I think this essentially only applies to the packages in the allow list. Why do we not want to document those as NNBD? Users will be using versions that do in fact support NNBD, so they probably want to see the interfaces as such on pub.dev?

Even if you aren't opted in yourself knowing that a null is not allowed for a given parameter, or that a function returns a nullable type is still useful?

[jcollins-g]

Then, on pub.dev, we can unconditionally pass the experiment flag.

[jakemac53]

Then, on pub.dev, we can unconditionally pass the experiment flag.

I am not sure we really want to do that though - I think we want things to blow up if somebody tries to publish NNBD code before it is stable? We want to tank the scores of those packages.

[jcollins-g]

I don't think it should be dartdoc's reponsibility to enforce that. Packages already have the capacity to override the NNBD package-allow list through a combination of a magic SDK constraint and the analysis options file, at least for the purposes of the analyzer and dartdoc allows for that.

[franklinyow]

How are we doing on this issue?

[devoncarew]

cc @srawlins

[srawlins]

Sorry I just took over for Janice on this issue; just got up-to-speed on the request.

It looks like the allow-list is publicly visible for tools, as @jakemac53 proposed above. I can start work on this.