Add the option for Roslyn LSP to support organize imports (#7935)

Author: JoeRobich

.vscode/launch.json

@@ -12,7 +12,7 @@
             "preLaunchTask": "buildDev"
         },
         {
-            "name": "Launch Current File slnWithCsproj Integration Tests",
+            "name": "[Roslyn] Run Current File Integration Tests",
             "type": "extensionHost",
             "request": "launch",
             "runtimeExecutable": "${execPath}",
@@ -39,7 +39,7 @@
             "internalConsoleOptions": "openOnSessionStart"
         },
         {
-            "name": "[DevKit] Launch Current File slnWithCsproj Integration Tests",
+            "name": "[DevKit] Run Current File Integration Tests",
             "type": "extensionHost",
             "request": "launch",
             "runtimeExecutable": "${execPath}",
@@ -93,7 +93,7 @@
             "internalConsoleOptions": "openOnSessionStart"
         },
         {
-            "name": "Omnisharp: Launch Current File Integration Tests",
+            "name": "[O#] Run Current File Integration Tests",
             "type": "extensionHost",
             "request": "launch",
             "runtimeExecutable": "${execPath}",
@@ -115,10 +115,10 @@
             "sourceMaps": true,
             "outFiles": ["${workspaceRoot}/dist/*.js", "${workspaceRoot}/out/test/**/*.js"],
             "resolveSourceMapLocations": ["${workspaceFolder}/**", "!**/node_modules/**"],
-            "preLaunchTask": "buildDev"
+            "preLaunchTask": "buildTest"
         },
         {
-            "name": "Omnisharp: Launch Current File Integration Tests [LSP]",
+            "name": "[O# LSP] Run Current File Integration Tests",
             "type": "extensionHost",
             "request": "launch",
             "runtimeExecutable": "${execPath}",
@@ -140,7 +140,7 @@
             "sourceMaps": true,
             "outFiles": ["${workspaceRoot}/dist/*.js", "${workspaceRoot}/out/test/**/*.js"],
             "resolveSourceMapLocations": ["${workspaceFolder}/**", "!**/node_modules/**"],
-            "preLaunchTask": "buildDev"
+            "preLaunchTask": "buildTest"
         },
         {
             "type": "node",

CHANGELOG.md

@@ -5,7 +5,7 @@
 
 # 2.63.x
 * Bump xamlTools to 17.14.35716.216 (PR: [#7932](https://github.com/dotnet/vscode-csharp/pull/7932))
-* Update Roslyn to 4.14.0-1.25065.8 (PR: [#7927](https://github.com/dotnet/vscode-csharp/pull/7927))
+* Update Roslyn to 4.14.0-1.25072.1 (PR: [#7935](https://github.com/dotnet/vscode-csharp/pull/7935))
   * Remove explicit recursion in the json detection analyzer (#76764) (PR: [#76764](https://github.com/dotnet/roslyn/pull/76764))
   * Consider silly cyclic assignment in scoped variance (#76261) (PR: [#76261](https://github.com/dotnet/roslyn/pull/76261))
   * Fix ordering of 'params' vs 'scoped' in metadata as source (#76745) (PR: [#76745](https://github.com/dotnet/roslyn/pull/76745))
@@ -20,6 +20,32 @@
   * Adding checks for mutable structs. (#76711) (PR: [#76711](https://github.com/dotnet/roslyn/pull/76711))
   * Stash and restore original culture in CultureNormalizer (#76713) (PR: [#76713](https://github.com/dotnet/roslyn/pull/76713))
   * Add option for choosing stdio as the LSP communication channel (#76437) (PR: [#76437](https://github.com/dotnet/roslyn/pull/76437))
+  * Support organizing imports as part of LSP document formatting  (PR: [#76806](https://github.com/dotnet/roslyn/pull/76806))
+  * Improve collapsing of members followed by pp directives (PR: [#76837](https://github.com/dotnet/roslyn/pull/76837))
+  * Load razor assembly directly: (PR: [#76808](https://github.com/dotnet/roslyn/pull/76808))
+  * Special case inlining a collection expr into a spreaded element (PR: [#76823](https://github.com/dotnet/roslyn/pull/76823))
+  * Do not offer to simplify interpolations when using formattable strings (PR: [#76830](https://github.com/dotnet/roslyn/pull/76830))
+  * Add support for outlining switch expressions (PR: [#76827](https://github.com/dotnet/roslyn/pull/76827))
+  * Do no offer to make fields readonly if they are a struct and are written to through an indexer (PR: [#76825](https://github.com/dotnet/roslyn/pull/76825))
+  * Do not offer to inline a decl into a switch arm when it is referenced outside of it. (PR: [#76822](https://github.com/dotnet/roslyn/pull/76822))
+  * Fix gen-method generating into top level. (PR: [#76821](https://github.com/dotnet/roslyn/pull/76821))
+  * Fix 'invert if' refactor to properly enclose #region/#endregion blocks (PR: [#74145](https://github.com/dotnet/roslyn/pull/74145))
+  * Do not offer use-conditional when it would cause name collisions (PR: [#76807](https://github.com/dotnet/roslyn/pull/76807))
+  * Remove unnecessary cast in one conditional expression branch, based on the other branch and outer conversion. (PR: [#76798](https://github.com/dotnet/roslyn/pull/76798))
+  * Convert a return value to return type even if it has errors (PR: [#76699](https://github.com/dotnet/roslyn/pull/76699))
+  * Add EmbeddedAttribute API for source generators (PR: [#76583](https://github.com/dotnet/roslyn/pull/76583))
+  * Fix formatting when doing a 'fix all' with 'use auto prop' (PR: [#76791](https://github.com/dotnet/roslyn/pull/76791))
+  * Initialize naming style preferences when language is added to workspace (PR: [#76795](https://github.com/dotnet/roslyn/pull/76795))
+  * Support target type completion tags in object creation contexts (PR: [#76786](https://github.com/dotnet/roslyn/pull/76786))
+  * Fix 'use conditional expression' where it was causing a null-ref warning. (PR: [#76792](https://github.com/dotnet/roslyn/pull/76792))
+  * Keep comments on an 'else' keyword when converting to conditional expressions. (PR: [#76789](https://github.com/dotnet/roslyn/pull/76789))
+  * VB: Don't capture conditional access receiver into a temp local during lowering. (PR: [#76712](https://github.com/dotnet/roslyn/pull/76712))
+  * Update regex parsing to latest .Net core parsing (and diagnostic messages). (PR: [#76269](https://github.com/dotnet/roslyn/pull/76269))
+  * Forbid pointer types as instance fields in records (PR: [#76588](https://github.com/dotnet/roslyn/pull/76588))
+  * Field-backed properties: report diagnostic for variable named field declared in accessor (PR: [#76671](https://github.com/dotnet/roslyn/pull/76671))
+  * Update 'use nameof instead of typeof' to support generic types (PR: [#76780](https://github.com/dotnet/roslyn/pull/76780))
+  * Add feature to convert from an explicitly typed lambda to an implicitly typed one. (PR: [#76770](https://github.com/dotnet/roslyn/pull/76770))
+  * Support modifiers with simple lambda parameters. (PR: [#75400](https://github.com/dotnet/roslyn/pull/75400))
 * Update Razor to 9.0.0-preview.25064.4 (PR: [#7927](https://github.com/dotnet/vscode-csharp/pull/7927))
   * Wire up the UseRoslynTokenizer feature properly (#11386) (PR: [#11386](https://github.com/dotnet/razor/pull/11386))
   * New Razor document formatting engine (#11364) (PR: [#11364](https://github.com/dotnet/razor/pull/11364))

package.json

@@ -37,7 +37,7 @@
     }
   },
   "defaults": {
-    "roslyn": "4.14.0-1.25065.8",
+    "roslyn": "4.14.0-1.25072.1",
     "omniSharp": "1.39.12",
     "razor": "9.0.0-preview.25064.4",
     "razorOmnisharp": "7.0.0-preview.23363.1",
@@ -739,6 +739,11 @@
             "default": true,
             "description": "%configuration.dotnet.autoInsert.enableAutoInsert%"
           },
+          "dotnet.formatting.organizeImportsOnFormat": {
+            "type": "boolean",
+            "default": false,
+            "description": "%configuration.dotnet.formatting.organizeImportsOnFormat%"
+          },
           "dotnet.typeMembers.memberInsertionLocation": {
             "type": "string",
             "enum": [
@@ -1696,11 +1701,6 @@
             "default": false,
             "description": "%configuration.omnisharp.enableLspDriver%"
           },
-          "omnisharp.organizeImportsOnFormat": {
-            "type": "boolean",
-            "default": false,
-            "description": "%configuration.omnisharp.organizeImportsOnFormat%"
-          },
           "omnisharp.enableAsyncCompletion": {
             "type": "boolean",
             "default": false,

package.nls.json

@@ -24,6 +24,7 @@
   "command.dotnet.test.debugTestsInContext": "Debug Tests in Context",
   "command.dotnet.restartServer": "Restart Language Server",
   "configuration.dotnet.autoInsert.enableAutoInsert": "Enable automatic insertion of documentation comments.",
+  "configuration.dotnet.formatting.organizeImportsOnFormat": "Specifies whether 'using' directives should be grouped and sorted during document formatting. (Previously `omnisharp.organizeImportsOnFormat`)",
   "configuration.dotnet.defaultSolution.description": "The path of the default solution to be opened in the workspace, or set to 'disable' to skip it. (Previously `omnisharp.defaultLaunchSolution`)",
   "configuration.dotnet.server.path": "Specifies the absolute path to the server (LSP or O#) executable. When left empty the version pinned to the C# Extension is used. (Previously `omnisharp.path`)",
   "configuration.dotnet.server.componentPaths": "Allows overriding the folder path for built in components of the language server (for example, override the .roslynDevKit path in the extension directory to use locally built components)",
@@ -116,7 +117,6 @@
   "configuration.omnisharp.enableEditorConfigSupport": "Enables support for reading code style, naming convention and analyzer settings from .editorconfig.",
   "configuration.omnisharp.enableDecompilationSupport": "Enables support for decompiling external references instead of viewing metadata.",
   "configuration.omnisharp.enableLspDriver": "Enables support for the experimental language protocol based engine (requires reload to setup bindings correctly)",
-  "configuration.omnisharp.organizeImportsOnFormat": "Specifies whether 'using' directives should be grouped and sorted during document formatting.",
   "configuration.omnisharp.enableAsyncCompletion": "(EXPERIMENTAL) Enables support for resolving completion edits asynchronously. This can speed up time to show the completion list, particularly override and partial method completion lists, at the cost of slight delays after inserting a completion item. Most completion items will have no noticeable impact with this feature, but typing immediately after inserting an override or partial method completion, before the insert is completed, can have unpredictable results.",
   "configuration.omnisharp.dotNetCliPaths": "Paths to a local download of the .NET CLI to use for running any user code.",
   "configuration.omnisharp.razor.plugin.path": "Overrides the path to the Razor plugin dll.",

src/omnisharp/server.ts

@@ -458,7 +458,7 @@ export class OmniSharpServer {
             args.push('FormattingOptions:EnableEditorConfigSupport=true');
         }
 
-        if (omnisharpOptions.organizeImportsOnFormat === true) {
+        if (commonOptions.organizeImportsOnFormat === true) {
             args.push('FormattingOptions:OrganizeImports=true');
         }
 

src/shared/migrateOptions.ts

@@ -100,6 +100,10 @@ export const migrateOptions = [
         oldName: 'dotnet.implementType.propertyGenerationBehavior',
         newName: 'dotnet.typeMembers.propertyGenerationBehavior',
     },
+    {
+        oldName: 'omnisharp.organizeImportsOnFormat',
+        newName: 'dotnet.formatting.organizeImportsOnFormat',
+    },
 ];
 
 export async function MigrateOptions(vscode: vscode): Promise<void> {

src/shared/options.ts

@@ -17,6 +17,7 @@ export interface CommonOptions {
     readonly defaultSolution: string;
     readonly unitTestDebuggingOptions: object;
     readonly runSettingsPath: string;
+    readonly organizeImportsOnFormat: boolean;
 }
 
 export interface OmnisharpServerOptions {
@@ -34,7 +35,6 @@ export interface OmnisharpServerOptions {
     readonly enableImportCompletion: boolean;
     readonly enableAsyncCompletion: boolean;
     readonly analyzeOpenDocumentsOnly: boolean;
-    readonly organizeImportsOnFormat: boolean;
     readonly disableMSBuildDiagnosticWarning: boolean;
     readonly showOmnisharpLogOnError: boolean;
     readonly minFindSymbolsFilterLength: number;
@@ -157,6 +157,9 @@ class CommonOptionsImpl implements CommonOptions {
     public get runSettingsPath() {
         return readOption<string>('dotnet.unitTests.runSettingsPath', '', 'omnisharp.testRunSettings');
     }
+    public get organizeImportsOnFormat() {
+        return readOption<boolean>('dotnet.formatting.organizeImportsOnFormat', false);
+    }
 }
 
 class OmnisharpOptionsImpl implements OmnisharpServerOptions {
@@ -227,9 +230,6 @@ class OmnisharpOptionsImpl implements OmnisharpServerOptions {
         const analyzeOpenDocumentsOnlyNewOption = diagnosticAnalysisScope == 'openFiles';
         return analyzeOpenDocumentsOnlyLegacyOption || analyzeOpenDocumentsOnlyNewOption;
     }
-    public get organizeImportsOnFormat() {
-        return readOption<boolean>('omnisharp.organizeImportsOnFormat', false);
-    }
     public get disableMSBuildDiagnosticWarning() {
         return readOption<boolean>('omnisharp.disableMSBuildDiagnosticWarning', false);
     }
@@ -483,7 +483,6 @@ export const OmnisharpOptionsThatTriggerReload: ReadonlyArray<keyof OmnisharpSer
     'enableEditorConfigSupport',
     'enableDecompilationSupport',
     'enableImportCompletion',
-    'organizeImportsOnFormat',
     'enableAsyncCompletion',
     'useModernNet',
     'enableLspDriver',

test/lsptoolshost/integrationTests/formatting.integration.test.ts

@@ -12,19 +12,19 @@ import {
     expectText,
     openFileInWorkspaceAsync,
 } from './integrationHelpers';
-import { describe, beforeAll, beforeEach, afterAll, test, afterEach } from '@jest/globals';
+import { describe, beforeEach, afterAll, test, afterEach } from '@jest/globals';
 import { formatDocumentAsync, formatOnTypeAsync, formatRangeAsync } from './formattingTestHelpers';
 
 describe(`Formatting Tests`, () => {
-    beforeAll(async () => {
+    beforeEach(async () => {
+        await setOrganizeImportsOnFormat(undefined);
         await activateCSharpExtension();
-    });
 
-    beforeEach(async () => {
         await openFileInWorkspaceAsync(path.join('src', 'app', 'Formatting.cs'));
     });
 
     afterAll(async () => {
+        await setOrganizeImportsOnFormat(undefined);
         await testAssetWorkspace.cleanupWorkspace();
     });
 
@@ -36,6 +36,8 @@ describe(`Formatting Tests`, () => {
         await formatDocumentAsync();
 
         const expectedText = [
+            'using Options;',
+            'using System;',
             'namespace Formatting;',
             'class DocumentFormatting',
             '{',
@@ -54,9 +56,11 @@ describe(`Formatting Tests`, () => {
     });
 
     test('Document range formatting formats only the range', async () => {
-        await formatRangeAsync(new vscode.Range(3, 0, 5, 0));
+        await formatRangeAsync(new vscode.Range(5, 0, 7, 0));
 
         const expectedText = [
+            'using Options;',
+            'using System;',
             'namespace Formatting;',
             'class DocumentFormatting',
             '{',
@@ -75,9 +79,11 @@ describe(`Formatting Tests`, () => {
 
     test('Document on type formatting formats the typed location', async () => {
         // The server expects the position to be the position after the inserted character `;`
-        await formatOnTypeAsync(new vscode.Position(7, 37), ';');
+        await formatOnTypeAsync(new vscode.Position(9, 37), ';');
 
         const expectedText = [
+            'using Options;',
+            'using System;',
             'namespace Formatting;',
             'class DocumentFormatting',
             '{',
@@ -91,4 +97,60 @@ describe(`Formatting Tests`, () => {
         ];
         await expectText(vscode.window.activeTextEditor!.document, expectedText);
     });
+
+    test('Document formatting can organize imports', async () => {
+        await setOrganizeImportsOnFormat(true);
+        await activateCSharpExtension();
+
+        await formatDocumentAsync();
+
+        const expectedText = [
+            'using System;',
+            'using Options;',
+            'namespace Formatting;',
+            'class DocumentFormatting',
+            '{',
+            '    public int Property1',
+            '    {',
+            '        get; set;',
+            '    }',
+            '',
+            '    public void Method1()',
+            '    {',
+            '        System.Console.Write("");',
+            '    }',
+            '}',
+        ];
+        await expectText(vscode.window.activeTextEditor!.document, expectedText);
+    });
+
+    test('Document range formatting does not organize imports', async () => {
+        await setOrganizeImportsOnFormat(true);
+        await activateCSharpExtension();
+
+        await formatRangeAsync(new vscode.Range(0, 0, 7, 0));
+
+        const expectedText = [
+            'using Options;',
+            'using System;',
+            'namespace Formatting;',
+            'class DocumentFormatting',
+            '{',
+            '    public int Property1',
+            '    {',
+            '        get; set;',
+            '    }',
+            '',
+            '    public void Method1() {',
+            '            System.Console.Write("");',
+            '    }',
+            '}',
+        ];
+        await expectText(vscode.window.activeTextEditor!.document, expectedText);
+    });
+
+    async function setOrganizeImportsOnFormat(value: boolean | undefined) {
+        const dotnetConfig = vscode.workspace.getConfiguration('dotnet');
+        await dotnetConfig.update('formatting.organizeImportsOnFormat', value, true);
+    }
 });

test/lsptoolshost/integrationTests/testAssets/slnWithCsproj/src/app/Formatting.cs

@@ -1,3 +1,5 @@
+using Options;
+using System;
 namespace Formatting;
 class DocumentFormatting
 {

test/lsptoolshost/unitTests/configurationMiddleware.test.ts

@@ -267,6 +267,12 @@ const testData = [
         declareInPackageJson: true,
         section: editorBehaviorSection,
     },
+    {
+        serverOption: 'csharp|formatting.dotnet_organize_imports_on_format',
+        vsCodeConfiguration: 'dotnet.formatting.organizeImportsOnFormat',
+        declareInPackageJson: true,
+        section: editorBehaviorSection,
+    },
 ];
 
 describe('Server option name to vscode configuration name test', () => {