Skip to content

Commit

Permalink
[analysis_server] Preserve existing quote kinds when updating string …
Browse files Browse the repository at this point in the history
…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]>
  • Loading branch information
DanTup authored and Commit Queue committed Jan 8, 2025
1 parent fc95268 commit 11f53fd
Show file tree
Hide file tree
Showing 2 changed files with 322 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,13 @@ class EditArgumentHandler extends SharedMessageHandler<EditArgumentParams, Null>
);
}

var argument = parameterArguments[parameter];
var valueExpression =
argument is NamedExpression ? argument.expression : argument;

// Determine whether a value for this parameter is editable.
var notEditableReason = getNotEditableReason(
argument: parameterArguments[parameter],
argument: valueExpression,
positionalIndex: positionalParameterIndexes[parameter],
numPositionals: numPositionals,
numSuppliedPositionals: numSuppliedPositionals,
Expand All @@ -119,7 +123,11 @@ class EditArgumentHandler extends SharedMessageHandler<EditArgumentParams, Null>
}

// Compute the new expression for this argument.
var newValueCode = _computeValueCode(parameter, params.edit);
var newValueCode = _computeValueCode(
parameter,
valueExpression,
params.edit,
);

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

/// Computes the string of Dart code (including quotes) for [value].
String _computeStringValueCode(String value) {
const singleQuote = "'";
const doubleQuote = '"';

// Use single quotes unless the string contains a single quote and no
// double quotes.
var surroundingQuote =
value.contains(singleQuote) && !value.contains(doubleQuote)
? doubleQuote
: singleQuote;

// TODO(dantup): Determine if we already have reusable code for this
// anywhere.
// TODO(dantup): Decide whether we should write multiline and/or raw strings
// for some cases.
var escaped = value
.replaceAll(r'\', r'\\') // Escape backslashes
.replaceAll(
surroundingQuote,
'\\$surroundingQuote',
) // Escape surrounding quotes we'll use
.replaceAll('\r', r'\r')
.replaceAll('\n', r'\n')
.replaceAll(r'$', r'\$');

return '$surroundingQuote$escaped$surroundingQuote';
}

/// Computes the string of Dart code that should be used as the new value
/// for this argument.
///
/// This is purely the expression for the value and does not take into account
/// the parameter name or any commas that may be required.
ErrorOr<String> _computeValueCode(
FormalParameterElement parameter,
Expression? argument,
ArgumentEdit edit,
) {
// TODO(dantup): Should we accept arbitrary strings for all values? For
Expand Down Expand Up @@ -202,9 +182,17 @@ class EditArgumentHandler extends SharedMessageHandler<EditArgumentParams, Null>
return success(value.toString());
} else if (type.isDartCoreBool && value is bool) {
return success(value.toString());
} else if (type.isDartCoreString && value is String) {
return success(_computeStringValueCode(value));
} else if (type case InterfaceType(
} else if (parameter.type.isDartCoreString && value is String) {
var simpleString = argument is SimpleStringLiteral ? argument : null;
return success(
computeStringValueCode(
value,
preferSingleQuotes: simpleString?.isSingleQuoted ?? true,
preferMultiline: simpleString?.isMultiline ?? false,
preferRaw: simpleString?.isRaw ?? false,
),
);
} else if (parameter.type case InterfaceType(
:EnumElement2 element3,
) when value is String?) {
var allowedValues = getQualifiedEnumConstantNames(element3);
Expand Down Expand Up @@ -398,4 +386,47 @@ class EditArgumentHandler extends SharedMessageHandler<EditArgumentParams, Null>
newCode.toString(),
);
}

/// Computes the string of Dart code (including quotes) for the String
/// [value].
///
/// [preferSingleQuotes], [preferMultiline] and [preferRaw] are used to
/// control the kinds of delimeters used for the string but are not
/// guaranteed because the contents of the strings might prevent some
/// delimeters (for example raw strings can't be used where there need to be
/// escape sequences).
static String computeStringValueCode(
String value, {
bool preferSingleQuotes = true,
bool preferMultiline = false,
bool preferRaw = false,
}) {
var quoteCharacter = preferSingleQuotes ? "'" : '"';
var useMultiline = preferMultiline /* && value.contains('\n') ??? */;
var numQuotes = useMultiline ? 3 : 1;
var surroundingQuote = quoteCharacter * numQuotes;
// Only use raw if requested _and_ the string doesn't contain the
// quotes that'll be used to surround it or newlines.
var useRaw =
preferRaw &&
!value.contains(surroundingQuote) &&
!value.contains('\r') &&
!value.contains('\n');

// Escape non-quote characters.
if (!useRaw) {
value = value
.replaceAll(r'\', r'\\') // Escape backslashes
.replaceAll('\r', r'\r')
.replaceAll('\n', r'\n')
.replaceAll(r'$', r'\$');
}

// Escape quotes.
var escapedSurroundingQuote = '\\$quoteCharacter' * numQuotes;
value = value.replaceAll(surroundingQuote, escapedSurroundingQuote);

var prefix = useRaw ? 'r' : '';
return '$prefix$surroundingQuote$value$surroundingQuote';
}
}
Loading

0 comments on commit 11f53fd

Please sign in to comment.