-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Fix interpolated string refactoring to work for Console.Write* #55521
Fix interpolated string refactoring to work for Console.Write* #55521
Conversation
...ertToInterpolatedString/AbstractConvertPlaceholderToInterpolatedStringRefactoringProvider.cs
Outdated
Show resolved
Hide resolved
.OfType<IMethodSymbol>() | ||
.Where(ShouldIncludeFormatMethod) | ||
.ToImmutableArray(); | ||
var consoleType = semanticModel.Compilation.GetTypeByMetadataName(typeof(Console).FullName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we eventually support all methods mentioned in composite formatting?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe. The intention of this PR was to open that possibility up easily without adding them, since the original issue and design only mentioned console.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AppendFormat
is fun because you would want it to substitute with Append($"")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yea, I'm really thinking there should be some sort of abstraction here for all of that...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated included ones in this PR
...atures/CSharpTest/ConvertToInterpolatedString/ConvertPlaceholderToInterpolatedStringTests.cs
Outdated
Show resolved
Hide resolved
...atures/CSharpTest/ConvertToInterpolatedString/ConvertPlaceholderToInterpolatedStringTests.cs
Outdated
Show resolved
Hide resolved
...atures/CSharpTest/ConvertToInterpolatedString/ConvertPlaceholderToInterpolatedStringTests.cs
Outdated
Show resolved
Hide resolved
...atures/CSharpTest/ConvertToInterpolatedString/ConvertPlaceholderToInterpolatedStringTests.cs
Outdated
Show resolved
Hide resolved
.OfType<IMethodSymbol>() | ||
.Where(ShouldIncludeFormatMethod) | ||
.ToImmutableArray(); | ||
var consoleType = semanticModel.Compilation.GetTypeByMetadataName(typeof(Console).FullName); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AppendFormat
is fun because you would want it to substitute with Append($"")
...ertToInterpolatedString/AbstractConvertPlaceholderToInterpolatedStringRefactoringProvider.cs
Outdated
Show resolved
Hide resolved
@@ -35,18 +35,22 @@ public override async Task ComputeRefactoringsAsync(CodeRefactoringContext conte | |||
var semanticModel = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false); | |||
|
|||
var stringType = semanticModel.Compilation.GetSpecialType(SpecialType.System_String); | |||
if (stringType == null) | |||
|
|||
if (stringType is null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This if condition is unreachable. Only GetTypeByMetadataName
can return null. For special type if it doesn't exist, the compiler returns an error symbol.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. Looks like the initial code was wrong then. Fixing
Fixes #55053