Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure formatter can always get a newline character #38579

Merged
merged 3 commits into from
May 19, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/harness/fourslashImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down
4 changes: 4 additions & 0 deletions src/harness/fourslashInterfaceImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -560,6 +560,10 @@ namespace FourSlashInterface {
public noMoveToNewFile(): void {
this.state.noMoveToNewFile();
}

public organizeImports(newContent: string) {
this.state.verifyOrganizeImports(newContent);
}
}

export class Edit {
Expand Down
7 changes: 4 additions & 3 deletions src/services/formatting/formatting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ namespace ts.formatting {
export interface FormatContext {
readonly options: FormatCodeSettings;
readonly getRules: RulesMap;
readonly host?: FormattingHost;
andrewbranch marked this conversation as resolved.
Show resolved Hide resolved
}

export interface TextRangeWithKind<T extends SyntaxKind = SyntaxKind> extends TextRange {
Expand Down Expand Up @@ -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[] {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions src/services/formatting/rulesMap.ts
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
18 changes: 9 additions & 9 deletions src/services/services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);
Expand Down Expand Up @@ -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) {
Expand All @@ -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<number>(errorCodes, equateValues, compareValues), errorCode => {
cancellationToken.throwIfCancellationRequested();
Expand All @@ -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 });
}
Expand All @@ -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<ApplyCodeActionCommandResult>;
Expand Down Expand Up @@ -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,
};
Expand Down
5 changes: 5 additions & 0 deletions src/services/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
//
Expand Down
6 changes: 3 additions & 3 deletions src/services/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 | undefined, formatSettings?: FormatCodeSettings) {
return formatSettings?.newLineCharacter ||
host?.getNewLine?.() ||
carriageReturnLineFeed;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ interface Array<T> {}`
cancellationToken: { throwIfCancellationRequested: noop, isCancellationRequested: returnFalse },
preferences: emptyOptions,
host: notImplementedHost,
formatContext: formatting.getFormatContext(testFormatSettings)
formatContext: formatting.getFormatContext(testFormatSettings, notImplementedHost)
};

const diagnostics = languageService.getSuggestionDiagnostics(f.path);
Expand Down
4 changes: 2 additions & 2 deletions src/testRunner/unittests/services/extract/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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));
Expand Down
2 changes: 1 addition & 1 deletion src/testRunner/unittests/services/textChanges.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 2 additions & 0 deletions tests/cases/fourslash/fourslash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,8 @@ declare namespace FourSlashInterface {
noMoveToNewFile(): void;

generateTypes(...options: GenerateTypesOptions[]): void;

organizeImports(newContent: string): void;
}
class edit {
backspace(count?: number): void;
Expand Down
22 changes: 22 additions & 0 deletions tests/cases/fourslash/organizeImportsNoFormatOptions.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/// <reference path="fourslash.ts" />

// #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;
}`);