dotnet / roslyn

The Roslyn .NET compiler provides C# and Visual Basic languages with rich code analysis APIs.
https://docs.microsoft.com/dotnet/csharp/roslyn-sdk/
MIT License
18.96k stars 4.02k forks source link

Automatic IsNullOrWhiteSpace and IsNullOrEmpty generation in German uses unescaped double quotes, generates broken code #51338

Closed vsfeedback closed 3 years ago

vsfeedback commented 3 years ago

This issue has been moved from a ticket on Developer Community.


Microsoft Visual Studio Enterprise 2019 Version 16.8.4

If you auto generate IsNullOrWhiteSpace or IsNullOrEmpty checks from a constructor in the german version of VS2019 it generates code with double quotes in a string, without escaping them first. The english version would use single quotes and it would therefore be not an issue. In German we use double quotes instead of single quotes.

Please replace the inner double quotes with singles or escape them with \" this is an error that is probably going to happen in other generation methods too. This is the generated code:

if (string. IsNullOrWhiteSpace(ConnectionString))
{
      throw new ArgumentException($""{nameof(ConnectionString)}" darf nicht NULL oder ein Leerraumzeichen sein.", nameof(ConnectionString));
}

It should be

if (string. IsNullOrWhiteSpace(ConnectionString))
{
      throw new ArgumentException($"\"{nameof(ConnectionString)}\" darf nicht NULL oder ein Leerraumzeichen sein.", nameof(ConnectionString));
}

Same with IsNullOrEmpty.

Thanks, -VR


Original Comments

Feedback Bot on 2/17/2021, 07:22 PM:

We have directed your feedback to the appropriate engineering team for further evaluation. The team will review the feedback and notify you about the next steps.


Original Solutions

(no solutions)

Youssef1313 commented 3 years ago

Related part of code:

https://github.com/dotnet/roslyn/blob/afd10305a37c0ffb2cfb2c2d8446154c68cfa87a/src/Features/Core/Portable/InitializeParameter/AbstractAddParameterCheckCodeRefactoringProvider.cs#L546-L572

Youssef1313 commented 3 years ago

https://github.com/dotnet/roslyn/blob/afd10305a37c0ffb2cfb2c2d8446154c68cfa87a/src/Features/Core/Portable/InitializeParameter/AbstractAddParameterCheckCodeRefactoringProvider.cs#L583-L586

In this method, should the SyntaxGenerator itself be responsible for escaping? or is it the responsibility of callers of SyntaxGenerator (i.e. the above method).

@CyrusNajmabadi Any clue?

CyrusNajmabadi commented 3 years ago

Flipped this around in my head a while and came up with this:

if this is SyntaxGenerator, i think it should be responsible. It basically never makes sense to generate bogus strings that will just hork the doc.

davidwengier commented 3 years ago

I don't think this is a SyntaxGenerator issue. That method is intended to take string literals and return interpolated string nodes. This bug is caused by taking a UI string and assuming it will be a valid string literal.