String interpolation refactoring doesn't work on Console.WriteLine

[WhiteBlackGoose]

SDK: 6.0.100-preview.6.21355.2
VS: 17.0p2

[Youssef1313]

This is the expected behavior of this code refactoring. It keeps the semantics the same. Its main usage is when you have a long string containing a bunch of braces and you want to add some variables in it. See #48219

I guess the intent should be clearer by updating the title to "Convert to interpolated string (keep semantics)" or "Convert to an equivalent interpolated string" or whatever makes it clear.

[333fred]

Not sure I agree with that assessment. Sure, it calls a different method, but the effect is the same. We should recognize this and offer the refactoring (imo).

[Youssef1313]

@333fred That would be a separate analyzer with a codefix?

ie, I consider this issue as a request for a new analyzer and a codefix to handle this case, not to modify the existing refactoring.

[333fred]

ie, I consider this issue as a request for a new analyzer and a codefix to handle this case, not to modify the existing refactoring.

I'd defer to @CyrusNajmabadi or someone else on the IDE team to answer that question. Personally, I view it as the same thing, but I'm not a decision-maker here :)

[jmarolf]

We have this refactoring defined for methods like string.Format

We should update the refactoring to handle all other .NET "formatting" methods.

We just need to expand this list

roslyn/src/Features/Core/Portable/ConvertToInterpolatedString/AbstractConvertPlaceholderToInterpolatedStringRefactoringProvider.cs

Lines 43 to 48 in 3a5457c

	var formatMethods = stringType
	.GetMembers(nameof(string.Format))
	.OfType<IMethodSymbol>()
	.Where(ShouldIncludeFormatMethod)
	.ToImmutableArray();

[Youssef1313]

It looks like the same title "Convert to interpolated string" is used by different codefixes that exist for different purposes. I find this very confusing. Should this be tracked as part of this issue or do I need to create a new issue?

cc @jmarolf @CyrusNajmabadi

[jmarolf]

I would argue that we should only show a single refactoring for this case

that converts this:

Console.Writeline("Quack quack {0}", "duck");

to this:

Console.Writeline($"Quack quack {"duck"}");

I think it is fine to have the same text for different refactorings as long as it does what the user expects.