From 5b29b141fa0dc12f16592803ccd926526a45f409 Mon Sep 17 00:00:00 2001 From: TypeScript Bot Date: Tue, 19 May 2020 14:40:23 -0700 Subject: [PATCH] Cherry-pick PR #38579 into release-3.9 (#38666) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Component commits: 0a696c902d Ensure formatter can always get a newline character ab09d67b49 Make FormatContext.host optional since it’s not necessary if format options are all applied 90923e2050 Make FormattingHost required again Co-authored-by: Andrew Branch --- 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 | 5 +++++ src/services/utilities.ts | 6 ++--- .../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, 61 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 c0d386291a5a6..b88ed9559577d 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1480,7 +1480,7 @@ namespace ts { position, { name, source }, host, - (formattingOptions && formatting.getFormatContext(formattingOptions))!, // TODO: GH#18217 + (formattingOptions && formatting.getFormatContext(formattingOptions, host))!, // TODO: GH#18217 preferences, cancellationToken, ); @@ -1818,16 +1818,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) { @@ -1849,7 +1849,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(); @@ -1861,7 +1861,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 }); } @@ -1870,13 +1870,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; @@ -2119,7 +2119,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 24d04fb438e2f..a8e4b43e3f3b4 100644 --- a/src/services/types.ts +++ b/src/services/types.ts @@ -201,6 +201,11 @@ namespace ts { has(dependencyName: string, inGroups?: PackageJsonDependencyGroup): boolean; } + /** @internal */ + export interface FormattingHost { + getNewLine?(): string; + } + // // Public interface of the host of a language service instance. // diff --git a/src/services/utilities.ts b/src/services/utilities.ts index 58f53198f7905..2afad719556f8 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -2085,9 +2085,9 @@ namespace ts { /** * The default is CRLF. */ - export function getNewLineOrDefaultFromHost(host: LanguageServiceHost | LanguageServiceShimHost, formatSettings?: FormatCodeSettings) { - return (formatSettings && formatSettings.newLineCharacter) || - (host.getNewLine && host.getNewLine()) || + export function getNewLineOrDefaultFromHost(host: FormattingHost, formatSettings?: FormatCodeSettings) { + return formatSettings?.newLineCharacter || + 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; +}`);