Update bind-to-field for primary constructors

This updates the fix so that in a primary constructor, instead of
generating a new field declaration when there is no existing field, it
instead converts the parameter into a declaring parameter.

It also restricts the assist so that it isn't produced if the selection
is inside a default value expression.

Because this and bind-all-to-fields share so much code, I also fixed
the latter.

I added tests for both fixes. I renamed some of the tests to better
conform with our style, but didn't rename all of them.

Closes https://github.com/dart-lang/sdk/issues/63035
Closes https://github.com/dart-lang/sdk/issues/63034

Change-Id: I1bdbd6f2b987eecb81b2fa64aed91c21d4757b53
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/493482
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
This commit is contained in:
Brian Wilkerson
2026-04-07 10:59:34 -07:00
committed by Commit Queue
parent 26fc53e109
commit 39607f64c6
3 changed files with 145 additions and 38 deletions
@@ -5,6 +5,7 @@
import 'package:analysis_server/src/services/correction/assist.dart';
import 'package:analysis_server_plugin/edit/dart/correction_producer.dart';
import 'package:analyzer/dart/ast/ast.dart';
import 'package:analyzer/dart/ast/token.dart';
import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/dart/element/type.dart';
import 'package:analyzer/source/source_range.dart';
@@ -36,8 +37,14 @@ class BindToField extends ResolvedCorrectionProducer {
Future<void> compute(ChangeBuilder builder) async {
var parameter = node.thisOrAncestorOfType<FormalParameter>();
if (parameter != null) {
// Don't propose an assist for super parameters and super parameters with
// a default value because they can't be bound to a field.
if (parameter is DefaultFormalParameter) {
var separator = parameter.separator;
if (separator != null && node.offset >= separator.offset) {
// Don't propose the assist if the selection is inside the default
// value.
return;
}
}
await tryReplacingParameter(file, builder, parameter, libraryElement2);
}
}
@@ -48,19 +55,54 @@ class BindToField extends ResolvedCorrectionProducer {
FormalParameter parameter,
LibraryElement libraryElement2,
) async {
if (parameter is SuperFormalParameter) {
return;
}
if (parameter is DefaultFormalParameter &&
parameter.parameter is SuperFormalParameter) {
if (parameter is SuperFormalParameter ||
(parameter is DefaultFormalParameter &&
parameter.parameter is SuperFormalParameter)) {
// Don't propose the assist for super parameters because they can't be
// bound to a field.
return;
}
if (parameter is FieldFormalParameter) {
// Don't propose the assist if the parameter is already a field formal
// parameter.
return;
}
await _replaceParameterWithThis(file, builder, parameter, libraryElement2);
}
static (
List<ConstructorInitializer> initializers,
Token? factoryKeyword,
Token? constKeyword,
CompilationUnitMember? container,
)?
_constructorParts(FormalParameter parameter) {
var constructor = parameter.thisOrAncestorOfType<ConstructorDeclaration>();
if (constructor != null) {
var container = constructor.thisOrAncestorOfType<CompilationUnitMember>();
return (
constructor.initializers,
constructor.factoryKeyword,
constructor.constKeyword,
container,
);
}
var primaryConstructor = parameter
.thisOrAncestorOfType<PrimaryConstructorDeclaration>();
if (primaryConstructor != null) {
var body = primaryConstructor.body;
var container = primaryConstructor
.thisOrAncestorOfType<CompilationUnitMember>();
return (
body?.initializers ?? [],
null as Token?,
primaryConstructor.constKeyword,
container,
);
}
return null;
}
static SourceRange _replaceable(FormalParameter parameter) {
return switch (parameter) {
SimpleFormalParameter() => range.startEnd(
@@ -80,22 +122,22 @@ class BindToField extends ResolvedCorrectionProducer {
FormalParameter parameter,
LibraryElement libraryElement2,
) async {
var constructor = parameter.thisOrAncestorOfType<ConstructorDeclaration>();
if (constructor == null) {
// Not a constructor.
var constructorParts = _constructorParts(parameter);
if (constructorParts == null) {
return;
}
if (constructor.childEntities.any(
var (initializers, factoryKeyword, constKeyword, container) =
constructorParts;
if (initializers.any(
(element) => element is RedirectingConstructorInvocation,
)) {
// A redirecting constructor.
return;
}
if (constructor.factoryKeyword != null) {
if (factoryKeyword != null) {
// A factory constructor.
return;
}
var container = constructor.thisOrAncestorOfType<CompilationUnitMember>();
if (container == null ||
(container is! ClassDeclaration && container is! EnumDeclaration)) {
// Not a class or enum.
@@ -133,6 +175,10 @@ class BindToField extends ResolvedCorrectionProducer {
type,
libraryElement2,
);
if (fixType == _FixType.addVar) {
builder.addSimpleInsertion(parameter.offset, 'var ');
return;
}
if (fixType != _FixType.noop) {
builder.addSimpleReplacement(
_replaceable(parameter),
@@ -141,7 +187,7 @@ class BindToField extends ResolvedCorrectionProducer {
}
if (fixType == _FixType.replaceWithThisAndNewField) {
builder.insertField(container, (builder) {
var isFinal = constructor.constKeyword != null || parameter.isFinal;
var isFinal = constKeyword != null || parameter.isFinal;
builder.writeFieldDeclaration(
name.lexeme,
isFinal: isFinal,
@@ -182,6 +228,11 @@ class BindToField extends ResolvedCorrectionProducer {
return _FixType.replaceWithThis;
}
} else {
var declaration = parameter
.thisOrAncestorOfType<PrimaryConstructorDeclaration>();
if (declaration != null) {
return _FixType.addVar;
}
return _FixType.replaceWithThisAndNewField;
}
}
@@ -189,4 +240,4 @@ class BindToField extends ResolvedCorrectionProducer {
}
}
enum _FixType { replaceWithThis, replaceWithThisAndNewField, noop }
enum _FixType { replaceWithThis, replaceWithThisAndNewField, addVar, noop }
@@ -42,6 +42,16 @@ class A {
''');
}
Future<void>
test_typed_multiple_constructor_parameter_primaryConstructor() async {
await resolveTestCode('''
class C(int ^i, String s);
''');
await assertHasAssist('''
class C(var int i, var String s);
''');
}
Future<void> test_typed_multiple_constructor_parameters() async {
await resolveTestCode('''
class A {
@@ -165,6 +175,19 @@ class B {
''');
}
Future<void> test_with_existing_fields_primaryConstructor() async {
await resolveTestCode('''
class Foo({this.a, bool ^b = false}) {
final String? a;
}
''');
await assertHasAssist('''
class Foo({this.a, var bool b = false}) {
final String? a;
}
''');
}
Future<void> test_with_two_parameters() async {
await resolveTestCode('''
class Foo {
@@ -21,7 +21,39 @@ class BindToFieldTest extends AssistProcessorTest {
@override
AssistKind get kind => DartAssistKind.bindToField;
Future<void> test_class_constructor_same_named_field() async {
Future<void> test_class_primaryConstructor_requiredPositional() async {
await resolveTestCode('''
class C(int ^i);
''');
await assertHasAssist('''
class C(var int i);
''');
}
Future<void> test_class_primaryConstructor_sameNamedField() async {
await resolveTestCode('''
class C(int ^i) {
int? i;
}
''');
await assertHasAssist('''
class C(this.i) {
int? i;
}
''');
}
Future<void>
test_class_secondaryConstructor_optionalPositional_inDefault() async {
await resolveTestCode('''
class C {
C([int i = ^0]);
}
''');
await assertNoAssist();
}
Future<void> test_class_secondaryConstructor_sameNamedField() async {
await resolveTestCode('''
class A {
int? i;
@@ -38,7 +70,8 @@ class A {
''');
}
Future<void> test_class_constructor_same_named_field_wrong_type() async {
Future<void>
test_class_secondaryConstructor_sameNamedField_wrongType() async {
await resolveTestCode('''
class A {
String? i;
@@ -49,7 +82,7 @@ class A {
await assertNoAssist();
}
Future<void> test_class_constructor_same_named_method() async {
Future<void> test_class_secondaryConstructor_sameNamedMethod() async {
await resolveTestCode('''
class A {
void i(){}
@@ -60,7 +93,7 @@ class A {
await assertNoAssist();
}
Future<void> test_enum_constructor() async {
Future<void> test_enum_secondaryConstructor() async {
await resolveTestCode('''
enum A {
e(3);
@@ -79,7 +112,7 @@ enum A {
''');
}
Future<void> test_enum_constructor_same_name() async {
Future<void> test_enum_secondaryConstructor_sameNamedField() async {
await resolveTestCode('''
enum A {
i(3);
@@ -90,7 +123,7 @@ enum A {
await assertNoAssist();
}
Future<void> test_factory_constructor_does_not_apply() async {
Future<void> test_factoryConstructor_doesNotApply() async {
await resolveTestCode('''
class A {
A();
@@ -100,23 +133,6 @@ class A {
await assertNoAssist();
}
Future<void> test_final_constructor_parameter() async {
await resolveTestCode('''
// @dart = 3.10
class A {
A(final ^i);
}
''');
await assertHasAssist('''
// @dart = 3.10
class A {
final i;
A(this.i);
}
''');
}
Future<void> test_imported_type() async {
await resolveTestCode('''
import 'dart:core' as core;
@@ -256,6 +272,23 @@ class A {
''');
}
Future<void> test_secondaryConstructor_finalParameter() async {
await resolveTestCode('''
// @dart = 3.10
class A {
A(final ^i);
}
''');
await assertHasAssist('''
// @dart = 3.10
class A {
final i;
A(this.i);
}
''');
}
Future<void> test_static_method_parameter_does_not_apply() async {
await resolveTestCode('''
class A {