'should lint' should not be an analysis options property

[srawlins]

Right now, AnalysisOptions has a bool get lint property, that dictates whether lint rules should be run. It is used it oh so very few places:

1. analysis_update_options.dart, in the legacy protocol; is this ever used? Can we... not?
2. library_analyzer.dart, which decides whether or not to run lint rules based on this property. Same for pubspec_validator.dart.
3. dart2wasm's dry_run.dart tool (sets the property to true).
4. linter's tool/checks/driver.dart and test_linter tools (sets the property to true).
5. dartdocs package_builder.dart (sets the property to false, for performance).

It is ill-fitting to put this property on AnalysisOptions, because each use treats it as a global/process-wide setting. It is not a property that can be set in an analysis_options.yaml file; it is just bolted onto the side.

Setting this property is also done in AnalysisContextCollectionImpl.new's updateAnalysisOptions4 parameter, in 4, 5. It is manually updated in 1, 3, and 4, by walking all analysis options files found in DriverBasedAnalysisContext.allAnalysisOptions.

[srawlins]

We have a few choices here, if we want to change this. I see:

• Remove the setting, treating it as always "true" in library_analyzer.dart; it would no longer be available in the legacy protocol, and DartDoc would pay a small performance price; when I implemented the setting eons ago, I clocked a 5% time savings when disabling lint rules.
• Move the location of setting from AnalysisOptions. I'm not sure the best place to move it to. AnalysisContextCollectionImpl seems fine, because we construct that in DartDoc. However, library_analyzer.dart is the primary reader of the property, which doesn't exactly have access to the AnalysisContextCollectionImpl. Maybe the data could be piped through, or maybe it can be stored on LibraryContext or AnalysisDriver or AnalysisContext. 🤷

[srawlins]

CC @bwilkerson @scheglov

[bwilkerson]

Given that it's being used by several of our tools it seems like we ought to keep it, but there doesn't seem to be a clearly better place for it. Also, I'm not sure how much the tech-debt is really costing us.

[scheglov]

It works, though :-)
Not nice, but works.
But we could invent some collection-wide options class, which as already should have done, because of enableIndex.

[srawlins]

It works, it's not P1. I find AnalysisContextCollectionImpl.new's updateAnalysisOptions4 very unintuitive. And this API requires that some fields on AnalysisOptions be mutable, which I find confusing. (Why should/would AnalysisOptions.lint change through the duration of analysis?)

Anyways, I did the investigation and wanted to write up the issue before I lose the info.