DAS plugins: Support configurable severity

This introduces a concept of a "configured severity," based on the
design at #57034, such that
an analysis rule can be configured in analysis options to be one of:
"disable," "enable," "info," "warning," or "error."

We actually don't have any validation of what is inside a plugin
section in analysis options; that should come next.

Closes #59644

Change-Id: I56f1f51ef01adee7e6c042d8df9a974e7e5a11a8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/443003
Reviewed-by: Konstantin Shcheglov <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Samuel Rawlins <[email protected]>

Author: srawlins

pkg/analysis_server_plugin/lib/src/plugin_server.dart

@@ -16,10 +16,12 @@ import 'package:analysis_server_plugin/src/registry.dart';
 import 'package:analyzer/analysis_rule/rule_context.dart';
 import 'package:analyzer/dart/analysis/analysis_context.dart';
 import 'package:analyzer/dart/analysis/analysis_context_collection.dart';
+import 'package:analyzer/dart/analysis/analysis_options.dart';
 import 'package:analyzer/dart/analysis/results.dart';
 import 'package:analyzer/dart/analysis/session.dart';
 import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/diagnostic/diagnostic.dart';
+import 'package:analyzer/error/error.dart';
 import 'package:analyzer/error/listener.dart';
 import 'package:analyzer/file_system/file_system.dart';
 import 'package:analyzer/file_system/overlay_file_system.dart';
@@ -30,6 +32,7 @@ import 'package:analyzer/src/dart/analysis/byte_store.dart';
 import 'package:analyzer/src/dart/analysis/file_content_cache.dart';
 import 'package:analyzer/src/dart/element/type_system.dart';
 import 'package:analyzer/src/ignore_comments/ignore_info.dart';
+import 'package:analyzer/src/lint/config.dart';
 import 'package:analyzer/src/lint/linter.dart';
 import 'package:analyzer/src/lint/linter_visitor.dart';
 import 'package:analyzer/src/lint/registry.dart';
@@ -351,8 +354,11 @@ class PluginServer {
       null,
     );
 
