analyzer: allow LintCode instances to set their own severity

Work towards #53402

Letting plugin authors set the default severity of a lint code is a
top request. Users can still customize severities with analysis
options.

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

Author: srawlins

pkg/analysis_server_plugin/lib/src/plugin_server.dart

@@ -377,7 +377,7 @@ class PluginServer {
         AnalysisRuleVisitor(nodeRegistry, shouldPropagateExceptions: true));
 
     var ignoreInfo = IgnoreInfo.forDart(unitResult.unit, unitResult.content);
-    var errors = listener.errors.where((e) {
+    var diagnostics = listener.errors.where((e) {
       var pluginName = pluginCodeMapping[e.errorCode.name];
       if (pluginName == null) {
         // If [e] is somehow not mapped, something is wrong; but don't mark it
@@ -389,27 +389,40 @@ class PluginServer {
 
     // The list of the `AnalysisError`s and their associated
     // `protocol.AnalysisError`s.
-    var errorsAndProtocolErrors = [
-      for (var e in errors)
+    var diagnosticsAndProtocolErrors = [
+      for (var diagnostic in diagnostics)
         (
-          diagnostic: e,
+          diagnostic: diagnostic,
           protocolError: protocol.AnalysisError(
-            protocol.AnalysisErrorSeverity.INFO,
+            _severityOf(diagnostic),
             protocol.AnalysisErrorType.STATIC_WARNING,
-            _locationFor(currentUnit.unit, path, e),
-            e.message,
-            e.errorCode.name,
-            correction: e.correctionMessage,
+            _locationFor(currentUnit.unit, path, diagnostic),
+            diagnostic.message,
+            diagnostic.errorCode.name,
+            correction: diagnostic.correctionMessage,
             // TODO(srawlins): Use a valid value here.
             hasFix: true,
           )
         ),
     ];
     _recentState[path] = (
       analysisContext: analysisContext,
-      errors: [...errorsAndProtocolErrors],
+      errors: [...diagnosticsAndProtocolErrors],
     );
-    return errorsAndProtocolErrors.map((e) => e.protocolError).toList();
+    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;
+    }
   }
 
   /// Invokes [fn] first for priority analysis contexts, then for the rest.

pkg/analysis_server_plugin/test/src/lint_rules.dart

@@ -41,6 +41,27 @@ class NoDoublesRule extends AnalysisRule {
   }
 }
 
+class NoDoublesWarningRule extends AnalysisRule {
+  static const LintCode code = LintCode(
+    'no_doubles_warning',
+    'No doubles message',
+    severity: DiagnosticSeverity.WARNING,
+  );
+
+  NoDoublesWarningRule()
+      : super(name: 'no_doubles_warning', description: 'No doubles message');
+
+  @override
+  DiagnosticCode get diagnosticCode => code;
+
+  @override
+  void registerNodeProcessors(
+      NodeLintRegistry registry, LinterContext context) {
+    var visitor = _NoDoublesVisitor(this);
+    registry.addDoubleLiteral(this, visitor);
+  }
+}
+
 class _NoBoolsVisitor extends SimpleAstVisitor<void> {
   final AnalysisRule rule;
 

pkg/analysis_server_plugin/test/src/plugin_server_test.dart

@@ -144,7 +144,22 @@ bool b = false;
     expect(result.fixes.first.fixes, hasLength(4));
   }
 
-  Future<void> test_lintDiagnosticsAreDisabledByDefault() async {
+  Future<void> test_lintCodesCanHaveCustomSeverity() async {
+    writeAnalysisOptionsWithPlugin({'no_doubles_warning': true});
+    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.WARNING,
+    );
+  }
+
+  Future<void> test_lintRulesAreDisabledByDefault() async {
     writeAnalysisOptionsWithPlugin();
     newFile(filePath, 'double x = 3.14;');
     await channel
@@ -154,7 +169,7 @@ bool b = false;
     expect(params.errors, isEmpty);
   }
 
-  Future<void> test_lintDiagnosticsCanBeEnabled() async {
+  Future<void> test_lintRulesCanBeEnabled() async {
     writeAnalysisOptionsWithPlugin({'no_doubles': true});
     newFile(filePath, 'double x = 3.14;');
     await channel
@@ -234,7 +249,7 @@ bool b = false;
     _expectAnalysisError(params.errors.single, message: 'No bools message');
   }
 
-  Future<void> test_warningDiagnosticsAreEnabledByDefault() async {
+  Future<void> test_warningRulesAreEnabledByDefault() async {
     writeAnalysisOptionsWithPlugin();
     newFile(filePath, 'bool b = false;');
     await channel
@@ -245,7 +260,7 @@ bool b = false;
     _expectAnalysisError(params.errors.single, message: 'No bools message');
   }
 
-  Future<void> test_warningDiagnosticsCanBeDisabled() async {
+  Future<void> test_warningRulesCanBeDisabled() async {
     writeAnalysisOptionsWithPlugin({'no_bools': false});
     newFile(filePath, 'bool b = false;');
     await channel
@@ -270,13 +285,16 @@ plugins:
     newAnalysisOptionsYamlFile(packagePath, buffer.toString());
   }
 
-  void _expectAnalysisError(protocol.AnalysisError error,
-      {required String message}) {
+  void _expectAnalysisError(
+    protocol.AnalysisError error, {
+    required String message,
+    protocol.AnalysisErrorSeverity severity =
+        protocol.AnalysisErrorSeverity.INFO,
+  }) {
     expect(
       error,
       isA<protocol.AnalysisError>()
-          .having((e) => e.severity, 'severity',
-              protocol.AnalysisErrorSeverity.INFO)
+          .having((e) => e.severity, 'severity', severity)
           .having(
               (e) => e.type, 'type', protocol.AnalysisErrorType.STATIC_WARNING)
           .having((e) => e.message, 'message', message),
@@ -313,6 +331,7 @@ class _NoLiteralsPlugin extends Plugin {
   void register(PluginRegistry registry) {
     registry.registerWarningRule(NoBoolsRule());
     registry.registerLintRule(NoDoublesRule());
+    registry.registerLintRule(NoDoublesWarningRule());
     registry.registerFixForRule(NoBoolsRule.code, _WrapInQuotes.new);
     registry.registerAssist(_InvertBoolean.new);
   }

pkg/analyzer/api.txt

@@ -4436,7 +4436,7 @@ package:analyzer/error/error.dart:
     compareTo (method: int Function(DiagnosticType))
     toString (method: String Function())
   LintCode (class extends DiagnosticCode):
-    new (constructor: LintCode Function(String, String, {String? correctionMessage, bool hasPublishedDocs, String? uniqueName}))
+    new (constructor: LintCode Function(String, String, {String? correctionMessage, bool hasPublishedDocs, DiagnosticSeverity severity, String? uniqueName}))
     errorSeverity (getter: DiagnosticSeverity)
     hashCode (getter: int)
     type (getter: DiagnosticType)

pkg/analyzer/lib/src/dart/error/lint_codes.dart

@@ -8,29 +8,29 @@ library;
 import 'package:_fe_analyzer_shared/src/base/analyzer_public_api.dart';
 import 'package:analyzer/error/error.dart';
 
-/// Defines style and best practice recommendations.
+/// Diagnostic codes which are not reported by default.
 ///
-/// Unlike [WarningCode]s, which are akin to traditional static warnings from a
-/// compiler, lint recommendations focus on matters of avoiding errors,
-/// unintended code, maintainability, style and other best practices that might
-/// be aggregated to define a project's style guide.
+/// Lint codes are only reported when a lint rule (either a first-party lint
+/// rule, or one declared in an analyzer plugin) is enabled.
 @AnalyzerPublicApi(message: 'exported by lib/error/error.dart')
 class LintCode extends DiagnosticCode {
+  @override
+  final DiagnosticSeverity errorSeverity;
+
   const LintCode(
     String name,
     String problemMessage, {
     super.correctionMessage,
     super.hasPublishedDocs,
     String? uniqueName,
-  }) : super(
+    DiagnosticSeverity severity = DiagnosticSeverity.INFO,
+  }) : errorSeverity = severity,
+       super(
          problemMessage: problemMessage,
          name: name,
          uniqueName: uniqueName ?? 'LintCode.$name',
        );
 
-  @override
-  DiagnosticSeverity get errorSeverity => DiagnosticSeverity.INFO;
-
   @override
   int get hashCode => uniqueName.hashCode;