[CMSR] Initial support for super formal parameters.

There are still holes for already `super.positional` in the target
constructor, and constructors that invoke it.

Change-Id: I5bd7d93947b1792c05a41c7b51426a8f7b589bca
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/350163
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
This commit is contained in:
Konstantin Shcheglov 2024-02-05 22:00:27 +00:00 committed by Commit Queue
parent 90d41f4f2f
commit baf84040e1
2 changed files with 284 additions and 3 deletions

View file

@ -22,6 +22,7 @@ import 'package:analyzer/src/utilities/extensions/collection.dart';
import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart';
import 'package:analyzer_plugin/utilities/change_builder/change_builder_dart.dart';
import 'package:analyzer_plugin/utilities/range_factory.dart';
import 'package:collection/collection.dart';
/// Analyzes the selection in [refactoringContext], and either returns
/// a [Available], or [NotAvailable].
@ -112,6 +113,9 @@ final class FormalParameterState {
/// [FormalParameterUpdate] to specify the same parameter.
final int id;
/// The element, used internally for `super` conversion.
final ParameterElement element;
/// The current kind of the formal parameter.
final FormalParameterKind kind;
@ -138,6 +142,7 @@ final class FormalParameterState {
FormalParameterState({
required this.id,
required this.element,
required this.kind,
required this.positionalIndex,
required this.name,
@ -158,9 +163,13 @@ class FormalParameterUpdate {
// TODO(scheglov): We might need `defaultValueText` added.
final FormalParameterKind kind;
/// Whether the formal parameter should be made `super`.
final bool withSuper;
FormalParameterUpdate({
required this.id,
required this.kind,
this.withSuper = false,
});
}
@ -561,6 +570,7 @@ class _SelectionAnalyzer {
formalParameterStateList.add(
FormalParameterState(
id: formalParameterId++,
element: parameterElement,
kind: kind,
positionalIndex: kind.isPositional ? positionalIndex++ : null,
name: nameToken.lexeme,
@ -695,12 +705,17 @@ class _SignatureUpdater {
/// Replaces [argumentList] with new code that has arguments as requested
/// by the formal parameter updates, reordering, changing kind, etc.
Future<ChangeStatus> updateArguments({
required Set<FormalParameterUpdate> excludedFormalParameters,
required ResolvedUnitResult resolvedUnit,
required ArgumentList argumentList,
required ChangeBuilder builder,
}) async {
final formalParameters = signatureUpdate.formalParameters
.whereNot(excludedFormalParameters.contains)
.toList();
final frameworkStatus = await framework.writeArguments(
formalParameterUpdates: signatureUpdate.formalParameters.map((update) {
formalParameterUpdates: formalParameters.map((update) {
// TODO(scheglov): Maybe support adding formal parameters.
final existing = selectionState.formalParameters[update.id];
final reference = _asFrameworkFormalParameterReference(existing);
@ -767,11 +782,21 @@ class _SignatureUpdater {
}
/// Returns the code with the `required` modifier.
String withRequired(FormalParameter existing) {
String withRequired(
FormalParameter existing, {
required bool withSuper,
}) {
final notDefault = existing.notDefault;
final requiredToken = notDefault.requiredKeyword;
if (requiredToken != null) {
return utils.getNodeText(existing);
} else if (withSuper) {
var nameToken = notDefault.name!;
var before = utils.getRangeText(
range.startStart(notDefault, nameToken),
);
// TODO(scheglov): what if already `super`?
return 'required ${before}super.${nameToken.lexeme}';
} else {
final after = utils.getNodeText(notDefault);
return 'required $after';
@ -832,7 +857,10 @@ class _SignatureUpdater {
final text = withoutRequired(existing);
optionalPositionalWrites.add(text);
case FormalParameterKind.requiredNamed:
final text = withRequired(existing);
final text = withRequired(
existing,
withSuper: update.withSuper,
);
namedWrites.add(text);
case FormalParameterKind.optionalNamed:
final text = withoutRequired(existing);
@ -963,7 +991,21 @@ class _SignatureUpdater {
return ChangeStatusFailure();
}
final excludedParameters = <FormalParameterUpdate>{};
if (invocation is SuperConstructorInvocation) {
var result = await _rewriteToNamedSuper(
builder: builder,
unitResult: unitResult,
invocation: invocation,
excludedParameters: excludedParameters,
);
if (result is! ChangeStatusSuccess) {
return result;
}
}
final result = await updateArguments(
excludedFormalParameters: excludedParameters,
resolvedUnit: unitResult,
argumentList: argumentList,
builder: builder,
@ -1077,6 +1119,114 @@ class _SignatureUpdater {
named: named,
);
}
/// Attempts to rewrite explicit arguments to `super()` invocation into
/// implicit arguments using `{super.name}` formal parameters.
Future<ChangeStatus> _rewriteToNamedSuper({
required ChangeBuilder builder,
required ResolvedUnitResult unitResult,
required SuperConstructorInvocation invocation,
required Set<FormalParameterUpdate> excludedParameters,
}) async {
var argumentList = invocation.argumentList;
final constructorDeclaration = invocation.parent;
if (constructorDeclaration is! ConstructorDeclaration) {
return ChangeStatusSuccess();
}
final parameterElementsToSuper = <ParameterElement>{};
for (final update in signatureUpdate.formalParameters) {
if (update.kind.isNamed) {
final existing = selectionState.formalParameters[update.id];
final reference = _asFrameworkFormalParameterReference(existing);
final argument = reference.argumentFrom(argumentList);
if (argument is! SimpleIdentifier) {
continue;
}
final parameterElement = argument.staticElement;
if (parameterElement is! ParameterElement) {
continue;
}
// We need the names to be the same to use `super.named`.
// If not, we still can use explicit argument to `super()`.
// TODO(scheglov): can we have a lint for this?
if (argument.name != existing.name) {
continue;
}
excludedParameters.add(update);
parameterElementsToSuper.add(parameterElement);
}
}
// Prepare for recursive update of the constructor.
final availability = analyzeAvailability(
refactoringContext: AbstractRefactoringContext(
searchEngine: searchEngine,
startSessions: refactoringContext.startSessions,
resolvedLibraryResult: refactoringContext.resolvedLibraryResult,
resolvedUnitResult: unitResult,
selectionOffset: constructorDeclaration.offset,
selectionLength: 0,
includeExperimental: true,
),
);
if (availability is! Available) {
return ChangeStatusFailure();
}
final selection = await analyzeSelection(
available: availability,
);
if (selection is! ValidSelectionState) {
return ChangeStatusFailure();
}
var formalParameterUpdatesNotNamed = <FormalParameterUpdate>[];
var formalParameterUpdatesNamed = <FormalParameterUpdate>[];
for (var formalParameter in selection.formalParameters) {
final element = formalParameter.element;
// TODO(scheglov): what if already named?
if (parameterElementsToSuper.contains(element)) {
formalParameterUpdatesNamed.add(
FormalParameterUpdate(
id: formalParameter.id,
kind: FormalParameterKind.requiredNamed,
withSuper: true,
),
);
} else {
formalParameterUpdatesNotNamed.add(
FormalParameterUpdate(
id: formalParameter.id,
kind: formalParameter.kind,
),
);
}
}
var status = await computeSourceChange(
selectionState: selection,
signatureUpdate: MethodSignatureUpdate(
formalParameters: [
...formalParameterUpdatesNotNamed,
...formalParameterUpdatesNamed,
],
formalParametersTrailingComma:
signatureUpdate.formalParametersTrailingComma,
argumentsTrailingComma: signatureUpdate.argumentsTrailingComma,
),
builder: builder,
);
if (status is! ChangeStatusSuccess) {
return status;
}
return ChangeStatusSuccess();
}
}
extension on AstNode {

View file

@ -1221,6 +1221,137 @@ void f() {
''');
}
Future<void>
test_classConstructor_requiredPositional_toRequiredNamed_canSuper() async {
await _analyzeValidSelection(r'''
class A {
^A(int a, int b, int c);
}
class B extends A {
B(int a, int b, int c) : super(a, b, c);
}
void f() {
A(0, 1, 2);
B(3, 4, 5);
}
''');
final signatureUpdate = MethodSignatureUpdate(
formalParameters: [
FormalParameterUpdate(
id: 0,
kind: FormalParameterKind.requiredPositional,
),
FormalParameterUpdate(
id: 2,
kind: FormalParameterKind.requiredPositional,
),
FormalParameterUpdate(
id: 1,
kind: FormalParameterKind.requiredNamed,
),
],
formalParametersTrailingComma: TrailingComma.always,
argumentsTrailingComma: ArgumentsTrailingComma.ifPresent,
);
await _assertUpdate(signatureUpdate, r'''
>>>>>>> /home/test/lib/test.dart
class A {
A(
int a,
int c, {
required int b,
});
}
class B extends A {
B(
int a,
int c, {
required int super.b,
}) : super(a, c);
}
void f() {
A(0, 2, b: 1);
B(3, 5, b: 4);
}
''');
}
Future<void>
test_classConstructor_requiredPositional_toRequiredNamed_canSuper_differentName() async {
await _analyzeValidSelection(r'''
class A {
^A(int a, int b, int c, int d);
}
class B extends A {
B(int a, int x, int c, int d) : super(a, x, c, d);
}
void f() {
A(0, 1, 2, 3);
B(4, 5, 6, 7);
}
''');
final signatureUpdate = MethodSignatureUpdate(
formalParameters: [
FormalParameterUpdate(
id: 0,
kind: FormalParameterKind.requiredPositional,
),
FormalParameterUpdate(
id: 3,
kind: FormalParameterKind.requiredPositional,
),
FormalParameterUpdate(
id: 1,
kind: FormalParameterKind.requiredNamed,
),
FormalParameterUpdate(
id: 2,
kind: FormalParameterKind.requiredNamed,
),
],
formalParametersTrailingComma: TrailingComma.always,
argumentsTrailingComma: ArgumentsTrailingComma.ifPresent,
);
// The operation succeeded, even though `int x` has different name than
// `int b` in `A`. We cannot use `{super.x}`, but we still can forward
// `x` explicitly with `super(b: x)`.
await _assertUpdate(signatureUpdate, r'''
>>>>>>> /home/test/lib/test.dart
class A {
A(
int a,
int d, {
required int b,
required int c,
});
}
class B extends A {
B(
int a,
int x,
int d, {
required int super.c,
}) : super(a, d, b: x);
}
void f() {
A(0, 3, b: 1, c: 2);
B(4, 5, 7, c: 6);
}
''');
}
Future<void> test_classConstructor_superConstructorInvocation_named() async {
await _analyzeValidSelection(r'''
class A {