Convert ChangeArgumentName to a producer
The ChangeArgumentName "fix" is one of a small number that can produce
multiple fixes. I'm proposing that we support these by adding objects
that can dynamically produce zero or more CorrectionProducers depending
on the situation being flagged as an error.
Change-Id: I37c06ffefd46f4b8478d127d1b537b3bc840e024
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/148360
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
diff --git a/pkg/analysis_server/lib/src/services/correction/dart/abstract_producer.dart b/pkg/analysis_server/lib/src/services/correction/dart/abstract_producer.dart
index 00fa1c3..234e13d 100644
--- a/pkg/analysis_server/lib/src/services/correction/dart/abstract_producer.dart
+++ b/pkg/analysis_server/lib/src/services/correction/dart/abstract_producer.dart
@@ -20,15 +20,8 @@
import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
import 'package:meta/meta.dart';
-abstract class CorrectionProducer {
- CorrectionProducerContext _context;
-
- /// The most deeply nested node that completely covers the highlight region of
- /// the diagnostic, or `null` if there is no diagnostic, such a node does not
- /// exist, or if it hasn't been computed yet. Use [coveredNode] to access this
- /// field.
- AstNode _coveredNode;
-
+/// An object that can compute a correction (fix or assist).
+abstract class CorrectionProducer extends _AbstractCorrectionProducer {
/// Return the arguments that should be used when composing the message for an
/// assist, or `null` if the assist message has no parameters or if this
/// producer doesn't support assists.
@@ -38,6 +31,90 @@
/// if this producer doesn't support assists.
AssistKind get assistKind => null;
+ /// Return the arguments that should be used when composing the message for a
+ /// fix, or `null` if the fix message has no parameters or if this producer
+ /// doesn't support fixes.
+ List<Object> get fixArguments => null;
+
+ /// Return the fix kind that should be used to build a fix, or `null` if this
+ /// producer doesn't support fixes.
+ FixKind get fixKind => null;
+
+ Future<void> compute(DartChangeBuilder builder);
+}
+
+class CorrectionProducerContext {
+ final int selectionOffset;
+ final int selectionLength;
+ final int selectionEnd;
+
+ final CompilationUnit unit;
+ final CorrectionUtils utils;
+ final String file;
+
+ final TypeProvider typeProvider;
+ final Flutter flutter;
+
+ final AnalysisSession session;
+ final AnalysisSessionHelper sessionHelper;
+ final ResolvedUnitResult resolvedResult;
+ final ChangeWorkspace workspace;
+
+ final Diagnostic diagnostic;
+
+ AstNode _node;
+
+ CorrectionProducerContext({
+ @required this.resolvedResult,
+ @required this.workspace,
+ this.diagnostic,
+ this.selectionOffset = -1,
+ this.selectionLength = 0,
+ }) : file = resolvedResult.path,
+ flutter = Flutter.of(resolvedResult),
+ session = resolvedResult.session,
+ sessionHelper = AnalysisSessionHelper(resolvedResult.session),
+ typeProvider = resolvedResult.typeProvider,
+ selectionEnd = (selectionOffset ?? 0) + (selectionLength ?? 0),
+ unit = resolvedResult.unit,
+ utils = CorrectionUtils(resolvedResult);
+
+ AstNode get node => _node;
+
+ /// Return `true` the lint with the given [name] is enabled.
+ bool isLintEnabled(String name) {
+ var analysisOptions = session.analysisContext.analysisOptions;
+ return analysisOptions.isLintEnabled(name);
+ }
+
+ bool setupCompute() {
+ final locator = NodeLocator(selectionOffset, selectionEnd);
+ _node = locator.searchWithin(resolvedResult.unit);
+ return _node != null;
+ }
+}
+
+/// An object that can dynamically compute multiple corrections (fixes or
+/// assists).
+abstract class MultiCorrectionProducer extends _AbstractCorrectionProducer {
+ /// Return each of the individual producers generated by this producer.
+ Iterable<CorrectionProducer> get producers;
+}
+
+/// The behavior shared by [CorrectionProducer] and [MultiCorrectionProducer].
+abstract class _AbstractCorrectionProducer {
+ /// The context used to produce corrections.
+ CorrectionProducerContext _context;
+
+ /// The most deeply nested node that completely covers the highlight region of
+ /// the diagnostic, or `null` if there is no diagnostic, such a node does not
+ /// exist, or if it hasn't been computed yet. Use [coveredNode] to access this
+ /// field.
+ AstNode _coveredNode;
+
+ /// Initialize a newly created producer.
+ _AbstractCorrectionProducer();
+
/// The most deeply nested node that completely covers the highlight region of
/// the diagnostic, or `null` if there is no diagnostic or if such a node does
/// not exist.
@@ -66,15 +143,6 @@
String get file => _context.file;
- /// Return the arguments that should be used when composing the message for a
- /// fix, or `null` if the fix message has no parameters or if this producer
- /// doesn't support fixes.
- List<Object> get fixArguments => null;
-
- /// Return the fix kind that should be used to build a fix, or `null` if this
- /// producer doesn't support fixes.
- FixKind get fixKind => null;
-
Flutter get flutter => _context.flutter;
AstNode get node => _context.node;
@@ -85,14 +153,15 @@
int get selectionOffset => _context.selectionOffset;
+ AnalysisSessionHelper get sessionHelper => _context.sessionHelper;
+
TypeProvider get typeProvider => _context.typeProvider;
CompilationUnit get unit => _context.unit;
CorrectionUtils get utils => _context.utils;
- Future<void> compute(DartChangeBuilder builder);
-
+ /// Configure this producer based on the [context].
void configure(CorrectionProducerContext context) {
_context = context;
}
@@ -151,54 +220,3 @@
return false;
}
}
-
-class CorrectionProducerContext {
- final int selectionOffset;
- final int selectionLength;
- final int selectionEnd;
-
- final CompilationUnit unit;
- final CorrectionUtils utils;
- final String file;
-
- final TypeProvider typeProvider;
- final Flutter flutter;
-
- final AnalysisSession session;
- final AnalysisSessionHelper sessionHelper;
- final ResolvedUnitResult resolvedResult;
- final ChangeWorkspace workspace;
-
- final Diagnostic diagnostic;
-
- AstNode _node;
-
- CorrectionProducerContext({
- @required this.resolvedResult,
- @required this.workspace,
- this.diagnostic,
- this.selectionOffset = -1,
- this.selectionLength = 0,
- }) : file = resolvedResult.path,
- flutter = Flutter.of(resolvedResult),
- session = resolvedResult.session,
- sessionHelper = AnalysisSessionHelper(resolvedResult.session),
- typeProvider = resolvedResult.typeProvider,
- selectionEnd = (selectionOffset ?? 0) + (selectionLength ?? 0),
- unit = resolvedResult.unit,
- utils = CorrectionUtils(resolvedResult);
-
- AstNode get node => _node;
-
- /// Return `true` the lint with the given [name] is enabled.
- bool isLintEnabled(String name) {
- var analysisOptions = session.analysisContext.analysisOptions;
- return analysisOptions.isLintEnabled(name);
- }
-
- bool setupCompute() {
- final locator = NodeLocator(selectionOffset, selectionEnd);
- _node = locator.searchWithin(resolvedResult.unit);
- return _node != null;
- }
-}
diff --git a/pkg/analysis_server/lib/src/services/correction/dart/change_argument_name.dart b/pkg/analysis_server/lib/src/services/correction/dart/change_argument_name.dart
new file mode 100644
index 0000000..6ec06fc
--- /dev/null
+++ b/pkg/analysis_server/lib/src/services/correction/dart/change_argument_name.dart
@@ -0,0 +1,88 @@
+// Copyright (c) 2020, 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/dart/abstract_producer.dart';
+import 'package:analysis_server/src/services/correction/executable_parameters.dart';
+import 'package:analysis_server/src/services/correction/fix.dart';
+import 'package:analysis_server/src/services/correction/levenshtein.dart';
+import 'package:analyzer/dart/ast/ast.dart';
+import 'package:analyzer_plugin/utilities/change_builder/change_builder_dart.dart';
+import 'package:analyzer_plugin/utilities/fixes/fixes.dart';
+import 'package:analyzer_plugin/utilities/range_factory.dart';
+
+class ChangeArgumentName extends MultiCorrectionProducer {
+ static const maxDistance = 4;
+
+ @override
+ Iterable<CorrectionProducer> get producers sync* {
+ var names = _getNamedParameterNames();
+ if (names == null || names.isEmpty) {
+ return;
+ }
+ SimpleIdentifier argumentName = node;
+ var invalidName = argumentName.name;
+ for (var proposedName in names) {
+ var distance = _computeDistance(invalidName, proposedName);
+ if (distance <= maxDistance) {
+ // TODO(brianwilkerson) Create a way to use the distance as part of the
+ // computation of the priority (so that closer names sort first).
+ yield _ChangeName(argumentName, proposedName);
+ }
+ }
+ }
+
+ int _computeDistance(String current, String proposal) {
+ if ((current == 'child' && proposal == 'children') ||
+ (current == 'children' && proposal == 'child')) {
+ // Special case handling for 'child' and 'children' is unnecessary if
+ // `maxDistance >= 3`, but is included to prevent regression in case the
+ // value is changed to improve results.
+ return 1;
+ }
+ return levenshtein(current, proposal, maxDistance, caseSensitive: false);
+ }
+
+ List<String> _getNamedParameterNames() {
+ var namedExpression = node?.parent?.parent;
+ if (node is SimpleIdentifier &&
+ namedExpression is NamedExpression &&
+ namedExpression.name == node.parent &&
+ namedExpression.parent is ArgumentList) {
+ var parameters = ExecutableParameters(
+ sessionHelper,
+ namedExpression.parent.parent,
+ );
+ return parameters?.namedNames;
+ }
+ return null;
+ }
+
+ /// Return an instance of this class. Used as a tear-off in `FixProcessor`.
+ static ChangeArgumentName newInstance() => ChangeArgumentName();
+}
+
+/// A correction processor that can make one of the possible change computed by
+/// the [ChangeArgumentName] producer.
+class _ChangeName extends CorrectionProducer {
+ /// The name of the argument being changed.
+ final SimpleIdentifier argumentName;
+
+ /// The name to which the argument name will be changed.
+ final String proposedName;
+
+ _ChangeName(this.argumentName, this.proposedName);
+
+ @override
+ List<Object> get fixArguments => [proposedName];
+
+ @override
+ FixKind get fixKind => DartFixKind.CHANGE_ARGUMENT_NAME;
+
+ @override
+ Future<void> compute(DartChangeBuilder builder) async {
+ await builder.addFileEdit(file, (builder) {
+ builder.addSimpleReplacement(range.node(argumentName), proposedName);
+ });
+ }
+}
diff --git a/pkg/analysis_server/lib/src/services/correction/executable_parameters.dart b/pkg/analysis_server/lib/src/services/correction/executable_parameters.dart
new file mode 100644
index 0000000..522ca6d
--- /dev/null
+++ b/pkg/analysis_server/lib/src/services/correction/executable_parameters.dart
@@ -0,0 +1,88 @@
+// Copyright (c) 2014, 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 'dart:async';
+import 'dart:core';
+
+import 'package:analyzer/dart/ast/ast.dart';
+import 'package:analyzer/dart/element/element.dart';
+import 'package:analyzer/src/dart/analysis/session_helper.dart';
+
+/// [ExecutableElement], its parameters, and operations on them.
+class ExecutableParameters {
+ final AnalysisSessionHelper sessionHelper;
+ final ExecutableElement executable;
+
+ final List<ParameterElement> required = [];
+ final List<ParameterElement> optionalPositional = [];
+ final List<ParameterElement> named = [];
+
+ factory ExecutableParameters(
+ AnalysisSessionHelper sessionHelper, AstNode invocation) {
+ Element element;
+ // This doesn't handle FunctionExpressionInvocation.
+ if (invocation is Annotation) {
+ element = invocation.element;
+ } else if (invocation is InstanceCreationExpression) {
+ element = invocation.staticElement;
+ } else if (invocation is MethodInvocation) {
+ element = invocation.methodName.staticElement;
+ } else if (invocation is ConstructorReferenceNode) {
+ element = invocation.staticElement;
+ }
+ if (element is ExecutableElement && !element.isSynthetic) {
+ return ExecutableParameters._(sessionHelper, element);
+ } else {
+ return null;
+ }
+ }
+
+ ExecutableParameters._(this.sessionHelper, this.executable) {
+ for (var parameter in executable.parameters) {
+ if (parameter.isRequiredPositional) {
+ required.add(parameter);
+ } else if (parameter.isOptionalPositional) {
+ optionalPositional.add(parameter);
+ } else if (parameter.isNamed) {
+ named.add(parameter);
+ }
+ }
+ }
+
+ /// Return the path of the file in which the executable is declared.
+ String get file => executable.source.fullName;
+
+ /// Return the names of the named parameters.
+ List<String> get namedNames {
+ return named.map((parameter) => parameter.name).toList();
+ }
+
+ /// Return the [FormalParameterList] of the [executable], or `nul be found.
+ Future<FormalParameterList> getParameterList() async {
+ var result = await sessionHelper.getElementDeclaration(executable);
+ var targetDeclaration = result.node;
+ if (targetDeclaration is ConstructorDeclaration) {
+ return targetDeclaration.parameters;
+ } else if (targetDeclaration is FunctionDeclaration) {
+ var function = targetDeclaration.functionExpression;
+ return function.parameters;
+ } else if (targetDeclaration is MethodDeclaration) {
+ return targetDeclaration.parameters;
+ }
+ return null;
+ }
+
+ /// Return the [FormalParameter] of the [element] in [FormalParameterList],
+ /// or `null` if it can't be found.
+ Future<FormalParameter> getParameterNode(ParameterElement element) async {
+ var result = await sessionHelper.getElementDeclaration(element);
+ var declaration = result.node;
+ for (var node = declaration; node != null; node = node.parent) {
+ if (node is FormalParameter && node.parent is FormalParameterList) {
+ return node;
+ }
+ }
+ return null;
+ }
+}
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 12d3153..db30359 100644
--- a/pkg/analysis_server/lib/src/services/correction/fix_internal.dart
+++ b/pkg/analysis_server/lib/src/services/correction/fix_internal.dart
@@ -25,6 +25,7 @@
import 'package:analysis_server/src/services/correction/dart/add_return_type.dart';
import 'package:analysis_server/src/services/correction/dart/add_static.dart';
import 'package:analysis_server/src/services/correction/dart/add_type_annotation.dart';
+import 'package:analysis_server/src/services/correction/dart/change_argument_name.dart';
import 'package:analysis_server/src/services/correction/dart/convert_add_all_to_spread.dart';
import 'package:analysis_server/src/services/correction/dart/convert_conditional_expression_to_if_element.dart';
import 'package:analysis_server/src/services/correction/dart/convert_documentation_into_line.dart';
@@ -100,6 +101,9 @@
/// A predicate is a one-argument function that returns a boolean value.
typedef ElementPredicate = bool Function(Element argument);
+/// A function that can be executed to create a multi-correction producer.
+typedef MultiProducerGenerator = MultiCorrectionProducer Function();
+
/// A function that can be executed to create a correction producer.
typedef ProducerGenerator = CorrectionProducer Function();
@@ -349,6 +353,16 @@
// LintNames.use_rethrow_when_possible : [],
};
+ /// A map from error codes to a list of generators used to create multiple
+ /// correction producers used to build fixes for those diagnostics. The
+ /// generators used for lint rules are in the [lintMultiProducerMap].
+ static const Map<ErrorCode, List<MultiProducerGenerator>>
+ nonLintMultiProducerMap = {
+ CompileTimeErrorCode.UNDEFINED_NAMED_PARAMETER: [
+ ChangeArgumentName.newInstance
+ ],
+ };
+
/// A map from error codes to a list of generators used to create the
/// correction producers used to build fixes for those diagnostics. The
/// generators used for lint rules are in the [lintProducerMap].
@@ -802,7 +816,6 @@
}
if (errorCode == CompileTimeErrorCode.UNDEFINED_NAMED_PARAMETER) {
await _addFix_addMissingParameterNamed();
- await _addFix_changeArgumentName();
}
if (errorCode == StaticTypeWarningCode.ILLEGAL_ASYNC_RETURN_TYPE) {
await _addFix_illegalAsyncReturnType();
@@ -1217,57 +1230,6 @@
}
}
- Future<void> _addFix_changeArgumentName() async {
- const maxDistance = 4;
-
- List<String> getNamedParameterNames() {
- var namedExpression = node?.parent?.parent;
- if (node is SimpleIdentifier &&
- namedExpression is NamedExpression &&
- namedExpression.name == node.parent &&
- namedExpression.parent is ArgumentList) {
- var parameters = _ExecutableParameters(
- sessionHelper,
- namedExpression.parent.parent,
- );
- return parameters?.namedNames;
- }
- return null;
- }
-
- int computeDistance(String current, String proposal) {
- if ((current == 'child' && proposal == 'children') ||
- (current == 'children' && proposal == 'child')) {
- // Special case handling for 'child' and 'children' is unnecessary if
- // `maxDistance >= 3`, but is included to prevent regression in case the
- // value is changed to improve results.
- return 1;
- }
- return levenshtein(current, proposal, maxDistance, caseSensitive: false);
- }
-
- var names = getNamedParameterNames();
- if (names == null || names.isEmpty) {
- return;
- }
-
- SimpleIdentifier argumentName = node;
- var invalidName = argumentName.name;
- for (var proposedName in names) {
- var distance = computeDistance(invalidName, proposedName);
- if (distance <= maxDistance) {
- var changeBuilder = _newDartChangeBuilder();
- await changeBuilder.addFileEdit(file, (builder) {
- builder.addSimpleReplacement(range.node(argumentName), proposedName);
- });
- // TODO(brianwilkerson) Create a way to use the distance as part of the
- // computation of the priority (so that closer names sort first).
- _addFixFromBuilder(changeBuilder, DartFixKind.CHANGE_ARGUMENT_NAME,
- args: [proposedName]);
- }
- }
- }
-
Future<void> _addFix_changeToNearestPreciseValue() async {
IntegerLiteral integer = node;
var lexeme = integer.literal.lexeme;
@@ -4271,10 +4233,8 @@
Future<void> compute(CorrectionProducer producer) async {
producer.configure(context);
-
var builder = _newDartChangeBuilder();
await producer.compute(builder);
-
_addFixFromBuilder(builder, producer.fixKind,
args: producer.fixArguments);
}
@@ -4294,6 +4254,16 @@
await compute(generator());
}
}
+ var multiGenerators = nonLintMultiProducerMap[errorCode];
+ if (multiGenerators != null) {
+ for (var multiGenerator in multiGenerators) {
+ var multiProducer = multiGenerator();
+ multiProducer.configure(context);
+ for (var producer in multiProducer.producers) {
+ await compute(producer);
+ }
+ }
+ }
}
}
