Fix a false positive in convert_to_super_parameters

Unlike normal parameters, super parameters are visible in the body of
the constructor. So if the normal parameter is referenced it can't be
converted.

Change-Id: I6a48e0d6de896b25d7a9d25f6d20ae7ad01f82c3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/237502
Reviewed-by: Phil Quitslund <pquitslund@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
This commit is contained in:
Brian Wilkerson 2022-03-16 04:19:03 +00:00 committed by Commit Bot
parent 8a43a891f3
commit cd5933c833
2 changed files with 72 additions and 30 deletions

View file

@ -8,6 +8,7 @@ import 'package:analysis_server/src/services/correction/fix.dart';
import 'package:analysis_server/src/utilities/extensions/range_factory.dart';
import 'package:analyzer/dart/analysis/features.dart';
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/ast/visitor.dart';
import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/source/source_range.dart';
import 'package:analyzer_plugin/utilities/assist/assist.dart';
@ -62,6 +63,7 @@ class ConvertToSuperParameters extends CorrectionProducer {
// set to `null` if a positional argument is found that can't be converted
// because either all of the positional parameters must be converted or none
// of them can be converted.
var referencedParameters = _referencedParameters(constructor);
var parameterMap = _parameterMap(constructor.parameters);
List<_ParameterData>? positional = [];
var named = <_ParameterData>[];
@ -71,12 +73,12 @@ class ConvertToSuperParameters extends CorrectionProducer {
argumentIndex++) {
var argument = arguments[argumentIndex];
if (argument is NamedExpression) {
var parameterAndElement =
_parameterFor(parameterMap, argument.expression);
if (parameterAndElement != null && parameterAndElement.isNamed) {
var parameter = _parameterFor(parameterMap, argument.expression);
if (parameter != null &&
parameter.isNamed &&
!referencedParameters.contains(parameter.element)) {
var data = _dataForParameter(
parameterAndElement.parameter,
parameterAndElement.element,
parameter,
argumentIndex,
argument.staticParameterElement,
);
@ -85,13 +87,14 @@ class ConvertToSuperParameters extends CorrectionProducer {
}
}
} else if (positional != null) {
var parameterAndElement = _parameterFor(parameterMap, argument);
if (parameterAndElement == null || !parameterAndElement.isPositional) {
var parameter = _parameterFor(parameterMap, argument);
if (parameter == null ||
!parameter.isPositional ||
referencedParameters.contains(parameter.element)) {
positional = null;
} else {
var data = _dataForParameter(
parameterAndElement.parameter,
parameterAndElement.element,
parameter,
argumentIndex,
argument.staticParameterElement,
);
@ -175,10 +178,7 @@ class ConvertToSuperParameters extends CorrectionProducer {
/// If the [parameter] can be converted into a super initializing formal
/// parameter then return the data needed to do so.
_ParameterData? _dataForParameter(
_Parameter parameter,
ParameterElement thisParameter,
int argumentIndex,
_ParameterData? _dataForParameter(_Parameter parameter, int argumentIndex,
ParameterElement? superParameter) {
if (superParameter == null) {
return null;
@ -186,7 +186,7 @@ class ConvertToSuperParameters extends CorrectionProducer {
// If the type of the `thisParameter` isn't a subtype of the type of the
// super parameter, then the change isn't appropriate.
var superType = superParameter.type;
var thisType = thisParameter.type;
var thisType = parameter.element.type;
if (!typeSystem.isSubtypeOf(thisType, superType)) {
return null;
}
@ -200,7 +200,7 @@ class ConvertToSuperParameters extends CorrectionProducer {
return _ParameterData(
argumentIndex: argumentIndex,
defaultValueRange: _defaultValueRange(
parameter.parameter, superParameter, thisParameter),
parameter.parameter, superParameter, parameter.element),
nameOffset: identifier.offset,
parameterIndex: parameter.index,
typeToDelete: superType == thisType ? _type(parameter.parameter) : null,
@ -259,16 +259,16 @@ class ConvertToSuperParameters extends CorrectionProducer {
return true;
}
/// Return the parameter element corresponding to the [expression], or `null`
/// if the expression isn't a simple reference to one of the parameters in the
/// Return the parameter corresponding to the [expression], or `null` if the
/// expression isn't a simple reference to one of the parameters in the
/// constructor being converted.
_ParameterAndElement? _parameterFor(
_Parameter? _parameterFor(
Map<ParameterElement, _Parameter> parameterMap, Expression expression) {
if (expression is SimpleIdentifier) {
var element = expression.staticElement;
var parameter = parameterMap[element];
if (element is ParameterElement && parameter != null) {
return _ParameterAndElement(element, parameter);
if (parameter != null) {
return parameter;
}
}
return null;
@ -284,12 +284,21 @@ class ConvertToSuperParameters extends CorrectionProducer {
var parameter = parameters[i];
var element = parameter.declaredElement;
if (element != null) {
map[element] = _Parameter(parameter, i);
map[element] = _Parameter(parameter, element, i);
}
}
return map;
}
/// Return a set containing the elements of all of the parameters that are
/// referenced in the body of the [constructor].
Set<ParameterElement> _referencedParameters(
ConstructorDeclaration constructor) {
var collector = _ReferencedParameterCollector();
constructor.body.accept(collector);
return collector.foundParameters;
}
/// Return the invocation of the super constructor.
SuperConstructorInvocation? _superInvocation(
ConstructorDeclaration constructor) {
@ -337,18 +346,11 @@ class ConvertToSuperParameters extends CorrectionProducer {
class _Parameter {
final FormalParameter parameter;
final int index;
_Parameter(this.parameter, this.index);
}
/// A data class used to avoid a null check.
class _ParameterAndElement {
final ParameterElement element;
final _Parameter parameter;
final int index;
_ParameterAndElement(this.element, this.parameter);
_Parameter(this.parameter, this.element, this.index);
bool get isNamed => element.isNamed;
@ -385,6 +387,18 @@ class _ParameterData {
required this.argumentIndex});
}
class _ReferencedParameterCollector extends RecursiveAstVisitor<void> {
final Set<ParameterElement> foundParameters = {};
@override
void visitSimpleIdentifier(SimpleIdentifier node) {
var element = node.staticElement;
if (element is ParameterElement) {
foundParameters.add(element);
}
}
}
/// Information about the ranges of text that need to be removed in order to
/// remove a type annotation.
class _TypeData {

View file

@ -283,6 +283,34 @@ class B extends A {
await assertNoAssistAt('B(');
}
Future<void> test_invalid_referencedInBody_named() async {
await resolveTestCode('''
class A {
A({int? x});
}
class B extends A {
B({int? x}) : super(x: x) {
print(x);
}
}
''');
await assertNoAssistAt('B(');
}
Future<void> test_invalid_referencedInBody_positional() async {
await resolveTestCode('''
class A {
A(int x);
}
class B extends A {
B(int x) : super(x) {
print(x);
}
}
''');
await assertNoAssistAt('B(');
}
Future<void> test_mixed_first() async {
await resolveTestCode('''
class A {