Skip to content

Commit 11f53fd

Browse files
DanTupCommit Queue
authored andcommitted
[analysis_server] Preserve existing quote kinds when updating string arguments
The original code tried to predict the "best" kind of quotes based on the string value (for example changing to double quotes when the string contained a single quote). Since, we decided it would be better to try to preserve the existing quotes the user picked (this results in less of the string changing, and preserves user preferences/lints). In some cases we can't preserve the exact delimeters, because if the string is a raw string but the new value contains the delimeter, we can't escape it, so we drop the `r` and escape anything that requires it. Change-Id: I9a224ade5edd5bd558310b78be132fc9addd3ed3 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/398885 Commit-Queue: Brian Wilkerson <[email protected]> Reviewed-by: Brian Wilkerson <[email protected]> Reviewed-by: Elliott Brooks <[email protected]>
1 parent fc95268 commit 11f53fd

File tree

2 files changed

+322
-37
lines changed

2 files changed

+322
-37
lines changed

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

Lines changed: 65 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -101,9 +101,13 @@ class EditArgumentHandler extends SharedMessageHandler<EditArgumentParams, Null>
101101
);
102102
}
103103

104+
var argument = parameterArguments[parameter];
105+
var valueExpression =
106+
argument is NamedExpression ? argument.expression : argument;
107+
104108
// Determine whether a value for this parameter is editable.
105109
var notEditableReason = getNotEditableReason(
106-
argument: parameterArguments[parameter],
110+
argument: valueExpression,
107111
positionalIndex: positionalParameterIndexes[parameter],
108112
numPositionals: numPositionals,
109113
numSuppliedPositionals: numSuppliedPositionals,
@@ -119,7 +123,11 @@ class EditArgumentHandler extends SharedMessageHandler<EditArgumentParams, Null>
119123
}
120124

121125
// Compute the new expression for this argument.
122-
var newValueCode = _computeValueCode(parameter, params.edit);
126+
var newValueCode = _computeValueCode(
127+
parameter,
128+
valueExpression,
129+
params.edit,
130+
);
123131

124132
// Build the edit and send it to the client.
125133
var workspaceEdit = await newValueCode.mapResult(
@@ -139,42 +147,14 @@ class EditArgumentHandler extends SharedMessageHandler<EditArgumentParams, Null>
139147
});
140148
}
141149

142-
/// Computes the string of Dart code (including quotes) for [value].
143-
String _computeStringValueCode(String value) {
144-
const singleQuote = "'";
145-
const doubleQuote = '"';
146-
147-
// Use single quotes unless the string contains a single quote and no
148-
// double quotes.
149-
var surroundingQuote =
150-
value.contains(singleQuote) && !value.contains(doubleQuote)
151-
? doubleQuote
152-
: singleQuote;
153-
154-
// TODO(dantup): Determine if we already have reusable code for this
155-
// anywhere.
156-
// TODO(dantup): Decide whether we should write multiline and/or raw strings
157-
// for some cases.
158-
var escaped = value
159-
.replaceAll(r'\', r'\\') // Escape backslashes
160-
.replaceAll(
161-
surroundingQuote,
162-
'\\$surroundingQuote',
163-
) // Escape surrounding quotes we'll use
164-
.replaceAll('\r', r'\r')
165-
.replaceAll('\n', r'\n')
166-
.replaceAll(r'$', r'\$');
167-
168-
return '$surroundingQuote$escaped$surroundingQuote';
169-
}
170-
171150
/// Computes the string of Dart code that should be used as the new value
172151
/// for this argument.
173152
///
174153
/// This is purely the expression for the value and does not take into account
175154
/// the parameter name or any commas that may be required.
176155
ErrorOr<String> _computeValueCode(
177156
FormalParameterElement parameter,
157+
Expression? argument,
178158
ArgumentEdit edit,
179159
) {
180160
// TODO(dantup): Should we accept arbitrary strings for all values? For
@@ -202,9 +182,17 @@ class EditArgumentHandler extends SharedMessageHandler<EditArgumentParams, Null>
202182
return success(value.toString());
203183
} else if (type.isDartCoreBool && value is bool) {
204184
return success(value.toString());
205-
} else if (type.isDartCoreString && value is String) {
206-
return success(_computeStringValueCode(value));
207-
} else if (type case InterfaceType(
185+
} else if (parameter.type.isDartCoreString && value is String) {
186+
var simpleString = argument is SimpleStringLiteral ? argument : null;
187+
return success(
188+
computeStringValueCode(
189+
value,
190+
preferSingleQuotes: simpleString?.isSingleQuoted ?? true,
191+
preferMultiline: simpleString?.isMultiline ?? false,
192+
preferRaw: simpleString?.isRaw ?? false,
193+
),
194+
);
195+
} else if (parameter.type case InterfaceType(
208196
:EnumElement2 element3,
209197
) when value is String?) {
210198
var allowedValues = getQualifiedEnumConstantNames(element3);
@@ -398,4 +386,47 @@ class EditArgumentHandler extends SharedMessageHandler<EditArgumentParams, Null>
398386
newCode.toString(),
399387
);
400388
}
389+
390+
/// Computes the string of Dart code (including quotes) for the String
391+
/// [value].
392+
///
393+
/// [preferSingleQuotes], [preferMultiline] and [preferRaw] are used to
394+
/// control the kinds of delimeters used for the string but are not
395+
/// guaranteed because the contents of the strings might prevent some
396+
/// delimeters (for example raw strings can't be used where there need to be
397+
/// escape sequences).
398+
static String computeStringValueCode(
399+
String value, {
400+
bool preferSingleQuotes = true,
401+
bool preferMultiline = false,
402+
bool preferRaw = false,
403+
}) {
404+
var quoteCharacter = preferSingleQuotes ? "'" : '"';
405+
var useMultiline = preferMultiline /* && value.contains('\n') ??? */;
406+
var numQuotes = useMultiline ? 3 : 1;
407+
var surroundingQuote = quoteCharacter * numQuotes;
408+
// Only use raw if requested _and_ the string doesn't contain the
409+
// quotes that'll be used to surround it or newlines.
410+
var useRaw =
411+
preferRaw &&
412+
!value.contains(surroundingQuote) &&
413+
!value.contains('\r') &&
414+
!value.contains('\n');
415+
416+
// Escape non-quote characters.
417+
if (!useRaw) {
418+
value = value
419+
.replaceAll(r'\', r'\\') // Escape backslashes
420+
.replaceAll('\r', r'\r')
421+
.replaceAll('\n', r'\n')
422+
.replaceAll(r'$', r'\$');
423+
}
424+
425+
// Escape quotes.
426+
var escapedSurroundingQuote = '\\$quoteCharacter' * numQuotes;
427+
value = value.replaceAll(surroundingQuote, escapedSurroundingQuote);
428+
429+
var prefix = useRaw ? 'r' : '';
430+
return '$prefix$surroundingQuote$value$surroundingQuote';
431+
}
401432
}

0 commit comments

Comments
 (0)