Skip to content

Commit a1699ec

Browse files
ellietteCommit Queue
authored andcommitted
[Property Editor] Use dot-shorthand syntax in enum edits when possible
Bug: #60727 Change-Id: I41388422b0c317e7500166d29ba4f9e7b656a24f Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/441862 Reviewed-by: Brian Wilkerson <[email protected]> Commit-Queue: Elliott Brooks <[email protected]>
1 parent 289278b commit a1699ec

File tree

3 files changed

+149
-14
lines changed

3 files changed

+149
-14
lines changed

pkg/analysis_server/lib/src/lsp/handlers/custom/editable_arguments/editable_arguments_mixin.dart

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import 'package:analysis_server/src/computer/computer_documentation.dart';
66
import 'package:analysis_server/src/utilities/extensions/numeric.dart';
7+
import 'package:analyzer/dart/analysis/features.dart';
78
import 'package:analyzer/dart/analysis/results.dart';
89
import 'package:analyzer/dart/element/element.dart';
910
import 'package:analyzer/src/dart/ast/ast.dart';
@@ -35,6 +36,35 @@ mixin EditableArgumentsMixin {
3536
return dartDoc?.full;
3637
}
3738

39+
/// Returns the name of an enum constant prefixed with only a dot.
40+
///
41+
/// If the dot-shorthands feature is not enabled, this method returns null.
42+
String? getDotShorthandEnumConstantName(FieldElement enumConstant) {
43+
if (!_supportsDotShorthandSyntax(enumConstant)) {
44+
return null;
45+
}
46+
47+
var name = enumConstant.name;
48+
return name != null ? '.$name' : null;
49+
}
50+
51+
/// Returns an enum constant [FieldElement] of the given [element] matching
52+
/// the provided fully qualified name.
53+
///
54+
/// This method iterates through all constants of the [element] and compares
55+
/// their fully qualified names against the [matching] string.
56+
FieldElement? getEnumConstantMatching(
57+
EnumElement element, {
58+
required String matching,
59+
}) {
60+
for (var enumConstant in element.constants) {
61+
if (getQualifiedEnumConstantName(enumConstant) == matching) {
62+
return enumConstant;
63+
}
64+
}
65+
return null;
66+
}
67+
3868
/// Gets the argument list at [offset] that can be edited.
3969
EditableInvocationInfo? getInvocationInfo(
4070
ResolvedUnitResult result,
@@ -161,6 +191,11 @@ mixin EditableArgumentsMixin {
161191
List<String> getQualifiedEnumConstantNames(EnumElement element) =>
162192
element.constants.map(getQualifiedEnumConstantName).nonNulls.toList();
163193

194+
/// Determines whether or not the dot-shortands feature is enabled for the
195+
/// given [element].
196+
bool _supportsDotShorthandSyntax(Element element) =>
197+
element.library?.featureSet.isEnabled(Feature.dot_shorthands) ?? false;
198+
164199
/// Returns the name of an enum constant prefixed with the enum name.
165200
static String? getQualifiedEnumConstantName(FieldElement enumConstant) {
166201
var enumName = enumConstant.enclosingElement.name;

pkg/analysis_server/lib/src/lsp/handlers/custom/editable_arguments/handler_edit_argument.dart

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,29 @@ class EditArgumentHandler extends SharedMessageHandler<EditArgumentParams, Null>
164164
});
165165
}
166166

