DAS plugins: Support plugin-namespaced inline ignore comments
Fixes https://github.com/dart-lang/sdk/issues/59647
Change-Id: I303c83c3a486d6eb495e890007127215357e2864
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/406686
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
diff --git a/pkg/analysis_server_plugin/lib/src/plugin_server.dart b/pkg/analysis_server_plugin/lib/src/plugin_server.dart
index dc4032b..778f25e 100644
--- a/pkg/analysis_server_plugin/lib/src/plugin_server.dart
+++ b/pkg/analysis_server_plugin/lib/src/plugin_server.dart
@@ -26,6 +26,7 @@
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 @@
}) 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,7 +245,7 @@
}
}
- Future<List<protocol.AnalysisError>> _computeLints(
+ Future<List<protocol.AnalysisError>> _computeDiagnostics(
AnalysisContext analysisContext,
String path, {
required AnalysisOptionsImpl analysisOptions,
@@ -252,11 +253,11 @@
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 @@
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 @@
// `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(
diff --git a/pkg/analysis_server_plugin/pubspec.yaml b/pkg/analysis_server_plugin/pubspec.yaml
index b871245..6871bdd 100644
--- a/pkg/analysis_server_plugin/pubspec.yaml
+++ b/pkg/analysis_server_plugin/pubspec.yaml
@@ -7,7 +7,7 @@
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
diff --git a/pkg/analysis_server_plugin/test/src/plugin_server_test.dart b/pkg/analysis_server_plugin/test/src/plugin_server_test.dart
index 61a7f7d..9c92c9b 100644
--- a/pkg/analysis_server_plugin/test/src/plugin_server_test.dart
+++ b/pkg/analysis_server_plugin/test/src/plugin_server_test.dart
@@ -48,6 +48,33 @@
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;');
diff --git a/pkg/analyzer/lib/src/ignore_comments/ignore_info.dart b/pkg/analyzer/lib/src/ignore_comments/ignore_info.dart
index 279a448..1e713f3 100644
--- a/pkg/analyzer/lib/src/ignore_comments/ignore_info.dart
+++ b/pkg/analyzer/lib/src/ignore_comments/ignore_info.dart
@@ -18,7 +18,7 @@
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.
@@ -26,12 +26,18 @@
/// 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 @@
: 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 @@
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 @@
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 @@
// 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 @@
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 @@
}
}
hasIgnoredElements = true;
- yield IgnoredDiagnosticName(word, this.offset + wordOffset);
+ yield IgnoredDiagnosticName(word, this.offset + wordOffset,
+ pluginName: pluginName);
}
if (offset == lexeme.length) return;
diff --git a/pkg/analyzer/lib/src/utilities/extensions/string.dart b/pkg/analyzer/lib/src/utilities/extensions/string.dart
index 93cb8c6..4d8bb29 100644
--- a/pkg/analyzer/lib/src/utilities/extensions/string.dart
+++ b/pkg/analyzer/lib/src/utilities/extensions/string.dart
@@ -11,6 +11,8 @@
/// character.
bool get isEOL => this == 0x0D || this == 0x0A;
+ bool get isEqual => this == 0x3D;
+
bool get isLetter =>
(this >= 0x41 && this <= 0x5A) || (this >= 0x61 && this <= 0x7A);
@@ -18,6 +20,8 @@
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;
diff --git a/pkg/analyzer/test/src/ignore_comments/ignore_info_test.dart b/pkg/analyzer/test/src/ignore_comments/ignore_info_test.dart
index 49a24d8..ec8d71d 100644
--- a/pkg/analyzer/test/src/ignore_comments/ignore_info_test.dart
+++ b/pkg/analyzer/test/src/ignore_comments/ignore_info_test.dart
@@ -35,6 +35,11 @@
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 @@
_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 @@
_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 @@
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(
