analyzer: Use Registry.ruleRegistry instead of a rule provider in LinterRuleOptionsValidator.

The theme of this change is that the Registry.ruleRegistry is always
the source of truth for what lint rules are registered, even in tests.
Before this change, there was a mechanism for passing a "rule
provider" around different classes and functions, which is a function
that returns a list of lint rules. This mechanism only existed for one
test, `options_rule_validator_test.dart`.

So this change instead makes that test use Registry.ruleRegistry, and
removes all of the rule provider mechanisms. All in all:

* Remove the LintRuleProvider type alias.
* Remove the LinterRuleOptionsValidator constructor's `provider`
  parameter and corresponding field.
* Remove the `provider` parameter from `analyzeAnalysisOptions()`.
* Remove the `provider` parameter from `OptionsFileValidator()`.
* In AbstractAnalysisOptionsTest, remove the `provider` parameter from
  `assertErrorsInCode`.
* Rename `_validatePluginsOption` to `_validateLegacyPluginsOption`
  while I'm there.
* In order to register the test LintRule classes, they need a proper
  `lintCode` property. So create a super class for them, simplifying
  each test LintRule class.


Change-Id: Ic153bba006fe0fe2a94abb08ac55723ffb2c356e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/393907
Reviewed-by: Phil Quitslund <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>

Author: srawlins

pkg/analysis_server/test/services/linter/linter_test.dart

