analyzer plugins: change rules options key to diagnostics

Also allow warning diagnostics to be disabled.

In many places, the RuleConfig is unchanged because currently the
`linter/rules` section parsing is shared with the `plugin/<plugin-name>/diagnostics` section parsing.

They will probably diverge at some point, but as long as they are the
same, we can use the same Registry class to identify enabled and
disabled rules/diagnostics.

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

Author: srawlins

pkg/analysis_server_plugin/lib/src/plugin_server.dart

@@ -257,7 +257,8 @@ class PluginServer {
     for (var configuration in analysisOptions.pluginConfigurations) {
       if (!configuration.isEnabled) continue;
       // TODO(srawlins): Namespace rules by their plugin, to avoid collisions.
-      var rules = Registry.ruleRegistry.enabled(configuration.ruleConfigs);
+      var rules =
+          Registry.ruleRegistry.enabled(configuration.diagnosticConfigs);
       for (var rule in rules) {
         rule.reporter = errorReporter;
         var timer = enableTiming ? analysisRuleTimers.getTimer(rule) : null;

pkg/analysis_server_plugin/test/src/plugin_server_error_test.dart

@@ -36,7 +36,7 @@ class PluginServerErrorTest extends PluginServerTestBase {
     newAnalysisOptionsYamlFile(packagePath, '''
 plugins:
   no_bools:
-    rules:
+    diagnostics:
       - no_bools
 ''');
   }

pkg/analysis_server_plugin/test/src/plugin_server_test.dart

@@ -69,7 +69,7 @@ class PluginServerTest extends PluginServerTestBase {
     expect(fixes[0].fixes, hasLength(1));
   }
 
-  Future<void> test_lintRulesAreDisabledByDefault() async {
+  Future<void> test_lintDiagnosticsAreDisabledByDefault() async {
     writeAnalysisOptionsWithPlugin();
     newFile(filePath, 'double x = 3.14;');
     await channel
@@ -81,7 +81,7 @@ class PluginServerTest extends PluginServerTestBase {
     expect(params.errors, isEmpty);
   }
 
-  Future<void> test_lintRulesCanBeEnabled() async {
+  Future<void> test_lintDiagnosticsCanBeEnabled() async {
     writeAnalysisOptionsWithPlugin({'no_doubles': true});
     newFile(filePath, 'double x = 3.14;');
     await channel
@@ -169,7 +169,7 @@ class PluginServerTest extends PluginServerTestBase {
     _expectAnalysisError(params.errors.single, message: 'No bools message');
   }
 
-  Future<void> test_warningRulesAreEnabledByDefault() async {
+  Future<void> test_warningDiagnosticsAreEnabledByDefault() async {
     writeAnalysisOptionsWithPlugin();
     newFile(filePath, 'bool b = false;');
     await channel
@@ -182,9 +182,7 @@ class PluginServerTest extends PluginServerTestBase {
     _expectAnalysisError(params.errors.single, message: 'No bools message');
   }
 
-  Future<void> test_warningRulesCannotBeDisabled() async {
-    // TODO(srawlins): A warning should be reported in the analysis options file
-    // for this.
+  Future<void> test_warningDiagnosticsCanBeDisabled() async {
     writeAnalysisOptionsWithPlugin({'no_bools': false});
     newFile(filePath, 'bool b = false;');
     await channel
@@ -193,19 +191,19 @@ class PluginServerTest extends PluginServerTestBase {
         .map((n) => protocol.AnalysisErrorsParams.fromNotification(n))
         .where((p) => p.file == filePath));
     var params = await paramsQueue.next;
-    _expectAnalysisError(params.errors.single, message: 'No bools message');
+    expect(params.errors, isEmpty);
   }
 
   void writeAnalysisOptionsWithPlugin(
-      [Map<String, bool> ruleConfiguration = const {}]) {
+      [Map<String, bool> diagnosticConfiguration = const {}]) {
     var buffer = StringBuffer('''
 plugins:
   no_literals:
-    rules:
+    diagnostics:
 ''');
-    for (var MapEntry(key: ruleName, value: isEnabled)
-        in ruleConfiguration.entries) {
-      buffer.writeln('      $ruleName: $isEnabled');
+    for (var MapEntry(key: diagnosticName, value: isEnabled)
+        in diagnosticConfiguration.entries) {
+      buffer.writeln('      $diagnosticName: $isEnabled');
     }
     newAnalysisOptionsYamlFile(packagePath, buffer.toString());
   }

pkg/analyzer/lib/dart/analysis/analysis_options.dart

@@ -79,15 +79,15 @@ final class PluginConfiguration {
   /// The name of the plugin being configured.
   final String name;
 
-  /// The list of specified [RuleConfig]s.
-  final Map<String, RuleConfig> ruleConfigs;
+  /// The list of specified [DiagnosticConfig]s.
+  final Map<String, DiagnosticConfig> diagnosticConfigs;
 
   /// Whether the plugin is enabled.
   final bool isEnabled;
 
   PluginConfiguration({
     required this.name,
-    required this.ruleConfigs,
+    required this.diagnosticConfigs,
     required this.isEnabled,
   });
 }

pkg/analyzer/lib/src/dart/analysis/analysis_options.dart

@@ -209,24 +209,24 @@ final class AnalysisOptionsBuilder {
         // TODO(srawlins): There may be value in storing the version constraint,
         // `value`.
         pluginConfigurations.add(PluginConfiguration(
-            name: pluginName, ruleConfigs: const {}, isEnabled: true));
+            name: pluginName, diagnosticConfigs: const {}, isEnabled: true));
         return;
       }
 
       if (pluginNode is! YamlMap) {
         return;
       }
 
-      var rules = pluginNode.valueAt(AnalyzerOptions.rules);
-      if (rules == null) {
+      var diagnostics = pluginNode.valueAt(AnalyzerOptions.diagnostics);
+      if (diagnostics == null) {
         return;
       }
 
-      var ruleConfigurations = parseRulesSection(rules);
+      var diagnosticConfigurations = parseDiagnosticsSection(diagnostics);
 
       pluginConfigurations.add(PluginConfiguration(
         name: pluginName,
-        ruleConfigs: ruleConfigurations,
+        diagnosticConfigs: diagnosticConfigurations,
         // TODO(srawlins): Implement `enabled: false`.
         isEnabled: true,
       ));

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

@@ -6,24 +6,9 @@ import 'package:analyzer/src/task/options.dart';
 import 'package:analyzer/src/util/yaml.dart';
 import 'package:yaml/yaml.dart';
 
-/// Parses [optionsMap] into [RuleConfig]s mapped from their names, returning
-/// them, or `null` if [optionsMap] does not have `linter` map.
-Map<String, RuleConfig>? parseLinterSection(YamlMap optionsMap) {
-  var options = optionsMap.valueAt('linter');
-  // Quick check of basic contract.
-  if (options is YamlMap) {
-    var rulesNode = options.valueAt(AnalyzerOptions.rules);
-    return {
-      if (rulesNode != null) ...parseRulesSection(rulesNode),
-    };
-  }
-
-  return null;
-}
-
 /// Returns the [RuleConfig]s that are parsed from [value], which can be either
 /// a YAML list or a YAML map, mapped from each rule's name.
-Map<String, RuleConfig> parseRulesSection(YamlNode value) {
+Map<String, RuleConfig> parseDiagnosticsSection(YamlNode value) {
   // For example:
   //
   // ```yaml
@@ -72,6 +57,21 @@ Map<String, RuleConfig> parseRulesSection(YamlNode value) {
   return ruleConfigs;
 }
 
+/// Parses [optionsMap] into [RuleConfig]s mapped from their names, returning
+/// them, or `null` if [optionsMap] does not have `linter` map.
+Map<String, RuleConfig>? parseLinterSection(YamlMap optionsMap) {
+  var options = optionsMap.valueAt('linter');
+  // Quick check of basic contract.
+  if (options is YamlMap) {
+    var rulesNode = options.valueAt(AnalyzerOptions.rules);
+    return {
+      if (rulesNode != null) ...parseDiagnosticsSection(rulesNode),
+    };
+  }
+
+  return null;
+}
+
 RuleConfig? _parseRuleConfig(dynamic configKey, YamlNode configNode,
     {String? group}) {
   // For example: `{unnecessary_getters: false}`.
@@ -84,6 +84,15 @@ RuleConfig? _parseRuleConfig(dynamic configKey, YamlNode configNode,
   return null;
 }
 
+/// An alias for a [RuleConfig], but which is configured under a 'diagnostics'
+/// key in an analysis options file.
+///
+/// In an analyzer plugin, diagnostics are enabled and disabled via their name.
+/// (For the built-in lint diagnostics, which are configured in an analysis
+/// options file's top-level 'linter' key, diagnostics are enabled and disabled
+/// via the name of the lint rule that reports the diagnostic.)
+typedef DiagnosticConfig = RuleConfig;
+
 /// The configuration of a single analysis rule within an analysis options file.
 class RuleConfig {
   /// Whether this rule is enabled or disabled in this configuration.

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

@@ -48,8 +48,9 @@ class Registry with IterableMixin<AnalysisRule> {
   ///
   /// enables `my_rule`.
   Iterable<AnalysisRule> enabled(Map<String, RuleConfig> ruleConfigs) => [
-        // All warning rules (which cannot be disabled).
-        ..._warningRules.values,
+        // All warning rules that haven't explicitly been disabled.
+        ..._warningRules.values
+            .where((rule) => ruleConfigs[rule.name]?.isEnabled ?? true),
         // All lint rules that have explicitly been enabled.
         ..._lintRules.values
             .where((rule) => ruleConfigs[rule.name]?.isEnabled ?? false),

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

@@ -283,6 +283,9 @@ class AnalyzerOptions {
   // Linter options.
   static const String rules = 'rules';
 
+  /// Plugin options.
+  static const String diagnostics = 'diagnostics';
+
   static const String propagateLinterExceptions = 'propagate-linter-exceptions';
 
   /// Ways to say `true` or `false`.