refactor dartfix lint fixes

This replaces several very similar lint fixes with a single BasicFixLintTask
thereby making it easier to add new fixes for existing lints.
In addition, the dartfix test names have been normalized
to match the corresponding DartFix info and additional tests added.

Change-Id: Ia3d1e0ffcb14d34e0af5d49650888eb2c49a9b42
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/108520
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Dan Rubel <danrubel@google.com>
This commit is contained in:
Dan Rubel 2019-07-09 21:42:55 +00:00 committed by commit-bot@chromium.org
parent 09fc76bc51
commit 8bb8a29f8b
8 changed files with 139 additions and 290 deletions

View file

@ -1,4 +1,4 @@
// Copyright (c) 2018, the Dart project authors. Please see the AUTHORS file
// Copyright (c) 2019, 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.
@ -13,49 +13,43 @@ import 'package:analyzer/dart/analysis/results.dart';
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/error/error.dart';
import 'package:analyzer/src/lint/registry.dart';
import 'package:analyzer_plugin/utilities/assist/assist.dart';
class PreferIntLiteralsFix extends FixLintTask {
static void task(DartFixRegistrar registrar, DartFixListener listener) {
registrar.registerLintTask(
Registry.ruleRegistry['prefer_int_literals'],
new PreferIntLiteralsFix(listener),
);
}
class BasicFixLintTask extends FixLintTask {
final AssistKind assistKind;
final nodes = <AstNode>[];
final literalsToConvert = <DoubleLiteral>[];
PreferIntLiteralsFix(DartFixListener listener) : super(listener);
BasicFixLintTask(this.assistKind, DartFixListener listener) : super(listener);
@override
Future<void> applyLocalFixes(ResolvedUnitResult result) async {
while (literalsToConvert.isNotEmpty) {
DoubleLiteral literal = literalsToConvert.removeLast();
while (nodes.isNotEmpty) {
AstNode node = nodes.removeLast();
AssistProcessor processor = new AssistProcessor(
new DartAssistContextImpl(
DartChangeWorkspace(listener.server.currentSessions),
result,
literal.offset,
0,
node.offset,
node.length,
),
);
List<Assist> assists =
await processor.computeAssist(DartAssistKind.CONVERT_TO_INT_LITERAL);
final location =
listener.locationFor(result, literal.offset, literal.length);
List<Assist> assists = await processor.computeAssist(assistKind);
final location = listener.locationFor(result, node.offset, node.length);
if (assists.isNotEmpty) {
for (Assist assist in assists) {
listener.addSourceChange(
'Replace a double literal with an int literal',
location,
assist.change);
assist.kind.message, location, assist.change);
}
} else {
// TODO(danrubel): If assists is empty, then determine why
// assist could not be performed and report that in the description.
listener.addRecommendation(
'Could not replace a double literal with an int literal', location);
'Fix not found: ${assistKind.message}', location);
}
}
return null;
}
@override
@ -68,7 +62,39 @@ class PreferIntLiteralsFix extends FixLintTask {
void reportErrorForNode(ErrorCode errorCode, AstNode node,
[List<Object> arguments]) {
if (source.fullName != null) {
literalsToConvert.add(node);
nodes.add(node);
}
}
static void preferForElementsToMapFromIterable(
DartFixRegistrar registrar, DartFixListener listener) {
registrar.registerLintTask(
Registry.ruleRegistry['prefer_for_elements_to_map_fromIterable'],
new BasicFixLintTask(DartAssistKind.CONVERT_TO_FOR_ELEMENT, listener),
);
}
static void preferIfElementsToConditionalExpressions(
DartFixRegistrar registrar, DartFixListener listener) {
registrar.registerLintTask(
Registry.ruleRegistry['prefer_if_elements_to_conditional_expressions'],
new BasicFixLintTask(DartAssistKind.CONVERT_TO_IF_ELEMENT, listener),
);
}
static void preferIntLiterals(
DartFixRegistrar registrar, DartFixListener listener) {
registrar.registerLintTask(
Registry.ruleRegistry['prefer_int_literals'],
new BasicFixLintTask(DartAssistKind.CONVERT_TO_INT_LITERAL, listener),
);
}
static void preferSpreadCollections(
DartFixRegistrar registrar, DartFixListener listener) {
registrar.registerLintTask(
Registry.ruleRegistry['prefer_spread_collections'],
new BasicFixLintTask(DartAssistKind.CONVERT_TO_SPREAD, listener),
);
}
}

View file

@ -8,11 +8,8 @@ import 'package:analysis_server/src/edit/fix/dartfix_listener.dart';
import 'package:analysis_server/src/edit/fix/dartfix_registrar.dart';
import 'package:analysis_server/src/edit/fix/fix_error_task.dart';
import 'package:analysis_server/src/edit/fix/non_nullable_fix.dart';
import 'package:analysis_server/src/edit/fix/prefer_for_elements_to_map_fromIterable_fix.dart';
import 'package:analysis_server/src/edit/fix/prefer_if_elements_to_conditional_expressions_fix.dart';
import 'package:analysis_server/src/edit/fix/prefer_int_literals_fix.dart';
import 'package:analysis_server/src/edit/fix/prefer_mixin_fix.dart';
import 'package:analysis_server/src/edit/fix/prefer_spread_collections_fix.dart';
import 'package:analysis_server/src/edit/fix/basic_fix_lint_task.dart';
const allFixes = <DartFixInfo>[
//
@ -64,7 +61,7 @@ For example, this
will be converted to
const double myDouble = 8;''',
PreferIntLiteralsFix.task,
BasicFixLintTask.preferIntLiterals,
),
const DartFixInfo(
'use-spread-collections',
@ -78,7 +75,7 @@ For example, this
will be converted to
var l1 = ['b'];
var l2 = ['a', ...l1];''',
PreferSpreadCollectionsFix.task,
BasicFixLintTask.preferSpreadCollections,
isDefault: false,
),
const DartFixInfo(
@ -91,7 +88,7 @@ For example, this
will be converted to
f(bool b) => ['a', if (b) 'c' else 'd', 'e'];''',
PreferIfElementsToConditionalExpressionsFix.task,
BasicFixLintTask.preferIfElementsToConditionalExpressions,
isDefault: false,
),
const DartFixInfo(
@ -104,7 +101,7 @@ For example, this
will be converted to
<int, int>{ for(int i in [1, 2, 3]) i : i * 2, }''',
PreferForElementsToMapFromIterableFix.task,
BasicFixLintTask.preferForElementsToMapFromIterable,
isDefault: false,
),
//

View file

@ -1,69 +0,0 @@
// Copyright (c) 2019, 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/plugin/edit/assist/assist_core.dart';
import 'package:analysis_server/src/edit/fix/dartfix_listener.dart';
import 'package:analysis_server/src/edit/fix/dartfix_registrar.dart';
import 'package:analysis_server/src/edit/fix/fix_lint_task.dart';
import 'package:analysis_server/src/services/correction/assist.dart';
import 'package:analysis_server/src/services/correction/assist_internal.dart';
import 'package:analysis_server/src/services/correction/change_workspace.dart';
import 'package:analyzer/dart/analysis/results.dart';
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/error/error.dart';
import 'package:analyzer/src/lint/registry.dart';
class PreferForElementsToMapFromIterableFix extends FixLintTask {
final List<AstNode> nodes = <AstNode>[];
PreferForElementsToMapFromIterableFix(DartFixListener listener)
: super(listener);
@override
Future<void> applyLocalFixes(ResolvedUnitResult result) async {
while (nodes.isNotEmpty) {
AstNode node = nodes.removeLast();
AssistProcessor processor = new AssistProcessor(
new DartAssistContextImpl(
DartChangeWorkspace(listener.server.currentSessions),
result,
node.offset,
node.length),
);
List<Assist> assists =
await processor.computeAssist(DartAssistKind.CONVERT_TO_FOR_ELEMENT);
final location = listener.locationFor(result, node.offset, node.length);
if (assists.isNotEmpty) {
for (Assist assist in assists) {
listener.addSourceChange(
assist.kind.message, location, assist.change);
}
} else {
listener.addRecommendation(
'Convert to for elements assist not found', location);
}
}
return null;
}
@override
Future<void> applyRemainingFixes() {
return null;
}
@override
void reportErrorForNode(ErrorCode errorCode, AstNode node,
[List<Object> arguments]) {
nodes.add(node);
}
static void task(DartFixRegistrar registrar, DartFixListener listener) {
registrar.registerLintTask(
Registry.ruleRegistry['prefer_for_elements_to_map_fromIterable'],
new PreferForElementsToMapFromIterableFix(listener),
);
}
}

View file

@ -1,69 +0,0 @@
// Copyright (c) 2019, 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/plugin/edit/assist/assist_core.dart';
import 'package:analysis_server/src/edit/fix/dartfix_listener.dart';
import 'package:analysis_server/src/edit/fix/dartfix_registrar.dart';
import 'package:analysis_server/src/edit/fix/fix_lint_task.dart';
import 'package:analysis_server/src/services/correction/assist.dart';
import 'package:analysis_server/src/services/correction/assist_internal.dart';
import 'package:analysis_server/src/services/correction/change_workspace.dart';
import 'package:analyzer/dart/analysis/results.dart';
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/error/error.dart';
import 'package:analyzer/src/lint/registry.dart';
class PreferIfElementsToConditionalExpressionsFix extends FixLintTask {
final List<AstNode> nodes = <AstNode>[];
PreferIfElementsToConditionalExpressionsFix(DartFixListener listener)
: super(listener);
@override
Future<void> applyLocalFixes(ResolvedUnitResult result) async {
while (nodes.isNotEmpty) {
AstNode node = nodes.removeLast();
AssistProcessor processor = new AssistProcessor(
new DartAssistContextImpl(
DartChangeWorkspace(listener.server.currentSessions),
result,
node.offset,
node.length),
);
List<Assist> assists =
await processor.computeAssist(DartAssistKind.CONVERT_TO_IF_ELEMENT);
final location = listener.locationFor(result, node.offset, node.length);
if (assists.isNotEmpty) {
for (Assist assist in assists) {
listener.addSourceChange(
assist.kind.message, location, assist.change);
}
} else {
listener.addRecommendation(
'Convert to if elements assist not found', location);
}
}
return null;
}
@override
Future<void> applyRemainingFixes() {
return null;
}
@override
void reportErrorForNode(ErrorCode errorCode, AstNode node,
[List<Object> arguments]) {
nodes.add(node);
}
static void task(DartFixRegistrar registrar, DartFixListener listener) {
registrar.registerLintTask(
Registry.ruleRegistry['prefer_if_elements_to_conditional_expressions'],
new PreferIfElementsToConditionalExpressionsFix(listener),
);
}
}

View file

@ -1,68 +0,0 @@
// Copyright (c) 2019, 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/plugin/edit/assist/assist_core.dart';
import 'package:analysis_server/src/edit/fix/dartfix_listener.dart';
import 'package:analysis_server/src/edit/fix/dartfix_registrar.dart';
import 'package:analysis_server/src/edit/fix/fix_lint_task.dart';
import 'package:analysis_server/src/services/correction/assist.dart';
import 'package:analysis_server/src/services/correction/assist_internal.dart';
import 'package:analysis_server/src/services/correction/change_workspace.dart';
import 'package:analyzer/dart/analysis/results.dart';
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/error/error.dart';
import 'package:analyzer/src/lint/registry.dart';
class PreferSpreadCollectionsFix extends FixLintTask {
final List<AstNode> nodes = <AstNode>[];
PreferSpreadCollectionsFix(DartFixListener listener) : super(listener);
@override
Future<void> applyLocalFixes(ResolvedUnitResult result) async {
while (nodes.isNotEmpty) {
AstNode node = nodes.removeLast();
AssistProcessor processor = new AssistProcessor(
new DartAssistContextImpl(
DartChangeWorkspace(listener.server.currentSessions),
result,
node.offset,
0),
);
List<Assist> assists =
await processor.computeAssist(DartAssistKind.CONVERT_TO_SPREAD);
final location = listener.locationFor(result, node.offset, node.length);
if (assists.isNotEmpty) {
for (Assist assist in assists) {
listener.addSourceChange(
assist.kind.message, location, assist.change);
}
} else {
listener.addRecommendation(
'Convert to spread assist not found', location);
}
}
return null;
}
@override
Future<void> applyRemainingFixes() {
return null;
}
@override
void reportErrorForNode(ErrorCode errorCode, AstNode node,
[List<Object> arguments]) {
nodes.add(node);
}
static void task(DartFixRegistrar registrar, DartFixListener listener) {
registrar.registerLintTask(
Registry.ruleRegistry['prefer_spread_collections'],
new PreferSpreadCollectionsFix(listener),
);
}
}

View file

@ -93,36 +93,6 @@ f(bool b) {
''');
}
test_dartfix_convertClassToMixin() async {
addTestFile('''
class A {}
class B extends A {}
class C with B {}
''');
createProject();
EditDartfixResult result = await performFix();
expect(result.suggestions, hasLength(1));
expectSuggestion(result.suggestions[0], 'mixin', 17, 1);
expectEdits(result.edits, '''
class A {}
mixin B implements A {}
class C with B {}
''');
}
test_dartfix_convertToIntLiteral() async {
addTestFile('''
const double myDouble = 42.0;
''');
createProject();
EditDartfixResult result = await performFix();
expect(result.suggestions, hasLength(1));
expectSuggestion(result.suggestions[0], 'int literal', 24, 4);
expectEdits(result.edits, '''
const double myDouble = 42;
''');
}
test_dartfix_excludedSource() async {
// Add analysis options to exclude the lib directory then reanalyze
newFile('/project/analysis_options.yaml', content: '''
@ -142,6 +112,25 @@ const double myDouble = 42.0;
expect(result.edits, hasLength(0));
}
test_dartfix_fixNamedConstructorTypeArgs() async {
addTestFile('''
class A<T> { A.from(Object obj) { } }
main() {
print(new A.from<String>([]));
}
''');
createProject();
EditDartfixResult result = await performFix();
expect(result.suggestions, hasLength(1));
expectSuggestion(result.suggestions[0], 'type arguments', 65, 8);
expectEdits(result.edits, '''
class A<T> { A.from(Object obj) { } }
main() {
print(new A<String>.from([]));
}
''');
}
test_dartfix_map_for_elements() async {
// Add analysis options to enable ui as code
newFile('/project/analysis_options.yaml', content: '''
@ -169,25 +158,6 @@ f(Iterable<int> i) {
''');
}
test_dartfix_moveTypeArgumentToClass() async {
addTestFile('''
class A<T> { A.from(Object obj) { } }
main() {
print(new A.from<String>([]));
}
''');
createProject();
EditDartfixResult result = await performFix();
expect(result.suggestions, hasLength(1));
expectSuggestion(result.suggestions[0], 'type arguments', 65, 8);
expectEdits(result.edits, '''
class A<T> { A.from(Object obj) { } }
main() {
print(new A<String>.from([]));
}
''');
}
test_dartfix_non_nullable() async {
// Add analysis options to enable non-nullable analysis
newFile('/project/analysis_options.yaml', content: '''
@ -326,7 +296,69 @@ const double myDouble = 42;
''');
}
test_dartfix_spread_collections() async {
test_dartfix_preferForElementsToMapFromIterable() async {
addTestFile('''
var m =
Map<int, int>.fromIterable([1, 2, 3], key: (i) => i, value: (i) => i * 2);
''');
createProject();
EditDartfixResult result =
await performFix(includedFixes: ['map-for-elements']);
expect(result.suggestions, hasLength(1));
expectSuggestion(
result.suggestions[0], "Convert to a 'for' element", 10, 73);
expectEdits(result.edits, '''
var m =
{ for (var i in [1, 2, 3]) i : i * 2 };
''');
}
test_dartfix_preferIfElementsToConditionalExpressions() async {
addTestFile('''
f(bool b) => ['a', b ? 'c' : 'd', 'e'];
''');
createProject();
EditDartfixResult result =
await performFix(includedFixes: ['collection-if-elements']);
expect(result.suggestions, hasLength(1));
expectSuggestion(
result.suggestions[0], "Convert to an 'if' element", 19, 13);
expectEdits(result.edits, '''
f(bool b) => ['a', if (b) 'c' else 'd', 'e'];
''');
}
test_dartfix_preferIntLiterals() async {
addTestFile('''
const double myDouble = 42.0;
''');
createProject();
EditDartfixResult result = await performFix();
expect(result.suggestions, hasLength(1));
expectSuggestion(result.suggestions[0], 'int literal', 24, 4);
expectEdits(result.edits, '''
const double myDouble = 42;
''');
}
test_dartfix_preferMixin() async {
addTestFile('''
class A {}
class B extends A {}
class C with B {}
''');
createProject();
EditDartfixResult result = await performFix();
expect(result.suggestions, hasLength(1));
expectSuggestion(result.suggestions[0], 'mixin', 17, 1);
expectEdits(result.edits, '''
class A {}
mixin B implements A {}
class C with B {}
''');
}
test_dartfix_preferSpreadCollections() async {
// Add analysis options to enable ui as code
newFile('/project/analysis_options.yaml', content: '''
analyzer:

View file

@ -43,6 +43,6 @@ main() {
final suggestions = driver.result.suggestions;
expect(suggestions, hasLength(1));
expectDoesNotHaveSuggestion(suggestions, 'Convert MyMixin to a mixin');
expectHasSuggestion(suggestions, 'Replace a double literal');
expectHasSuggestion(suggestions, 'Convert to an int literal');
});
}

View file

@ -44,7 +44,7 @@ main() {
final suggestions = driver.result.suggestions;
expect(suggestions, hasLength(2));
expectHasSuggestion(suggestions, 'Convert MyMixin to a mixin');
expectHasSuggestion(suggestions, 'Replace a double literal');
expectHasSuggestion(suggestions, 'Convert to an int literal');
expect(driver.result.edits, hasLength(1));
for (SourceEdit edit in driver.result.edits[0].edits) {