Enable in-line ignores per 'type' such as 'lint'

This uses the class ErrorType which may be an outdated field on ErrorCode, but it works for a prototype.

Fixes https://github.com/dart-lang/sdk/issues/46957

Change-Id: I98de8faf7f3ad738e9e27b0d5dd5add531b78b77
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/211460
Commit-Queue: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Phil Quitslund <pquitslund@google.com>
This commit is contained in:
Sam Rawlins
2021-09-10 01:30:27 +00:00
committed by commit-bot@chromium.org
parent 73d90bd400
commit e319cfa6aa
6 changed files with 233 additions and 68 deletions
@@ -416,16 +416,7 @@ class LibraryAnalyzer {
return false;
}
int errorLine = lineInfo.getLocation(error.offset).lineNumber;
String name = code.name;
if (ignoreInfo.ignoredAt(name, errorLine)) {
return true;
}
String uniqueName = code.uniqueName;
int period = uniqueName.indexOf('.');
if (period >= 0) {
uniqueName = uniqueName.substring(period + 1);
}
return uniqueName != name && ignoreInfo.ignoredAt(uniqueName, errorLine);
return ignoreInfo.ignoredAt(code, errorLine);
}
return errors.where((AnalysisError e) => !isIgnored(e)).toList();
@@ -413,7 +413,7 @@ class LibraryAnalyzer {
bool isIgnored(AnalysisError error) {
int errorLine = lineInfo.getLocation(error.offset).lineNumber;
return ignoreInfo.ignoredAt(error.errorCode.name, errorLine);
return ignoreInfo.ignoredAt(error.errorCode, errorLine);
}
return errors.where((AnalysisError e) => !isIgnored(e)).toList();
@@ -46,28 +46,44 @@ class IgnoreValidator {
// Report and remove any un-ignorable or duplicated names.
//
var namesIgnoredForFile = <String>{};
var typesIgnoredForFile = <String>{};
var unignorable = <DiagnosticName>[];
var duplicated = <DiagnosticName>[];
for (var ignoredName in ignoredForFile) {
var name = ignoredName.name;
if (_unignorableNames.contains(name)) {
unignorable.add(ignoredName);
} else if (!namesIgnoredForFile.add(name)) {
duplicated.add(ignoredName);
var duplicated = <IgnoredElement>[];
for (var ignoredElement in ignoredForFile) {
if (ignoredElement is DiagnosticName) {
var name = ignoredElement.name;
if (_unignorableNames.contains(name)) {
unignorable.add(ignoredElement);
} else if (!namesIgnoredForFile.add(name)) {
duplicated.add(ignoredElement);
}
} else if (ignoredElement is DiagnosticType) {
if (!typesIgnoredForFile.add(ignoredElement.type)) {
duplicated.add(ignoredElement);
}
}
}
_reportUnknownAndDuplicateIgnores(unignorable, duplicated, ignoredForFile);
for (var ignoredOnLine in ignoredOnLineMap.values) {
var namedIgnoredOnLine = <String>{};
var unignorable = <DiagnosticName>[];
var duplicated = <DiagnosticName>[];
for (var ignoredName in ignoredOnLine) {
var name = ignoredName.name;
if (_unignorableNames.contains(name)) {
unignorable.add(ignoredName);
} else if (namesIgnoredForFile.contains(name) ||
!namedIgnoredOnLine.add(name)) {
duplicated.add(ignoredName);
var typesIgnoredOnLine = <String>{};
var unignorable = <IgnoredElement>[];
var duplicated = <IgnoredElement>[];
for (var ignoredElement in ignoredOnLine) {
if (ignoredElement is DiagnosticName) {
var name = ignoredElement.name;
if (_unignorableNames.contains(name)) {
unignorable.add(ignoredElement);
} else if (namesIgnoredForFile.contains(name) ||
!namedIgnoredOnLine.add(name)) {
duplicated.add(ignoredElement);
}
} else if (ignoredElement is DiagnosticType) {
var type = ignoredElement.type;
if (typesIgnoredForFile.contains(type) ||
!typesIgnoredOnLine.add(type)) {
duplicated.add(ignoredElement);
}
}
}
_reportUnknownAndDuplicateIgnores(unignorable, duplicated, ignoredOnLine);
@@ -96,8 +112,8 @@ class IgnoreValidator {
/// Report the names that are [unignorable] or [duplicated] and remove them
/// from the [list] of names from which they were extracted.
void _reportUnknownAndDuplicateIgnores(List<DiagnosticName> unignorable,
List<DiagnosticName> duplicated, List<DiagnosticName> list) {
void _reportUnknownAndDuplicateIgnores(List<IgnoredElement> unignorable,
List<IgnoredElement> duplicated, List<IgnoredElement> list) {
// TODO(brianwilkerson) Uncomment the code below after the unignorable
// ignores in the Flutter code base have been cleaned up.
// for (var unignorableName in unignorable) {
@@ -106,16 +122,26 @@ class IgnoreValidator {
// unignorableName.offset, name.length, [name]);
// list.remove(unignorableName);
// }
for (var ignoredName in duplicated) {
var name = ignoredName.name;
_errorReporter.reportErrorForOffset(
HintCode.DUPLICATE_IGNORE, ignoredName.offset, name.length, [name]);
list.remove(ignoredName);
for (var ignoredElement in duplicated) {
if (ignoredElement is DiagnosticName) {
var name = ignoredElement.name;
_errorReporter.reportErrorForOffset(HintCode.DUPLICATE_IGNORE,
ignoredElement.offset, name.length, [name]);
list.remove(ignoredElement);
} else if (ignoredElement is DiagnosticType) {
_errorReporter.reportErrorForOffset(
HintCode.DUPLICATE_IGNORE,
ignoredElement.offset,
ignoredElement.length,
[ignoredElement.type],
);
list.remove(ignoredElement);
}
}
}
/// Report the [ignoredNames] as being unnecessary.
void _reportUnnecessaryIgnores(List<DiagnosticName> ignoredNames) {
void _reportUnnecessaryIgnores(List<IgnoredElement> ignoredNames) {
// TODO(brianwilkerson) Uncomment the code below after the unnecessary
// ignores in the Flutter code base have been cleaned up.
// for (var ignoredName in ignoredNames) {
@@ -140,8 +166,9 @@ extension on AnalysisError {
}
}
extension on List<DiagnosticName> {
extension on List<IgnoredElement> {
void removeByName(String name) {
removeWhere((ignoredName) => ignoredName.name == name);
removeWhere((ignoredElement) =>
ignoredElement is DiagnosticName && ignoredElement.name == name);
}
}
@@ -4,14 +4,14 @@
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/ast/token.dart';
import 'package:analyzer/error/error.dart';
import 'package:analyzer/src/dart/ast/token.dart';
/// The name and location of a diagnostic name in an ignore comment.
class DiagnosticName {
class DiagnosticName implements IgnoredElement {
/// The name of the diagnostic being ignored.
final String name;
/// The offset of the diagnostic in the source file.
final int offset;
/// Initialize a newly created diagnostic name to have the given [name] and
@@ -19,10 +19,40 @@ class DiagnosticName {
DiagnosticName(this.name, this.offset);
/// Return `true` if this diagnostic name matches the given error code.
bool matches(String errorCode) => name == errorCode;
@override
bool matches(ErrorCode errorCode) {
if (name == errorCode.name.toLowerCase()) {
return true;
}
var uniqueName = errorCode.uniqueName;
var period = uniqueName.indexOf('.');
if (period >= 0) {
uniqueName = uniqueName.substring(period + 1);
}
return name == uniqueName.toLowerCase();
}
}
/// Information about analysis `//ignore:` and `//ignore_for_file` comments
class DiagnosticType implements IgnoredElement {
final String type;
final int offset;
final int length;
DiagnosticType(String type, this.offset, this.length)
: type = type.toLowerCase();
@override
bool matches(ErrorCode errorCode) =>
type == errorCode.type.name.toLowerCase();
}
abstract class IgnoredElement {
bool matches(ErrorCode errorCode);
}
/// Information about analysis `//ignore:` and `//ignore_for_file:` comments
/// within a source file.
class IgnoreInfo {
/// A regular expression for matching 'ignore' comments.
@@ -36,13 +66,13 @@ class IgnoreInfo {
static final RegExp IGNORE_FOR_FILE_MATCHER =
RegExp(r'//[ ]*ignore_for_file:');
/// A table mapping line numbers to the diagnostics that are ignored on that
/// line.
final Map<int, List<DiagnosticName>> _ignoredOnLine = {};
/// A table mapping line numbers to the elements (diagnostics and diagnostic
/// types) that are ignored on that line.
final Map<int, List<IgnoredElement>> _ignoredOnLine = {};
/// A list containing all of the diagnostics that are ignored for the whole
/// file.
final List<DiagnosticName> _ignoredForFile = [];
/// A list containing all of the elements (diagnostics and diagnostic types)
/// that are ignored for the whole file.
final List<IgnoredElement> _ignoredForFile = [];
/// Initialize a newly created instance of this class to represent the ignore
/// comments in the given compilation [unit].
@@ -62,9 +92,9 @@ class IgnoreInfo {
}
_ignoredOnLine
.putIfAbsent(lineNumber, () => [])
.addAll(comment.diagnosticNames);
.addAll(comment.ignoredElements);
} else if (lexeme.contains('ignore_for_file:')) {
_ignoredForFile.addAll(comment.diagnosticNames);
_ignoredForFile.addAll(comment.ignoredElements);
}
}
}
@@ -75,26 +105,24 @@ class IgnoreInfo {
/// Return a list containing all of the diagnostics that are ignored for the
/// whole file.
List<DiagnosticName> get ignoredForFile => _ignoredForFile.toList();
List<IgnoredElement> get ignoredForFile => _ignoredForFile.toList();
/// Return a table mapping line numbers to the diagnostics that are ignored on
/// that line.
Map<int, List<DiagnosticName>> get ignoredOnLine {
Map<int, List<DiagnosticName>> ignoredOnLine = {};
Map<int, List<IgnoredElement>> get ignoredOnLine {
Map<int, List<IgnoredElement>> ignoredOnLine = {};
for (var entry in _ignoredOnLine.entries) {
ignoredOnLine[entry.key] = entry.value.toList();
}
return ignoredOnLine;
}
/// Return `true` if the [errorCode] (case-insensitive) is ignored at the
/// given [line].
bool ignoredAt(String errorCode, int line) {
/// Return `true` if the [errorCode] is ignored at the given [line].
bool ignoredAt(ErrorCode errorCode, int line) {
var ignoredDiagnostics = _ignoredOnLine[line];
if (ignoredForFile.isEmpty && ignoredDiagnostics == null) {
return false;
}
errorCode = errorCode.toLowerCase();
if (ignoredForFile.any((name) => name.matches(errorCode))) {
return true;
}
@@ -135,21 +163,28 @@ extension on CommentToken {
/// more restrictive in this test.
static final _errorCodeNameRegExp = RegExp(r'^[a-zA-Z][_a-z0-9A-Z]*$');
static final _errorTypeRegExp =
RegExp(r'^type[ ]*=[ ]*lint', caseSensitive: false);
/// Return the diagnostic names contained in this comment, assuming that it is
/// a correctly formatted ignore comment.
Iterable<DiagnosticName> get diagnosticNames sync* {
bool isValidErrorCodeName(String text) {
return text.contains(_errorCodeNameRegExp);
}
Iterable<IgnoredElement> get ignoredElements sync* {
int offset = lexeme.indexOf(':') + 1;
var names = lexeme.substring(offset).split(',');
offset += this.offset;
for (var name in names) {
var trimmedName = name.trim();
if (trimmedName.isNotEmpty && isValidErrorCodeName(trimmedName)) {
var innerOffset = name.indexOf(trimmedName);
yield DiagnosticName(trimmedName.toLowerCase(), offset + innerOffset);
if (trimmedName.isNotEmpty) {
if (trimmedName.contains(_errorCodeNameRegExp)) {
var innerOffset = name.indexOf(trimmedName);
yield DiagnosticName(trimmedName.toLowerCase(), offset + innerOffset);
} else {
var match = _errorTypeRegExp.matchAsPrefix(trimmedName);
if (match != null) {
var innerOffset = name.indexOf(trimmedName);
yield DiagnosticType('lint', offset + innerOffset, name.length);
}
}
}
offset += name.length + 1;
}
@@ -2,7 +2,9 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
import 'package:analyzer/error/error.dart';
import 'package:analyzer/src/error/codes.dart';
import 'package:linter/src/rules/avoid_types_as_parameter_names.dart';
import 'package:test_reflective_loader/test_reflective_loader.dart';
import '../src/dart/resolution/context_collection_resolution.dart';
@@ -15,13 +17,26 @@ main() {
@reflectiveTest
class ErrorSuppressionTest extends PubPackageResolutionTest {
static final ErrorCode _lintCode = AvoidTypesAsParameterNames().lintCode;
String get ignoredCode => 'unused_element';
@override
void setUp() {
super.setUp();
writeTestPackageAnalysisOptionsFile(
AnalysisOptionsFileConfig(
experiments: experiments,
lints: ['avoid_types_as_parameter_names'],
),
);
}
test_error_code_mismatch() async {
await assertErrorsInCode('''
// ignore: $ignoredCode
int x = '';
int _y = 0; //CONST_INITIALIZED_WITH_NON_CONSTANT_VALUE
int _y = 0; //INVALID_ASSIGNMENT
''', [
error(CompileTimeErrorCode.INVALID_ASSIGNMENT, 34, 2),
error(HintCode.UNUSED_ELEMENT, 42, 2),
@@ -225,6 +240,62 @@ int y = (0 as int);
]);
}
test_type_ignore_badType() async {
await assertErrorsInCode('''
// ignore: type=wrong
void f(arg1(int)) {} // AVOID_TYPES_AS_PARAMETER_NAMES
''', [
error(_lintCode, 34, 3),
]);
}
test_type_ignore_match() async {
await assertNoErrorsInCode('''
// ignore: type=lint
void f(arg1(int)) {} // AVOID_TYPES_AS_PARAMETER_NAMES
''');
}
test_type_ignore_mismatch() async {
await assertErrorsInCode('''
// ignore: type=lint
int _x = 1;
''', [
error(HintCode.UNUSED_ELEMENT, 25, 2),
]);
}
test_type_ignoreForFile_match() async {
await assertNoErrorsInCode('''
// ignore_for_file: type=lint
void f(arg1(int)) {} // AVOID_TYPES_AS_PARAMETER_NAMES
''');
}
test_type_ignoreForFile_match_upperCase() async {
await assertNoErrorsInCode('''
// ignore_for_file: TYPE=LINT
void f(arg1(int)) {} // AVOID_TYPES_AS_PARAMETER_NAMES
''');
}
test_type_ignoreForFile_match_withWhitespace() async {
await assertNoErrorsInCode('''
// ignore_for_file: type = lint
void f(arg1(int)) {} // AVOID_TYPES_AS_PARAMETER_NAMES
''');
}
test_type_ignoreForFile_mismatch() async {
await assertErrorsInCode('''
// ignore_for_file: type=lint
int a = 0;
int _x = 1;
''', [
error(HintCode.UNUSED_ELEMENT, 45, 2),
]);
}
test_undefined_function_within_flutter_can_be_ignored() async {
await assertErrorsInFile(
'$workspaceRootPath/flutterlib/flutter.dart',
@@ -15,7 +15,18 @@ main() {
@reflectiveTest
class DuplicateIgnoreTest extends PubPackageResolutionTest {
test_file() async {
@override
void setUp() {
super.setUp();
writeTestPackageAnalysisOptionsFile(
AnalysisOptionsFileConfig(
experiments: experiments,
lints: ['avoid_types_as_parameter_names'],
),
);
}
test_name_file() async {
await assertErrorsInCode(r'''
// ignore_for_file: unused_local_variable, unused_local_variable
void f() {
@@ -26,7 +37,7 @@ void f() {
]);
}
test_line() async {
test_name_line() async {
await assertErrorsInCode(r'''
void f() {
// ignore: unused_local_variable, unused_local_variable
@@ -37,7 +48,7 @@ void f() {
]);
}
test_lineAndFile() async {
test_name_lineAndFile() async {
await assertErrorsInCode(r'''
// ignore_for_file: unused_local_variable
void f() {
@@ -48,4 +59,34 @@ void f() {
error(HintCode.DUPLICATE_IGNORE, 66, 21),
]);
}
test_type_file() async {
await assertErrorsInCode(r'''
// ignore_for_file: type=lint, TYPE=LINT
void f(arg1(int)) {} // AVOID_TYPES_AS_PARAMETER_NAMES
''', [
error(HintCode.DUPLICATE_IGNORE, 31, 10),
]);
}
test_type_line() async {
await assertErrorsInCode(r'''
void f() {}
// ignore: type=lint, TYPE=LINT
void g(arg1(int)) {} // AVOID_TYPES_AS_PARAMETER_NAMES
''', [
error(HintCode.DUPLICATE_IGNORE, 34, 10),
]);
}
test_type_lineAndFile() async {
await assertErrorsInCode(r'''
// ignore_for_file: type=lint
void f() {}
// ignore: type=lint
void g(arg1(int)) {} // AVOID_TYPES_AS_PARAMETER_NAMES
''', [
error(HintCode.DUPLICATE_IGNORE, 53, 10),
]);
}
}