Skip to content

Commit

Permalink
Convert ChangeArgumentName to a producer
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
bwilkerson authored and commit-bot@chromium.org committed May 17, 2020
1 parent 7a94d1d commit 86edfbe
Show file tree
Hide file tree
Showing 4 changed files with 289 additions and 125 deletions.
Expand Up @@ -20,15 +20,8 @@ import 'package:analyzer_plugin/utilities/change_builder/change_workspace.dart';
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.
Expand All @@ -38,6 +31,90 @@ abstract class CorrectionProducer {
/// 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.
Expand Down Expand Up @@ -66,15 +143,6 @@ abstract class CorrectionProducer {

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;
Expand All @@ -85,14 +153,15 @@ abstract class CorrectionProducer {

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;
}
Expand Down Expand Up @@ -151,54 +220,3 @@ abstract class CorrectionProducer {
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;
}
}
@@ -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);
});
}
}
@@ -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;
}
}

0 comments on commit 86edfbe

Please sign in to comment.