Extend dartdoc options to begin to handle experiment flags

[jcollins-g]

The beginnings of experiment flag implementation for https://github.com/dart-lang/sdk/blob/master/docs/process/experimental-flags.md.

Initial non-generated options list borrowed from the analyzer for consistency (https://github.com/dart-lang/sdk/blob/master/pkg/analyzer/lib/src/dart/analysis/experiments.dart).

[coveralls]

Coverage increased (+0.02%) to 92.666% when pulling ec85e3d on experiment-control into 27ae7bf on master.

[stereotype441]

Clarifying question: the long term plan with pkg/analyzer/lib/src/dart/analysis/experiments.dart is that it will be code-generated from a yaml description file in the SDK repo specifying what experiments are available, and metadata about them. The motivation for this is to make sure that when an experiment is created or retired, we don't forget to update all the tools. Given dartdoc's place in the ecosystem (and the fact that it's developed outside the SDK repo), it seems like it's more at risk of being forgotten than some of the other tools. Is your long term plan to generate lib/src/experiment_options.dart as well, or do you want it to stay hand-coded?

I'm wondering if it would be better for us to define an analyzer API that would let dartdoc ask about what experiments are available so that this information doesn't need to be duplicated in dartdoc source code. Or perhaps we should define an API to allow analyzer clients to access some of the experimental options parsing code in pkg/analyzer/lib/src/dart/analysis/experiments_impl.dart, so that you don't need to replicate this logic in dartdoc.

[jcollins-g]

@stereotype441 All good questions. First, the easier one: I was planning longer term to generate key parts of the code from a yaml file (DartdocExperimentOptionContext and createExperimentOptions), and have that generated against different yamls in the SDK vs in dartdoc's standalone mode.

The harder question you're getting at is the problem of synchronizing dartdoc and analyzer's configuration. Dartdoc has the possibility to have experiments turned off and on based on its dartdoc_options.yaml and that doesn't necessarily sync with the analyzer's configuration. I don't know what kind of API would be useful to synchronize options, but coming up with something here would be a good idea. In the past we just force-switched on analyzer flags we might at some point depend on with a dartdoc feature...

[jcollins-g]

Oh, as far as sharing the API for option validation, that seems like a more obviously good idea. I did wind up reimplementing what you did slightly differently with different behavior. For example, I took a last conflicting flag wins approach vs returning an error and I'm sure there's more subtlety in there that is really unnecessary.

[stereotype441]

The harder question you're getting at is the problem of synchronizing dartdoc and analyzer's configuration. Dartdoc has the possibility to have experiments turned off and on based on its dartdoc_options.yaml and that doesn't necessarily sync with the analyzer's configuration. I don't know what kind of API would be useful to synchronize options, but coming up with something here would be a good idea. In the past we just force-switched on analyzer flags we might at some point depend on with a dartdoc feature...

@jcollins-g The analyzer already has the problem of synchronizing options between the command-line and analysis_options.yaml files. The way I plan to handle it is by using validateFlagCombination to validate that the options specified in the two places are compatible, and then simply concatenate the options. You might consider a similar approach. (Note that validateFlagCombination is written and unit tested but I haven't yet written the code that uses it).

Another possibility would be to decide that dartdoc_options.yaml is not allowed to turn experiments on and off; it must be done either via the command line or analysis_options.yaml. This would have the advantage of being less work, and I think it's compatible with the intention of these experimental options (i.e. the fact that they allow the user to opt into experimental and potentially buggy behavior, don't guarantee backward compatibility, and are not intended to be used in production code). We could always reverse this decision at a later date if it we wind up deciding to support experimental options in a more durable way.

[stereotype441]

Oh, as far as sharing the API for option validation, that seems like a more obviously good idea. I did wind up reimplementing what you did slightly differently with different behavior. For example, I took a last conflicting flag wins approach vs returning an error and I'm sure there's more subtlety in there that is really unnecessary.

I can see two options for moving forward with this:

1. Dartdoc starts making use of the code in package:analyzer/src/dart/analysis/experiments.dart now, and once we see what functionality it needs, we can use that information to help us decide on the long-term API the analyzer wants to support; then once that API is implemented we can migrate dartdoc to it.
2. You could send a CL to the analyzer team proposing the public API that you need, and we can review and land it, then you can start using it.

WDYT?

[jcollins-g]

The harder question you're getting at is the problem of synchronizing dartdoc and analyzer's configuration. Dartdoc has the possibility to have experiments turned off and on based on its dartdoc_options.yaml and that doesn't necessarily sync with the analyzer's configuration. I don't know what kind of API would be useful to synchronize options, but coming up with something here would be a good idea. In the past we just force-switched on analyzer flags we might at some point depend on with a dartdoc feature...

@jcollins-g The analyzer already has the problem of synchronizing options between the command-line and analysis_options.yaml files. The way I plan to handle it is by using validateFlagCombination to validate that the options specified in the two places are compatible, and then simply concatenate the options. You might consider a similar approach. (Note that validateFlagCombination is written and unit tested but I haven't yet written the code that uses it).

Yes. The PR handles this already with a strict override of what's in the file because that's what's simple and already supported through DartdocOptionArgFile, but it would be quite possible to change.

Another possibility would be to decide that dartdoc_options.yaml is not allowed to turn experiments on and off; it must be done either via the command line or analysis_options.yaml. This would have the advantage of being less work, and I think it's compatible with the intention of these experimental options (i.e. the fact that they allow the user to opt into experimental and potentially buggy behavior, don't guarantee backward compatibility, and are not intended to be used in production code). We could always reverse this decision at a later date if it we wind up deciding to support experimental options in a more durable way.

I think subordinating Dartdoc's options to analyzer's was what I was trying to avoid, but perhaps that's not reasonable. It would be simpler to implement if we did it this way.

[jcollins-g]

Oh, as far as sharing the API for option validation, that seems like a more obviously good idea. I did wind up reimplementing what you did slightly differently with different behavior. For example, I took a last conflicting flag wins approach vs returning an error and I'm sure there's more subtlety in there that is really unnecessary.

I can see two options for moving forward with this:

1. Dartdoc starts making use of the code in package:analyzer/src/dart/analysis/experiments.dart now, and once we see what functionality it needs, we can use that information to help us decide on the long-term API the analyzer wants to support; then once that API is implemented we can migrate dartdoc to it.
2. You could send a CL to the analyzer team proposing the public API that you need, and we can review and land it, then you can start using it.

WDYT?

[jcollins-g]

Reopening with fixes based on our discussion. We now use ExperimentStatus class from analyzer to assemble options from the command line, and directly pass those in from our configuration to AnalysisOptions.

[stereotype441]

lgtm, thank you!