(fix) fall back to FormattingOptions (#554)

dummdidumm · web-flow · commit dd540d6696e9 · 2020-09-22T12:16:16.000+02:00
In case no prettier configs are found #539 BREAKING CHANGE: If user has no prettier config, he now gets the tab size / space/tabs-settings of his IDE, not the default prettier config
diff --git a/packages/language-server/src/plugins/PluginHost.ts b/packages/language-server/src/plugins/PluginHost.ts
@@ -18,6 +18,7 @@ import {
     CompletionItem,
     CompletionContext,
     WorkspaceEdit,
+    FormattingOptions,
 } from 'vscode-languageserver';
 import { LSConfig, LSConfigManager } from '../ls-config';
 import { DocumentManager } from '../lib/documents';
@@ -136,14 +137,21 @@ export class PluginHost implements LSProvider, OnWatchFileChanges {
         return result ?? completionItem;
     }
 
-    async formatDocument(textDocument: TextDocumentIdentifier): Promise<TextEdit[]> {
+    async formatDocument(
+        textDocument: TextDocumentIdentifier,
+        options: FormattingOptions,
+    ): Promise<TextEdit[]> {
         const document = this.getDocument(textDocument.uri);
         if (!document) {
             throw new Error('Cannot call methods on an unopened document');
         }
 
         return flatten(
-            await this.execute<TextEdit[]>('formatDocument', [document], ExecuteMode.Collect),
+            await this.execute<TextEdit[]>(
+                'formatDocument',
+                [document, options],
+                ExecuteMode.Collect,
+            ),
         );
     }
 
diff --git a/packages/language-server/src/plugins/interfaces.ts b/packages/language-server/src/plugins/interfaces.ts
@@ -9,6 +9,7 @@ import {
     CompletionList,
     DefinitionLink,
     Diagnostic,
+    FormattingOptions,
     Hover,
     Position,
     Range,
@@ -51,7 +52,7 @@ export interface CompletionsProvider<T extends TextDocumentIdentifier = any> {
 }
 
 export interface FormattingProvider {
-    formatDocument(document: Document): Resolvable<TextEdit[]>;
+    formatDocument(document: Document, options: FormattingOptions): Resolvable<TextEdit[]>;
 }
 
 export interface TagCompleteProvider {
diff --git a/packages/language-server/src/plugins/svelte/SveltePlugin.ts b/packages/language-server/src/plugins/svelte/SveltePlugin.ts
@@ -3,6 +3,7 @@ import {
     CodeActionContext,
     CompletionList,
     Diagnostic,
+    FormattingOptions,
     Hover,
     Position,
     Range,
@@ -35,11 +36,7 @@ export class SveltePlugin
         CodeActionsProvider {
     private docManager = new Map<Document, SvelteDocument>();
 
-    constructor(
-        private configManager: LSConfigManager,
-        private prettierConfig: any,
-        private editorConfig?: any,
-    ) {}
+    constructor(private configManager: LSConfigManager, private prettierConfig?: any) {}
 
     async getDiagnostics(document: Document): Promise<Diagnostic[]> {
         if (!this.featureEnabled('diagnostics')) {
@@ -62,7 +59,7 @@ export class SveltePlugin
         }
     }
 
-    async formatDocument(document: Document): Promise<TextEdit[]> {
+    async formatDocument(document: Document, options: FormattingOptions): Promise<TextEdit[]> {
         if (!this.featureEnabled('format')) {
             return [];
         }
@@ -74,9 +71,9 @@ export class SveltePlugin
             (await prettier.resolveConfig(filePath, { editorconfig: true })) ||
             this.prettierConfig ||
             // Be defensive here because IDEs other than VSCode might not have these settings
-            (this.editorConfig && this.editorConfig.tabSize && {
-                tabWidth: this.editorConfig.tabSize,
-                useTabs: !this.editorConfig.insertSpaces,
+            (options && {
+                tabWidth: options.tabSize,
+                useTabs: !options.insertSpaces,
             });
         // Take .prettierignore into account
         const fileInfo = await prettier.getFileInfo(filePath, {
diff --git a/packages/language-server/src/server.ts b/packages/language-server/src/server.ts
@@ -86,8 +86,9 @@ export function startServer(options?: LSOptions) {
     let sveltePlugin: SveltePlugin = undefined as any;
 
     connection.onInitialize((evt) => {
-        const workspaceUris = evt.workspaceFolders?.map(folder => folder.uri.toString())
-            ?? [evt.rootUri ?? ''];
+        const workspaceUris = evt.workspaceFolders?.map((folder) => folder.uri.toString()) ?? [
+            evt.rootUri ?? '',
+        ];
         Logger.log('Initialize language server at ', workspaceUris.join(', '));
         if (workspaceUris.length === 0) {
             Logger.error('No workspace path set');
@@ -98,8 +99,7 @@ export function startServer(options?: LSOptions) {
         pluginHost.register(
             (sveltePlugin = new SveltePlugin(
                 configManager,
-                evt.initializationOptions?.prettierConfig || {},
-                evt.initializationOptions?.editorConfig, // deliberatly don't fall back to empty object
+                evt.initializationOptions?.prettierConfig,
             )),
         );
         pluginHost.register(new HTMLPlugin(docManager, configManager));
@@ -206,7 +206,9 @@ export function startServer(options?: LSOptions) {
     connection.onCompletion((evt) =>
         pluginHost.getCompletions(evt.textDocument, evt.position, evt.context),
     );
-    connection.onDocumentFormatting((evt) => pluginHost.formatDocument(evt.textDocument));
+    connection.onDocumentFormatting((evt) =>
+        pluginHost.formatDocument(evt.textDocument, evt.options),
+    );
     connection.onRequest(TagCloseRequest.type, (evt) =>
         pluginHost.doTagComplete(evt.textDocument, evt.position),
     );
diff --git a/packages/language-server/test/plugins/svelte/SveltePlugin.test.ts b/packages/language-server/test/plugins/svelte/SveltePlugin.test.ts
@@ -3,13 +3,15 @@ import { SveltePlugin } from '../../../src/plugins';
 import { DocumentManager, Document } from '../../../src/lib/documents';
 import { Diagnostic, Range, DiagnosticSeverity } from 'vscode-languageserver';
 import { LSConfigManager } from '../../../src/ls-config';
+import * as importPackage from '../../../src/importPackage';
+import sinon from 'sinon';
 
 describe('Svelte Plugin', () => {
-    function setup(content: string) {
+    function setup(content: string, prettierConfig?: any) {
         const document = new Document('file:///hello.svelte', content);
         const docManager = new DocumentManager(() => document);
         const pluginManager = new LSConfigManager();
-        const plugin = new SveltePlugin(pluginManager, {});
+        const plugin = new SveltePlugin(pluginManager, prettierConfig);
         docManager.openDocument(<any>'some doc');
         return { plugin, document };
     }
@@ -43,4 +45,81 @@ describe('Svelte Plugin', () => {
 
         assert.deepStrictEqual(diagnostics, [diagnostic]);
     });
+
+    describe('#formatDocument', () => {
+        function stubPrettier(configExists: boolean) {
+            const formatStub = sinon.stub().returns('formatted');
+
+            sinon.stub(importPackage, 'importPrettier').returns(<any>{
+                resolveConfig: () => Promise.resolve(configExists && { fromConfig: true }),
+                getFileInfo: () => ({ ignored: false }),
+                format: formatStub,
+                getSupportInfo: () => ({ languages: [{ name: 'svelte' }] }),
+            });
+
+            return formatStub;
+        }
+
+        async function testFormat(configExists: boolean, fallbackPrettierConfigExists: boolean) {
+            const { plugin, document } = setup(
+                'unformatted',
+                fallbackPrettierConfigExists ? { fallbackConfig: true } : undefined,
+            );
+            const formatStub = stubPrettier(configExists);
+
+            const formatted = await plugin.formatDocument(document, {
+                insertSpaces: true,
+                tabSize: 4,
+            });
+            assert.deepStrictEqual(formatted, [
+                {
+                    newText: 'formatted',
+                    range: {
+                        end: {
+                            character: 11,
+                            line: 0,
+                        },
+                        start: {
+                            character: 0,
+                            line: 0,
+                        },
+                    },
+                },
+            ]);
+
+            return formatStub;
+        }
+
+        afterEach(() => {
+            sinon.restore();
+        });
+
+        it('should use config for formatting', async () => {
+            const formatStub = await testFormat(true, true);
+            sinon.assert.calledOnceWithExactly(formatStub, 'unformatted', {
+                fromConfig: true,
+                plugins: [],
+                parser: 'svelte',
+            });
+        });
+
+        it('should use prettier fallback config for formatting', async () => {
+            const formatStub = await testFormat(false, true);
+            sinon.assert.calledOnceWithExactly(formatStub, 'unformatted', {
+                fallbackConfig: true,
+                plugins: [],
+                parser: 'svelte',
+            });
+        });
+
+        it('should use FormattingOptions for formatting', async () => {
+            const formatStub = await testFormat(false, false);
+            sinon.assert.calledOnceWithExactly(formatStub, 'unformatted', {
+                tabWidth: 4,
+                useTabs: false,
+                plugins: [],
+                parser: 'svelte',
+            });
+        });
+    });
 });
diff --git a/packages/svelte-vscode/src/extension.ts b/packages/svelte-vscode/src/extension.ts
@@ -94,7 +94,6 @@ export function activate(context: ExtensionContext) {
         initializationOptions: {
             config: workspace.getConfiguration('svelte.plugin'),
             prettierConfig: workspace.getConfiguration('prettier'),
-            editorConfig: workspace.getConfiguration('editor', { languageId: 'svelte' }),
             dontFilterIncompleteCompletions: true, // VSCode filters client side and is smarter at it than us
         },
     };

Original file line number	Diff line number	Diff line change
`@@ -9,6 +9,7 @@ import {`
`9`	`9`	`CompletionList,`
`10`	`10`	`DefinitionLink,`
`11`	`11`	`Diagnostic,`
	`12`	`+ FormattingOptions,`
`12`	`13`	`Hover,`
`13`	`14`	`Position,`
`14`	`15`	`Range,`
`@@ -51,7 +52,7 @@ export interface CompletionsProvider<T extends TextDocumentIdentifier = any> {`
`51`	`52`	`}`
`52`	`53`
`53`	`54`	`export interface FormattingProvider {`
`54`		`- formatDocument(document: Document): Resolvable<TextEdit[]>;`
	`55`	`+ formatDocument(document: Document, options: FormattingOptions): Resolvable<TextEdit[]>;`
`55`	`56`	`}`
`56`	`57`
`57`	`58`	`export interface TagCompleteProvider {`