Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When converting to raw strings, do not change \r\n sequences if not explicitly requested by the user #76120

Merged
merged 3 commits into from
Dec 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading