Skip to content

Commit

Permalink
[fix] add remove_ignore correction producer
Browse files Browse the repository at this point in the history
Change-Id: I48ed5c612492e79e1b42ffd8e1522e960981fe63
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/404524
Reviewed-by: Brian Wilkerson <[email protected]>
Commit-Queue: Phil Quitslund <[email protected]>
  • Loading branch information
pq authored and Commit Queue committed Jan 16, 2025
1 parent b115ec0 commit fa2f2dd
Show file tree
Hide file tree
Showing 7 changed files with 219 additions and 12 deletions.
Original file line number Diff line number Diff line change
@@ -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<String> get fixArguments => [_diagnosticName];

@override
FixKind get fixKind => DartFixKind.REMOVE_IGNORED_DIAGNOSTIC;

@override
Future<void> 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!);
});
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
5 changes: 5 additions & 0 deletions pkg/analysis_server/lib/src/services/correction/fix.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -485,6 +486,11 @@ final _builtInLintProducers = <LintCode, List<ProducerGenerator>>{
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: [
Expand Down
Original file line number Diff line number Diff line change
@@ -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<void> 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<void> 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<void> 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<void> 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<void> 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<void> 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;
''');
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down
34 changes: 25 additions & 9 deletions pkg/analyzer/lib/src/error/ignore_validator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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,
);
}
}

Expand Down Expand Up @@ -172,7 +186,8 @@ class IgnoreValidator {
/// Report the [ignoredNames] as being unnecessary.
void _reportUnnecessaryOrRemovedOrDeprecatedIgnores(
List<IgnoredElement> ignoredNames,
{bool forFile = false}) {
{int? ignoredForFileCount,
int? ignoredOnLineCount}) {
if (!_validateUnnecessaryIgnores) return;

for (var ignoredName in ignoredNames) {
Expand Down Expand Up @@ -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;
}

Expand Down

0 comments on commit fa2f2dd

Please sign in to comment.