Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit d96dfeb

Browse filesBrowse 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
Copy full SHA for d96dfeb

16 files changed

+82-83Lines changed: 82 additions & 83 deletions
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎src/harness/fourslash.ts‎

Copy file name to clipboardExpand all lines: src/harness/fourslash.ts
+46-47Lines 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() {
Collapse file

‎src/harness/harness.ts‎

Copy file name to clipboardExpand all lines: src/harness/harness.ts
+2-1Lines 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 = "/";
Collapse file

‎src/harness/harnessLanguageService.ts‎

Copy file name to clipboardExpand all lines: src/harness/harnessLanguageService.ts
+1-1Lines 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[] {
Collapse file

‎src/server/client.ts‎

Copy file name to clipboardExpand all lines: src/server/client.ts
+2-1Lines 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);
Collapse file

‎src/server/protocol.ts‎

Copy file name to clipboardExpand all lines: src/server/protocol.ts
+1Lines 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

Collapse file

‎src/server/session.ts‎

Copy file name to clipboardExpand all lines: src/server/session.ts
+1-1Lines 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
Collapse file

‎src/services/services.ts‎

Copy file name to clipboardExpand all lines: src/services/services.ts
+1-1Lines 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
};
Collapse file

‎src/services/textChanges.ts‎

Copy file name to clipboardExpand all lines: src/services/textChanges.ts
+1-5Lines 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(
Collapse file

‎tests/cases/fourslash/codeFixAddMissingMember5.ts‎

Copy file name to clipboardExpand all lines: tests/cases/fourslash/codeFixAddMissingMember5.ts
+1-2Lines 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
Collapse file

‎tests/cases/fourslash/codeFixAddMissingMember7.ts‎

Copy file name to clipboardExpand all lines: tests/cases/fourslash/codeFixAddMissingMember7.ts
+1-2Lines 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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.