From 0a696c902dc0f9534c2eb4ae723b46fe870cc2dc Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Thu, 14 May 2020 12:17:51 -0700 Subject: [PATCH] Ensure formatter can always get a newline character --- src/harness/fourslashImpl.ts | 6 +++++ src/harness/fourslashInterfaceImpl.ts | 4 ++++ src/services/formatting/formatting.ts | 7 +++--- src/services/formatting/rulesMap.ts | 4 ++-- src/services/services.ts | 18 +++++++-------- src/services/types.ts | 7 ++++-- src/services/utilities.ts | 2 +- .../services/convertToAsyncFunction.ts | 2 +- .../unittests/services/extract/helpers.ts | 4 ++-- .../unittests/services/textChanges.ts | 2 +- tests/cases/fourslash/fourslash.ts | 2 ++ .../organizeImportsNoFormatOptions.ts | 22 +++++++++++++++++++ 12 files changed, 59 insertions(+), 21 deletions(-) create mode 100644 tests/cases/fourslash/organizeImportsNoFormatOptions.ts diff --git a/src/harness/fourslashImpl.ts b/src/harness/fourslashImpl.ts index 5436413701de3..dd4135ca40c15 100644 --- a/src/harness/fourslashImpl.ts +++ b/src/harness/fourslashImpl.ts @@ -515,6 +515,12 @@ namespace FourSlash { } } + public verifyOrganizeImports(newContent: string) { + const changes = this.languageService.organizeImports({ fileName: this.activeFile.fileName, type: "file" }, this.formatCodeSettings, ts.emptyOptions); + this.applyChanges(changes); + this.verifyFileContent(this.activeFile.fileName, newContent); + } + private raiseError(message: string): never { throw new Error(this.messageAtLastKnownMarker(message)); } diff --git a/src/harness/fourslashInterfaceImpl.ts b/src/harness/fourslashInterfaceImpl.ts index ca7ac8f58ffd3..f4905c00b84b5 100644 --- a/src/harness/fourslashInterfaceImpl.ts +++ b/src/harness/fourslashInterfaceImpl.ts @@ -560,6 +560,10 @@ namespace FourSlashInterface { public noMoveToNewFile(): void { this.state.noMoveToNewFile(); } + + public organizeImports(newContent: string) { + this.state.verifyOrganizeImports(newContent); + } } export class Edit { diff --git a/src/services/formatting/formatting.ts b/src/services/formatting/formatting.ts index 31aff6b210a5b..951ad5e5bba87 100644 --- a/src/services/formatting/formatting.ts +++ b/src/services/formatting/formatting.ts @@ -3,6 +3,7 @@ namespace ts.formatting { export interface FormatContext { readonly options: FormatCodeSettings; readonly getRules: RulesMap; + readonly host: FormattingHost; } export interface TextRangeWithKind extends TextRange { @@ -394,7 +395,7 @@ namespace ts.formatting { initialIndentation: number, delta: number, formattingScanner: FormattingScanner, - { options, getRules }: FormatContext, + { options, getRules, host }: FormatContext, requestKind: FormattingRequestKind, rangeContainsError: (r: TextRange) => boolean, sourceFile: SourceFileLike): TextChange[] { @@ -1193,7 +1194,7 @@ namespace ts.formatting { previousRange: TextRangeWithKind, previousStartLine: number, currentRange: TextRangeWithKind, - currentStartLine: number, + currentStartLine: number ): LineAction { const onLaterLine = currentStartLine !== previousStartLine; switch (rule.action) { @@ -1221,7 +1222,7 @@ namespace ts.formatting { // edit should not be applied if we have one line feed between elements const lineDelta = currentStartLine - previousStartLine; if (lineDelta !== 1) { - recordReplace(previousRange.end, currentRange.pos - previousRange.end, options.newLineCharacter!); + recordReplace(previousRange.end, currentRange.pos - previousRange.end, getNewLineOrDefaultFromHost(host, options)); return onLaterLine ? LineAction.None : LineAction.LineAdded; } break; diff --git a/src/services/formatting/rulesMap.ts b/src/services/formatting/rulesMap.ts index f466b397d20dd..ccee491040fc6 100644 --- a/src/services/formatting/rulesMap.ts +++ b/src/services/formatting/rulesMap.ts @@ -1,7 +1,7 @@ /* @internal */ namespace ts.formatting { - export function getFormatContext(options: FormatCodeSettings): FormatContext { - return { options, getRules: getRulesMap() }; + export function getFormatContext(options: FormatCodeSettings, host: FormattingHost): FormatContext { + return { options, getRules: getRulesMap(), host }; } let rulesMapCache: RulesMap | undefined; diff --git a/src/services/services.ts b/src/services/services.ts index f50d99e55d1a3..6668ee14f37dc 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1502,7 +1502,7 @@ namespace ts { position, { name, source }, host, - (formattingOptions && formatting.getFormatContext(formattingOptions))!, // TODO: GH#18217 + (formattingOptions && formatting.getFormatContext(formattingOptions, host))!, // TODO: GH#18217 preferences, cancellationToken, ); @@ -1840,16 +1840,16 @@ namespace ts { function getFormattingEditsForRange(fileName: string, start: number, end: number, options: FormatCodeOptions | FormatCodeSettings): TextChange[] { const sourceFile = syntaxTreeCache.getCurrentSourceFile(fileName); - return formatting.formatSelection(start, end, sourceFile, formatting.getFormatContext(toEditorSettings(options))); + return formatting.formatSelection(start, end, sourceFile, formatting.getFormatContext(toEditorSettings(options), host)); } function getFormattingEditsForDocument(fileName: string, options: FormatCodeOptions | FormatCodeSettings): TextChange[] { - return formatting.formatDocument(syntaxTreeCache.getCurrentSourceFile(fileName), formatting.getFormatContext(toEditorSettings(options))); + return formatting.formatDocument(syntaxTreeCache.getCurrentSourceFile(fileName), formatting.getFormatContext(toEditorSettings(options), host)); } function getFormattingEditsAfterKeystroke(fileName: string, position: number, key: string, options: FormatCodeOptions | FormatCodeSettings): TextChange[] { const sourceFile = syntaxTreeCache.getCurrentSourceFile(fileName); - const formatContext = formatting.getFormatContext(toEditorSettings(options)); + const formatContext = formatting.getFormatContext(toEditorSettings(options), host); if (!isInComment(sourceFile, position)) { switch (key) { @@ -1871,7 +1871,7 @@ namespace ts { synchronizeHostData(); const sourceFile = getValidSourceFile(fileName); const span = createTextSpanFromBounds(start, end); - const formatContext = formatting.getFormatContext(formatOptions); + const formatContext = formatting.getFormatContext(formatOptions, host); return flatMap(deduplicate(errorCodes, equateValues, compareValues), errorCode => { cancellationToken.throwIfCancellationRequested(); @@ -1883,7 +1883,7 @@ namespace ts { synchronizeHostData(); Debug.assert(scope.type === "file"); const sourceFile = getValidSourceFile(scope.fileName); - const formatContext = formatting.getFormatContext(formatOptions); + const formatContext = formatting.getFormatContext(formatOptions, host); return codefix.getAllFixes({ fixId, sourceFile, program, host, cancellationToken, formatContext, preferences }); } @@ -1892,13 +1892,13 @@ namespace ts { synchronizeHostData(); Debug.assert(scope.type === "file"); const sourceFile = getValidSourceFile(scope.fileName); - const formatContext = formatting.getFormatContext(formatOptions); + const formatContext = formatting.getFormatContext(formatOptions, host); return OrganizeImports.organizeImports(sourceFile, formatContext, host, program, preferences); } function getEditsForFileRename(oldFilePath: string, newFilePath: string, formatOptions: FormatCodeSettings, preferences: UserPreferences = emptyOptions): readonly FileTextChanges[] { - return ts.getEditsForFileRename(getProgram()!, oldFilePath, newFilePath, host, formatting.getFormatContext(formatOptions), preferences, sourceMapper); + return ts.getEditsForFileRename(getProgram()!, oldFilePath, newFilePath, host, formatting.getFormatContext(formatOptions, host), preferences, sourceMapper); } function applyCodeActionCommand(action: CodeActionCommand, formatSettings?: FormatCodeSettings): Promise; @@ -2141,7 +2141,7 @@ namespace ts { endPosition, program: getProgram()!, host, - formatContext: formatting.getFormatContext(formatOptions!), // TODO: GH#18217 + formatContext: formatting.getFormatContext(formatOptions!, host), // TODO: GH#18217 cancellationToken, preferences, }; diff --git a/src/services/types.ts b/src/services/types.ts index 13c68364664d8..d40b94ae065fd 100644 --- a/src/services/types.ts +++ b/src/services/types.ts @@ -203,12 +203,15 @@ namespace ts { has(dependencyName: string, inGroups?: PackageJsonDependencyGroup): boolean; } + export interface FormattingHost { + getNewLine?(): string; + } + // // Public interface of the host of a language service instance. // - export interface LanguageServiceHost extends GetEffectiveTypeRootsHost { + export interface LanguageServiceHost extends GetEffectiveTypeRootsHost, FormattingHost { getCompilationSettings(): CompilerOptions; - getNewLine?(): string; getProjectVersion?(): string; getScriptFileNames(): string[]; getScriptKind?(fileName: string): ScriptKind; diff --git a/src/services/utilities.ts b/src/services/utilities.ts index 617855c584a92..a5d733dd24ffc 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -2085,7 +2085,7 @@ namespace ts { /** * The default is CRLF. */ - export function getNewLineOrDefaultFromHost(host: LanguageServiceHost | LanguageServiceShimHost, formatSettings?: FormatCodeSettings) { + export function getNewLineOrDefaultFromHost(host: FormattingHost, formatSettings?: FormatCodeSettings) { return (formatSettings && formatSettings.newLineCharacter) || (host.getNewLine && host.getNewLine()) || carriageReturnLineFeed; diff --git a/src/testRunner/unittests/services/convertToAsyncFunction.ts b/src/testRunner/unittests/services/convertToAsyncFunction.ts index 999e1580a98c3..78a2f43f6c3d6 100644 --- a/src/testRunner/unittests/services/convertToAsyncFunction.ts +++ b/src/testRunner/unittests/services/convertToAsyncFunction.ts @@ -306,7 +306,7 @@ interface Array {}` cancellationToken: { throwIfCancellationRequested: noop, isCancellationRequested: returnFalse }, preferences: emptyOptions, host: notImplementedHost, - formatContext: formatting.getFormatContext(testFormatSettings) + formatContext: formatting.getFormatContext(testFormatSettings, notImplementedHost) }; const diagnostics = languageService.getSuggestionDiagnostics(f.path); diff --git a/src/testRunner/unittests/services/extract/helpers.ts b/src/testRunner/unittests/services/extract/helpers.ts index a998bf6f51057..a47875299946d 100644 --- a/src/testRunner/unittests/services/extract/helpers.ts +++ b/src/testRunner/unittests/services/extract/helpers.ts @@ -102,7 +102,7 @@ namespace ts { startPosition: selectionRange.pos, endPosition: selectionRange.end, host: notImplementedHost, - formatContext: formatting.getFormatContext(testFormatSettings), + formatContext: formatting.getFormatContext(testFormatSettings, notImplementedHost), preferences: emptyOptions, }; const rangeToExtract = refactor.extractSymbol.getRangeToExtract(sourceFile, createTextSpanFromRange(selectionRange)); @@ -164,7 +164,7 @@ namespace ts { startPosition: selectionRange.pos, endPosition: selectionRange.end, host: notImplementedHost, - formatContext: formatting.getFormatContext(testFormatSettings), + formatContext: formatting.getFormatContext(testFormatSettings, notImplementedHost), preferences: emptyOptions, }; const rangeToExtract = refactor.extractSymbol.getRangeToExtract(sourceFile, createTextSpanFromRange(selectionRange)); diff --git a/src/testRunner/unittests/services/textChanges.ts b/src/testRunner/unittests/services/textChanges.ts index 2d76b64696d64..5c3a103c23833 100644 --- a/src/testRunner/unittests/services/textChanges.ts +++ b/src/testRunner/unittests/services/textChanges.ts @@ -19,7 +19,7 @@ namespace ts { const newLineCharacter = getNewLineCharacter(printerOptions); function getRuleProvider(placeOpenBraceOnNewLineForFunctions: boolean): formatting.FormatContext { - return formatting.getFormatContext(placeOpenBraceOnNewLineForFunctions ? { ...testFormatSettings, placeOpenBraceOnNewLineForFunctions: true } : testFormatSettings); + return formatting.getFormatContext(placeOpenBraceOnNewLineForFunctions ? { ...testFormatSettings, placeOpenBraceOnNewLineForFunctions: true } : testFormatSettings, notImplementedHost); } // validate that positions that were recovered from the printed text actually match positions that will be created if the same text is parsed. diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index d9b47adc1fb7a..d7d4935118d0f 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -394,6 +394,8 @@ declare namespace FourSlashInterface { noMoveToNewFile(): void; generateTypes(...options: GenerateTypesOptions[]): void; + + organizeImports(newContent: string): void; } class edit { backspace(count?: number): void; diff --git a/tests/cases/fourslash/organizeImportsNoFormatOptions.ts b/tests/cases/fourslash/organizeImportsNoFormatOptions.ts new file mode 100644 index 0000000000000..2462644122238 --- /dev/null +++ b/tests/cases/fourslash/organizeImportsNoFormatOptions.ts @@ -0,0 +1,22 @@ +/// + +// #38548 + +////import { +//// stat, +//// statSync, +////} from "fs"; +////export function fakeFn() { +//// stat; +//// statSync; +////} + +format.setFormatOptions({}); + +// Default newline is carriage return. +// See `getNewLineOrDefaultFromHost()` in services/utilities.ts. +verify.organizeImports( +`import {\r\nstat,\r\nstatSync\r\n} from "fs";\r\nexport function fakeFn() { + stat; + statSync; +}`);