From 323d54f1b78622ba1251901e6676dc77582f6e0d Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Fri, 3 Dec 2021 13:39:13 +0100 Subject: [PATCH 1/4] (fix) Adhere to code formatting settings when organizing imports Add semicolon or not depending on Prettier config #1260 --- packages/language-server/src/ls-config.ts | 26 +++++++++++- .../src/plugins/svelte/SveltePlugin.ts | 42 +++++++------------ .../plugins/typescript/TypeScriptPlugin.ts | 3 +- .../features/CodeActionsProvider.ts | 14 ++++++- packages/language-server/src/utils.ts | 13 +++++- .../features/CodeActionsProvider.test.ts | 30 +++++++------ .../typescript/features/preferences.test.ts | 13 +++--- 7 files changed, 89 insertions(+), 52 deletions(-) diff --git a/packages/language-server/src/ls-config.ts b/packages/language-server/src/ls-config.ts index 1085b8348..e819297b9 100644 --- a/packages/language-server/src/ls-config.ts +++ b/packages/language-server/src/ls-config.ts @@ -1,6 +1,7 @@ -import { merge, get } from 'lodash'; +import { get, merge } from 'lodash'; import { UserPreferences } from 'typescript'; import { VSCodeEmmetConfig } from 'vscode-emmet-helper'; +import { returnObjectIfHasKeys } from './utils'; /** * Default config for the language server. @@ -340,6 +341,29 @@ export class LSConfigManager { return this.prettierConfig; } + /** + * Returns a merged Prettier config following these rules: + * - If `prettierFromFileConfig` exists, that one is returned + * - Else the Svelte extension's Prettier config is used as a starting point, + * and overridden by a possible Prettier config from the Prettier extension, + * or, if that doesn't exist, a possible fallback override. + */ + getMergedPrettierConfig( + prettierFromFileConfig: any, + overridesWhenNoPrettierConfig: any = {} + ): any { + return ( + returnObjectIfHasKeys(prettierFromFileConfig) || + merge( + {}, // merge into empty obj to not manipulate own config + this.get('svelte.format.config'), + returnObjectIfHasKeys(this.getPrettierConfig()) || + overridesWhenNoPrettierConfig || + {} + ) + ); + } + updateTsJsUserPreferences(config: Record): void { (['typescript', 'javascript'] as const).forEach((lang) => { if (config[lang]) { diff --git a/packages/language-server/src/plugins/svelte/SveltePlugin.ts b/packages/language-server/src/plugins/svelte/SveltePlugin.ts index 4233bcfb1..8869234f3 100644 --- a/packages/language-server/src/plugins/svelte/SveltePlugin.ts +++ b/packages/language-server/src/plugins/svelte/SveltePlugin.ts @@ -1,21 +1,22 @@ import { + CancellationToken, CodeAction, CodeActionContext, + CompletionContext, CompletionList, Diagnostic, FormattingOptions, Hover, Position, Range, - TextEdit, - WorkspaceEdit, SelectionRange, - CancellationToken, - CompletionContext + TextEdit, + WorkspaceEdit } from 'vscode-languageserver'; +import { getPackageInfo, importPrettier } from '../../importPackage'; import { Document } from '../../lib/documents'; +import { Logger } from '../../logger'; import { LSConfigManager, LSSvelteConfig } from '../../ls-config'; -import { getPackageInfo, importPrettier } from '../../importPackage'; import { CodeActionsProvider, CompletionsProvider, @@ -28,10 +29,8 @@ import { executeCommand, getCodeActions } from './features/getCodeActions'; import { getCompletions } from './features/getCompletions'; import { getDiagnostics } from './features/getDiagnostics'; import { getHoverInfo } from './features/getHoverInfo'; -import { SvelteCompileResult, SvelteDocument } from './SvelteDocument'; -import { Logger } from '../../logger'; import { getSelectionRange } from './features/getSelectionRanges'; -import { merge } from 'lodash'; +import { SvelteCompileResult, SvelteDocument } from './SvelteDocument'; export class SveltePlugin implements @@ -75,19 +74,14 @@ export class SveltePlugin const filePath = document.getFilePath()!; const prettier = importPrettier(filePath); // Try resolving the config through prettier and fall back to possible editor config - const config = - returnObjectIfHasKeys(await prettier.resolveConfig(filePath, { editorconfig: true })) || - merge( - {}, // merge into empty obj to not manipulate own config - this.configManager.get('svelte.format.config'), - returnObjectIfHasKeys(this.configManager.getPrettierConfig()) || - // Be defensive here because IDEs other than VSCode might not have these settings - (options && { - tabWidth: options.tabSize, - useTabs: !options.insertSpaces - }) || - {} - ); + const config = this.configManager.getMergedPrettierConfig( + await prettier.resolveConfig(filePath, { editorconfig: true }), + // Be defensive here because IDEs other than VSCode might not have these settings + options && { + tabWidth: options.tabSize, + useTabs: !options.insertSpaces + } + ); // If user has prettier-plugin-svelte 1.x, then remove `options` from the sort // order or else it will throw a config error (`options` was not present back then). if ( @@ -137,12 +131,6 @@ export class SveltePlugin .languages.some((l) => l.name === 'svelte'); return hasPluginLoadedAlready ? [] : [require.resolve('prettier-plugin-svelte')]; } - - function returnObjectIfHasKeys(obj: T | undefined): T | undefined { - if (Object.keys(obj || {}).length > 0) { - return obj; - } - } } async getCompletions( diff --git a/packages/language-server/src/plugins/typescript/TypeScriptPlugin.ts b/packages/language-server/src/plugins/typescript/TypeScriptPlugin.ts index 36edc19a5..898334ba4 100644 --- a/packages/language-server/src/plugins/typescript/TypeScriptPlugin.ts +++ b/packages/language-server/src/plugins/typescript/TypeScriptPlugin.ts @@ -100,7 +100,8 @@ export class TypeScriptPlugin this.completionProvider = new CompletionsProviderImpl(this.lsAndTsDocResolver); this.codeActionsProvider = new CodeActionsProviderImpl( this.lsAndTsDocResolver, - this.completionProvider + this.completionProvider, + configManager ); this.updateImportsProvider = new UpdateImportsProviderImpl(this.lsAndTsDocResolver); this.diagnosticsProvider = new DiagnosticsProviderImpl(this.lsAndTsDocResolver); diff --git a/packages/language-server/src/plugins/typescript/features/CodeActionsProvider.ts b/packages/language-server/src/plugins/typescript/features/CodeActionsProvider.ts index dfae0423a..77ba68bd2 100644 --- a/packages/language-server/src/plugins/typescript/features/CodeActionsProvider.ts +++ b/packages/language-server/src/plugins/typescript/features/CodeActionsProvider.ts @@ -10,6 +10,7 @@ import { TextEdit, WorkspaceEdit } from 'vscode-languageserver'; +import { importPrettier } from '../../../importPackage'; import { Document, getLineAtPosition, @@ -17,6 +18,7 @@ import { isRangeInTag, mapRangeToOriginal } from '../../../lib/documents'; +import { LSConfigManager } from '../../../ls-config'; import { flatten, getIndent, isNotNullOrUndefined, modifyLines, pathToUrl } from '../../../utils'; import { CodeActionsProvider } from '../../interfaces'; import { SnapshotFragment, SvelteSnapshotFragment } from '../DocumentSnapshot'; @@ -35,7 +37,8 @@ interface RefactorArgs { export class CodeActionsProviderImpl implements CodeActionsProvider { constructor( private readonly lsAndTsDocResolver: LSAndTSDocResolver, - private readonly completionProvider: CompletionsProviderImpl + private readonly completionProvider: CompletionsProviderImpl, + private readonly configManager: LSConfigManager ) {} async getCodeActions( @@ -77,12 +80,19 @@ export class CodeActionsProviderImpl implements CodeActionsProvider { return []; } + const prettierConfig = this.configManager.getMergedPrettierConfig( + await importPrettier(document.getFilePath()!).resolveConfig(document.getFilePath()!, { + editorconfig: true + }) + ); const changes = lang.organizeImports( { fileName: tsDoc.filePath, type: 'file' }, - {}, + { + semicolons: prettierConfig.semi ?? true + }, userPreferences ); diff --git a/packages/language-server/src/utils.ts b/packages/language-server/src/utils.ts index 3f8f98e65..ef159efb3 100644 --- a/packages/language-server/src/utils.ts +++ b/packages/language-server/src/utils.ts @@ -1,6 +1,6 @@ -import { URI } from 'vscode-uri'; -import { Position, Range } from 'vscode-languageserver'; import { Node } from 'vscode-html-languageservice'; +import { Position, Range } from 'vscode-languageserver'; +import { URI } from 'vscode-uri'; export function clamp(num: number, min: number, max: number): number { return Math.max(min, Math.min(max, num)); @@ -240,3 +240,12 @@ export function getIndent(text: string) { export function possiblyComponent(node: Node): boolean { return !!node.tag?.[0].match(/[A-Z]/); } + +/** + * If the object if it has entries, else undefined + */ +export function returnObjectIfHasKeys(obj: T | undefined): T | undefined { + if (Object.keys(obj || {}).length > 0) { + return obj; + } +} diff --git a/packages/language-server/test/plugins/typescript/features/CodeActionsProvider.test.ts b/packages/language-server/test/plugins/typescript/features/CodeActionsProvider.test.ts index 0cc6e543f..16460fb64 100644 --- a/packages/language-server/test/plugins/typescript/features/CodeActionsProvider.test.ts +++ b/packages/language-server/test/plugins/typescript/features/CodeActionsProvider.test.ts @@ -1,20 +1,20 @@ -import { DocumentManager, Document } from '../../../../src/lib/documents'; -import { LSAndTSDocResolver } from '../../../../src/plugins/typescript/LSAndTSDocResolver'; -import { CodeActionsProviderImpl } from '../../../../src/plugins/typescript/features/CodeActionsProvider'; -import { pathToUrl } from '../../../../src/utils'; -import ts from 'typescript'; -import * as path from 'path'; import * as assert from 'assert'; +import * as path from 'path'; +import ts from 'typescript'; import { - Range, - Position, - CodeActionKind, - TextDocumentEdit, + CancellationTokenSource, CodeAction, - CancellationTokenSource + CodeActionKind, + Position, + Range, + TextDocumentEdit } from 'vscode-languageserver'; -import { CompletionsProviderImpl } from '../../../../src/plugins/typescript/features/CompletionProvider'; +import { Document, DocumentManager } from '../../../../src/lib/documents'; import { LSConfigManager } from '../../../../src/ls-config'; +import { CodeActionsProviderImpl } from '../../../../src/plugins/typescript/features/CodeActionsProvider'; +import { CompletionsProviderImpl } from '../../../../src/plugins/typescript/features/CompletionProvider'; +import { LSAndTSDocResolver } from '../../../../src/plugins/typescript/LSAndTSDocResolver'; +import { pathToUrl } from '../../../../src/utils'; const testDir = path.join(__dirname, '..'); @@ -41,7 +41,11 @@ describe('CodeActionsProvider', () => { new LSConfigManager() ); const completionProvider = new CompletionsProviderImpl(lsAndTsDocResolver); - const provider = new CodeActionsProviderImpl(lsAndTsDocResolver, completionProvider); + const provider = new CodeActionsProviderImpl( + lsAndTsDocResolver, + completionProvider, + new LSConfigManager() + ); const filePath = getFullPath(filename); const document = docManager.openDocument({ uri: pathToUrl(filePath), diff --git a/packages/language-server/test/plugins/typescript/features/preferences.test.ts b/packages/language-server/test/plugins/typescript/features/preferences.test.ts index d6ab6c6c2..79f481fe3 100644 --- a/packages/language-server/test/plugins/typescript/features/preferences.test.ts +++ b/packages/language-server/test/plugins/typescript/features/preferences.test.ts @@ -1,7 +1,6 @@ -import ts from 'typescript'; import assert from 'assert'; import { join } from 'path'; - +import ts from 'typescript'; import { CodeActionContext, Diagnostic, @@ -11,11 +10,11 @@ import { TextDocumentEdit } from 'vscode-languageserver'; import { Document, DocumentManager } from '../../../../src/lib/documents'; +import { LSConfigManager, TSUserConfig } from '../../../../src/ls-config'; +import { CodeActionsProviderImpl } from '../../../../src/plugins/typescript/features/CodeActionsProvider'; import { CompletionsProviderImpl } from '../../../../src/plugins/typescript/features/CompletionProvider'; import { LSAndTSDocResolver } from '../../../../src/plugins/typescript/LSAndTSDocResolver'; import { pathToUrl } from '../../../../src/utils'; -import { CodeActionsProviderImpl } from '../../../../src/plugins/typescript/features/CodeActionsProvider'; -import { LSConfigManager, TSUserConfig } from '../../../../src/ls-config'; const testFilesDir = join(__dirname, '..', 'testfiles', 'preferences'); @@ -88,7 +87,8 @@ describe('ts user preferences', () => { const completionProvider = new CompletionsProviderImpl(lsAndTsDocResolver); const codeActionProvider = new CodeActionsProviderImpl( lsAndTsDocResolver, - completionProvider + completionProvider, + new LSConfigManager() ); const codeAction = await codeActionProvider.getCodeActions(document, range, context); @@ -188,7 +188,8 @@ describe('ts user preferences', () => { const completionProvider = new CompletionsProviderImpl(lsAndTsDocResolver); const codeActionProvider = new CodeActionsProviderImpl( lsAndTsDocResolver, - completionProvider + completionProvider, + new LSConfigManager() ); const codeAction = await codeActionProvider.getCodeActions(document, range, { From c9e64c13c62fb2bc76b48515cfe6c11435f5059c Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Fri, 3 Dec 2021 17:39:14 +0100 Subject: [PATCH 2/4] handle removal of semis trough regex --- .../features/CodeActionsProvider.ts | 23 ++++++++++++++----- 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/packages/language-server/src/plugins/typescript/features/CodeActionsProvider.ts b/packages/language-server/src/plugins/typescript/features/CodeActionsProvider.ts index 77ba68bd2..2d93b4b47 100644 --- a/packages/language-server/src/plugins/typescript/features/CodeActionsProvider.ts +++ b/packages/language-server/src/plugins/typescript/features/CodeActionsProvider.ts @@ -80,18 +80,22 @@ export class CodeActionsProviderImpl implements CodeActionsProvider { return []; } - const prettierConfig = this.configManager.getMergedPrettierConfig( - await importPrettier(document.getFilePath()!).resolveConfig(document.getFilePath()!, { - editorconfig: true - }) - ); + const useSemicolons = + this.configManager.getMergedPrettierConfig( + await importPrettier(document.getFilePath()!).resolveConfig( + document.getFilePath()!, + { + editorconfig: true + } + ) + ).semi ?? true; const changes = lang.organizeImports( { fileName: tsDoc.filePath, type: 'file' }, { - semicolons: prettierConfig.semi ?? true + semicolons: useSemicolons }, userPreferences ); @@ -102,6 +106,13 @@ export class CodeActionsProviderImpl implements CodeActionsProvider { return TextDocumentEdit.create( OptionalVersionedTextDocumentIdentifier.create(document.url, null), change.textChanges.map((edit) => { + if (!useSemicolons) { + // For some reason the TS language service ignores the formatOptions + // we set above, so remove possible semicolons here. + edit.newText = edit.newText.replace(/(;(\n))|(;(\r\n))/g, (match) => + match.substring(1) + ); + } const range = this.checkRemoveImportCodeActionRange( edit, fragment, From 6ae1b7b90582caf3b42a10d535b68dfcceb55fc7 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Fri, 3 Dec 2021 17:39:43 +0100 Subject: [PATCH 3/4] simplify --- .../src/plugins/typescript/features/CodeActionsProvider.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/language-server/src/plugins/typescript/features/CodeActionsProvider.ts b/packages/language-server/src/plugins/typescript/features/CodeActionsProvider.ts index 2d93b4b47..056ccfda7 100644 --- a/packages/language-server/src/plugins/typescript/features/CodeActionsProvider.ts +++ b/packages/language-server/src/plugins/typescript/features/CodeActionsProvider.ts @@ -109,7 +109,7 @@ export class CodeActionsProviderImpl implements CodeActionsProvider { if (!useSemicolons) { // For some reason the TS language service ignores the formatOptions // we set above, so remove possible semicolons here. - edit.newText = edit.newText.replace(/(;(\n))|(;(\r\n))/g, (match) => + edit.newText = edit.newText.replace(/(;\n)|(;\r\n)/g, (match) => match.substring(1) ); } From 2621e33b8e403fe9d3b7e7c52152dfb8a45f791b Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Wed, 8 Dec 2021 09:08:31 +0100 Subject: [PATCH 4/4] fix option setting --- .../plugins/typescript/features/CodeActionsProvider.ts | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/packages/language-server/src/plugins/typescript/features/CodeActionsProvider.ts b/packages/language-server/src/plugins/typescript/features/CodeActionsProvider.ts index 056ccfda7..135005e94 100644 --- a/packages/language-server/src/plugins/typescript/features/CodeActionsProvider.ts +++ b/packages/language-server/src/plugins/typescript/features/CodeActionsProvider.ts @@ -96,6 +96,8 @@ export class CodeActionsProviderImpl implements CodeActionsProvider { }, { semicolons: useSemicolons + ? ts.SemicolonPreference.Insert + : ts.SemicolonPreference.Remove }, userPreferences ); @@ -106,13 +108,6 @@ export class CodeActionsProviderImpl implements CodeActionsProvider { return TextDocumentEdit.create( OptionalVersionedTextDocumentIdentifier.create(document.url, null), change.textChanges.map((edit) => { - if (!useSemicolons) { - // For some reason the TS language service ignores the formatOptions - // we set above, so remove possible semicolons here. - edit.newText = edit.newText.replace(/(;\n)|(;\r\n)/g, (match) => - match.substring(1) - ); - } const range = this.checkRemoveImportCodeActionRange( edit, fragment,