diff --git a/pkg/analysis_server/lib/src/services/correction/dart/remove_ignore.dart b/pkg/analysis_server/lib/src/services/correction/dart/remove_ignore.dart new file mode 100644 index 00000000000..63abdee6622 --- /dev/null +++ b/pkg/analysis_server/lib/src/services/correction/dart/remove_ignore.dart @@ -0,0 +1,84 @@ +// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file +// 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:analysis_server/src/services/correction/fix.dart'; +import 'package:analysis_server_plugin/edit/dart/correction_producer.dart'; +import 'package:analyzer/source/source_range.dart'; +import 'package:analyzer/src/dart/ast/token.dart'; +import 'package:analyzer/src/ignore_comments/ignore_info.dart'; +import 'package:analyzer/src/utilities/extensions/ast.dart'; +import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart'; +import 'package:analyzer_plugin/utilities/fixes/fixes.dart'; + +class RemoveIgnore extends ResolvedCorrectionProducer { + String _diagnosticName = ''; + RemoveIgnore({required super.context}); + + @override + CorrectionApplicability get applicability => + CorrectionApplicability.singleLocation; + + @override + List get fixArguments => [_diagnosticName]; + + @override + FixKind get fixKind => DartFixKind.REMOVE_IGNORED_DIAGNOSTIC; + + @override + Future compute(ChangeBuilder builder) async { + var diagnostic = this.diagnostic; + if (diagnostic == null) return; + + var diagnosticOffset = diagnostic.problemMessage.offset; + + var comment = node.commentTokenCovering(diagnosticOffset); + if (comment is! CommentToken) return; + + SourceRange? rangeToDelete; + + var ignoredElements = comment.ignoredElements.toList(); + + for (var (index, ignoredElement) in ignoredElements.indexed) { + if (ignoredElement is! IgnoredDiagnosticName) continue; + + var ignoredElementOffset = ignoredElement.offset; + if (ignoredElement.offset == diagnosticOffset) { + _diagnosticName = ignoredElement.name; + var scanBack = index != 0; + var commentText = comment.lexeme; + if (scanBack) { + // Scan back for a preceding comma: + for (var offset = ignoredElementOffset; offset > -1; --offset) { + if (commentText[offset] == ',') { + var backSteps = ignoredElementOffset - offset; + rangeToDelete = SourceRange( + diagnosticOffset - backSteps, + _diagnosticName.length + backSteps, + ); + break; + } + } + } else { + // Scan forward for a trailing comma: + var chars = commentText.substring(ignoredElementOffset).indexOf(','); + if (chars == -1) return; + + // Eat the comma + chars++; + + // Eat a trailing space if needed + if (commentText[ignoredElementOffset + chars] == ' ') chars++; + + rangeToDelete = SourceRange(ignoredElementOffset, chars); + } + } + } + + if (rangeToDelete != null) { + await builder.addDartFileEdit(file, (builder) { + builder.addDeletion(rangeToDelete!); + }); + } + } +} diff --git a/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml b/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml index 478783c3e33..963bc3347c1 100644 --- a/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml +++ b/pkg/analysis_server/lib/src/services/correction/error_fix_status.yaml @@ -2424,13 +2424,15 @@ LintCode.unnecessary_getters_setters: LintCode.unnecessary_ignore: status: needsFix notes: |- - Remove the ignore comment (or one code in the comment). + Remove the ignore comment. LintCode.unnecessary_ignore_file: status: needsFix + notes: |- + Remove the ignore comment. LintCode.unnecessary_ignore_name: - status: needsFix + status: hasFix LintCode.unnecessary_ignore_name_file: - status: needsFix + status: hasFix LintCode.unnecessary_lambdas: status: hasFix LintCode.unnecessary_late: diff --git a/pkg/analysis_server/lib/src/services/correction/fix.dart b/pkg/analysis_server/lib/src/services/correction/fix.dart index 7d090430041..190b7f24f81 100644 --- a/pkg/analysis_server/lib/src/services/correction/fix.dart +++ b/pkg/analysis_server/lib/src/services/correction/fix.dart @@ -1266,6 +1266,11 @@ abstract final class DartFixKind { DartFixKindPriority.inFile, "Remove unnecessary '??' operators everywhere in file", ); + static const REMOVE_IGNORED_DIAGNOSTIC = FixKind( + 'dart.fix.remove.ignored.diagnostic', + DartFixKindPriority.standard, + 'Remove {0}', + ); static const REMOVE_INVOCATION = FixKind( 'dart.fix.remove.invocation', DartFixKindPriority.standard, diff --git a/pkg/analysis_server/lib/src/services/correction/fix_internal.dart b/pkg/analysis_server/lib/src/services/correction/fix_internal.dart index 598075187fb..efafc75505a 100644 --- a/pkg/analysis_server/lib/src/services/correction/fix_internal.dart +++ b/pkg/analysis_server/lib/src/services/correction/fix_internal.dart @@ -146,6 +146,7 @@ import 'package:analysis_server/src/services/correction/dart/remove_empty_else.d import 'package:analysis_server/src/services/correction/dart/remove_empty_statement.dart'; import 'package:analysis_server/src/services/correction/dart/remove_extends_clause.dart'; import 'package:analysis_server/src/services/correction/dart/remove_if_null_operator.dart'; +import 'package:analysis_server/src/services/correction/dart/remove_ignore.dart'; import 'package:analysis_server/src/services/correction/dart/remove_initializer.dart'; import 'package:analysis_server/src/services/correction/dart/remove_interpolation_braces.dart'; import 'package:analysis_server/src/services/correction/dart/remove_invocation.dart'; @@ -485,6 +486,11 @@ final _builtInLintProducers = >{ LinterLintCode.unnecessary_final_with_type: [ReplaceFinalWithVar.new], LinterLintCode.unnecessary_final_without_type: [ReplaceFinalWithVar.new], LinterLintCode.unnecessary_getters_setters: [MakeFieldPublic.new], + LinterLintCode.unnecessary_ignore_name: [RemoveIgnore.new], + LinterLintCode.unnecessary_ignore_name_file: [RemoveIgnore.new], + // TODO(pq): add => + // LinterLintCode.unnecessary_ignore: [RemoveComment.new], + // LinterLintCode.unnecessary_ignore_file: [RemoveComment.new], LinterLintCode.unnecessary_lambdas: [ReplaceWithTearOff.new], LinterLintCode.unnecessary_late: [RemoveUnnecessaryLate.new], LinterLintCode.unnecessary_library_directive: [ diff --git a/pkg/analysis_server/test/src/services/correction/fix/remove_ignore_test.dart b/pkg/analysis_server/test/src/services/correction/fix/remove_ignore_test.dart new file mode 100644 index 00000000000..6e6330586f7 --- /dev/null +++ b/pkg/analysis_server/test/src/services/correction/fix/remove_ignore_test.dart @@ -0,0 +1,92 @@ +// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file +// 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:analysis_server/src/services/correction/fix.dart'; +import 'package:analyzer_plugin/utilities/fixes/fixes.dart'; +import 'package:linter/src/lint_names.dart'; +import 'package:test_reflective_loader/test_reflective_loader.dart'; + +import 'fix_processor.dart'; + +void main() { + defineReflectiveSuite(() { + defineReflectiveTests(RemoveUnnecessaryIgnoreTest); + }); +} + +// TODO(pq): add bulk fix tests +@reflectiveTest +class RemoveUnnecessaryIgnoreTest extends FixProcessorLintTest { + @override + FixKind get kind => DartFixKind.REMOVE_IGNORED_DIAGNOSTIC; + + @override + String get lintCode => LintNames.unnecessary_ignore; + + Future test_file_first() async { + await resolveTestCode(''' +// ignore_for_file: unused_local_variable, return_of_invalid_type +int f() => null; +'''); + await assertHasFix(''' +// ignore_for_file: return_of_invalid_type +int f() => null; +'''); + } + + Future test_file_last() async { + await resolveTestCode(''' +// ignore_for_file: return_of_invalid_type, unused_local_variable +int f() => null; +'''); + await assertHasFix(''' +// ignore_for_file: return_of_invalid_type +int f() => null; +'''); + } + + Future test_file_middle() async { + await resolveTestCode(''' +// ignore_for_file: return_of_invalid_type, unused_local_variable, non_bool_negation_expression +int f() => !null; +'''); + await assertHasFix(''' +// ignore_for_file: return_of_invalid_type, non_bool_negation_expression +int f() => !null; +'''); + } + + Future test_line_first() async { + await resolveTestCode(''' +// ignore: unused_local_variable, return_of_invalid_type +int f() => null; +'''); + await assertHasFix(''' +// ignore: return_of_invalid_type +int f() => null; +'''); + } + + Future test_line_last() async { + await resolveTestCode(''' +// ignore: return_of_invalid_type, unused_local_variable +int f() => null; +'''); + await assertHasFix(''' +// ignore: return_of_invalid_type +int f() => null; +'''); + } + + Future test_line_middle() async { + await resolveTestCode(''' +// ignore: return_of_invalid_type, unused_local_variable, non_bool_negation_expression +int f() => !null; +'''); + await assertHasFix(''' +// ignore: return_of_invalid_type, non_bool_negation_expression +int f() => !null; +'''); + } +} diff --git a/pkg/analysis_server/test/src/services/correction/fix/test_all.dart b/pkg/analysis_server/test/src/services/correction/fix/test_all.dart index ac465354a9b..a58cb1170c1 100644 --- a/pkg/analysis_server/test/src/services/correction/fix/test_all.dart +++ b/pkg/analysis_server/test/src/services/correction/fix/test_all.dart @@ -187,6 +187,7 @@ import 'remove_empty_else_test.dart' as remove_empty_else; import 'remove_empty_statement_test.dart' as remove_empty_statement; import 'remove_extends_clause_test.dart' as remove_extends_clause; import 'remove_if_null_operator_test.dart' as remove_if_null_operator; +import 'remove_ignore_test.dart' as remove_ignore; import 'remove_initializer_test.dart' as remove_initializer; import 'remove_interpolation_braces_test.dart' as remove_interpolation_braces; import 'remove_invocation_test.dart' as remove_invocation; @@ -465,6 +466,7 @@ void main() { remove_empty_statement.main(); remove_extends_clause.main(); remove_if_null_operator.main(); + remove_ignore.main(); remove_initializer.main(); remove_interpolation_braces.main(); remove_invocation.main(); diff --git a/pkg/analyzer/lib/src/error/ignore_validator.dart b/pkg/analyzer/lib/src/error/ignore_validator.dart index 1a48304919f..6644c308b40 100644 --- a/pkg/analyzer/lib/src/error/ignore_validator.dart +++ b/pkg/analyzer/lib/src/error/ignore_validator.dart @@ -107,12 +107,21 @@ class IgnoreValidator { _reportUnignorableAndDuplicateIgnores( unignorable, duplicated, ignoredOnLine); } + + // If there's more than one ignore, the fix is to remove the name. + // Otherwise, the entire comment can be removed. + var ignoredForFileCount = ignoredForFile.length; + var ignoredOnLineCount = 0; + // // Remove all of the errors that are actually being ignored. // for (var error in _reportedErrors) { var lineNumber = _lineInfo.getLocation(error.offset).lineNumber; var ignoredOnLine = ignoredOnLineMap[lineNumber]; + if (ignoredOnLine != null) { + ignoredOnLineCount += ignoredOnLine.length; + } ignoredForFile.removeByName(error.ignoreName); ignoredForFile.removeByName(error.ignoreUniqueName); @@ -123,10 +132,15 @@ class IgnoreValidator { // // Report any remaining ignored names as being unnecessary. // - _reportUnnecessaryOrRemovedOrDeprecatedIgnores(ignoredForFile, - forFile: true); + _reportUnnecessaryOrRemovedOrDeprecatedIgnores( + ignoredForFile, + ignoredForFileCount: ignoredForFileCount, + ); for (var ignoredOnLine in ignoredOnLineMap.values) { - _reportUnnecessaryOrRemovedOrDeprecatedIgnores(ignoredOnLine); + _reportUnnecessaryOrRemovedOrDeprecatedIgnores( + ignoredOnLine, + ignoredOnLineCount: ignoredOnLineCount, + ); } } @@ -172,7 +186,8 @@ class IgnoreValidator { /// Report the [ignoredNames] as being unnecessary. void _reportUnnecessaryOrRemovedOrDeprecatedIgnores( List ignoredNames, - {bool forFile = false}) { + {int? ignoredForFileCount, + int? ignoredOnLineCount}) { if (!_validateUnnecessaryIgnores) return; for (var ignoredName in ignoredNames) { @@ -210,13 +225,14 @@ class IgnoreValidator { } late ErrorCode lintCode; - if (ignoredNames.length > 1) { - lintCode = forFile + + if (ignoredForFileCount != null) { + lintCode = ignoredForFileCount > 1 ? unnecessaryIgnoreNameFileLintCode - : unnecessaryIgnoreNameLocationLintCode; + : unnecessaryIgnoreFileLintCode; } else { - lintCode = forFile - ? unnecessaryIgnoreFileLintCode + lintCode = ignoredOnLineCount! > 1 + ? unnecessaryIgnoreNameLocationLintCode : unnecessaryIgnoreLocationLintCode; }