[analysis_server] Add isEditable to editableArguments results and make false for non-editable strings

Change-Id: Iac020bfe0d495f3b634b95c8c32409433e9181bc
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/396282
Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
Reviewed-by: Elliott Brooks <elliottbrooks@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
This commit is contained in:
Danny Tuppeny
2024-12-02 17:52:33 +00:00
committed by Commit Queue
parent 05130dcdef
commit dcb9ec10a1
4 changed files with 269 additions and 5 deletions
@@ -149,6 +149,18 @@ class EditableArgumentsHandler
};
}
/// Checks whether [argument] is editable and if not, returns a human-readable
/// description why.
String? _getNotEditableReason(Expression argument) {
return switch (argument) {
AdjacentStrings() => "Adjacent strings can't be edited",
StringInterpolation() => "Interpolated strings can't be edited",
SimpleStringLiteral() when argument.value.contains('\n') =>
"Strings containing newlines can't be edited",
_ => null,
};
}
/// Computes the values for a parameter and argument and returns them along
/// with a flag indicating if the default parameter value is being used.
_Values _getValues(
@@ -194,6 +206,11 @@ class EditableArgumentsHandler
Object? value;
List<String>? options;
// Check whether this value may be editable (for example is not an
// interpolated string).
var notEditableReason =
valueExpression != null ? _getNotEditableReason(valueExpression) : null;
if (parameter.type.isDartCoreDouble) {
type = 'double';
value =
@@ -232,11 +249,22 @@ class EditableArgumentsHandler
return null;
}
// If the value is not a literal, include the source as displayValue.
var displayValue =
valueExpression is! Literal ? valueExpression?.toSource() : null;
// Unless it turns out to match the value (converted to string).
if (displayValue != null && displayValue == value?.toString()) {
var isEditable = notEditableReason == null;
// Compute a displayValue.
String? displayValue;
if (!isEditable) {
// Not editable, so show the value or source in displayValue.
displayValue = value?.toString() ?? valueExpression?.toSource();
// And remove the value.
value = null;
} else if (valueExpression is! Literal) {
// Also provide the source if it was not a literal.
displayValue = valueExpression?.toSource();
}
// Never provide a displayValue if it's the same as value.
if (displayValue == value) {
displayValue = null;
}
@@ -251,6 +279,8 @@ class EditableArgumentsHandler
isRequired: parameter.isRequired,
isNullable:
parameter.type.nullabilitySuffix == NullabilitySuffix.question,
isEditable: notEditableReason == null,
notEditableReason: notEditableReason,
);
}
}
@@ -64,6 +64,8 @@ $content
Object? isDefault = anything,
Object? isRequired = anything,
Object? isNullable = anything,
Object? isEditable = anything,
Object? notEditableReason = anything,
Object? options = anything,
}) {
return isA<EditableArgument>()
@@ -75,6 +77,12 @@ $content
.having((arg) => arg.isDefault, 'isDefault', isDefault)
.having((arg) => arg.isRequired, 'isRequired', isRequired)
.having((arg) => arg.isNullable, 'isNullable', isNullable)
.having((arg) => arg.isEditable, 'isEditable', isEditable)
.having(
(arg) => arg.notEditableReason,
'notEditableReason',
notEditableReason,
)
.having((arg) => arg.options, 'options', options)
// Some extra checks that should be true for all.
.having(
@@ -84,6 +92,16 @@ $content
'different value and displayValues',
isTrue,
)
.having(
(arg) => (arg.notEditableReason == null) == arg.isEditable,
'notEditableReason must be supplied if isEditable=false',
isTrue,
)
.having(
(arg) => arg.value == null || arg.isEditable,
'isEditable must be true if there is a value',
isTrue,
)
.having(
(arg) =>
arg.type == 'enum'
@@ -129,6 +147,166 @@ class MyWidget extends StatelessWidget {
);
}
test_isEditable_false_string_adjacent() async {
var result = await getEditableArgumentsFor(r'''
class MyWidget extends StatelessWidget {
const MyWidget(String s);
@override
Widget build(BuildContext context) => MyW^idget('a' 'b');
}
''');
expect(
result,
hasArg(
isArg(
's',
type: 'string',
value: isNull,
displayValue: 'ab',
isDefault: false,
isEditable: false,
notEditableReason: "Adjacent strings can't be edited",
),
),
);
}
test_isEditable_false_string_interpolated() async {
var result = await getEditableArgumentsFor(r'''
class MyWidget extends StatelessWidget {
const MyWidget(String s);
@override
Widget build(BuildContext context) => MyW^idget('${context.runtimeType}');
}
''');
expect(
result,
hasArgs(
orderedEquals([
isArg(
's',
type: 'string',
value: isNull,
displayValue: r"'${context.runtimeType}'",
isEditable: false,
notEditableReason: "Interpolated strings can't be edited",
),
]),
),
);
}
test_isEditable_false_string_withNewlines() async {
var result = await getEditableArgumentsFor(r'''
class MyWidget extends StatelessWidget {
const MyWidget(String sEscaped, String sLiteral);
@override
Widget build(BuildContext context) => MyW^idget(
'a\nb',
"""
a
b
""",
);
}
''');
expect(
result,
hasArgs(
orderedEquals([
isArg(
'sEscaped',
type: 'string',
value: isNull,
displayValue: 'a\nb',
isEditable: false,
notEditableReason: "Strings containing newlines can't be edited",
),
isArg(
'sLiteral',
type: 'string',
value: isNull,
displayValue: 'a\nb\n',
isEditable: false,
notEditableReason: "Strings containing newlines can't be edited",
),
]),
),
);
}
test_isEditable_true_string_dollar_escaped() async {
var result = await getEditableArgumentsFor(r'''
class MyWidget extends StatelessWidget {
const MyWidget(String s);
@override
Widget build(BuildContext context) => MyW^idget('\${1}');
}
''');
expect(
result,
hasArg(
isArg(
's',
type: 'string',
value: r'${1}',
displayValue: isNull,
isEditable: true,
),
),
);
}
test_isEditable_true_string_dollar_raw() async {
var result = await getEditableArgumentsFor(r'''
class MyWidget extends StatelessWidget {
const MyWidget(String s);
@override
Widget build(BuildContext context) => MyW^idget(r'${1}');
}
''');
expect(
result,
hasArg(
isArg(
's',
type: 'string',
value: r'${1}',
displayValue: isNull,
isEditable: true,
),
),
);
}
test_isEditable_true_string_tripleQuoted_withoutNewlines() async {
var result = await getEditableArgumentsFor(r'''
class MyWidget extends StatelessWidget {
const MyWidget(String s);
@override
Widget build(BuildContext context) => MyW^idget("""string_value""");
}
''');
expect(
result,
hasArg(
isArg(
's',
type: 'string',
value: 'string_value',
displayValue: isNull,
isEditable: true,
),
),
);
}
test_isNullable() async {
failTestOnErrorDiagnostic = false;
var result = await getEditableArgumentsFor('''
@@ -488,6 +488,21 @@ List<LspEntity> getCustomClasses() {
'Whether this argument can be `null`. It is possible for an '
'argument to be required, but still allow an explicit `null`.',
),
field(
'isEditable',
type: 'boolean',
comment:
'Whether this argument can be add/edited. If not, '
'notEditableReason will contain an explanation for why.',
),
field(
'notEditableReason',
type: 'String',
canBeUndefined: true,
comment:
'If isEditable is false, contains a human-readable '
'description of why.',
),
field(
'options',
type: 'string',
@@ -1484,6 +1484,10 @@ class EditableArgument implements ToJsonable {
/// is no argument or because it is explicitly provided as the same value.
final bool isDefault;
/// Whether this argument can be add/edited. If not, notEditableReason may
/// contain an explanation for why.
final bool isEditable;
/// Whether this argument can be `null`. It is possible for an argument to be
/// required, but still allow an explicit `null`.
final bool isNullable;
@@ -1494,6 +1498,9 @@ class EditableArgument implements ToJsonable {
/// The name of the corresponding parameter.
final String name;
/// An optional reason for why isEditable is false.
final String? notEditableReason;
/// The set of values allowed for this argument if it is an enum. Values are
/// qualified in the form `EnumName.valueName`.
final List<String>? options;
@@ -1511,9 +1518,11 @@ class EditableArgument implements ToJsonable {
this.displayValue,
required this.hasArgument,
required this.isDefault,
required this.isEditable,
required this.isNullable,
required this.isRequired,
required this.name,
this.notEditableReason,
this.options,
required this.type,
this.value,
@@ -1523,9 +1532,11 @@ class EditableArgument implements ToJsonable {
displayValue,
hasArgument,
isDefault,
isEditable,
isNullable,
isRequired,
name,
notEditableReason,
lspHashCode(options),
type,
value,
@@ -1538,9 +1549,11 @@ class EditableArgument implements ToJsonable {
displayValue == other.displayValue &&
hasArgument == other.hasArgument &&
isDefault == other.isDefault &&
isEditable == other.isEditable &&
isNullable == other.isNullable &&
isRequired == other.isRequired &&
name == other.name &&
notEditableReason == other.notEditableReason &&
const DeepCollectionEquality().equals(options, other.options) &&
type == other.type &&
value == other.value;
@@ -1554,9 +1567,13 @@ class EditableArgument implements ToJsonable {
}
result['hasArgument'] = hasArgument;
result['isDefault'] = isDefault;
result['isEditable'] = isEditable;
result['isNullable'] = isNullable;
result['isRequired'] = isRequired;
result['name'] = name;
if (notEditableReason != null) {
result['notEditableReason'] = notEditableReason;
}
if (options != null) {
result['options'] = options;
}
@@ -1599,6 +1616,15 @@ class EditableArgument implements ToJsonable {
)) {
return false;
}
if (!_canParseBool(
obj,
reporter,
'isEditable',
allowsUndefined: false,
allowsNull: false,
)) {
return false;
}
if (!_canParseBool(
obj,
reporter,
@@ -1626,6 +1652,15 @@ class EditableArgument implements ToJsonable {
)) {
return false;
}
if (!_canParseString(
obj,
reporter,
'notEditableReason',
allowsUndefined: true,
allowsNull: false,
)) {
return false;
}
if (!_canParseListString(
obj,
reporter,
@@ -1655,12 +1690,16 @@ class EditableArgument implements ToJsonable {
final hasArgument = hasArgumentJson as bool;
final isDefaultJson = json['isDefault'];
final isDefault = isDefaultJson as bool;
final isEditableJson = json['isEditable'];
final isEditable = isEditableJson as bool;
final isNullableJson = json['isNullable'];
final isNullable = isNullableJson as bool;
final isRequiredJson = json['isRequired'];
final isRequired = isRequiredJson as bool;
final nameJson = json['name'];
final name = nameJson as String;
final notEditableReasonJson = json['notEditableReason'];
final notEditableReason = notEditableReasonJson as String?;
final optionsJson = json['options'];
final options =
(optionsJson as List<Object?>?)?.map((item) => item as String).toList();
@@ -1672,9 +1711,11 @@ class EditableArgument implements ToJsonable {
displayValue: displayValue,
hasArgument: hasArgument,
isDefault: isDefault,
isEditable: isEditable,
isNullable: isNullable,
isRequired: isRequired,
name: name,
notEditableReason: notEditableReason,
options: options,
type: type,
value: value,