fix (all) in file processor
Change-Id: I092d99ab8dd7e16fe351cf475a7c935a2546d67c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/183700
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Phil Quitslund <pquitslund@google.com>
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 97be45b..55d892e 100644
--- a/pkg/analysis_server/lib/src/services/correction/fix_internal.dart
+++ b/pkg/analysis_server/lib/src/services/correction/fix_internal.dart
@@ -271,6 +271,117 @@
}
}
+/// Computer for Dart "fix all in file" fixes.
+class FixInFileProcessor {
+ final DartFixContext context;
+ FixInFileProcessor(this.context);
+
+ Future<List<Fix>> compute() async {
+ var error = context.error;
+ var errors = context.resolveResult.errors.toList();
+ // Remove any analysis errors that don't have the expected error code name
+ errors.removeWhere((e) => error.errorCode.name != e.errorCode.name);
+
+ if (errors.length < 2) {
+ return const <Fix>[];
+ }
+
+ var instrumentationService = context.instrumentationService;
+ var workspace = context.workspace;
+ var resolveResult = context.resolveResult;
+
+ var generators = _getGenerators(
+ error.errorCode,
+ CorrectionProducerContext(
+ dartFixContext: context,
+ diagnostic: error,
+ resolvedResult: resolveResult,
+ selectionOffset: context.error.offset,
+ selectionLength: context.error.length,
+ workspace: workspace,
+ ));
+
+ var fixes = <Fix>[];
+ for (var generator in generators) {
+ var builder = ChangeBuilder(workspace: workspace);
+ for (var error in errors) {
+ var fixContext = DartFixContextImpl(
+ instrumentationService,
+ workspace,
+ resolveResult,
+ error,
+ (name) => [],
+ );
+ builder = await _fixError(fixContext, builder, generator(), error);
+ }
+ var sourceChange = builder.sourceChange;
+ if (sourceChange.edits.isNotEmpty) {
+ var fixKind = generator().fixKind;
+ // todo (pq): with a table redesign we can bail earlier.
+ if (fixKind.canBeAppliedTogether()) {
+ sourceChange.message = fixKind.appliedTogetherMessage;
+ fixes.add(Fix(fixKind, sourceChange));
+ }
+ }
+ }
+ return fixes;
+ }
+
+ Future<ChangeBuilder> _fixError(
+ DartFixContext fixContext,
+ ChangeBuilder builder,
+ CorrectionProducer producer,
+ AnalysisError diagnostic) async {
+ var context = CorrectionProducerContext(
+ applyingBulkFixes: true,
+ dartFixContext: fixContext,
+ diagnostic: diagnostic,
+ resolvedResult: fixContext.resolveResult,
+ selectionOffset: diagnostic.offset,
+ selectionLength: diagnostic.length,
+ workspace: fixContext.workspace,
+ );
+
+ var setupSuccess = context.setupCompute();
+ if (!setupSuccess) {
+ return null;
+ }
+
+ producer.configure(context);
+
+ try {
+ var localBuilder = builder.copy();
+ await producer.compute(localBuilder);
+ builder = localBuilder;
+ } on ConflictingEditException {
+ // If a conflicting edit was added in [compute], then the [localBuilder]
+ // is discarded and we revert to the previous state of the builder.
+ }
+
+ return builder;
+ }
+
+ List<ProducerGenerator> _getGenerators(
+ ErrorCode errorCode, CorrectionProducerContext context) {
+ var producers = <ProducerGenerator>[];
+ if (errorCode is LintCode) {
+ var generators = FixProcessor.lintProducerMap[errorCode.name];
+ if (generators != null) {
+ producers.addAll(generators);
+ }
+ } else {
+ var generators = FixProcessor.nonLintProducerMap[errorCode];
+ if (generators != null) {
+ if (generators != null) {
+ producers.addAll(generators);
+ }
+ }
+ // todo (pq): consider support for multiGenerators
+ }
+ return producers;
+ }
+}
+
/// The computer for Dart fixes.
class FixProcessor extends BaseProcessor {
/// A map from the names of lint rules to a list of generators used to create
diff --git a/pkg/analysis_server/test/src/services/correction/fix/fix_in_file_test.dart b/pkg/analysis_server/test/src/services/correction/fix/fix_in_file_test.dart
new file mode 100644
index 0000000..a73400b
--- /dev/null
+++ b/pkg/analysis_server/test/src/services/correction/fix/fix_in_file_test.dart
@@ -0,0 +1,74 @@
+// Copyright (c) 2021, 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:test/test.dart';
+import 'package:test_reflective_loader/test_reflective_loader.dart';
+
+import 'fix_processor.dart';
+
+void main() {
+ defineReflectiveSuite(() {
+ // defineReflectiveTests(MultiFixInFileTest);
+ defineReflectiveTests(SingleFixInFileTest);
+ });
+}
+
+//// todo (pq): update w/ a FixKind that we're sure we want to support as a file fix
+// @reflectiveTest
+// class MultiFixInFileTest extends FixInFileProcessorTest
+// with WithNullSafetyMixin {
+// Future<void> test_nullable() async {
+// await resolveTestCode('''
+// class C {
+// String? s;
+// String? s2;
+// C({String this.s, String this.s2});
+// }
+// ''');
+//
+// var fixes = await getFixes();
+// expect(fixes, hasLength(2));
+//
+// var addRequired =
+// fixes.where((f) => f.kind == DartFixKind.ADD_REQUIRED).first;
+// assertProduces(addRequired, '''
+// class C {
+// String? s;
+// String? s2;
+// C({required String this.s, required String this.s2});
+// }
+// ''');
+//
+// var makeNullable =
+// fixes.where((f) => f.kind == DartFixKind.MAKE_VARIABLE_NULLABLE).first;
+// assertProduces(makeNullable, '''
+// class C {
+// String? s;
+// String? s2;
+// C({String? this.s, String? this.s2});
+// }
+// ''');
+// }
+// }
+
+@reflectiveTest
+class SingleFixInFileTest extends FixInFileProcessorTest {
+ Future<void> test_isNull() async {
+ await resolveTestCode('''
+bool f(p, q) {
+ return p is Null && q is Null;
+}
+''');
+
+ var fixes = await getFixes();
+ expect(fixes, hasLength(1));
+ assertProduces(fixes.first, '''
+bool f(p, q) {
+ return p == null && q == null;
+}
+''');
+ }
+}
+
+/// todo (pq): add negative tests
diff --git a/pkg/analysis_server/test/src/services/correction/fix/fix_processor.dart b/pkg/analysis_server/test/src/services/correction/fix/fix_processor.dart
index 70b6a66..ac90224 100644
--- a/pkg/analysis_server/test/src/services/correction/fix/fix_processor.dart
+++ b/pkg/analysis_server/test/src/services/correction/fix/fix_processor.dart
@@ -25,6 +25,131 @@
export 'package:analyzer/src/test_utilities/package_config_file_builder.dart';
+abstract class BaseFixProcessorTest extends AbstractSingleUnitTest {
+ /// The errors in the file for which fixes are being computed.
+ List<AnalysisError> _errors;
+
+ /// The source change associated with the fix that was found, or `null` if
+ /// neither [assertHasFix] nor [assertHasFixAllFix] has been invoked.
+ SourceChange change;
+
+ /// The result of applying the [change] to the file content, or `null` if
+ /// neither [assertHasFix] nor [assertHasFixAllFix] has been invoked.
+ String resultCode;
+
+ /// The workspace in which fixes contributor operates.
+ ChangeWorkspace get workspace {
+ return DartChangeWorkspace([session]);
+ }
+
+ Future<List<AnalysisError>> _computeErrors() async {
+ if (_errors == null) {
+ if (testAnalysisResult != null) {
+ _errors = testAnalysisResult.errors;
+ }
+ if (_errors == null) {
+ var result = await session.getResolvedUnit(testFile);
+ _errors = result.errors;
+ }
+ }
+ return _errors;
+ }
+
+ /// Find the error that is to be fixed by computing the errors in the file,
+ /// using the [errorFilter] to filter out errors that should be ignored, and
+ /// expecting that there is a single remaining error. The error filter should
+ /// return `true` if the error should not be ignored.
+ Future<AnalysisError> _findErrorToFix(
+ bool Function(AnalysisError) errorFilter,
+ {int length}) async {
+ var errors = await _computeErrors();
+ if (errorFilter != null) {
+ if (errors.length == 1) {
+ fail('Unnecessary error filter');
+ }
+ errors = errors.where(errorFilter).toList();
+ }
+ if (errors.isEmpty) {
+ fail('Expected one error, found: none');
+ } else if (errors.length > 1) {
+ var buffer = StringBuffer();
+ buffer.writeln('Expected one error, found:');
+ for (var error in errors) {
+ buffer.writeln(' $error [${error.errorCode}]');
+ }
+ fail(buffer.toString());
+ }
+ return errors[0];
+ }
+
+ Future<AnalysisError> _findErrorToFixOfType(ErrorCode errorCode) async {
+ var errors = await _computeErrors();
+ for (var error in errors) {
+ if (error.errorCode == errorCode) {
+ return error;
+ }
+ }
+ return null;
+ }
+}
+
+/// A base class defining support for writing fix-in-file processor tests.
+abstract class FixInFileProcessorTest extends BaseFixProcessorTest {
+ void assertProduces(Fix fix, String expected) {
+ var fileEdits = fix.change.edits;
+ expect(fileEdits, hasLength(1));
+
+ var fileContent = testCode;
+ resultCode = SourceEdit.applySequence(fileContent, fileEdits[0].edits);
+ expect(resultCode, expected);
+ }
+
+ Future<List<Fix>> getFixes() async {
+ var errors = await _computeErrors();
+ expect(errors, isNotEmpty);
+ String errorCode;
+ for (var error in errors) {
+ errorCode ??= error.errorCode.name;
+ if (errorCode != error.errorCode.name) {
+ fail('Expected only errors of one type but found: $errors');
+ }
+ }
+
+ var fixes = await _computeFixes(errors.first);
+ return fixes;
+ }
+
+ @override
+ void setUp() {
+ super.setUp();
+ verifyNoTestUnitErrors = false;
+ useLineEndingsForPlatform = true;
+ }
+
+ /// Computes fixes for the given [error] in [testUnit].
+ Future<List<Fix>> _computeFixes(AnalysisError error) async {
+ var analysisContext = contextFor(testFile);
+
+ var tracker = DeclarationsTracker(MemoryByteStore(), resourceProvider);
+ tracker.addContext(analysisContext);
+
+ var context = DartFixContextImpl(
+ TestInstrumentationService(),
+ workspace,
+ testAnalysisResult,
+ error,
+ (name) {
+ var provider = TopLevelDeclarationsProvider(tracker);
+ provider.doTrackerWork();
+ return provider.get(analysisContext, testFile, name);
+ },
+ );
+
+ var fixes = await FixInFileProcessor(context).compute();
+ return fixes;
+ }
+}
+
/// A base class defining support for writing fix processor tests that are
/// specific to fixes associated with lints that use the FixKind.
abstract class FixProcessorLintTest extends FixProcessorTest {
@@ -60,26 +185,10 @@
}
/// A base class defining support for writing fix processor tests.
-abstract class FixProcessorTest extends AbstractSingleUnitTest {
- /// The errors in the file for which fixes are being computed.
- List<AnalysisError> _errors;
-
- /// The source change associated with the fix that was found, or `null` if
- /// neither [assertHasFix] nor [assertHasFixAllFix] has been invoked.
- SourceChange change;
-
- /// The result of applying the [change] to the file content, or `null` if
- /// neither [assertHasFix] nor [assertHasFixAllFix] has been invoked.
- String resultCode;
-
+abstract class FixProcessorTest extends BaseFixProcessorTest {
/// Return the kind of fixes being tested by this test class.
FixKind get kind;
- /// The workspace in which fixes contributor operates.
- ChangeWorkspace get workspace {
- return DartChangeWorkspace([session]);
- }
-
Future<void> assertHasFix(String expected,
{bool Function(AnalysisError) errorFilter,
int length,
@@ -266,19 +375,6 @@
}
}
- Future<List<AnalysisError>> _computeErrors() async {
- if (_errors == null) {
- if (testAnalysisResult != null) {
- _errors = testAnalysisResult.errors;
- }
- if (_errors == null) {
- var result = await session.getResolvedUnit(testFile);
- _errors = result.errors;
- }
- }
- return _errors;
- }
-
/// Computes fixes for the given [error] in [testUnit].
Future<List<Fix>> _computeFixes(AnalysisError error) async {
var analysisContext = contextFor(testFile);
@@ -300,43 +396,6 @@
return await DartFixContributor().computeFixes(context);
}
- /// Find the error that is to be fixed by computing the errors in the file,
- /// using the [errorFilter] to filter out errors that should be ignored, and
- /// expecting that there is a single remaining error. The error filter should
- /// return `true` if the error should not be ignored.
- Future<AnalysisError> _findErrorToFix(
- bool Function(AnalysisError) errorFilter,
- {int length}) async {
- var errors = await _computeErrors();
- if (errorFilter != null) {
- if (errors.length == 1) {
- fail('Unnecessary error filter');
- }
- errors = errors.where(errorFilter).toList();
- }
- if (errors.isEmpty) {
- fail('Expected one error, found: none');
- } else if (errors.length > 1) {
- var buffer = StringBuffer();
- buffer.writeln('Expected one error, found:');
- for (var error in errors) {
- buffer.writeln(' $error [${error.errorCode}]');
- }
- fail(buffer.toString());
- }
- return errors[0];
- }
-
- Future<AnalysisError> _findErrorToFixOfType(ErrorCode errorCode) async {
- var errors = await _computeErrors();
- for (var error in errors) {
- if (error.errorCode == errorCode) {
- return error;
- }
- }
- return null;
- }
-
List<Position> _findResultPositions(List<String> searchStrings) {
var positions = <Position>[];
for (var search in searchStrings) {
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 62832b7..41b5cce 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
@@ -81,6 +81,7 @@
import 'create_setter_test.dart' as create_setter;
import 'data_driven/test_all.dart' as data_driven;
import 'extend_class_for_mixin_test.dart' as extend_class_for_mixin;
+import 'fix_in_file_test.dart' as fix_in_file;
import 'fix_test.dart' as fix;
import 'import_async_test.dart' as import_async;
import 'import_library_prefix_test.dart' as import_library_prefix;
@@ -245,6 +246,7 @@
data_driven.main();
extend_class_for_mixin.main();
fix.main();
+ fix_in_file.main();
import_async.main();
import_library_prefix.main();
import_library_project.main();
