Skip to content

Don't normalize whitespace in fourslash tests #18447

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

Merged
2 commits merged into from
Sep 14, 2017
Merged
Show file tree
Hide file tree
Changes from all 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
93 changes: 46 additions & 47 deletions src/harness/fourslash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,6 @@
namespace FourSlash {
ts.disableIncrementalParsing = false;

function normalizeNewLines(s: string) {
return s.replace(/\r\n/g, "\n");
}

// Represents a parsed source file with metadata
export interface FourSlashFile {
// The contents of the file (with markers, etc stripped out)
Expand Down Expand Up @@ -364,7 +360,7 @@ namespace FourSlash {
baseIndentSize: 0,
indentSize: 4,
tabSize: 4,
newLineCharacter: Harness.IO.newLine(),
newLineCharacter: "\n",
convertTabsToSpaces: true,
indentStyle: ts.IndentStyle.Smart,
insertSpaceAfterCommaDelimiter: true,
Expand Down Expand Up @@ -1603,16 +1599,16 @@ namespace FourSlash {
}
}

public printCurrentFileState(makeWhitespaceVisible: boolean, makeCaretVisible: boolean) {
public printCurrentFileState(showWhitespace: boolean, makeCaretVisible: boolean) {
for (const file of this.testData.files) {
const active = (this.activeFile === file);
Harness.IO.log(`=== Script (${file.fileName}) ${(active ? "(active, cursor at |)" : "")} ===`);
let content = this.getFileContent(file.fileName);
if (active) {
content = content.substr(0, this.currentCaretPosition) + (makeCaretVisible ? "|" : "") + content.substr(this.currentCaretPosition);
}
if (makeWhitespaceVisible) {
content = TestState.makeWhitespaceVisible(content);
if (showWhitespace) {
content = makeWhitespaceVisible(content);
}
Harness.IO.log(content);
}
Expand Down Expand Up @@ -2128,10 +2124,8 @@ namespace FourSlash {

public verifyCurrentFileContent(text: string) {
const actual = this.getFileContent(this.activeFile.fileName);
if (normalizeNewLines(actual) !== normalizeNewLines(text)) {
throw new Error("verifyCurrentFileContent\n" +
"\tExpected: \"" + TestState.makeWhitespaceVisible(text) + "\"\n" +
"\t Actual: \"" + TestState.makeWhitespaceVisible(actual) + "\"");
if (actual !== text) {
throw new Error(`verifyCurrentFileContent failed:\n${showTextDiff(text, actual)}`);
}
}

Expand Down Expand Up @@ -2305,11 +2299,11 @@ namespace FourSlash {
const actualText = this.rangeText(ranges[0]);

const result = includeWhiteSpace
? normalizeNewLines(actualText) === normalizeNewLines(expectedText)
? actualText === expectedText
: this.removeWhitespace(actualText) === this.removeWhitespace(expectedText);

if (!result) {
this.raiseError(`Actual text doesn't match expected text. Actual:\n'${actualText}'\nExpected:\n'${expectedText}'`);
this.raiseError(`Actual range text doesn't match expected text.\n${showTextDiff(expectedText, actualText)}`);
}
}

Expand Down Expand Up @@ -2403,15 +2397,19 @@ namespace FourSlash {
const originalContent = scriptInfo.content;
for (const codeFix of codeFixes) {
this.applyEdits(codeFix.changes[0].fileName, codeFix.changes[0].textChanges, /*isFormattingEdit*/ false);
actualTextArray.push(this.normalizeNewlines(this.rangeText(ranges[0])));
let text = this.rangeText(ranges[0]);
// TODO:GH#18445 (remove this line to see errors in many `importNameCodeFix` tests)
text = text.replace(/\r\n/g, "\n");
actualTextArray.push(text);
scriptInfo.updateContent(originalContent);
}
const sortedExpectedArray = ts.map(expectedTextArray, str => this.normalizeNewlines(str)).sort();
const sortedExpectedArray = expectedTextArray.sort();
const sortedActualArray = actualTextArray.sort();
if (!ts.arrayIsEqualTo(sortedExpectedArray, sortedActualArray)) {
this.raiseError(
`Actual text array doesn't match expected text array. \nActual: \n'${sortedActualArray.join("\n\n")}'\n---\nExpected: \n'${sortedExpectedArray.join("\n\n")}'`);
}
ts.zipWith(sortedExpectedArray, sortedActualArray, (expected, actual, index) => {
if (expected !== actual) {
this.raiseError(`Import fix at index ${index} doesn't match.\n${showTextDiff(expected, actual)}`);
}
});
}

public verifyDocCommentTemplate(expected: ts.TextInsertion | undefined) {
Expand All @@ -2431,7 +2429,7 @@ namespace FourSlash {
}

if (actual.newText !== expected.newText) {
this.raiseError(`${name} failed - expected insertion:\n"${this.clarifyNewlines(expected.newText)}"\nactual insertion:\n"${this.clarifyNewlines(actual.newText)}"`);
this.raiseError(`${name} failed for expected insertion.\n${showTextDiff(expected.newText, actual.newText)}`);
}

if (actual.caretOffset !== expected.caretOffset) {
Expand All @@ -2440,17 +2438,6 @@ namespace FourSlash {
}
}

private clarifyNewlines(str: string) {
return str.replace(/\r?\n/g, lineEnding => {
const representation = lineEnding === "\r\n" ? "CRLF" : "LF";
return "# - " + representation + lineEnding;
});
}

private normalizeNewlines(str: string) {
return str.replace(/\r?\n/g, "\n");
}

public verifyBraceCompletionAtPosition(negative: boolean, openingBrace: string) {

const openBraceMap = ts.createMapFromTemplate<ts.CharacterCodes>({
Expand Down Expand Up @@ -2878,8 +2865,8 @@ namespace FourSlash {
}
const actualContent = this.getFileContent(this.activeFile.fileName);

if (this.normalizeNewlines(actualContent) !== this.normalizeNewlines(expectedContent)) {
this.raiseError(`verifyFileAfterApplyingRefactors failed: expected:\n${expectedContent}\nactual:\n${actualContent}`);
if (actualContent !== expectedContent) {
this.raiseError(`verifyFileAfterApplyingRefactors failed:\n${showTextDiff(expectedContent, actualContent)}`);
}
}

Expand Down Expand Up @@ -3014,10 +3001,6 @@ namespace FourSlash {
}
}

private static makeWhitespaceVisible(text: string) {
return text.replace(/ /g, "\u00B7").replace(/\r/g, "\u00B6").replace(/\n/g, "\u2193\n").replace(/\t/g, "\u2192\ ");
}

public setCancelled(numberOfCalls: number): void {
this.cancellationToken.setCancelled(numberOfCalls);
}
Expand Down Expand Up @@ -3319,12 +3302,7 @@ ${code}
let column = 1;

const flush = (lastSafeCharIndex: number) => {
if (lastSafeCharIndex === undefined) {
output = output + content.substr(lastNormalCharPosition);
}
else {
output = output + content.substr(lastNormalCharPosition, lastSafeCharIndex - lastNormalCharPosition);
}
output = output + content.substr(lastNormalCharPosition, lastSafeCharIndex === undefined ? undefined : lastSafeCharIndex - lastNormalCharPosition);
};

if (content.length > 0) {
Expand Down Expand Up @@ -3511,6 +3489,27 @@ ${code}
function toArray<T>(x: T | T[]): T[] {
return ts.isArray(x) ? x : [x];
}

function makeWhitespaceVisible(text: string) {
return text.replace(/ /g, "\u00B7").replace(/\r/g, "\u00B6").replace(/\n/g, "\u2193\n").replace(/\t/g, "\u2192\ ");
}

function showTextDiff(expected: string, actual: string): string {
// Only show whitespace if the difference is whitespace-only.
if (differOnlyByWhitespace(expected, actual)) {
expected = makeWhitespaceVisible(expected);
actual = makeWhitespaceVisible(actual);
}
return `Expected:\n${expected}\nActual:${actual}`;
}

function differOnlyByWhitespace(a: string, b: string) {
return stripWhitespace(a) === stripWhitespace(b);
}

function stripWhitespace(s: string): string {
return s.replace(/\s/g, "");
}
}

namespace FourSlashInterface {
Expand Down Expand Up @@ -4143,15 +4142,15 @@ namespace FourSlashInterface {
}

public printCurrentFileState() {
this.state.printCurrentFileState(/*makeWhitespaceVisible*/ false, /*makeCaretVisible*/ true);
this.state.printCurrentFileState(/*showWhitespace*/ false, /*makeCaretVisible*/ true);
}

public printCurrentFileStateWithWhitespace() {
this.state.printCurrentFileState(/*makeWhitespaceVisible*/ true, /*makeCaretVisible*/ true);
this.state.printCurrentFileState(/*showWhitespace*/ true, /*makeCaretVisible*/ true);
}

public printCurrentFileStateWithoutCaret() {
this.state.printCurrentFileState(/*makeWhitespaceVisible*/ false, /*makeCaretVisible*/ false);
this.state.printCurrentFileState(/*showWhitespace*/ false, /*makeCaretVisible*/ false);
}

public printCurrentQuickInfo() {
Expand Down
3 changes: 2 additions & 1 deletion src/harness/harness.ts
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,8 @@ namespace Harness {
export let IO: IO;

// harness always uses one kind of new line
const harnessNewLine = "\r\n";
// But note that `parseTestData` in `fourslash.ts` uses "\n"
export const harnessNewLine = "\r\n";

// Root for file paths that are stored in a virtual file system
export const virtualFileSystemRoot = "/";
Expand Down
2 changes: 1 addition & 1 deletion src/harness/harnessLanguageService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ namespace Harness.LanguageService {
}

public getNewLine(): string {
return "\r\n";
return harnessNewLine;
}

public getFilenames(): string[] {
Expand Down
3 changes: 2 additions & 1 deletion src/server/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -573,14 +573,15 @@ namespace ts.server {

getEditsForRefactor(
fileName: string,
_formatOptions: FormatCodeSettings,
formatOptions: FormatCodeSettings,
positionOrRange: number | TextRange,
refactorName: string,
actionName: string): RefactorEditInfo {

const args = this.createFileLocationOrRangeRequestArgs(positionOrRange, fileName) as protocol.GetEditsForRefactorRequestArgs;
args.refactor = refactorName;
args.action = actionName;
args.formatOptions = formatOptions;

const request = this.processRequest<protocol.GetEditsForRefactorRequest>(CommandNames.GetEditsForRefactor, args);
const response = this.processResponse<protocol.GetEditsForRefactorResponse>(request);
Expand Down
1 change: 1 addition & 0 deletions src/server/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,7 @@ namespace ts.server.protocol {
refactor: string;
/* The 'name' property from the refactoring action */
action: string;
formatOptions: FormatCodeSettings,
};


Expand Down
2 changes: 1 addition & 1 deletion src/server/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1488,7 +1488,7 @@ namespace ts.server {

const result = project.getLanguageService().getEditsForRefactor(
file,
this.projectService.getFormatCodeOptions(),
convertFormatOptions(args.formatOptions),
position || textRange,
args.refactor,
args.action
Expand Down
2 changes: 1 addition & 1 deletion src/services/services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2019,7 +2019,7 @@ namespace ts {
startPosition,
endPosition,
program: getProgram(),
newLineCharacter: host.getNewLine(),
newLineCharacter: formatOptions ? formatOptions.newLineCharacter : host.getNewLine(),
rulesProvider: getRuleProvider(formatOptions),
cancellationToken
};
Expand Down
6 changes: 1 addition & 5 deletions src/services/textChanges.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,16 +184,12 @@ namespace ts.textChanges {
return s;
}

function getNewlineKind(context: { newLineCharacter: string }) {
return context.newLineCharacter === "\n" ? NewLineKind.LineFeed : NewLineKind.CarriageReturnLineFeed;
}

export class ChangeTracker {
private changes: Change[] = [];
private readonly newLineCharacter: string;

public static fromContext(context: RefactorContext | CodeFixContext) {
return new ChangeTracker(getNewlineKind(context), context.rulesProvider);
return new ChangeTracker(context.newLineCharacter === "\n" ? NewLineKind.LineFeed : NewLineKind.CarriageReturnLineFeed, context.rulesProvider);
}

constructor(
Expand Down
3 changes: 1 addition & 2 deletions tests/cases/fourslash/codeFixAddMissingMember5.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,4 @@ verify.currentFileContentIs(`class C {
()=>{ this.foo === 10 };
}
}
C.foo = undefined;
`);
C.foo = undefined;` + "\r\n"); // TODO: GH#18445
3 changes: 1 addition & 2 deletions tests/cases/fourslash/codeFixAddMissingMember7.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,4 @@ verify.getAndApplyCodeFix(/*errorCode*/ undefined, /*index*/ 2)
verify.currentFileContentIs(`class C {
static p = ()=>{ this.foo === 10 };
}
C.foo = undefined;
`);
C.foo = undefined;` + "\r\n"); // TODO: GH#18445
5 changes: 3 additions & 2 deletions tests/cases/fourslash/codeFixSuperAfterThis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
//// super();
//// |]}
////}
// TODO: GH#18445
verify.rangeAfterCodeFix(`
super();
super();\r
this.a = 12;
`, /*includeWhiteSpace*/ true);
`, /*includeWhiteSpace*/ true);
3 changes: 2 additions & 1 deletion tests/cases/fourslash/codeFixSuperCall.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
//// constructor() {[|
//// |]}
////}
// TODO: GH#18445
verify.rangeAfterCodeFix(`
super();
super();\r
`, /*includeWhitespace*/ true);
2 changes: 1 addition & 1 deletion tests/cases/fourslash/extract-method14.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ edit.applyRefactor({
`function foo() {
var i = 10;
var __return: any;
({ __return, i } = n/*RENAME*/ewFunction(i));
({ __return, i } = /*RENAME*/newFunction(i));
return __return;
}
function newFunction(i) {
Expand Down
18 changes: 9 additions & 9 deletions tests/cases/fourslash/formatConflictDiff3Marker1.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@
////}

format.document();
verify.currentFileContentIs("class C {\r\n\
<<<<<<< HEAD\r\n\
v = 1;\r\n\
||||||| merged common ancestors\r\n\
v = 3;\r\n\
=======\r\n\
v = 2;\r\n\
>>>>>>> Branch - a\r\n\
}");
verify.currentFileContentIs(`class C {
<<<<<<< HEAD
v = 1;
||||||| merged common ancestors
v = 3;
=======
v = 2;
>>>>>>> Branch - a
}`);
14 changes: 7 additions & 7 deletions tests/cases/fourslash/formatConflictMarker1.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@
////}

format.document();
verify.currentFileContentIs("class C {\r\n\
<<<<<<< HEAD\r\n\
v = 1;\r\n\
=======\r\n\
v = 2;\r\n\
>>>>>>> Branch - a\r\n\
}");
verify.currentFileContentIs(`class C {
<<<<<<< HEAD
v = 1;
=======
v = 2;
>>>>>>> Branch - a
}`);
5 changes: 3 additions & 2 deletions tests/cases/fourslash/importNameCodeFixReExport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@
////x;|]

goTo.file("/b.ts");
verify.rangeAfterCodeFix(`import { x } from "./a";

// TODO:GH#18445
verify.rangeAfterCodeFix(`import { x } from "./a";\r
\r
export { x } from "./a";
x;`, /*includeWhiteSpace*/ true);