167+
/// Computes the appropriate enum value for the String [requestValue].
168+
///
169+
/// This method tries to use dot-shorthand syntax for the enum value when the
170+
/// [currentArgument] is a [DotShorthandPropertyAccess], a [SimpleIdentifier],
171+
/// or `null`.
172+
String _computeEnumValue({
173+
required String? requestValue,
174+
required FieldElement enumConstant,
175+
required Expression? currentArgument,
176+
}) {
177+
var preferDotShorthand =
178+
currentArgument is DotShorthandPropertyAccess ||
179+
currentArgument is SimpleIdentifier ||
180+
currentArgument == null;
181+
182+
var enumValue =
183+
preferDotShorthand
184+
? getDotShorthandEnumConstantName(enumConstant) ?? requestValue
185+
: requestValue;
186+
187+
return enumValue.toString();
188+
}
189+
167190
/// Computes the string of Dart code that should be used as the new value
168191
/// for this argument.
169192
///
@@ -188,7 +211,7 @@ class EditArgumentHandler extends SharedMessageHandler<EditArgumentParams, Null>
188211
} else {
189212
return error(
190213
ServerErrorCodes.EditArgumentInvalidValue,
191-
"The value for the parameter '${edit.name}' cannot be null",
214+
"The value for the parameter '${edit.name}' can't be null",
192215
);
193216
}
194217
}
@@ -211,14 +234,20 @@ class EditArgumentHandler extends SharedMessageHandler<EditArgumentParams, Null>
211234
);
212235
} else if (parameter.type case InterfaceType(
213236
:EnumElement element,
214-
) when value is String?) {
215-
var allowedValues = getQualifiedEnumConstantNames(element);
216-
if (allowedValues.contains(value)) {
217-
return success(value.toString());
237+
) when value is String) {
238+
var enumConstant = getEnumConstantMatching(element, matching: value);
239+
if (enumConstant != null) {
240+
return success(
241+
_computeEnumValue(
242+
requestValue: value,
243+
enumConstant: enumConstant,
244+
currentArgument: argument,
245+
),
246+
);
218247
} else {
219248
return error(
220249
ServerErrorCodes.EditArgumentInvalidValue,
221-
"The value for the parameter '${edit.name}' should be one of ${allowedValues.map((v) => "'$v'").join(', ')} but was '$value'",
250+
"The value for the parameter '${edit.name}' should be one of ${getQualifiedEnumConstantNames(element).map((v) => "'$v'").join(', ')} but was '$value'",
222251
);
223252
}
224253
} else {

pkg/analysis_server/test/shared/shared_edit_argument_tests.dart

Lines changed: 79 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -435,7 +435,7 @@ mixin SharedEditArgumentTests
435435
originalArgs: '(x: true)',
436436
edit: ArgumentEdit(name: 'x'),
437437
errorCode: ServerErrorCodes.EditArgumentInvalidValue,
438-
message: "The value for the parameter 'x' cannot be null",
438+
message: "The value for the parameter 'x' can't be null",
439439
);
440440
}
441441

@@ -483,7 +483,7 @@ mixin SharedEditArgumentTests
483483
originalArgs: '(x: 1.0)',
484484
edit: ArgumentEdit(name: 'x'),
485485
errorCode: ServerErrorCodes.EditArgumentInvalidValue,
486-
message: "The value for the parameter 'x' cannot be null",
486+
message: "The value for the parameter 'x' can't be null",
487487
);
488488
}
489489

@@ -523,6 +523,75 @@ mixin SharedEditArgumentTests
523523
);
524524
}
525525

