Skip to content

Tweak UTF-8 string literal analyzer and code fix after language change#61650

Merged
davidwengier merged 10 commits intodotnet:mainfrom
davidwengier:UTF8Strings
Jun 7, 2022
Merged

Tweak UTF-8 string literal analyzer and code fix after language change#61650
davidwengier merged 10 commits intodotnet:mainfrom
davidwengier:UTF8Strings

Conversation

@davidwengier
Copy link
Member

Fixes #61517

In response to the language spec change to make the type of "ABC"u8 be a ReadOnlySpan<byte> rather than a byte[], this updates the analyzer and code fix I wrote previously to:

  • Not be part of code cleanup or VS options
  • Append .ToArray() on the end of the string literals it creates
  • Still be an analyzer so that we can still educate people about new language features

@ghost ghost added the Area-IDE label Jun 2, 2022
@davidwengier davidwengier marked this pull request as ready for review June 2, 2022 07:00
@davidwengier davidwengier requested a review from a team as a code owner June 2, 2022 07:00
Copy link
Member

@Youssef1313 Youssef1313 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An array is implicitly convertible to a ReadOnlySpan<T>
The .ToArray call may be unnecessary if the variable, for example, is passed to a method that accepts ReadOnlySpan<T> and the implicit conversion is used, or if the method has both overloads.

This is out of scope for this PR, but it might be beneficial to consider. I'm not sure what would be the best design for this. A separate simplifier that's always applied after code fixes are invoked? An annotation (similar to AddImportsAnnotation)? A separate analyzer/codefix?

private static LiteralExpressionSyntax CreateUTF8String(SyntaxNode nodeToTakeTriviaFrom, string stringValue)
private static InvocationExpressionSyntax CreateUTF8String(SyntaxNode nodeToTakeTriviaFrom, string stringValue)
{
return CreateUTF8String(nodeToTakeTriviaFrom.GetLeadingTrivia(), stringValue, nodeToTakeTriviaFrom.GetTrailingTrivia());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return CreateUTF8String(nodeToTakeTriviaFrom.GetLeadingTrivia(), stringValue, nodeToTakeTriviaFrom.GetTrailingTrivia());
return CreateUTF8String(stringValue).WithTriviaFrom(nodeToTakeTrivia);

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per below, switching this to use WithTriviaFrom would work, but the method would have to pass two empty trivia lists for the leading and trailing trivia arguments, so not worth it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!!

}

private static LiteralExpressionSyntax CreateUTF8String(SyntaxTriviaList leadingTrivia, string stringValue, SyntaxTriviaList trailingTrivia)
private static InvocationExpressionSyntax CreateUTF8String(SyntaxTriviaList leadingTrivia, string stringValue, SyntaxTriviaList trailingTrivia)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can remove all trivia logic from this.

Copy link
Member Author

@davidwengier davidwengier Jun 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, not. This is called for the param array case which needs to pass in no leading trivia, and trailing trivia from the last argument.

@davidwengier davidwengier merged commit a97a567 into dotnet:main Jun 7, 2022
@ghost ghost added this to the Next milestone Jun 7, 2022
@davidwengier davidwengier deleted the UTF8Strings branch June 7, 2022 08:38
@RikkiGibson RikkiGibson modified the milestones: Next, 17.3 P3 Jun 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A change for natural type of a UTF-8 string literal to ReadOnlySpan<byte> breaks several IDE tests

4 participants