Do not allow client to modify whitespace for auto insert edits (#8588)

Author: dibarbet

CHANGELOG.md

@@ -4,7 +4,11 @@
 - Debug from .csproj and .sln [#5876](https://github.com/dotnet/vscode-csharp/issues/5876)
 
 # 2.91.x
-* Bump Roslyn to 5.0.0-2.25458.1 (PR: [#8593](https://github.com/dotnet/vscode-csharp/pull/8593))
+* Bump Roslyn to 5.0.0-2.25458.10 (PR: [#8588](https://github.com/dotnet/vscode-csharp/pull/8588))
+  * Move brace adjustment on enter to on auto insert in LSP(PR: [#80075](https://github.com/dotnet/roslyn/pull/80075))
+  * Avoid throwing when obsolete overload of GetUpdatesAsync is invoked with empty array(PR: [#80161](https://github.com/dotnet/roslyn/pull/80161))
+  * Bump patch version of MSBuild packages(PR: [#80156](https://github.com/dotnet/roslyn/pull/80156))
+  * Include category in Hot Reload log messages(PR: [#80160](https://github.com/dotnet/roslyn/pull/80160))
   * Store client's version for open docs (PR: [#80064](https://github.com/dotnet/roslyn/pull/80064))
   * Pass global properties and the binary log path via RPC to BuildHost (PR: [#80094](https://github.com/dotnet/roslyn/pull/80094))
   * Don't switch runtime / design time solutions if cohosting is on (PR: [#80065](https://github.com/dotnet/roslyn/pull/80065))

package-lock.json

@@ -40,7 +40,7 @@
     "workspace"
   ],
   "defaults": {
-    "roslyn": "5.0.0-2.25458.1",
+    "roslyn": "5.0.0-2.25458.10",
     "omniSharp": "1.39.14",
     "razor": "10.0.0-preview.25454.5",
     "razorOmnisharp": "7.0.0-preview.23363.1",
@@ -128,7 +128,7 @@
     "@types/semver": "7.3.13",
     "@types/tmp": "0.0.33",
     "@types/uuid": "^9.0.1",
-    "@types/vscode": "1.93.0",
+    "@types/vscode": "1.98.0",
     "@types/yauzl": "2.10.0",
     "@typescript-eslint/eslint-plugin": "^8.19.0",
     "@typescript-eslint/parser": "^8.19.0",
@@ -688,7 +688,7 @@
     }
   ],
   "engines": {
-    "vscode": "^1.93.0"
+    "vscode": "^1.98.0"
   },
   "activationEvents": [
     "onDebugInitialConfigurations",

package.json

@@ -5,7 +5,12 @@
 
 import * as vscode from 'vscode';
 
-import { FormattingOptions, LanguageClient, TextDocumentIdentifier } from 'vscode-languageclient/node';
+import {
+    FormattingOptions,
+    InsertTextFormat,
+    LanguageClient,
+    TextDocumentIdentifier,
+} from 'vscode-languageclient/node';
 import * as RoslynProtocol from '../server/roslynProtocol';
 import { RoslynLanguageServer } from '../server/roslynLanguageServer';
 
@@ -50,7 +55,21 @@ export function registerOnAutoInsert(languageServer: RoslynLanguageServer, langu
         }
 
         // Regular expression to match all whitespace characters except the newline character
-        const changeTrimmed = change.text.replace(/[^\S\n]+/g, '');
+        let changeTrimmed = change.text.replace(/[^\S\n]+/g, '');
+
+        // If the change is empty after removing whitespace, we don't need to process it.
+        if (changeTrimmed.length === 0) {
+            return;
+        }
+
+        // When hitting enter between braces, we can end up with two new lines added (one to move the cursor down to an empty line,
+        // and another to move the close brace to a new line below that).  We want to detect that edit as a single new line trigger.
+        //
+        // Since we already removed all whitespace except new lines above, we can just trim the string to remove new lines as well
+        // and check if there is anything left.  If not, we know the change is just whitespace and new lines and can set the trigger to the new line character.
+        if (changeTrimmed.trim() === '') {
+            changeTrimmed = '\n';
+        }
 
         if (!vsTriggerCharacters.includes(changeTrimmed)) {
             return;
@@ -98,12 +117,20 @@ async function applyAutoInsertEdit(
         const textEdit = response._vs_textEdit;
         const startPosition = new vscode.Position(textEdit.range.start.line, textEdit.range.start.character);
         const endPosition = new vscode.Position(textEdit.range.end.line, textEdit.range.end.character);
-        const docComment = new vscode.SnippetString(textEdit.newText);
-        const code: any = vscode;
-        const textEdits = [new code.SnippetTextEdit(new vscode.Range(startPosition, endPosition), docComment)];
+
+        let textEdits: (vscode.TextEdit | vscode.SnippetTextEdit)[] = [];
+        if (response._vs_textEditFormat === InsertTextFormat.Snippet) {
+            const docComment = new vscode.SnippetString(textEdit.newText);
+            const edit = vscode.SnippetTextEdit.replace(new vscode.Range(startPosition, endPosition), docComment);
+            // Roslyn already formats the snippet correctly - we don't want the client to try and change the whitespace.
+            edit.keepWhitespace = true;
+            textEdits = [edit];
+        } else {
+            textEdits = [vscode.TextEdit.replace(new vscode.Range(startPosition, endPosition), textEdit.newText)];
+        }
+
         const edit = new vscode.WorkspaceEdit();
         edit.set(uri, textEdits);
-
         const applied = vscode.workspace.applyEdit(edit);
         if (!applied) {
             throw new Error('Tried to apply an edit but an error occurred.');

src/lsptoolshost/autoInsert/onAutoInsert.ts

@@ -15,6 +15,7 @@ import {
     waitForExpectedResult,
 } from './integrationHelpers';
 import { describe, beforeAll, beforeEach, afterAll, test, expect, afterEach } from '@jest/globals';
+import { EOL } from 'os';
 
 describe(`OnAutoInsert Tests`, () => {
     beforeAll(async () => {
@@ -72,6 +73,43 @@ describe(`OnAutoInsert Tests`, () => {
             }
         );
     });
+
+    test('Enter inside braces fixes brace lines', async () => {
+        await vscode.window.activeTextEditor!.edit((editBuilder) => {
+            editBuilder.insert(new vscode.Position(11, 15), '\n');
+        });
+
+        // OnAutoInsert is triggered by the change event but completes asynchronously, so wait for the buffer to be updated.
+
+        const expectedLines = [
+            'class DocComments',
+            '{',
+            '    //',
+            '    string M(int param1, string param2)',
+            '    {',
+            '        return null;',
+            '    }',
+            '',
+            '    /// <summary>',
+            '',
+            '    /// </summary>',
+            '    void M2()',
+            '    {',
+            '        ',
+            '    }',
+            '}',
+            '',
+        ];
+
+        await waitForExpectedResult<string | undefined>(
+            async () => vscode.window.activeTextEditor?.document.getText(),
+            10000,
+            100,
+            (input) => {
+                expect(input).toBe(expectedLines.join(EOL));
+            }
+        );
+    });
 });
 
 function normalizeNewlines(text: string | undefined): string | undefined {