526+
Future<void> test_type_enum_dotshorthand_addNew() async {
527+
await _expectSimpleArgumentEdit(
528+
additionalCode: 'enum E { one, two }',
529+
params: '({ E? x })',
530+
originalArgs: '()',
531+
edit: ArgumentEdit(name: 'x', newValue: 'E.two'),
532+
expectedArgs: '(x: .two)',
533+
);
534+
}
535+
536+
Future<void> test_type_enum_dotshorthand_disabled_addNew() async {
537+
await _expectSimpleArgumentEdit(
538+
additionalCode: 'enum E { one, two }',
539+
params: '({ E? x })',
540+
originalArgs: '()',
541+
edit: ArgumentEdit(name: 'x', newValue: 'E.two'),
542+
expectedArgs: '(x: E.two)',
543+
fileComment: '// @dart = 3.8',
544+
);
545+
}
546+
547+
Future<void> test_type_enum_dotshorthand_disabled_replaceLiteral() async {
548+
await _expectSimpleArgumentEdit(
549+
additionalCode: 'enum E { one, two }',
550+
params: '({ E? x })',
551+
originalArgs: '(x: E.one)',
552+
edit: ArgumentEdit(name: 'x', newValue: 'E.two'),
553+
expectedArgs: '(x: E.two)',
554+
fileComment: '// @dart = 3.8',
555+
);
556+
}
557+
558+
Future<void> test_type_enum_dotshorthand_disabled_replaceNonLiteral() async {
559+
await _expectSimpleArgumentEdit(
560+
additionalCode: '''
561+
enum E { one, two }
562+
const E myConst = E.one;
563+
''',
564+
params: '({ E? x })',
565+
originalArgs: '(x: myConst)',
566+
edit: ArgumentEdit(name: 'x', newValue: 'E.two'),
567+
expectedArgs: '(x: E.two)',
568+
fileComment: '// @dart = 3.8',
569+
);
570+
}
571+
572+
Future<void> test_type_enum_dotshorthand_replaceLiteral() async {
573+
await _expectSimpleArgumentEdit(
574+
additionalCode: 'enum E { one, two }',
575+
params: '({ E? x })',
576+
originalArgs: '(x: .one)',
577+
edit: ArgumentEdit(name: 'x', newValue: 'E.two'),
578+
expectedArgs: '(x: .two)',
579+
);
580+
}
581+
582+
Future<void> test_type_enum_dotshorthand_replaceNonLiteral() async {
583+
await _expectSimpleArgumentEdit(
584+
additionalCode: '''
585+
enum E { one, two }
586+
const E myConst = .one;
587+
''',
588+
params: '({ E? x })',
589+
originalArgs: '(x: myConst)',
590+
edit: ArgumentEdit(name: 'x', newValue: 'E.two'),
591+
expectedArgs: '(x: .two)',
592+
);
593+
}
594+
526595
Future<void> test_type_enum_invalidType() async {
527596
await _expectFailedEdit(
528597
additionalCode: 'enum E { one, two }',
@@ -552,7 +621,7 @@ mixin SharedEditArgumentTests
552621
originalArgs: '(x: E.one)',
553622
edit: ArgumentEdit(name: 'x'),
554623
errorCode: ServerErrorCodes.EditArgumentInvalidValue,
555-
message: "The value for the parameter 'x' cannot be null",
624+
message: "The value for the parameter 'x' can't be null",
556625
);
557626
}
558627

@@ -575,7 +644,7 @@ const myConst = E.one;
575644
params: '({ E? x })',
576645
originalArgs: '(x: myConst)',
577646
edit: ArgumentEdit(name: 'x', newValue: 'E.two'),
578-
expectedArgs: '(x: E.two)',
647+
expectedArgs: '(x: .two)',
579648
);
580649
}
581650

@@ -604,7 +673,7 @@ const myConst = E.one;
604673
originalArgs: '(x: 1)',
605674
edit: ArgumentEdit(name: 'x'),
606675
errorCode: ServerErrorCodes.EditArgumentInvalidValue,
607-
message: "The value for the parameter 'x' cannot be null",
676+
message: "The value for the parameter 'x' can't be null",
608677
);
609678
}
610679

@@ -687,7 +756,7 @@ const myConst = E.one;
687756
originalArgs: "(x: 'a')",
688757
edit: ArgumentEdit(name: 'x'),
689758
errorCode: ServerErrorCodes.EditArgumentInvalidValue,
690-
message: "The value for the parameter 'x' cannot be null",
759+
message: "The value for the parameter 'x' can't be null",
691760
);
692761
}
693762

@@ -857,10 +926,11 @@ class MyWidget extends StatelessWidget {
857926
required String originalArgs,
858927
required ArgumentEdit edit,
859928
required String expectedArgs,
860-
String? additionalCode,
929+
String? additionalCode = '',
930+
String? fileComment = '',
861931
}) async {
862-
additionalCode ??= '';
863932
var content = '''
933+
$fileComment
864934
import 'package:flutter/widgets.dart';
865935
866936
$additionalCode
@@ -874,6 +944,7 @@ class MyWidget extends StatelessWidget {
874944
''';
875945
var expectedContent = '''
876946
>>>>>>>>>> lib/test.dart
947+
$fileComment
877948
import 'package:flutter/widgets.dart';
878949
879950
$additionalCode

0 commit comments

Comments
 (0)