-    // A mapping from each lint and warning code to its corresponding plugin.
-    var pluginCodeMapping = <String, String>{};
+    // A mapping from each diagnostic code to its corresponding plugin.
+    var pluginCodeMapping = <DiagnosticCode, String>{};
+
+    // A mapping from each diagnostic code to its configured severity.
+    var severityMapping = <DiagnosticCode, protocol.AnalysisErrorSeverity?>{};
 
     for (var configuration in analysisOptions.pluginConfigurations) {
       if (!configuration.isEnabled) continue;
@@ -362,14 +368,13 @@ class PluginServer {
       for (var rule in rules) {
         rule.reporter = diagnosticReporter;
         // TODO(srawlins): Enable timing similar to what the linter package's
-        // `benchhmark.dart` script does.
+        // `benchmark.dart` script does.
         rule.registerNodeProcessors(nodeRegistry, context);
       }
       for (var code in rules.expand((r) => r.diagnosticCodes)) {
-        var existingPlugin = pluginCodeMapping[code.name];
-        if (existingPlugin == null) {
-          pluginCodeMapping[code.name] = configuration.name;
-        }
+        pluginCodeMapping.putIfAbsent(code, () => configuration.name);
+        severityMapping.putIfAbsent(
+            code, () => _configuredSeverity(configuration, code));
       }
     }
 
@@ -379,7 +384,7 @@ class PluginServer {
 
     var ignoreInfo = IgnoreInfo.forDart(unitResult.unit, unitResult.content);
     var diagnostics = listener.diagnostics.where((e) {
-      var pluginName = pluginCodeMapping[e.diagnosticCode.name];
+      var pluginName = pluginCodeMapping[e.diagnosticCode];
       if (pluginName == null) {
         // If [e] is somehow not mapped, something is wrong; but don't mark it
         // as ignored.
@@ -395,7 +400,8 @@ class PluginServer {
         (
           diagnostic: diagnostic,
           protocolError: protocol.AnalysisError(
-            _severityOf(diagnostic),
+            severityMapping[diagnostic.diagnosticCode] ??
+                protocol.AnalysisErrorSeverity.INFO,
             protocol.AnalysisErrorType.STATIC_WARNING,
             _locationFor(currentUnit.unit, path, diagnostic),
             diagnostic.message,
@@ -413,17 +419,27 @@ class PluginServer {
     return diagnosticsAndProtocolErrors.map((e) => e.protocolError).toList();
   }
 
-  /// Converts the severity of [diagnostic] into a
-  /// [protocol.AnalysisErrorSeverity].
-  protocol.AnalysisErrorSeverity _severityOf(Diagnostic diagnostic) {
-    try {
-      return protocol.AnalysisErrorSeverity.values
-          .byName(diagnostic.severity.name.toUpperCase());
-    } catch (_) {
-      assert(false, 'Invalid severity: ${diagnostic.severity}');
-      // Return the default severity of `LintCode`.
-      return protocol.AnalysisErrorSeverity.INFO;
+  /// Converts the severity of [code] into a [protocol.AnalysisErrorSeverity].
+  protocol.AnalysisErrorSeverity? _configuredSeverity(
+      PluginConfiguration configuration, DiagnosticCode code) {
+    var configuredSeverity =
+        configuration.diagnosticConfigs[code.name]?.severity;
+    if (configuredSeverity != null &&
+        configuredSeverity != ConfiguredSeverity.enable) {
+      var severityName = configuredSeverity.name.toUpperCase();
+      var severity =
+          protocol.AnalysisErrorSeverity.values.asNameMap()[severityName];
+      assert(severity != null,
+          'Invalid configured severity: ${configuredSeverity.name}');
+      return severity;
     }
+
+    // Fall back to the declared severity of [code].
+    var severityName = code.severity.name.toUpperCase();
+    var severity =
+        protocol.AnalysisErrorSeverity.values.asNameMap()[code.severity.name];
+    assert(severity != null, 'Invalid severity: $severityName');
+    return severity;
   }
 
   /// Invokes [fn] first for priority analysis contexts, then for the rest.

pkg/analysis_server_plugin/test/src/plugin_server_test.dart

@@ -146,7 +146,7 @@ bool b = false;
   }
 
   Future<void> test_lintCodesCanHaveCustomSeverity() async {
-    writeAnalysisOptionsWithPlugin({'no_doubles_warning': true});
+    writeAnalysisOptionsWithPlugin({'no_doubles_warning': 'enable'});
     newFile(filePath, 'double x = 3.14;');
     await channel
         .sendRequest(protocol.AnalysisSetContextRootsParams([contextRoot]));
@@ -160,6 +160,21 @@ bool b = false;
     );
   }
 
+  Future<void> test_lintCodesCanHaveConfigurableSeverity() async {
+    writeAnalysisOptionsWithPlugin({'no_doubles_warning': 'error'});
+    newFile(filePath, 'double x = 3.14;');
+    await channel
+        .sendRequest(protocol.AnalysisSetContextRootsParams([contextRoot]));
+    var paramsQueue = _analysisErrorsParams;
+    var params = await paramsQueue.next;
+    expect(params.errors, hasLength(1));
+    _expectAnalysisError(
+      params.errors.single,
+      message: 'No doubles message',
+      severity: protocol.AnalysisErrorSeverity.ERROR,
+    );
+  }
+
   Future<void> test_lintRulesAreDisabledByDefault() async {
     writeAnalysisOptionsWithPlugin();
     newFile(filePath, 'double x = 3.14;');
@@ -171,7 +186,7 @@ bool b = false;
   }
 
   Future<void> test_lintRulesCanBeEnabled() async {
-    writeAnalysisOptionsWithPlugin({'no_doubles': true});
+    writeAnalysisOptionsWithPlugin({'no_doubles': 'enable'});
     newFile(filePath, 'double x = 3.14;');
     await channel
         .sendRequest(protocol.AnalysisSetContextRootsParams([contextRoot]));
@@ -276,7 +291,7 @@ bool b = false;
   }
 
   Future<void> test_warningRulesCanBeDisabled() async {
-    writeAnalysisOptionsWithPlugin({'no_bools': false});
+    writeAnalysisOptionsWithPlugin({'no_bools': 'disable'});
     newFile(filePath, 'bool b = false;');
     await channel
         .sendRequest(protocol.AnalysisSetContextRootsParams([contextRoot]));
@@ -286,16 +301,16 @@ bool b = false;
   }
 
   void writeAnalysisOptionsWithPlugin(
-      [Map<String, bool> diagnosticConfiguration = const {}]) {
+      [Map<String, String> diagnosticConfiguration = const {}]) {
     var buffer = StringBuffer('''
 plugins:
   no_literals:
     path: some/path
     diagnostics:
 ''');
-    for (var MapEntry(key: diagnosticName, value: isEnabled)
+    for (var MapEntry(key: diagnosticName, value: enablement)
         in diagnosticConfiguration.entries) {
-      buffer.writeln('      $diagnosticName: $isEnabled');
+      buffer.writeln('      $diagnosticName: $enablement');
     }
     newAnalysisOptionsYamlFile(packagePath, buffer.toString());
   }

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

@@ -19,7 +19,10 @@ Map<String, RuleConfig> parseDiagnosticsSection(YamlNode value) {
     return {
       for (var ruleNode in value.nodes)
         if (ruleNode case YamlScalar(value: String ruleName))
-          ruleName: RuleConfig._(name: ruleName, isEnabled: true),
+          ruleName: RuleConfig._(
+            name: ruleName,
+            severity: ConfiguredSeverity.enable,
+          ),
     };
   }
 
@@ -81,7 +84,14 @@ RuleConfig? _parseRuleConfig(
   // For example: `{unnecessary_getters: false}`.
   if (configKey case YamlScalar(value: String ruleName)) {
     if (configNode case YamlScalar(value: bool isEnabled)) {
-      return RuleConfig._(name: ruleName, isEnabled: isEnabled, group: group);
+      var severity =
+          isEnabled ? ConfiguredSeverity.enable : ConfiguredSeverity.disable;
+      return RuleConfig._(name: ruleName, group: group, severity: severity);
+    } else if (configNode case YamlScalar(value: String severityString)) {
+      var severity =
+          ConfiguredSeverity.values.asNameMap()[severityString] ??
+          ConfiguredSeverity.enable;
+      return RuleConfig._(name: ruleName, group: group, severity: severity);
     }
   }
 
@@ -97,18 +107,39 @@ RuleConfig? _parseRuleConfig(
 /// via the name of the lint rule that reports the diagnostic.)
 typedef DiagnosticConfig = RuleConfig;
 
+/// The possible values for an analysis rule's configured severity.
+enum ConfiguredSeverity {
+  /// A severity indicating the rule is simply disabled.
+  disable,
+
+  /// A severity indicating the rule is enabled with its default severity.
+  enable,
+
+  /// A severity indicating the rule is enabled with the 'info' severity.
+  info,
+
+  /// A severity indicating the rule is enabled with the 'warning' severity.
+  warning,
+
+  /// A severity indicating the rule is enabled with the 'error' severity.
+  error,
+}
+
 /// The configuration of a single analysis rule within an analysis options file.
 class RuleConfig {
-  /// Whether this rule is enabled or disabled in this configuration.
-  final bool isEnabled;
-
   /// The name of the group under which this configuration is found.
   final String? group;
 
   /// The name of the rule.
   final String name;
 
-  RuleConfig._({required this.name, required this.isEnabled, this.group});
+  /// The rule's severity in this configuration.
+  final ConfiguredSeverity severity;
+
+  RuleConfig._({required this.name, this.group, required this.severity});
+
+  /// Whether this rule is enabled or disabled in this configuration.
+  bool get isEnabled => severity != ConfiguredSeverity.disable;
 
   /// Returns whether [ruleName] is disabled in this configuration.
   bool disables(String ruleName) => ruleName == name && !isEnabled;

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

@@ -13,9 +13,10 @@ class Registry with IterableMixin<AbstractAnalysisRule> {
   /// The default registry to be used by clients.
   static final Registry ruleRegistry = Registry();
 
-  /// A table mapping rule names to rules.
+  /// A table mapping lint rule names to rules.
   final Map<String, AbstractAnalysisRule> _lintRules = {};
 
+  /// A table mapping warning rule names to rules.
   final Map<String, AbstractAnalysisRule> _warningRules = {};
 
   /// A table mapping unique names to lint codes.