@@ -20,16 +20,14 @@ void main() {
 
 @reflectiveTest
 class LinterRuleOptionsValidatorTest {
-  final LinterRuleOptionsValidator validator = LinterRuleOptionsValidator();
-
-  final AnalysisOptionsProvider optionsProvider = AnalysisOptionsProvider();
-
   late RecordingErrorListener recorder;
 
   late ErrorReporter reporter;
 
   List<AnalysisError> get errors => recorder.errors;
 
+  LinterRuleOptionsValidator get validator => LinterRuleOptionsValidator();
+
   void setUp() {
     registerLintRules();
     recorder = RecordingErrorListener();
@@ -72,7 +70,7 @@ linter:
   }
 
   void validate(String source, List<ErrorCode> expected) {
-    var options = optionsProvider.getOptionsFromString(source);
+    var options = AnalysisOptionsProvider().getOptionsFromString(source);
     validator.validate(reporter, options);
     expect(
       errors.map((AnalysisError e) => e.errorCode),

pkg/analyzer/lib/src/lint/options_rule_validator.dart

@@ -14,23 +14,18 @@ import 'package:collection/collection.dart';
 import 'package:pub_semver/pub_semver.dart';
 import 'package:yaml/yaml.dart';
 
-/// Rule provider.
-typedef LintRuleProvider = Iterable<LintRule> Function();
-
 /// Validates `linter` rule configurations.
 class LinterRuleOptionsValidator extends OptionsValidator {
   static const linter = 'linter';
   static const rulesKey = 'rules';
 
-  final LintRuleProvider ruleProvider;
   final VersionConstraint? sdkVersionConstraint;
   final bool sourceIsOptionsForContextRoot;
 
   LinterRuleOptionsValidator({
-    LintRuleProvider? provider,
     this.sdkVersionConstraint,
     this.sourceIsOptionsForContextRoot = true,
-  }) : ruleProvider = provider ?? (() => Registry.ruleRegistry.rules);
+  });
 
   bool currentSdkAllows(Version? since) {
     if (since == null) return true;
@@ -39,8 +34,8 @@ class LinterRuleOptionsValidator extends OptionsValidator {
     return sdk.allows(since);
   }
 
-  LintRule? getRegisteredLint(Object value) =>
-      ruleProvider().firstWhereOrNull((rule) => rule.name == value);
+  LintRule? getRegisteredLint(Object value) => Registry.ruleRegistry.rules
+      .firstWhereOrNull((rule) => rule.name == value);
 
   bool isDeprecatedInCurrentSdk(DeprecatedState state) =>
       currentSdkAllows(state.since);
@@ -52,13 +47,12 @@ class LinterRuleOptionsValidator extends OptionsValidator {
 
   @override
   List<AnalysisError> validate(ErrorReporter reporter, YamlMap options) {
-    List<AnalysisError> errors = <AnalysisError>[];
     var node = options.valueAt(linter);
     if (node is YamlMap) {
       var rules = node.valueAt(rulesKey);
       _validateRules(rules, reporter);
     }
-    return errors;
+    return const [];
   }
 
   void _validateRules(YamlNode? rules, ErrorReporter reporter) {

pkg/analyzer/lib/src/task/options.dart

@@ -26,9 +26,8 @@ List<AnalysisError> analyzeAnalysisOptions(
   String content,
   SourceFactory sourceFactory,
   String contextRoot,
-  VersionConstraint? sdkVersionConstraint, {
-  LintRuleProvider? provider,
-}) {
+  VersionConstraint? sdkVersionConstraint,
+) {
   List<AnalysisError> errors = [];
   Source initialSource = source;
   SourceSpan? initialIncludeSpan;
@@ -71,13 +70,12 @@ List<AnalysisError> analyzeAnalysisOptions(
   }
 
   // Validates the specified options and any included option files.
-  void validate(Source source, YamlMap options, LintRuleProvider? provider) {
+  void validate(Source source, YamlMap options) {
     var sourceIsOptionsForContextRoot = initialIncludeSpan == null;
     var validationErrors = OptionsFileValidator(
       source,
       sdkVersionConstraint: sdkVersionConstraint,
       sourceIsOptionsForContextRoot: sourceIsOptionsForContextRoot,
-      provider: provider,
     ).validate(options);
     addDirectErrorOrIncludedError(validationErrors, source,
         sourceIsOptionsForContextRoot: sourceIsOptionsForContextRoot);
@@ -87,7 +85,7 @@ List<AnalysisError> analyzeAnalysisOptions(
       // Validate the 'plugins' option in [options], understanding that no other
       // options are included.
       addDirectErrorOrIncludedError(
-          _validatePluginsOption(source, options: options), source,
+          _validateLegacyPluginsOption(source, options: options), source,
           sourceIsOptionsForContextRoot: sourceIsOptionsForContextRoot);
       return;
     }
@@ -145,12 +143,12 @@ List<AnalysisError> analyzeAnalysisOptions(
       try {
         var includedOptions =
             optionsProvider.getOptionsFromString(includedSource.contents.data);
-        validate(includedSource, includedOptions, provider);
+        validate(includedSource, includedOptions);
         firstPluginName ??= _firstPluginName(includedOptions);
         // Validate the 'plugins' option in [options], taking into account any
         // plugins enabled by [includedOptions].
         addDirectErrorOrIncludedError(
-          _validatePluginsOption(source,
+          _validateLegacyPluginsOption(source,
               options: options, firstEnabledPluginName: firstPluginName),
           source,
           sourceIsOptionsForContextRoot: sourceIsOptionsForContextRoot,
@@ -189,7 +187,7 @@ List<AnalysisError> analyzeAnalysisOptions(
 
   try {
     YamlMap options = optionsProvider.getOptionsFromString(content);
-    validate(source, options, provider);
+    validate(source, options);
   } on OptionsFormatException catch (e) {
     SourceSpan span = e.span!;
     errors.add(
@@ -205,8 +203,8 @@ List<AnalysisError> analyzeAnalysisOptions(
   return errors;
 }
 
-/// Returns the name of the first plugin, if one is specified in [options],
-/// otherwise `null`.
+/// Returns the name of the first legacy plugin, if one is specified in
+/// [options], otherwise `null`.
 String? _firstPluginName(YamlMap options) {
   var analyzerMap = options.valueAt(AnalyzerOptions.analyzer);
   if (analyzerMap is! YamlMap) {
@@ -224,9 +222,9 @@ String? _firstPluginName(YamlMap options) {
   }
 }
 
-/// Validates the 'plugins' options in [options], given
+/// Validates the legacy 'plugins' options in [options], given
 /// [firstEnabledPluginName].
-List<AnalysisError> _validatePluginsOption(
+List<AnalysisError> _validateLegacyPluginsOption(
   Source source, {
   required YamlMap options,
   String? firstEnabledPluginName,
@@ -365,16 +363,14 @@ class OptionsFileValidator {
 
   OptionsFileValidator(
     this._source, {
-    required VersionConstraint? sdkVersionConstraint,
+    VersionConstraint? sdkVersionConstraint,
     required bool sourceIsOptionsForContextRoot,
-    LintRuleProvider? provider,
   }) : _validators = [
           AnalyzerOptionsValidator(),
           _CodeStyleOptionsValidator(),
           _FormatterOptionsValidator(),
           _LinterOptionsValidator(),
           LinterRuleOptionsValidator(
-            provider: provider,
             sdkVersionConstraint: sdkVersionConstraint,
             sourceIsOptionsForContextRoot: sourceIsOptionsForContextRoot,
           ),

pkg/analyzer/test/src/diagnostics/analysis_options/analysis_options_test_support.dart

@@ -6,7 +6,6 @@ import 'package:_fe_analyzer_shared/src/base/errors.dart';
 import 'package:analyzer/src/context/source.dart';
 import 'package:analyzer/src/file_system/file_system.dart';
 import 'package:analyzer/src/generated/source.dart';
-import 'package:analyzer/src/lint/options_rule_validator.dart';
 import 'package:analyzer/src/task/options.dart';
 import 'package:analyzer/src/test_utilities/resource_provider_mixin.dart';
 import 'package:pub_semver/pub_semver.dart';
@@ -19,10 +18,7 @@ abstract class AbstractAnalysisOptionsTest with ResourceProviderMixin {
   VersionConstraint? get sdkVersionConstraint => null;
 
   Future<void> assertErrorsInCode(
-    String code,
-    List<ExpectedError> expectedErrors, {
-    LintRuleProvider? provider,
-  }) async {
+      String code, List<ExpectedError> expectedErrors) async {
     var path = convertPath('/analysis_options.yaml');
     newFile(path, code);
     var diagnostics = analyzeAnalysisOptions(
@@ -31,7 +27,6 @@ abstract class AbstractAnalysisOptionsTest with ResourceProviderMixin {
       sourceFactory,
       '/',
       sdkVersionConstraint,
-      provider: provider,
     );
     var errorListener = GatheringErrorListener();
     errorListener.addAll(diagnostics);