From 77075df2d7c63d4dcdf3989d1b80bb90a09cd7c4 Mon Sep 17 00:00:00 2001 From: Tim van der Lippe Date: Tue, 19 May 2020 12:11:29 +0100 Subject: [PATCH 1/2] Fix debug command for Node debugging If you would run with `--inspect=true`, the following error would be thrown: [12:08:13] > node --inspect-brk=true TypeScript/node_modules/mocha/bin/_mocha -R scripts/failed-tests -O "reporter=mocha-fivemat-progress-reporter" -g "implementsJSDocReferencesDeclarationEmit" --colors -t 0 built/local/run.js Unable to resolve "true": unknown node or service --- CONTRIBUTING.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d6ecd1d66ae..80f8f4b2fde 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -152,10 +152,10 @@ You can specify which browser to use for debugging. Currently, Chrome and IE are gulp runtests-browser --tests=2dArrays --browser=chrome ``` -You can debug with VS Code or Node instead with `gulp runtests --inspect=true`: +You can debug with VS Code or Node instead with `gulp runtests --inspect`: ```Shell -gulp runtests --tests=2dArrays --inspect=true +gulp runtests --tests=2dArrays --inspect ``` You can also use the [provided VS Code launch configuration](./.vscode/launch.template.json) to launch a debug session for an open test file. Rename the file 'launch.json', open the test file of interest, and launch the debugger from the debug panel (or press F5). From 707e9770564865d10168e3722eec4eb9e20d32db Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Tue, 19 May 2020 13:14:32 -0700 Subject: [PATCH 2/2] Ensure formatter can always get a newline character (#38579) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Ensure formatter can always get a newline character * Make FormatContext.host optional since it’s not necessary if format options are all applied * Make FormattingHost required again --- 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 5436413701d..dd4135ca40c 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 ca7ac8f58ff..f4905c00b84 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 31aff6b210a..951ad5e5bba 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 f466b397d20..ccee491040f 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 f50d99e55d1..6668ee14f37 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 13c68364664..fd8ade8f8e6 100644 --- a/src/services/types.ts +++ b/src/services/types.ts @@ -203,6 +203,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 617855c584a..b1799167c43 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 999e1580a98..78a2f43f6c3 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 a998bf6f510..a4787529994 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 2d76b64696d..5c3a103c238 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 d9b47adc1fb..d7d4935118d 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 00000000000..24626441222 --- /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; +}`);