Initial new union of Dart Analysis Fixes, the initial fix is to remove all unused imports in a file.
Change-Id: I259e796b2603234955a1950b2c886e6f08a697bc
Reviewed-on: https://dart-review.googlesource.com/56220
Commit-Queue: Jaime Wren <jwren@google.com>
Reviewed-by: Jaime Wren <jwren@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
diff --git a/pkg/analysis_server/lib/plugin/edit/fix/fix_core.dart b/pkg/analysis_server/lib/plugin/edit/fix/fix_core.dart
index e0af23c..031d864 100644
--- a/pkg/analysis_server/lib/plugin/edit/fix/fix_core.dart
+++ b/pkg/analysis_server/lib/plugin/edit/fix/fix_core.dart
@@ -72,6 +72,12 @@
AnalysisError get error;
/**
+ * All of the errors in the file. This is used to compute additional fixes
+ * such "Fix all instances in file."
+ */
+ List<AnalysisError> get errors;
+
+ /**
* The [ResourceProvider] to access files and folders.
*/
ResourceProvider get resourceProvider;
diff --git a/pkg/analysis_server/lib/src/edit/edit_domain.dart b/pkg/analysis_server/lib/src/edit/edit_domain.dart
index 1af301c..86e8dce 100644
--- a/pkg/analysis_server/lib/src/edit/edit_domain.dart
+++ b/pkg/analysis_server/lib/src/edit/edit_domain.dart
@@ -526,12 +526,18 @@
CompilationUnit unit = result.unit;
LineInfo lineInfo = result.lineInfo;
int requestLine = lineInfo.getLocation(offset).lineNumber;
+ List<engine.AnalysisError> errorsCopy = new List.from(result.errors);
for (engine.AnalysisError error in result.errors) {
int errorLine = lineInfo.getLocation(error.offset).lineNumber;
if (errorLine == requestLine) {
AstProvider astProvider = new AstProviderForDriver(driver);
DartFixContext context = new _DartFixContextImpl(
- server.resourceProvider, result.driver, astProvider, unit, error);
+ server.resourceProvider,
+ result.driver,
+ astProvider,
+ unit,
+ error,
+ errorsCopy);
List<Fix> fixes =
await new DefaultFixContributor().internalComputeFixes(context);
if (fixes.isNotEmpty) {
@@ -679,8 +685,11 @@
@override
final engine.AnalysisError error;
+ @override
+ final List<engine.AnalysisError> errors;
+
_DartFixContextImpl(this.resourceProvider, this.analysisDriver,
- this.astProvider, this.unit, this.error);
+ this.astProvider, this.unit, this.error, this.errors);
@override
GetTopLevelDeclarations get getTopLevelDeclarations =>
diff --git a/pkg/analysis_server/lib/src/services/correction/fix.dart b/pkg/analysis_server/lib/src/services/correction/fix.dart
index cdcdc36..9603adb 100644
--- a/pkg/analysis_server/lib/src/services/correction/fix.dart
+++ b/pkg/analysis_server/lib/src/services/correction/fix.dart
@@ -212,8 +212,9 @@
const FixKind('REMOVE_UNUSED_CATCH', 50, "Remove unused 'catch' clause");
static const REMOVE_UNUSED_CATCH_STACK = const FixKind(
'REMOVE_UNUSED_CATCH_STACK', 50, "Remove unused stack trace variable");
- static const REMOVE_UNUSED_IMPORT =
- const FixKind('REMOVE_UNUSED_IMPORT', 50, "Remove unused import");
+ static const REMOVE_UNUSED_IMPORT = const FixKind(
+ 'REMOVE_UNUSED_IMPORT', 50, "Remove unused import",
+ appliedTogetherMessage: "Remove all unused imports in this file");
static const RENAME_TO_CAMEL_CASE =
const FixKind('RENAME_TO_CAMEL_CASE', 50, "Rename to '{0}'");
static const REPLACE_BOOLEAN_WITH_BOOL = const FixKind(
@@ -266,10 +267,15 @@
@override
final AnalysisError error;
- FixContextImpl(this.resourceProvider, this.analysisDriver, this.error);
+ @override
+ final List<AnalysisError> errors;
+
+ FixContextImpl(
+ this.resourceProvider, this.analysisDriver, this.error, this.errors);
FixContextImpl.from(FixContext other)
: resourceProvider = other.resourceProvider,
analysisDriver = other.analysisDriver,
- error = other.error;
+ error = other.error,
+ errors = other.errors;
}
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 3df2168..da9de25 100644
--- a/pkg/analysis_server/lib/src/services/correction/fix_internal.dart
+++ b/pkg/analysis_server/lib/src/services/correction/fix_internal.dart
@@ -84,11 +84,97 @@
try {
FixProcessor processor = new FixProcessor(context);
List<Fix> fixes = await processor.compute();
- return fixes;
+ List<Fix> fixAllFixes = await _computeFixAllFixes(context, fixes);
+ return new List.from(fixes)..addAll(fixAllFixes);
} on CancelCorrectionException {
return Fix.EMPTY_LIST;
}
}
+
+ Future<List<Fix>> _computeFixAllFixes(
+ DartFixContext context, List<Fix> fixes) async {
+ final AnalysisError analysisError = context.error;
+ final List<AnalysisError> allAnalysisErrors = context.errors;
+
+ // Validate inputs:
+ // - return if no fixes
+ // - return if no other analysis errors
+ if (fixes.isEmpty || allAnalysisErrors.length < 2) {
+ return Fix.EMPTY_LIST;
+ }
+
+ // Remove any analysis errors that don't have the expected error code name
+ allAnalysisErrors
+ .removeWhere((e) => analysisError.errorCode.name != e.errorCode.name);
+ if (allAnalysisErrors.length < 2) {
+ return Fix.EMPTY_LIST;
+ }
+
+ // A map between each FixKind and the List of associated fixes
+ final HashMap<FixKind, List<Fix>> map = new HashMap();
+
+ // Populate the HashMap by looping through all AnalysisErrors, creating a
+ // new FixProcessor to compute the other fixes that can be applied with this
+ // one.
+ // For each fix, put the fix into the HashMap.
+ for (int i = 0; i < allAnalysisErrors.length; i++) {
+ final FixContext fixContextI = new FixContextImpl(
+ context.resourceProvider,
+ context.analysisDriver,
+ allAnalysisErrors[i],
+ allAnalysisErrors);
+ final DartFixContextImpl dartFixContextI = new DartFixContextImpl(
+ fixContextI, context.astProvider, context.unit);
+ final FixProcessor processorI = new FixProcessor(dartFixContextI);
+ final List<Fix> fixesListI = await processorI.compute();
+ for (Fix f in fixesListI) {
+ if (!map.containsKey(f.kind)) {
+ map[f.kind] = new List<Fix>()..add(f);
+ } else {
+ map[f.kind].add(f);
+ }
+ }
+ }
+
+ // For each FixKind in the HashMap, union each list together, then return
+ // the set of unioned Fixes.
+ final List<Fix> result = new List<Fix>();
+ map.forEach((FixKind kind, List<Fix> fixesListJ) {
+ if (fixesListJ.first.kind.canBeAppliedTogether()) {
+ Fix unionFix = _unionFixList(fixesListJ);
+ if (unionFix != null) {
+ result.add(unionFix);
+ }
+ }
+ });
+ return result;
+ }
+
+ Fix _unionFixList(List<Fix> fixList) {
+ if (fixList == null || fixList.isEmpty) {
+ return null;
+ } else if (fixList.length == 1) {
+ return fixList[0];
+ }
+ final SourceChange sourceChange =
+ new SourceChange(fixList[0].kind.appliedTogetherMessage);
+ sourceChange.edits = new List.from(fixList[0].change.edits);
+ final List<SourceEdit> edits = new List<SourceEdit>();
+ edits.addAll(fixList[0].change.edits[0].edits);
+ sourceChange.linkedEditGroups =
+ new List.from(fixList[0].change.linkedEditGroups);
+ for (int i = 1; i < fixList.length; i++) {
+ edits.addAll(fixList[i].change.edits[0].edits);
+ sourceChange.linkedEditGroups..addAll(fixList[i].change.linkedEditGroups);
+ }
+ // Sort the list of SourceEdits so that when the edits are applied, they
+ // are applied from the end of the file to the top of the file.
+ edits.sort((s1, s2) => s2.offset - s1.offset);
+
+ sourceChange.edits[0].edits = edits;
+
+ return new Fix(fixList[0].kind, sourceChange);
+ }
}
/**
diff --git a/pkg/analysis_server/test/services/correction/fix_test.dart b/pkg/analysis_server/test/services/correction/fix_test.dart
index 62b0d84..b0d8f2c 100644
--- a/pkg/analysis_server/test/services/correction/fix_test.dart
+++ b/pkg/analysis_server/test/services/correction/fix_test.dart
@@ -100,6 +100,27 @@
await _assertNoFix(kind, error);
}
+ assertHasFixAllFix(ErrorCode errorCode, FixKind kind, String expected,
+ {String target}) async {
+ AnalysisError error = await _findErrorToFixOfType(errorCode);
+ fix = await _assertHasFix(kind, error, hasFixAllFix: true);
+ change = fix.change;
+
+ // apply to "file"
+ List<SourceFileEdit> fileEdits = change.edits;
+ expect(fileEdits, hasLength(1));
+
+ String fileContent = testCode;
+ if (target != null) {
+ expect(fileEdits.first.file, convertPath(target));
+ fileContent = getFile(target).readAsStringSync();
+ }
+
+ resultCode = SourceEdit.applySequence(fileContent, change.edits[0].edits);
+ // verify
+ expect(resultCode, expected);
+ }
+
List<LinkedEditSuggestion> expectedSuggestions(
LinkedEditSuggestionKind kind, List<String> values) {
return values.map((value) {
@@ -115,14 +136,22 @@
/**
* Computes fixes and verifies that there is a fix of the given kind.
*/
- Future<Fix> _assertHasFix(FixKind kind, AnalysisError error) async {
- List<Fix> fixes = await _computeFixes(error);
- for (Fix fix in fixes) {
- if (fix.kind == kind) {
- return fix;
- }
+ Future<Fix> _assertHasFix(FixKind kind, AnalysisError error,
+ {bool hasFixAllFix: false}) async {
+ if (hasFixAllFix && !kind.canBeAppliedTogether()) {
+ fail(
+ 'Expected to find and return fix-all FixKind for $kind, but kind.canBeAppliedTogether is ${kind.canBeAppliedTogether}');
}
- fail('Expected to find fix $kind in\n${fixes.join('\n')}');
+ List<Fix> fixes = await _computeFixes(error);
+ Fix firstMatchingFix = fixes.firstWhere((fix) => fix.kind == kind);
+ Fix lastMatchingFix = fixes.lastWhere((fix) => fix.kind == kind);
+ if (firstMatchingFix == null) {
+ fail('Expected to find fix $kind in\n${fixes.join('\n')}');
+ } else if (lastMatchingFix == null &&
+ !lastMatchingFix.kind.canBeAppliedTogether()) {
+ fail('Expected to find fix $kind in\n${fixes.join('\n')}');
+ }
+ return !hasFixAllFix ? firstMatchingFix : lastMatchingFix;
}
void _assertLinkedGroup(LinkedEditGroup group, List<String> expectedStrings,
@@ -151,8 +180,13 @@
* Computes fixes for the given [error] in [testUnit].
*/
Future<List<Fix>> _computeFixes(AnalysisError error) async {
- DartFixContext fixContext = new _DartFixContextImpl(resourceProvider,
- driver, new AstProviderForDriver(driver), testUnit, error);
+ DartFixContext fixContext = new _DartFixContextImpl(
+ resourceProvider,
+ driver,
+ new AstProviderForDriver(driver),
+ testUnit,
+ error,
+ await _computeErrors());
return await new DefaultFixContributor().internalComputeFixes(fixContext);
}
@@ -189,6 +223,14 @@
return errors[0];
}
+ Future<AnalysisError> _findErrorToFixOfType(ErrorCode errorCode) async {
+ List<AnalysisError> errors = await _computeErrors();
+ if (errorFilter != null) {
+ errors = errors.where(errorFilter).toList();
+ }
+ return errors.firstWhere((error) => errorCode == error.errorCode);
+ }
+
List<Position> _findResultPositions(List<String> searchStrings) {
List<Position> positions = <Position>[];
for (String search in searchStrings) {
@@ -5119,6 +5161,51 @@
''');
}
+ test_removeUnusedImport_all() async {
+ await resolveTestUnit('''
+import 'dart:math';
+import 'dart:math';
+import 'dart:math';
+main() {
+}
+''');
+ await assertHasFixAllFix(
+ HintCode.UNUSED_IMPORT, DartFixKind.REMOVE_UNUSED_IMPORT, '''
+main() {
+}
+''');
+ }
+
+ test_removeUnusedImport_all_diverseImports() async {
+ // TODO (jwren) add an additional test to verify the used, unused, used,
+ // unused ... permutations.
+ await resolveTestUnit('''
+import 'dart:math';
+import 'dart:math';
+import 'dart:async';
+main() {
+}
+''');
+ await assertHasFixAllFix(
+ HintCode.UNUSED_IMPORT, DartFixKind.REMOVE_UNUSED_IMPORT, '''
+main() {
+}
+''');
+ }
+
+ test_removeUnusedImport_all_singleLine() async {
+ await resolveTestUnit('''
+import 'dart:math'; import 'dart:math'; import 'dart:math';
+main() {
+}
+''');
+ await assertHasFixAllFix(
+ HintCode.UNUSED_IMPORT, DartFixKind.REMOVE_UNUSED_IMPORT, '''
+main() {
+}
+''');
+ }
+
test_replaceVarWithDynamic() async {
errorFilter = (AnalysisError error) {
return error.errorCode == ParserErrorCode.VAR_AS_TYPE_NAME;
@@ -7923,8 +8010,11 @@
@override
final AnalysisError error;
+ @override
+ final List<AnalysisError> errors;
+
_DartFixContextImpl(this.resourceProvider, this.analysisDriver,
- this.astProvider, this.unit, this.error);
+ this.astProvider, this.unit, this.error, this.errors);
@override
GetTopLevelDeclarations get getTopLevelDeclarations =>
diff --git a/pkg/analyzer_plugin/lib/utilities/fixes/fixes.dart b/pkg/analyzer_plugin/lib/utilities/fixes/fixes.dart
index 865d169..3fc7fde7 100644
--- a/pkg/analyzer_plugin/lib/utilities/fixes/fixes.dart
+++ b/pkg/analyzer_plugin/lib/utilities/fixes/fixes.dart
@@ -139,11 +139,30 @@
final String message;
/**
- * Initialize a newly created kind of fix to have the given [name],
- * [priority] and [message].
+ * A human-readable description of the changes that will be applied by this
+ * kind of 'applied together' fix.
*/
- const FixKind(this.name, this.priority, this.message);
+ final String appliedTogetherMessage;
+
+ /**
+ * The change can be made with other fixes of this [FixKind].
+ */
+ bool canBeAppliedTogether() => appliedTogetherMessage != null;
+
+ /**
+ * Initialize a newly created kind of fix to have the given [name],
+ * [priority], [message], and optionally [canBeAppliedTogether] and
+ * [appliedTogetherMessage].
+ */
+ const FixKind(this.name, this.priority, this.message,
+ {this.appliedTogetherMessage: null});
@override
String toString() => name;
+
+ @override
+ bool operator ==(o) => o is FixKind && o.name == name;
+
+ @override
+ int get hashCode => name.hashCode;
}
