Skip to content

Commit

Permalink
DAS: Support "Create field" and "Create missing overrides" fixes for …
Browse files Browse the repository at this point in the history
…enums.

Fixes #55264

Fields added to enums are final.

Also:

* Fix a bug in CreateField - fields created from function-typed field
  formal parameters now have the right type.
* Fix a "bug" in CreateField - fields created from a field formal
  parameter in a const constructor are now final.
* Add more tests for CreateField.

Change-Id: I7d1344f6214c89e6351d3b5ed18b71ac97b9a310
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/362420
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Samuel Rawlins <srawlins@google.com>
  • Loading branch information
srawlins authored and Commit Queue committed Apr 12, 2024
1 parent 7e81e59 commit dd1fc29
Show file tree
Hide file tree
Showing 6 changed files with 334 additions and 54 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class CreateField extends CreateFieldOrGetter {
if (targetElement.library.isInSdk) {
return;
}
// Prepare target ClassDeclaration.
// Prepare target `ClassDeclaration`.
var targetDeclarationResult =
await sessionHelper.getElementDeclaration(targetElement);
if (targetDeclarationResult == null) {
Expand All @@ -75,38 +75,54 @@ class CreateField extends CreateFieldOrGetter {
if (targetNode is! CompilationUnitMember) {
return;
}
if (!(targetNode is ClassDeclaration || targetNode is MixinDeclaration)) {
if (!(targetNode is ClassDeclaration ||
targetNode is EnumDeclaration ||
targetNode is MixinDeclaration)) {
return;
}
// Build field source.
var targetSource = targetElement.source;
var targetFile = targetSource.fullName;
await builder.addDartFileEdit(targetFile, (builder) {
builder.insertField(targetNode, (builder) {
builder.writeFieldDeclaration(_fieldName,
isStatic: staticModifier,
nameGroupName: 'NAME',
type: fieldType,
typeGroupName: 'TYPE');
builder.writeFieldDeclaration(
_fieldName,
isFinal: targetNode is EnumDeclaration,
isStatic: staticModifier,
nameGroupName: 'NAME',
type: fieldType,
typeGroupName: 'TYPE',
);
});
});
}

Future<void> _proposeFromFieldFormalParameter(
ChangeBuilder builder, FieldFormalParameter parameter) async {
var targetClassNode = parameter.thisOrAncestorOfType<ClassDeclaration>();
if (targetClassNode == null) {
var constructor = parameter.thisOrAncestorOfType<ConstructorDeclaration>();
if (constructor == null) {
return;
}
var container = constructor.thisOrAncestorOfType<CompilationUnitMember>();
if (container == null) {
return;
}
if (container is! ClassDeclaration && container is! EnumDeclaration) {
return;
}

_fieldName = parameter.name.lexeme;

// Add proposal.
await builder.addDartFileEdit(file, (builder) {
var fieldType = parameter.type?.type;
builder.insertField(targetClassNode, (builder) {
builder.writeFieldDeclaration(_fieldName,
nameGroupName: 'NAME', type: fieldType, typeGroupName: 'TYPE');
builder.insertField(container, (builder) {
builder.writeFieldDeclaration(
_fieldName,
isFinal: constructor.constKeyword != null,
nameGroupName: 'NAME',
type: parameter.declaredElement?.type,
typeGroupName: 'TYPE',
);
});
});
}
Expand All @@ -117,22 +133,18 @@ class CreateField extends CreateFieldOrGetter {
return;
}
_fieldName = nameNode.name;
// prepare target Expression
Expression? target;
{
var nameParent = nameNode.parent;
if (nameParent is PrefixedIdentifier) {
target = nameParent.prefix;
} else if (nameParent is PropertyAccess) {
target = nameParent.realTarget;
}
}
// prepare target ClassElement
// Prepare target `Expression`.
var target = switch (nameNode.parent) {
PrefixedIdentifier(:var prefix) => prefix,
PropertyAccess(:var realTarget) => realTarget,
_ => null,
};
// Prepare target `ClassElement`.
var staticModifier = false;
InterfaceElement? targetClassElement;
if (target != null) {
targetClassElement = getTargetInterfaceElement(target);
// maybe static
// Maybe static.
if (target is Identifier) {
var targetIdentifier = target;
var targetElement = targetIdentifier.staticElement;
Expand All @@ -147,6 +159,14 @@ class CreateField extends CreateFieldOrGetter {
}

var fieldTypeNode = climbPropertyAccess(nameNode);
var fieldTypeParent = fieldTypeNode.parent;
if (targetClassElement is EnumElement &&
fieldTypeParent is AssignmentExpression &&
fieldTypeNode == fieldTypeParent.leftHandSide) {
// Any field on an enum must be final; creating a final field does not
// make sense when seen in an assignment expression.
return;
}
var fieldType = inferUndefinedExpressionType(fieldTypeNode);

await _addDeclaration(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import 'package:analysis_server/src/services/correction/dart/abstract_producer.d
import 'package:analysis_server/src/services/correction/fix.dart';
import 'package:analysis_server/src/utilities/strings.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/src/error/inheritance_override.dart';
import 'package:analyzer_plugin/utilities/change_builder/change_builder_core.dart';
Expand All @@ -22,13 +23,17 @@ class CreateMissingOverrides extends ResolvedCorrectionProducer {

@override
Future<void> compute(ChangeBuilder builder) async {
final targetClass = node;
if (targetClass is! ClassDeclaration) {
final targetDeclaration = node;
if (targetDeclaration is! NamedCompilationUnitMember) {
return;
}
if (targetDeclaration is! ClassDeclaration &&
targetDeclaration is! EnumDeclaration) {
return;
}
var signatures = [
...InheritanceOverrideVerifier.missingOverrides(targetClass),
...InheritanceOverrideVerifier.missingMustBeOverridden(targetClass)
...InheritanceOverrideVerifier.missingOverrides(targetDeclaration),
...InheritanceOverrideVerifier.missingMustBeOverridden(targetDeclaration)
];
// Sort by name, getters before setters.
signatures.sort((ExecutableElement a, ExecutableElement b) {
Expand All @@ -45,7 +50,7 @@ class CreateMissingOverrides extends ResolvedCorrectionProducer {

var prefix = utils.oneIndent;
await builder.addDartFileEdit(file, (builder) {
builder.insertIntoUnitMember(targetClass, (builder) {
builder.insertIntoUnitMember(targetDeclaration, (builder) {
// Separator management.
var numOfMembersWritten = 0;
void addSeparatorBetweenDeclarations() {
Expand Down Expand Up @@ -80,6 +85,10 @@ class CreateMissingOverrides extends ResolvedCorrectionProducer {
builder.write(eol);
// Add field.
builder.write(prefix);
if (targetDeclaration is EnumDeclaration) {
builder.write(Keyword.FINAL.lexeme);
builder.write(' ');
}
builder.writeType(element.returnType, required: true);
builder.write(' ');
builder.write(element.name);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,111 @@ import 'fix_processor.dart';

void main() {
defineReflectiveSuite(() {
defineReflectiveTests(CreateFieldTest);
defineReflectiveTests(CreateFieldEnumTest);
defineReflectiveTests(CreateFieldMixinTest);
defineReflectiveTests(CreateFieldTest);
});
}

@reflectiveTest
class CreateFieldEnumTest extends FixProcessorTest {
@override
FixKind get kind => DartFixKind.CREATE_FIELD;

Future<void> test_initializingFormal_dynamic() async {
await resolveTestCode('''
enum E {
one(1), two(2);
const E(dynamic this.f);
}
''');
await assertHasFix('''
enum E {
one(1), two(2);
final dynamic f;
const E(dynamic this.f);
}
''');
}

Future<void> test_initializingFormal_withoutType() async {
await resolveTestCode('''
enum E {
one(1), two(2);
const E(this.f);
}
''');
await assertHasFix('''
enum E {
one(1), two(2);
final dynamic f;
const E(this.f);
}
''');
}

Future<void> test_initializingFormal_withType() async {
await resolveTestCode('''
enum E {
one(1), two(2);
const E(int this.f);
}
''');
await assertHasFix('''
enum E {
one(1), two(2);
final int f;
const E(int this.f);
}
''');
}

Future<void> test_usedAsGetter() async {
await resolveTestCode('''
enum E {
one, two;
}
int f(E e) {
return e.a;
}
''');
await assertHasFix('''
enum E {
one, two;
final int a;
}
int f(E e) {
return e.a;
}
''');
}

Future<void> test_usedAsSetter() async {
await resolveTestCode('''
enum E {
one, two;
}
void f(E e) {
e.a = 1;
}
''');
await assertNoFix();
}
}

@reflectiveTest
class CreateFieldMixinTest extends FixProcessorTest {
@override
Expand Down Expand Up @@ -363,6 +463,81 @@ void f() {
await assertNoFix();
}

Future<void> test_initializingFormal_functionTyped() async {
await resolveTestCode('''
class C {
C(String this.text());
}
''');
await assertHasFix('''
class C {
String Function() text;
C(String this.text());
}
''');
}

Future<void> test_initializingFormal_typeVariable() async {
await resolveTestCode('''
class C<T> {
C(T this.text);
}
''');
await assertHasFix('''
class C<T> {
T text;
C(T this.text);
}
''');
}

Future<void> test_initializingFormal_withDefaultValue() async {
await resolveTestCode('''
class C {
C([String this.text = '']);
}
''');
await assertHasFix('''
class C {
String text;
C([String this.text = '']);
}
''');
}

Future<void> test_initializingFormal_withType() async {
await resolveTestCode('''
class C {
C(String this.text);
}
''');
await assertHasFix('''
class C {
String text;
C(String this.text);
}
''');
}

Future<void> test_initializingFormal_withType_constConstuctor() async {
await resolveTestCode('''
class C {
const C(String this.text);
}
''');
await assertHasFix('''
class C {
final String text;
const C(String this.text);
}
''');
}

Future<void> test_inPart_self() async {
await resolveTestCode('''
part of lib;
Expand Down Expand Up @@ -400,21 +575,6 @@ class C {
''');
}

Future<void> test_invalidInitializer_withType() async {
await resolveTestCode('''
class C {
C(String this.text);
}
''');
await assertHasFix('''
class C {
String text;
C(String this.text);
}
''');
}

Future<void> test_objectPattern_explicitName_variablePattern_typed() async {
await resolveTestCode('''
void f(Object? x) {
Expand Down
Loading

0 comments on commit dd1fc29

Please sign in to comment.