Skip to content

Commit

Permalink
When converting to raw strings, do not change \r\n sequences if not e…
Browse files Browse the repository at this point in the history
…xplicitly requested by the user (#76120)
  • Loading branch information
CyrusNajmabadi authored Dec 2, 2024
2 parents 31ac7e2 + e0bf0f3 commit b1d6734
Show file tree
Hide file tree
Showing 8 changed files with 207 additions and 55 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,14 @@

namespace Microsoft.CodeAnalysis.CSharp.ConvertToRawString;

internal readonly struct CanConvertParams(CodeActionPriority priority, bool canBeSingleLine, bool canBeMultiLineWithoutLeadingWhiteSpaces)
internal readonly struct CanConvertParams(
CodeActionPriority priority,
bool canBeSingleLine,
bool canBeMultiLineWithoutLeadingWhiteSpaces,
bool containsEscapedEndOfLineCharacter)
{
public CodeActionPriority Priority { get; } = priority;
public bool CanBeSingleLine { get; } = canBeSingleLine;
public bool CanBeMultiLineWithoutLeadingWhiteSpaces { get; } = canBeMultiLineWithoutLeadingWhiteSpaces;
public bool ContainsEscapedEndOfLineCharacter { get; } = containsEscapedEndOfLineCharacter;
}
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ protected override bool CanConvert(

var priority = CodeActionPriority.Low;
var canBeSingleLine = true;
var containsEscapedEndOfLineCharacter = false;
foreach (var content in stringExpression.Contents)
{
if (content is InterpolationSyntax interpolation)
Expand All @@ -86,8 +87,10 @@ protected override bool CanConvert(
var characters = TryConvertToVirtualChars(interpolation.FormatClause.FormatStringToken);

// Ensure that all characters in the string are those we can convert.
if (!ConvertToRawStringHelpers.CanConvert(characters))
if (!ConvertToRawStringHelpers.CanConvert(characters, out var chunkContainsEscapedEndOfLineCharacter))
return false;

containsEscapedEndOfLineCharacter |= chunkContainsEscapedEndOfLineCharacter;
}

if (canBeSingleLine && !document.Text.AreOnSameLine(interpolation.OpenBraceToken, interpolation.CloseBraceToken))
Expand All @@ -98,9 +101,11 @@ protected override bool CanConvert(
var characters = TryConvertToVirtualChars(interpolatedStringText);

// Ensure that all characters in the string are those we can convert.
if (!ConvertToRawStringHelpers.CanConvert(characters))
if (!ConvertToRawStringHelpers.CanConvert(characters, out var chunkContainsEscapedEndOfLineCharacter))
return false;

containsEscapedEndOfLineCharacter |= chunkContainsEscapedEndOfLineCharacter;

if (canBeSingleLine)
{
// a single line raw string cannot contain a newline.
Expand Down Expand Up @@ -167,7 +172,8 @@ protected override bool CanConvert(
canBeMultiLineWithoutLeadingWhiteSpaces = !cleaned.IsEquivalentTo(converted);
}

convertParams = new CanConvertParams(priority, canBeSingleLine, canBeMultiLineWithoutLeadingWhiteSpaces);
convertParams = new CanConvertParams(
priority, canBeSingleLine, canBeMultiLineWithoutLeadingWhiteSpaces, containsEscapedEndOfLineCharacter);
return true;
}

Expand All @@ -178,7 +184,7 @@ protected override InterpolatedStringExpressionSyntax Convert(
SyntaxFormattingOptions formattingOptions,
CancellationToken cancellationToken)
{
if (kind == ConvertToRawKind.SingleLine)
if ((kind & ConvertToRawKind.SingleLine) == ConvertToRawKind.SingleLine)
return ConvertToSingleLineRawString();

var indentationOptions = new IndentationOptions(formattingOptions);
Expand Down Expand Up @@ -231,7 +237,7 @@ InterpolatedStringExpressionSyntax ConvertToMultiLineRawIndentedString(ParsedDoc
var rawStringExpression = GetInitialMultiLineRawInterpolatedString(stringExpression, formattingOptions);

// If requested, cleanup the whitespace in the expression.
var cleanedExpression = kind == ConvertToRawKind.MultiLineWithoutLeadingWhitespace
var cleanedExpression = (kind & ConvertToRawKind.MultiLineWithoutLeadingWhitespace) == ConvertToRawKind.MultiLineWithoutLeadingWhitespace
? CleanInterpolatedString(rawStringExpression, cancellationToken)
: rawStringExpression;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ private static bool CanConvertStringLiteral(SyntaxToken token, out CanConvertPar

var characters = CSharpVirtualCharService.Instance.TryConvertToVirtualChars(token);

if (!ConvertToRawStringHelpers.CanConvert(characters))
if (!ConvertToRawStringHelpers.CanConvert(characters, out var containsEscapedEndOfLineCharacter))
return false;

// TODO(cyrusn): Should we offer this on empty strings... seems undesirable as you'd end with a gigantic
Expand Down Expand Up @@ -100,7 +100,8 @@ private static bool CanConvertStringLiteral(SyntaxToken token, out CanConvertPar
// invoking this on lots of strings that they have no interest in converting.
var priority = AllEscapesAreQuotes(characters) ? CodeActionPriority.Default : CodeActionPriority.Low;

convertParams = new CanConvertParams(priority, canBeSingleLine, canBeMultiLineWithoutLeadingWhiteSpaces);
convertParams = new CanConvertParams(
priority, canBeSingleLine, canBeMultiLineWithoutLeadingWhiteSpaces, containsEscapedEndOfLineCharacter);
return true;

static bool HasLeadingWhitespace(VirtualCharSequence characters)
Expand Down Expand Up @@ -144,7 +145,7 @@ private static SyntaxToken GetReplacementToken(
var characters = CSharpVirtualCharService.Instance.TryConvertToVirtualChars(token);
Contract.ThrowIfTrue(characters.IsDefaultOrEmpty);

if (kind == ConvertToRawKind.SingleLine)
if ((kind & ConvertToRawKind.SingleLine) == ConvertToRawKind.SingleLine)
return ConvertToSingleLineRawString();

var indentationOptions = new IndentationOptions(formattingOptions);
Expand Down Expand Up @@ -202,7 +203,7 @@ SyntaxToken ConvertToSingleLineRawString()
SyntaxToken ConvertToMultiLineRawIndentedString(string indentation)
{
// If the user asked to remove whitespace then do so now.
if (kind == ConvertToRawKind.MultiLineWithoutLeadingWhitespace)
if ((kind & ConvertToRawKind.MultiLineWithoutLeadingWhitespace) == ConvertToRawKind.MultiLineWithoutLeadingWhitespace)
characters = CleanupWhitespace(characters);

// Have to make sure we have a delimiter longer than any quote sequence in the string.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Composition;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Globalization;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
Expand All @@ -22,24 +23,22 @@
namespace Microsoft.CodeAnalysis.CSharp.ConvertToRawString;

[ExportCodeRefactoringProvider(LanguageNames.CSharp, Name = PredefinedCodeRefactoringProviderNames.ConvertToRawString), Shared]
internal sealed partial class ConvertStringToRawStringCodeRefactoringProvider : SyntaxEditorBasedCodeRefactoringProvider
[method: ImportingConstructor]
[method: SuppressMessage("RoslynDiagnosticsReliability", "RS0033:Importing constructor should be [Obsolete]", Justification = "Used in test code: https://github.com/dotnet/roslyn/issues/42814")]
internal sealed partial class ConvertStringToRawStringCodeRefactoringProvider() : SyntaxEditorBasedCodeRefactoringProvider
{
private static readonly BidirectionalMap<ConvertToRawKind, string> s_kindToEquivalenceKeyMap =
new(
[
KeyValuePairUtil.Create(ConvertToRawKind.SingleLine, nameof(ConvertToRawKind.SingleLine)),
KeyValuePairUtil.Create(ConvertToRawKind.MultiLineIndented, nameof(ConvertToRawKind.MultiLineIndented)),
KeyValuePairUtil.Create(ConvertToRawKind.MultiLineWithoutLeadingWhitespace, nameof(ConvertToRawKind.MultiLineWithoutLeadingWhitespace)),
]);
private static readonly string[] s_kindToEquivalenceKeyMap;

private static readonly ImmutableArray<IConvertStringProvider> s_convertStringProviders = [ConvertRegularStringToRawStringProvider.Instance, ConvertInterpolatedStringToRawStringProvider.Instance];

[ImportingConstructor]
[SuppressMessage("RoslynDiagnosticsReliability", "RS0033:Importing constructor should be [Obsolete]", Justification = "Used in test code: https://github.com/dotnet/roslyn/issues/42814")]
public ConvertStringToRawStringCodeRefactoringProvider()
static ConvertStringToRawStringCodeRefactoringProvider()
{
var count = (int)ConvertToRawKind.ContainsEscapedEndOfLineCharacter * 2;
s_kindToEquivalenceKeyMap = new string[count];
for (var i = 0; i < count; i++)
s_kindToEquivalenceKeyMap[i] = i.ToString(CultureInfo.InvariantCulture);
}

private static readonly ImmutableArray<IConvertStringProvider> s_convertStringProviders = [ConvertRegularStringToRawStringProvider.Instance, ConvertInterpolatedStringToRawStringProvider.Instance];

protected override ImmutableArray<FixAllScope> SupportedFixAllScopes => AllFixAllScopes;

private static bool CanConvert(
Expand Down Expand Up @@ -84,13 +83,17 @@ public override async Task ComputeRefactoringsAsync(CodeRefactoringContext conte

var priority = convertParams.Priority;

var kind = convertParams.ContainsEscapedEndOfLineCharacter
? ConvertToRawKind.ContainsEscapedEndOfLineCharacter
: 0;

if (convertParams.CanBeSingleLine)
{
context.RegisterRefactoring(
CodeAction.Create(
CSharpFeaturesResources.Convert_to_raw_string,
cancellationToken => UpdateDocumentAsync(document, parentExpression, ConvertToRawKind.SingleLine, provider, cancellationToken),
s_kindToEquivalenceKeyMap[ConvertToRawKind.SingleLine],
cancellationToken => UpdateDocumentAsync(document, parentExpression, kind | ConvertToRawKind.SingleLine, provider, cancellationToken),
s_kindToEquivalenceKeyMap[(int)(kind | ConvertToRawKind.SingleLine)],
priority),
token.Span);
}
Expand All @@ -99,8 +102,8 @@ public override async Task ComputeRefactoringsAsync(CodeRefactoringContext conte
context.RegisterRefactoring(
CodeAction.Create(
CSharpFeaturesResources.Convert_to_raw_string,
cancellationToken => UpdateDocumentAsync(document, parentExpression, ConvertToRawKind.MultiLineIndented, provider, cancellationToken),
s_kindToEquivalenceKeyMap[ConvertToRawKind.MultiLineIndented],
cancellationToken => UpdateDocumentAsync(document, parentExpression, kind | ConvertToRawKind.MultiLineIndented, provider, cancellationToken),
s_kindToEquivalenceKeyMap[(int)(kind | ConvertToRawKind.MultiLineIndented)],
priority),
token.Span);

Expand All @@ -109,8 +112,8 @@ public override async Task ComputeRefactoringsAsync(CodeRefactoringContext conte
context.RegisterRefactoring(
CodeAction.Create(
CSharpFeaturesResources.without_leading_whitespace_may_change_semantics,
cancellationToken => UpdateDocumentAsync(document, parentExpression, ConvertToRawKind.MultiLineWithoutLeadingWhitespace, provider, cancellationToken),
s_kindToEquivalenceKeyMap[ConvertToRawKind.MultiLineWithoutLeadingWhitespace],
cancellationToken => UpdateDocumentAsync(document, parentExpression, kind | ConvertToRawKind.MultiLineWithoutLeadingWhitespace, provider, cancellationToken),
s_kindToEquivalenceKeyMap[(int)(kind | ConvertToRawKind.MultiLineWithoutLeadingWhitespace)],
priority),
token.Span);
}
Expand Down Expand Up @@ -141,7 +144,7 @@ protected override async Task FixAllAsync(
{
// Get the kind to be fixed from the equivalenceKey for the FixAll operation
Debug.Assert(equivalenceKey != null);
var kind = s_kindToEquivalenceKeyMap[equivalenceKey];
var kind = (ConvertToRawKind)int.Parse(equivalenceKey, CultureInfo.InvariantCulture);

var formattingOptions = await document.GetSyntaxFormattingOptionsAsync(cancellationToken).ConfigureAwait(false);
var parsedDocument = await ParsedDocument.CreateAsync(document, cancellationToken).ConfigureAwait(false);
Expand All @@ -155,8 +158,15 @@ protected override async Task FixAllAsync(
if (!CanConvert(parsedDocument, expression, formattingOptions, out var canConvertParams, out var provider, cancellationToken))
continue;

// Ensure we have a matching kind to fix for this literal
var hasMatchingKind = kind switch
// If the user started on a string that didn't have an explicit \n in it, then don't update other string
// literals that do. Note: if they did start on a string with an explicit \n, then we should update all
// other literals, regardless of whether they had an explicit \n or not.
if ((kind & ConvertToRawKind.ContainsEscapedEndOfLineCharacter) == 0 && canConvertParams.ContainsEscapedEndOfLineCharacter)
continue;

// Ensure we have a matching kind to fix for this literal.
// Remove the end of line flag so we can trivially switch on the rest below.
var hasMatchingKind = (kind & ~ConvertToRawKind.ContainsEscapedEndOfLineCharacter) switch
{
ConvertToRawKind.SingleLine => canConvertParams.CanBeSingleLine,
ConvertToRawKind.MultiLineIndented => !canConvertParams.CanBeSingleLine,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,16 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;

namespace Microsoft.CodeAnalysis.CSharp.ConvertToRawString;

[Flags]
internal enum ConvertToRawKind
{
SingleLine,
MultiLineIndented,
MultiLineWithoutLeadingWhitespace,
SingleLine = 1 << 0,
MultiLineIndented = 1 << 1,
MultiLineWithoutLeadingWhitespace = 1 << 2,

ContainsEscapedEndOfLineCharacter = 1 << 3,
}
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,33 @@ public static bool IsInDirective(SyntaxNode? node)
return false;
}

public static bool CanConvert(VirtualCharSequence characters)
=> !characters.IsDefault && characters.All(static ch => CanConvert(ch));
/// <summary>
/// Returns if this sequence of characters can be converted to a raw string. If it can, also returns if it
/// contained an explicitly escaped newline (like <c>\r\n</c>) within it. It it can't convert, then the value of
/// <paramref name="containsEscapedEndOfLineCharacter"/> is undefined.
/// </summary>
public static bool CanConvert(VirtualCharSequence characters, out bool containsEscapedEndOfLineCharacter)
{
containsEscapedEndOfLineCharacter = false;
if (characters.IsDefault)
return false;

foreach (var ch in characters)
{
if (!CanConvert(ch))
return false;

// Look for *explicit* usages of sequences like \r or \n. These are multi character representations of
// newlines. If we see these, we only want to fix these up in a fix-all if the original string contained
// those as well.
if (ch.Span.Length > 1 && SyntaxFacts.IsNewLine((char)ch.Value))
containsEscapedEndOfLineCharacter = true;
}

return true;
}

public static bool CanConvert(VirtualChar ch)
private static bool CanConvert(VirtualChar ch)
{
// Don't bother with unpaired surrogates. This is just a legacy language corner case that we don't care to
// even try having support for.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -270,10 +270,7 @@ void M()
var singleLine1 = $"a";
var singleLine2 = @$"goo""bar";
var multiLine1 = $"""
goo
bar
""";
var multiLine1 = $"goo\r\nbar";
var multiLine2 = $"""
goo
bar
Expand All @@ -296,10 +293,7 @@ void M2()
var singleLine1 = $"a";
var singleLine2 = @$"goo""bar";
var multiLine1 = $"""
goo
bar
""";
var multiLine1 = $"goo\r\nbar";
var multiLine2 = $"""
goo
bar
Expand Down
Loading

0 comments on commit b1d6734

Please sign in to comment.