Skip to content

Commit d96dfeb

Browse files
author
Andy
authored
Don't normalize whitespace in fourslash tests (microsoft#18447)
* Don't normalize whitespace in fourslash tests * Only render whitespace when the diff is text-only
1 parent cf53743 commit d96dfeb

16 files changed

+82
-83
lines changed

‎src/harness/fourslash.ts‎

Lines changed: 46 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,6 @@
2222
namespace FourSlash {
2323
ts.disableIncrementalParsing = false;
2424

25-
function normalizeNewLines(s: string) {
26-
return s.replace(/\r\n/g, "\n");
27-
}
28-
2925
// Represents a parsed source file with metadata
3026
export interface FourSlashFile {
3127
// The contents of the file (with markers, etc stripped out)
@@ -364,7 +360,7 @@ namespace FourSlash {
364360
baseIndentSize: 0,
365361
indentSize: 4,
366362
tabSize: 4,
367-
newLineCharacter: Harness.IO.newLine(),
363+
newLineCharacter: "\n",
368364
convertTabsToSpaces: true,
369365
indentStyle: ts.IndentStyle.Smart,
370366
insertSpaceAfterCommaDelimiter: true,
@@ -1603,16 +1599,16 @@ namespace FourSlash {
16031599
}
16041600
}
16051601

1606-
public printCurrentFileState(makeWhitespaceVisible: boolean, makeCaretVisible: boolean) {
1602+
public printCurrentFileState(showWhitespace: boolean, makeCaretVisible: boolean) {
16071603
for (const file of this.testData.files) {
16081604
const active = (this.activeFile === file);
16091605
Harness.IO.log(`=== Script (${file.fileName}) ${(active ? "(active, cursor at |)" : "")} ===`);
16101606
let content = this.getFileContent(file.fileName);
16111607
if (active) {
16121608
content = content.substr(0, this.currentCaretPosition) + (makeCaretVisible ? "|" : "") + content.substr(this.currentCaretPosition);
16131609
}
1614-
if (makeWhitespaceVisible) {
1615-
content = TestState.makeWhitespaceVisible(content);
1610+
if (showWhitespace) {
1611+
content = makeWhitespaceVisible(content);
16161612
}
16171613
Harness.IO.log(content);
16181614
}
@@ -2128,10 +2124,8 @@ namespace FourSlash {
21282124

21292125
public verifyCurrentFileContent(text: string) {
21302126
const actual = this.getFileContent(this.activeFile.fileName);
2131-
if (normalizeNewLines(actual) !== normalizeNewLines(text)) {
2132-
throw new Error("verifyCurrentFileContent\n" +
2133-
"\tExpected: \"" + TestState.makeWhitespaceVisible(text) + "\"\n" +
2134-
"\t Actual: \"" + TestState.makeWhitespaceVisible(actual) + "\"");
2127+
if (actual !== text) {
2128+
throw new Error(`verifyCurrentFileContent failed:\n${showTextDiff(text, actual)}`);
21352129
}
21362130
}
21372131

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

23072301
const result = includeWhiteSpace
2308-
? normalizeNewLines(actualText) === normalizeNewLines(expectedText)
2302+
? actualText === expectedText
23092303
: this.removeWhitespace(actualText) === this.removeWhitespace(expectedText);
23102304

23112305
if (!result) {
2312-
this.raiseError(`Actual text doesn't match expected text. Actual:\n'${actualText}'\nExpected:\n'${expectedText}'`);
2306+
this.raiseError(`Actual range text doesn't match expected text.\n${showTextDiff(expectedText, actualText)}`);
23132307
}
23142308
}
23152309

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

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

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

24372435
if (actual.caretOffset !== expected.caretOffset) {
@@ -2440,17 +2438,6 @@ namespace FourSlash {
24402438
}
24412439
}
24422440

2443-
private clarifyNewlines(str: string) {
2444-
return str.replace(/\r?\n/g, lineEnding => {
2445-
const representation = lineEnding === "\r\n" ? "CRLF" : "LF";
2446-
return "# - " + representation + lineEnding;
2447-
});
2448-
}
2449-
2450-
private normalizeNewlines(str: string) {
2451-
return str.replace(/\r?\n/g, "\n");
2452-
}
2453-
24542441
public verifyBraceCompletionAtPosition(negative: boolean, openingBrace: string) {
24552442

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

2881-
if (this.normalizeNewlines(actualContent) !== this.normalizeNewlines(expectedContent)) {
2882-
this.raiseError(`verifyFileAfterApplyingRefactors failed: expected:\n${expectedContent}\nactual:\n${actualContent}`);
2868+
if (actualContent !== expectedContent) {
2869+
this.raiseError(`verifyFileAfterApplyingRefactors failed:\n${showTextDiff(expectedContent, actualContent)}`);
28832870
}
28842871
}
28852872

@@ -3014,10 +3001,6 @@ namespace FourSlash {
30143001
}
30153002
}
30163003

3017-
private static makeWhitespaceVisible(text: string) {
3018-
return text.replace(/ /g, "\u00B7").replace(/\r/g, "\u00B6").replace(/\n/g, "\u2193\n").replace(/\t/g, "\u2192\ ");
3019-
}
3020-
30213004
public setCancelled(numberOfCalls: number): void {
30223005
this.cancellationToken.setCancelled(numberOfCalls);
30233006
}
@@ -3319,12 +3302,7 @@ ${code}
33193302
let column = 1;
33203303

33213304
const flush = (lastSafeCharIndex: number) => {
3322-
if (lastSafeCharIndex === undefined) {
3323-
output = output + content.substr(lastNormalCharPosition);
3324-
}
3325-
else {
3326-
output = output + content.substr(lastNormalCharPosition, lastSafeCharIndex - lastNormalCharPosition);
3327-
}
3305+
output = output + content.substr(lastNormalCharPosition, lastSafeCharIndex === undefined ? undefined : lastSafeCharIndex - lastNormalCharPosition);
33283306
};
33293307

33303308
if (content.length > 0) {
@@ -3511,6 +3489,27 @@ ${code}
35113489
function toArray<T>(x: T | T[]): T[] {
35123490
return ts.isArray(x) ? x : [x];
35133491
}
3492+
3493+
function makeWhitespaceVisible(text: string) {
3494+
return text.replace(/ /g, "\u00B7").replace(/\r/g, "\u00B6").replace(/\n/g, "\u2193\n").replace(/\t/g, "\u2192\ ");
3495+
}
3496+
3497+
function showTextDiff(expected: string, actual: string): string {
3498+
// Only show whitespace if the difference is whitespace-only.
3499+
if (differOnlyByWhitespace(expected, actual)) {
3500+
expected = makeWhitespaceVisible(expected);
3501+
actual = makeWhitespaceVisible(actual);
3502+
}
3503+
return `Expected:\n${expected}\nActual:${actual}`;
3504+
}
3505+
3506+
function differOnlyByWhitespace(a: string, b: string) {
3507+
return stripWhitespace(a) === stripWhitespace(b);
3508+
}
3509+
3510+
function stripWhitespace(s: string): string {
3511+
return s.replace(/\s/g, "");
3512+
}
35143513
}
35153514

35163515
namespace FourSlashInterface {
@@ -4143,15 +4142,15 @@ namespace FourSlashInterface {
41434142
}
41444143

41454144
public printCurrentFileState() {
4146-
this.state.printCurrentFileState(/*makeWhitespaceVisible*/ false, /*makeCaretVisible*/ true);
4145+
this.state.printCurrentFileState(/*showWhitespace*/ false, /*makeCaretVisible*/ true);
41474146
}
41484147

41494148
public printCurrentFileStateWithWhitespace() {
4150-
this.state.printCurrentFileState(/*makeWhitespaceVisible*/ true, /*makeCaretVisible*/ true);
4149+
this.state.printCurrentFileState(/*showWhitespace*/ true, /*makeCaretVisible*/ true);
41514150
}
41524151

41534152
public printCurrentFileStateWithoutCaret() {
4154-
this.state.printCurrentFileState(/*makeWhitespaceVisible*/ false, /*makeCaretVisible*/ false);
4153+
this.state.printCurrentFileState(/*showWhitespace*/ false, /*makeCaretVisible*/ false);
41554154
}
41564155

41574156
public printCurrentQuickInfo() {

‎src/harness/harness.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -500,7 +500,8 @@ namespace Harness {
500500
export let IO: IO;
501501

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

505506
// Root for file paths that are stored in a virtual file system
506507
export const virtualFileSystemRoot = "/";

‎src/harness/harnessLanguageService.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ namespace Harness.LanguageService {
130130
}
131131

132132
public getNewLine(): string {
133-
return "\r\n";
133+
return harnessNewLine;
134134
}
135135

136136
public getFilenames(): string[] {

‎src/server/client.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -573,14 +573,15 @@ namespace ts.server {
573573

574574
getEditsForRefactor(
575575
fileName: string,
576-
_formatOptions: FormatCodeSettings,
576+
formatOptions: FormatCodeSettings,
577577
positionOrRange: number | TextRange,
578578
refactorName: string,
579579
actionName: string): RefactorEditInfo {
580580

581581
const args = this.createFileLocationOrRangeRequestArgs(positionOrRange, fileName) as protocol.GetEditsForRefactorRequestArgs;
582582
args.refactor = refactorName;
583583
args.action = actionName;
584+
args.formatOptions = formatOptions;
584585

585586
const request = this.processRequest<protocol.GetEditsForRefactorRequest>(CommandNames.GetEditsForRefactor, args);
586587
const response = this.processResponse<protocol.GetEditsForRefactorResponse>(request);

‎src/server/protocol.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -494,6 +494,7 @@ namespace ts.server.protocol {
494494
refactor: string;
495495
/* The 'name' property from the refactoring action */
496496
action: string;
497+
formatOptions: FormatCodeSettings,
497498
};
498499

499500

‎src/server/session.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1488,7 +1488,7 @@ namespace ts.server {
14881488

14891489
const result = project.getLanguageService().getEditsForRefactor(
14901490
file,
1491-
this.projectService.getFormatCodeOptions(),
1491+
convertFormatOptions(args.formatOptions),
14921492
position || textRange,
14931493
args.refactor,
14941494
args.action

‎src/services/services.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2019,7 +2019,7 @@ namespace ts {
20192019
startPosition,
20202020
endPosition,
20212021
program: getProgram(),
2022-
newLineCharacter: host.getNewLine(),
2022+
newLineCharacter: formatOptions ? formatOptions.newLineCharacter : host.getNewLine(),
20232023
rulesProvider: getRuleProvider(formatOptions),
20242024
cancellationToken
20252025
};

‎src/services/textChanges.ts‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -184,16 +184,12 @@ namespace ts.textChanges {
184184
return s;
185185
}
186186

187-
function getNewlineKind(context: { newLineCharacter: string }) {
188-
return context.newLineCharacter === "\n" ? NewLineKind.LineFeed : NewLineKind.CarriageReturnLineFeed;
189-
}
190-
191187
export class ChangeTracker {
192188
private changes: Change[] = [];
193189
private readonly newLineCharacter: string;
194190

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

199195
constructor(

‎tests/cases/fourslash/codeFixAddMissingMember5.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,5 +17,4 @@ verify.currentFileContentIs(`class C {
1717
()=>{ this.foo === 10 };
1818
}
1919
}
20-
C.foo = undefined;
21-
`);
20+
C.foo = undefined;` + "\r\n"); // TODO: GH#18445

‎tests/cases/fourslash/codeFixAddMissingMember7.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,5 +13,4 @@ verify.getAndApplyCodeFix(/*errorCode*/ undefined, /*index*/ 2)
1313
verify.currentFileContentIs(`class C {
1414
static p = ()=>{ this.foo === 10 };
1515
}
16-
C.foo = undefined;
17-
`);
16+
C.foo = undefined;` + "\r\n"); // TODO: GH#18445

0 commit comments

Comments
 (0)