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>
diff --git a/pkg/analyzer/lib/src/dart/analysis/library_analyzer.dart b/pkg/analyzer/lib/src/dart/analysis/library_analyzer.dart index b5bdf0d..a0caf52 100644 --- a/pkg/analyzer/lib/src/dart/analysis/library_analyzer.dart +++ b/pkg/analyzer/lib/src/dart/analysis/library_analyzer.dart
@@ -416,16 +416,7 @@ 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();
diff --git a/pkg/analyzer/lib/src/dart/micro/library_analyzer.dart b/pkg/analyzer/lib/src/dart/micro/library_analyzer.dart index 359d754..2b2efd4 100644 --- a/pkg/analyzer/lib/src/dart/micro/library_analyzer.dart +++ b/pkg/analyzer/lib/src/dart/micro/library_analyzer.dart
@@ -413,7 +413,7 @@ 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();
diff --git a/pkg/analyzer/lib/src/error/ignore_validator.dart b/pkg/analyzer/lib/src/error/ignore_validator.dart index 5b9844a..0c0d4ec 100644 --- a/pkg/analyzer/lib/src/error/ignore_validator.dart +++ b/pkg/analyzer/lib/src/error/ignore_validator.dart
@@ -46,28 +46,44 @@ // 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 @@ /// 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 @@ // 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 List<DiagnosticName> { +extension on List<IgnoredElement> { void removeByName(String name) { - removeWhere((ignoredName) => ignoredName.name == name); + removeWhere((ignoredElement) => + ignoredElement is DiagnosticName && ignoredElement.name == name); } }
diff --git a/pkg/analyzer/lib/src/ignore_comments/ignore_info.dart b/pkg/analyzer/lib/src/ignore_comments/ignore_info.dart index b2e5b12..4b37ce5 100644 --- a/pkg/analyzer/lib/src/ignore_comments/ignore_info.dart +++ b/pkg/analyzer/lib/src/ignore_comments/ignore_info.dart
@@ -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 @@ 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 @@ 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 @@ } _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 @@ /// 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 @@ /// 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; }
diff --git a/pkg/analyzer/test/generated/error_suppression_test.dart b/pkg/analyzer/test/generated/error_suppression_test.dart index b7a1e9c..01bcf82 100644 --- a/pkg/analyzer/test/generated/error_suppression_test.dart +++ b/pkg/analyzer/test/generated/error_suppression_test.dart
@@ -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 @@ @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 @@ ]); } + 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',
diff --git a/pkg/analyzer/test/src/diagnostics/duplicate_ignore_test.dart b/pkg/analyzer/test/src/diagnostics/duplicate_ignore_test.dart index 66d84e5..98eace4 100644 --- a/pkg/analyzer/test/src/diagnostics/duplicate_ignore_test.dart +++ b/pkg/analyzer/test/src/diagnostics/duplicate_ignore_test.dart
@@ -15,7 +15,18 @@ @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 @@ ]); } - test_line() async { + test_name_line() async { await assertErrorsInCode(r''' void f() { // ignore: unused_local_variable, unused_local_variable @@ -37,7 +48,7 @@ ]); } - test_lineAndFile() async { + test_name_lineAndFile() async { await assertErrorsInCode(r''' // ignore_for_file: unused_local_variable void f() { @@ -48,4 +59,34 @@ 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), + ]); + } }