DAS plugins: Support plugin-namespaced inline ignore comments

Fixes #59647

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

Author: srawlins

pkg/analysis_server_plugin/lib/src/plugin_server.dart

@@ -26,6 +26,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/analysis/session.dart';
 import 'package:analyzer/src/dart/element/type_system.dart';
+import 'package:analyzer/src/ignore_comments/ignore_info.dart';
 import 'package:analyzer/src/lint/linter.dart';
 import 'package:analyzer/src/lint/linter_visitor.dart';
 import 'package:analyzer/src/lint/registry.dart';
@@ -217,7 +218,7 @@ class PluginServer {
   }) async {
     var file = _resourceProvider.getFile(path);
     var analysisOptions = analysisContext.getAnalysisOptionsForFile(file);
-    var diagnostics = await _computeLints(
+    var diagnostics = await _computeDiagnostics(
       analysisContext,
       path,
       analysisOptions: analysisOptions as AnalysisOptionsImpl,
@@ -244,19 +245,19 @@ class PluginServer {
     }
   }
 
-  Future<List<protocol.AnalysisError>> _computeLints(
+  Future<List<protocol.AnalysisError>> _computeDiagnostics(
     AnalysisContext analysisContext,
     String path, {
     required AnalysisOptionsImpl analysisOptions,
   }) async {
     var libraryResult =
         await analysisContext.currentSession.getResolvedLibrary(path);
     if (libraryResult is! ResolvedLibraryResult) {
-      return [];
+      return const [];
     }
     var unitResult = await analysisContext.currentSession.getResolvedUnit(path);
     if (unitResult is! ResolvedUnitResult) {
-      return [];
+      return const [];
     }
     var listener = RecordingErrorListener();
     var errorReporter = ErrorReporter(
@@ -293,6 +294,9 @@ class PluginServer {
       null,
     );
 
+    // A mapping from each lint and warning code to its corresponding plugin.
+    var pluginCodeMapping = <String, String>{};
+
     for (var configuration in analysisOptions.pluginConfigurations) {
       if (!configuration.isEnabled) continue;
       // TODO(srawlins): Namespace rules by their plugin, to avoid collisions.
@@ -304,15 +308,33 @@ class PluginServer {
         // `benchhmark.dart` script does.
         rule.registerNodeProcessors(nodeRegistry, context);
       }
+      for (var code in rules.expand((r) => r.lintCodes)) {
+        var existingPlugin = pluginCodeMapping[code.name];
+        if (existingPlugin == null) {
+          pluginCodeMapping[code.name] = configuration.name;
+        }
+      }
     }
 
     context.currentUnit = currentUnit;
     currentUnit.unit.accept(
         AnalysisRuleVisitor(nodeRegistry, shouldPropagateExceptions: true));
+
+    var ignoreInfo = IgnoreInfo.forDart(unitResult.unit, unitResult.content);
+    var errors = 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
+        // as ignored.
+        return true;
+      }
+      return !ignoreInfo.ignored(e, pluginName: pluginName);
+    });
+
     // The list of the `AnalysisError`s and their associated
     // `protocol.AnalysisError`s.
     var errorsAndProtocolErrors = [
-      for (var e in listener.errors)
+      for (var e in errors)
         (
           error: e,
           protocolError: protocol.AnalysisError(

pkg/analysis_server_plugin/pubspec.yaml

@@ -7,7 +7,7 @@ environment:
   sdk: '>=3.3.0 <4.0.0'
 
 dependencies:
-  analyzer: ^7.2.0
+  analyzer: ^7.3.0
   analyzer_plugin: ^0.13.0
   meta: ^1.16.0
 

pkg/analysis_server_plugin/test/src/plugin_server_test.dart

@@ -48,6 +48,33 @@ class PluginServerTest extends PluginServerTestBase {
     await startPlugin();
   }
 
+  Future<void> test_diagnosticsCanBeIgnored() async {
+    writeAnalysisOptionsWithPlugin();
+    newFile(filePath, '''
+// ignore: no_literals/no_bools
+bool b = false;
+''');
+    await channel
+        .sendRequest(protocol.AnalysisSetContextRootsParams([contextRoot]));
+    var paramsQueue = _analysisErrorsParams;
+    var params = await paramsQueue.next;
+    expect(params.errors, isEmpty);
+  }
+
+  Future<void> test_diagnosticsCanBeIgnored_forFile() async {
+    writeAnalysisOptionsWithPlugin();
+    newFile(filePath, '''
+bool b = false;
+
+// ignore_for_file: no_literals/no_bools
+''');
+    await channel
+        .sendRequest(protocol.AnalysisSetContextRootsParams([contextRoot]));
+    var paramsQueue = _analysisErrorsParams;
+    var params = await paramsQueue.next;
+    expect(params.errors, isEmpty);
+  }
+
   Future<void> test_handleAnalysisSetContextRoots() async {
     writeAnalysisOptionsWithPlugin();
     newFile(filePath, 'bool b = false;');

pkg/analyzer/lib/src/ignore_comments/ignore_info.dart

@@ -18,20 +18,26 @@ class IgnoredDiagnosticComment implements IgnoredElement {
   IgnoredDiagnosticComment(this.text, this.offset);
 
   @override
-  bool matches(ErrorCode errorCode) => false;
+  bool _matches(ErrorCode errorCode, {String? pluginName}) => false;
 }
 
 /// The name and location of a diagnostic name in an ignore comment.
 class IgnoredDiagnosticName implements IgnoredElement {
   /// The name of the diagnostic being ignored.
   final String name;
 
+  final String? pluginName;
+
   final int offset;
 
-  IgnoredDiagnosticName(String name, this.offset) : name = name.toLowerCase();
+  IgnoredDiagnosticName(String name, this.offset, {this.pluginName})
+      : name = name.toLowerCase();
 
   @override
-  bool matches(ErrorCode errorCode) {
+  bool _matches(ErrorCode errorCode, {String? pluginName}) {
+    if (this.pluginName != pluginName) {
+      return false;
+    }
     if (name == errorCode.name.toLowerCase()) {
       return true;
     }
@@ -56,7 +62,8 @@ class IgnoredDiagnosticType implements IgnoredElement {
       : type = type.toLowerCase();
 
   @override
-  bool matches(ErrorCode errorCode) {
+  bool _matches(ErrorCode errorCode, {String? pluginName}) {
+    // Ignore 'pluginName'; it is irrelevant in an IgnoredDiagnosticType.
     return switch (errorCode.type) {
       ErrorType.HINT => type == 'hint',
       ErrorType.LINT => type == 'lint',
@@ -69,7 +76,7 @@ class IgnoredDiagnosticType implements IgnoredElement {
 
 sealed class IgnoredElement {
   /// Returns whether this matches the given [errorCode].
-  bool matches(ErrorCode errorCode);
+  bool _matches(ErrorCode errorCode, {String? pluginName});
 }
 
 /// Information about analysis `//ignore:` and `//ignore_for_file:` comments
@@ -178,24 +185,27 @@ class IgnoreInfo {
     return ignoredOnLine;
   }
 
-  bool ignored(AnalysisError error) {
+  /// Whether [error] is ignored via an inline "ignore" comment.
+  bool ignored(AnalysisError error, {String? pluginName}) {
     var line = _lineInfo.getLocation(error.offset).lineNumber;
-    return ignoredAt(error.errorCode, line);
+    return _ignoredAt(error.errorCode, line, pluginName: pluginName);
   }
 
   /// Returns whether the [errorCode] is ignored at the given [line].
-  bool ignoredAt(ErrorCode errorCode, int line) {
+  bool _ignoredAt(ErrorCode errorCode, int line, {String? pluginName}) {
     var ignoredDiagnostics = _ignoredOnLine[line];
     if (ignoredForFile.isEmpty && ignoredDiagnostics == null) {
       return false;
     }
-    if (ignoredForFile.any((name) => name.matches(errorCode))) {
+    if (ignoredForFile
+        .any((name) => name._matches(errorCode, pluginName: pluginName))) {
       return true;
     }
     if (ignoredDiagnostics == null) {
       return false;
     }
-    return ignoredDiagnostics.any((name) => name.matches(errorCode));
+    return ignoredDiagnostics
+        .any((name) => name._matches(errorCode, pluginName: pluginName));
   }
 }
 
@@ -259,8 +269,8 @@ extension CommentTokenExtension on CommentToken {
         // Parse diagnostic type.
         skipPastWhitespace();
         if (offset == lexeme.length) return;
-        var equalSign = lexeme.codeUnitAt(offset);
-        if (equalSign != 0x3D) return;
+        var nextChar = lexeme.codeUnitAt(offset);
+        if (!nextChar.isEqual) return;
         offset++;
         skipPastWhitespace();
         if (offset == lexeme.length) return;
@@ -293,6 +303,30 @@ extension CommentTokenExtension on CommentToken {
         yield IgnoredDiagnosticType(
             type, this.offset + wordOffset, offset - wordOffset);
       } else {
+        String? pluginName;
+        if (offset < lexeme.length) {
+          var nextChar = lexeme.codeUnitAt(offset);
+          if (nextChar.isSlash) {
+            // We may be looking at a plugin-name-prefixed code, like
+            // 'plugin_one/foo'.
+            pluginName = word;
+            offset++;
+            if (offset == lexeme.length) return;
+            var nameOffset = offset;
+            readWord();
+            word = lexeme.substring(nameOffset, offset);
+            if (nameOffset == offset) {
+              // There is a non-word (other characters) at `offset`.
+              if (hasIgnoredElements) {
+                yield IgnoredDiagnosticComment(
+                  lexeme.substring(offset),
+                  this.offset + nameOffset,
+                );
+              }
+              return;
+            }
+          }
+        }
         if (offset < lexeme.length) {
           var nextChar = lexeme.codeUnitAt(offset);
           if (!nextChar.isSpace && !nextChar.isComma) {
@@ -306,7 +340,8 @@ extension CommentTokenExtension on CommentToken {
           }
         }
         hasIgnoredElements = true;
-        yield IgnoredDiagnosticName(word, this.offset + wordOffset);
+        yield IgnoredDiagnosticName(word, this.offset + wordOffset,
+            pluginName: pluginName);
       }
 
       if (offset == lexeme.length) return;

pkg/analyzer/lib/src/utilities/extensions/string.dart

@@ -11,13 +11,17 @@ extension IntExtension on int {
   /// character.
   bool get isEOL => this == 0x0D || this == 0x0A;
 
+  bool get isEqual => this == 0x3D;
+
   bool get isLetter =>
       (this >= 0x41 && this <= 0x5A) || (this >= 0x61 && this <= 0x7A);
 
   bool get isLetterOrDigit => isLetter || isDigit;
 
   bool get isLetterOrDigitOrUnderscore => isLetter || isDigit || this == 0x5F;
 
+  bool get isSlash => this == 0x2F;
+
   /// Whether this, as an ASCII character, is a space or tab character.
   bool get isSpace => this == 0x20 || this == 0x09;
 

pkg/analyzer/test/src/ignore_comments/ignore_info_test.dart

@@ -35,6 +35,11 @@ class IgnoreInfoTest extends PubPackageResolutionTest {
     expect(ignoredElements, isEmpty);
   }
 
+  test_noIgnores_trailingSlash() async {
+    var ignoredElements = await _parseIgnoredElements('// ignore: /');
+    expect(ignoredElements, isEmpty);
+  }
+
   test_noIgnores_trailingWhitespace() async {
     var ignoredElements = await _parseIgnoredElements('// ignore: ');
     expect(ignoredElements, isEmpty);
@@ -59,6 +64,24 @@ class IgnoreInfoTest extends PubPackageResolutionTest {
     _expectIgnoredName(ignoredElements[0], name: 'foo', offset: 10);
   }
 
+  test_pluginName() async {
+    var ignoredElements =
+        await _parseIgnoredElements('// ignore: plugin_one/foo');
+    expect(ignoredElements, hasLength(1));
+    _expectIgnoredName(ignoredElements[0],
+        name: 'foo', offset: 11, pluginName: 'plugin_one');
+  }
+
+  test_pluginName_multiple() async {
+    var ignoredElements = await _parseIgnoredElements(
+        '// ignore: plugin_one/foo, plugin_two/bar');
+    expect(ignoredElements, hasLength(2));
+    _expectIgnoredName(ignoredElements[0],
+        name: 'foo', offset: 11, pluginName: 'plugin_one');
+    _expectIgnoredName(ignoredElements[1],
+        name: 'bar', offset: 27, pluginName: 'plugin_two');
+  }
+
   test_trailingComma() async {
     var ignoredElements = await _parseIgnoredElements('// ignore: foo,');
     expect(ignoredElements, hasLength(1));
@@ -78,6 +101,16 @@ class IgnoreInfoTest extends PubPackageResolutionTest {
     _expectIgnoredName(ignoredElements[0], name: 'foo', offset: 11);
   }
 
+  test_trailingSlash() async {
+    var ignoredElements = await _parseIgnoredElements('// ignore: foo/');
+    expect(ignoredElements, isEmpty);
+  }
+
+  test_trailingSlashAndSpace() async {
+    var ignoredElements = await _parseIgnoredElements('// ignore: foo/ ');
+    expect(ignoredElements, isEmpty);
+  }
+
   test_trailingSpace() async {
     var ignoredElements = await _parseIgnoredElements('// ignore: foo ');
     expect(ignoredElements, hasLength(1));
@@ -135,12 +168,14 @@ class IgnoreInfoTest extends PubPackageResolutionTest {
     IgnoredElement element, {
     required String name,
     required int offset,
+    String? pluginName,
   }) =>
       expect(
         element,
         isA<IgnoredDiagnosticName>()
             .having((e) => e.name, 'name', name)
-            .having((e) => e.offset, 'offset', offset),
+            .having((e) => e.offset, 'offset', offset)
+            .having((e) => e.pluginName, 'pluginName', pluginName),
       );
 
   void _expectIgnoredType(