Roslyn: Need auto-fixer for ordering of $@ versus @$

[jcouv]

Discussed here: https://github.com/dotnet/csharplang/issues/1630#issuecomment-422250220

[Neme12]

Meh. I'm thinking a nicer solution would have been for the language to only support one ordering but for the IDE to fix it up as you're typing if you get it wrong. Adding a code style for this seems very strange, because I don't think this is something people will care or have opinions about, they just want to get it right when typing. But now the language introduces 2 ways of writing the same thing here, so a code style would indeed be appropriate, because people want consistency 😕

[sharwell]

I have no objection to the inclusion of a code style analyzer. However, the preferred ordering should not be configurable. $@ is the strictly-preferred form as it is (and will always be) more widely supported.

[jcouv]

Updating issue to reflect discussion in https://github.com/dotnet/csharplang/issues/1630#issuecomment-422952283
We're planning to keep the language change (ie. allow @$), but it's good to have more "aggressive" tooling support to auto-fix to $@.

On further reflection, I'm thinking we can leave the code fixer, just in case you open a project that still has a @$ in it and you want to fix them.

[Neme12]

On further reflection, I'm thinking we can leave the code fixer, just in case you open a project that still has a @$ in it and you want to fix them.

Yes, I assumed it would be left in for cases other than forward typing. For example what if you have an existing interpolated string and add a @ in front of it? Then the code fix would still be useful (I assume the auto-fixer wouldn't trigger in that case.)

[Neme12]

Should we open a new issue for an auto-fixer? It seems unrelated to the code style. Or should it always be turned on if the code style is active, as opposed to having a separate checkbox that users could turn off while still having the code style?

[jcouv]

I updated the title for this issue. I think if we have an auto-fixer and the existing refactoring, then we're sufficiently covered.
Do you think we'd also need a code style on top of that?

(cc @CyrusNajmabadi)

[sharwell]

Do you think we'd also need a code style on top of that?

No, a code style is more code/configuration/test configurations to maintain. A simple diagnostic is fine, and if it needs to be turned off the existing rule set editor and/or future .editorconfig support for diagnostic IDs will be sufficient. There is no need to add more complexity to Tools>Options.

[Neme12]

@sharwell Should the diagnostic be a suggestion or a warning by default? Also, do you have an opinion about the auto-fixer when typing? Is it appropriate for Roslyn? Should there be an option for it? Can you bring this to a design meeting if necessary? Thanks

[Neme12]

I have the auto fixer working locally:

it is reordered as soon as " is pressed (and therefore it's clear that this actually parses as an interpolated verbatim string)

[Neme12]

Is there any chance that issues that have a PR submitted for them would be at least a little more prioritized over some other issues that aren't likely to get any attention soon (for example #28762, #24786)? Thanks.

[Neme12]

I'm not asking for prioritization over features that the team is interested in or has prioritized. My point is: there is a lot of open issues, suggestions & requests that won't likely be implemented unless an external contributor takes the work because it is not a priority for the team. This issue might fall in that category too (I don't know and that's not up to me to determine), but I'm asking whether within this group, issues that have an active PR could get a minor preference. It's OK if issues that nobody is trying to fix or even commenting on take months until a design resolution, but it would be nicer to not take as long if there's an actual PR and activity on that issue.

[sharwell]

@Neme12 Assuming you are talking about design priority, I agree. This will show up for us nicely if we label the issue as in-review (which I'll do now for the ones you pointed out).

[Neme12]

@sharwell I'm sorry, I think there was a misunderstanding. I mentioned #28762 and #24786 as examples of issues that aren't in review and don't need to be prioritized (as opposed to this one).

[dpoeschl]

Design Meeting Notes:

• The auto-fix as you type the quote sounds good.

[Neme12]

@dpoeschl There was another question for the design review brought by @sharwell when implementing this in #30074, which is: should we auto-fix even when inserting the @ or $ characters? Can you please bring that up?

❓ How hard would it be to handle the two remaining cases?

1. Inserting @ in front of $"
2. Inserting $ in the middle of @"

[dpoeschl]

Design Meeting Notes Part 2:
Let's implement the straightforward case regardless, but some thoughts:

1. Inserting @ in front of $"

This could interfere with typing valid constructs in front of the existing string (escaped identifiers, another verbatim string). Let's not do this.

1. Inserting $ in the middle of @"

We're not immediately aware of when typing this $ would mean anything else, but would still prefer to wait for feedback before adding this part.