Roslyn: Complete Statement should not move semicolon outside of lambda statements

[chborl]

I encounter this issue a lot when converting single line lambda expressions into multiline (block).
For example, typing a semicolon after ToString() inserts the semicolon at the end of the line:
var items = new[] { 1, 2, 3 };
items.Select(i => { i.ToString())

This issue comes from comments added to https://developercommunity.visualstudio.com/content/problem/605241/typing-a-semicolon-within-a-function-argument-list.html

[aaubry]

I have upgraded to version 16.2.0 and this specific problem has been fixed, although the behaviour is still inconsistent. If I remove the opening braces, typing the semicolon after ToString() places the semicolon at the end of the line instead of at the location of the cursor...

c# var items = new[] { 1, 2, 3 }; items.Select(i => i.ToString())

[CyrusNajmabadi]

That seems "correct" to me. It's not a block anymore. And i => i.ToString(); would be invalid. So it makes sense to me that you would get items.Select(i => i.ToString()); here.

[aaubry]

I disagree. If my cursor is placed at a given location, I expect that typing any key will type that key at that location.
In that case I know that I will need to add the braces, but since my cursor is already at the end of the line, it is more efficient to type the semicolon first, followed by the closing brace, then go the the start of the line and add the opening brace and the return keyword.
I don't want an editor that attempts to be clever and try to anticipate my intention. The editor should be predictable and, importantly, behave consistently with other similar editors.

[CyrusNajmabadi]

I disagree. If my cursor is placed at a given location, I expect that typing any key will type that key at that location.

I mean... this is the point of hte feature. To allow you to type semicolon and have it complete the statement appropriately, so you don't have to jump to the end.

Sounds like it would be valuable for you to have a way to turn this off. @chborl is there such a way.

In that case I know that I will need to add the braces, but since my cursor is already at the end of the line, it is more efficient to type the semicolon first, followed by the closing brace, then go the the start of the line and add the opening brace and the return keyword.

Sure. Any system that does anything beyond inserting exactly what you type might have some ill effects in some cases. We balance that versus the benefits the feature provides.

I don't want an editor that attempts to be clever and try to anticipate my intention.

That's a reasonable thing to want. It's worth pointing out that that's not what the VS ide does though. For example, it will commonly do things like brace completion, or intellisense completion, etc. which do not end up inserting what you've typed into the editor. In all these cases, it's very possible for what is inserted to not match what you desired. That's accepted, in exchange for the high value these still provide for the majority of cases though.

[aaubry]

I get this, and I understand that some people may want this on. But usually these helpers can be disabled.
Personally I'd prefer to not have stuff like brace completion because in many cases the IDE gets it wrong and ends up inserting braces or parenthesis at the wrong location.
For braces at least I can use Ctrl+Z and the editor reverts to the state before adding extra stuff. But not for this one. I would expect that Undo would put the semicolon where I typed it, but instead it completely removed the character. So, in practice it means that it is impossible to type the semicolon at some locations, unless I copy and paste one from somewhere else.

[CyrusNajmabadi]

I get this, and I understand that some people may want this on. But usually these helpers can be disabled.

Fair point. @chborl Can you disable this feature? If not, can we add an option to allow it to be disabled?

For braces at least I can use Ctrl+Z and the editor reverts to the state before adding extra stuff. But not for this one.

That seems wrong as well. @chborl is this the expected behavior of the feature? I woudl also expect that 'undo' put you back to the state the editor would be in if it hasn't moved the semicolon around. Can you clarify the expected behavior? If this is a bug, can we file an issue (or use this issue) to track addressing this part?

[jmarolf]

as @aaubry points out, the normal design in VS for features like these is to go ahead and do the "smart" thing but put the original action on the undo buffer. We don't do this for things like skipping ", or ')' if they are added. Perhaps a simple redesign of the undo buffer behavior would help here instead of polluting VS with even more options.

[aaubry]

That would be a reasonable solution, although I would really like a single setting to disable all the "smart" features. Not the ones that are triggered explicitly like Intellisense or snippets, but everything that adds more stuff than I added.
I don't want to sound too negative. Visual Studio is by far the best IDE that I have used. But not everyone types code in the order that these smart features expect, and in those cases the results are disastrous. A different example, many times I want to add parenthesis around something. If the cursor is closer to the end, I add the closing parenthesis first. But if there's already a parenthesis there, closing something else, the editor thinks that I made a mistake and does not add it. This kind of stuff is really annoying. I don't want to be fighting with the editor.
Sorry for the rant, I feel like an old-timer who is unable to adapt to change.

[CyrusNajmabadi]

Not the ones that are triggered explicitly like Intellisense

Intellisense isn't triggered explicitly though.

That would be a reasonable solution, although I would really like a single setting to disable all the "smart" features.

I'd prefer multiple options (or just allowin ctrl-z to work here). I don't like the idea of forcing other users to have to disable many features they may like just because there's a single feature they don't. It feels like throwing out the baby with the bathwater.

[jmarolf]

I'd prefer multiple options (or just allowin ctrl-z to work here). I don't like the idea of forcing other users to have to disable many features they may like just because there's a single feature they don't. It feels like throwing out the baby with the bathwater.

yeah, this is my position. ctrl+z should _always_ work. I think we can also tweak behavior to make things "smarter" so that we pair back the heuristic and not do type-over in a lot more cases. I think a user finding these options and being able to reason about what exactly they should disable/enable to get the right behavior is going to be hard.

[CyrusNajmabadi]

yeah, this is my position. ctrl+z should always work

agreed.

I think we can also tweak behavior to make things "smarter" so that we pair back the heuristic and not do type-over in a lot more cases.

Sure, though in this case, i think teh typing scenario he outlined is exactly the one that this feature was designed for. i.e. all the requests from people saying "if i type semicolon in the middle of an expression where it would not be legal, just go complete my outermost statement". Isn't this eactly the sort of case the feature was designed for?

[sharwell]

Supporting undo for the feature is desired. We didn't support it initially because the conservative approach for the feature means we never trigger the new behavior on common coding paths.

@aaubry Note that if you press Ctrl+. on the => operator, one of the options is to use the block syntax for the lambda. This will add braces and the semicolon, and also add a return keyword if necessary.

[aaubry]

@sharwell thanks for the tip! I didn't know about that one.

[binki]

This was a recently introduced feature which I also find unintuitive and different from VS’s existing features. When VS inserts a closing curly brace for you, for example, it lets you keep typing: when you finally type the closing curly break, it lets you type through the auto-inserted one. Inserting the semicolon never used to jump to the end of the line, and I would often type it into a lambda while converting from single to multi-line lambdas.

I thought this behavior was an accidental side effect of a different change, not an intentional one. The other behaviors of VS that have been around for years at least all feel intentional instead of unexpected (like this one).

Could we have the default setting for this preference be off at the very least?

[binki]

Another example of this feature doing things that are not helpful at all:

new[]
{
    1,
    2,
    3,
}.Select(x => 2 * x).Where(x => x < 4).Select(x => 2 / x).ToList();

If your cursor position is after the 4, it inserts a semicolon prior to .Select(). That seems to not be in line with the intentions of the author who intentionally placed their cursor prior to the ) rather than after the )…

[CyrusNajmabadi]

I thought this behavior was an accidental side effect of a different change, not an intentional one.

Nope. Definitely intentional. Strongly requested feature from many people.

If your cursor position is after the 4, it inserts a semicolon prior to .Select()

Yup, seems odd. @chborl was it intentional that you don't walk up all the postfix expressions of the invocation you're in?

[chborl]

This is all great feedback. I opened a new issue for supporting undo in this feature. https://github.com/dotnet/roslyn/issues/37577

If your cursor position is after the 4, it inserts a semicolon prior to .Select().

This was intentional, with the thought that the user wanted to end the statement on the expression where they entered the semicolon. I've opened https://github.com/dotnet/roslyn/issues/37578 to track this issue for discussion and possible fix.

[vinnyrom]

Can you elaborate on the specific user scenarios this feature is intended to address? I suppose I could get on board if it only ever skipped-over auto-inserted ')' or '}' characters.

I find it odd that this feature would ever insert the semicolon immediately after an existing one. Maybe the insertion location shouldn't be changed if that would be the case.

I routinely run into issues when I type a semicolon instead of a comma. Instead of changing where the semicolon is inserted, have you considered auto-changing it to a comma if one would be valid at that location? Skipping-over only auto-inserted ')' or '}' characters would still take precedence.

[CyrusNajmabadi]

@vinnyrom Sounds like an interesting idea! Would you be interested in contributing that?

[jinujoseph]

fixed via https://github.com/dotnet/roslyn/pull/36960