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.

Sign up for GitHub

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

Simplify for loops in fourslash.ts #11894

Merged
2 commits merged into from
Oct 27, 2016
Merged

Simplify for loops in fourslash.ts #11894

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ca09ef4
Simplify for loops in fourslash.ts
Oct 27, 2016
eb45962
Rename to zipWith
Oct 27, 2016
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
7 changes: 7 additions & 0 deletions src/compiler/core.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,13 @@ namespace ts {
return undefined;
}

export function zip<T, U>(arrayA: T[], arrayB: U[], callback: (a: T, b: U, index: number) => void): void {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lodash and ramda call this zipWith, and have zip with the type (t: T[], u: U[]) => [T,U][] (as do underscore, F#, Python and Haskell). I'd call it zipWith

Debug.assert(arrayA.length === arrayB.length);
for (let i = 0; i < arrayA.length; i++) {
callback(arrayA[i], arrayB[i], i);
}
}

/**
* Iterates through `array` by index and performs the callback on each element of array until the callback
* returns a falsey value, then returns false.
Expand Down
141 changes: 48 additions & 93 deletions src/harness/fourslash.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -438,9 +438,8 @@ namespace FourSlash {
private getAllDiagnostics(): ts.Diagnostic[] {
const diagnostics: ts.Diagnostic[] = [];

const fileNames = this.languageServiceAdapterHost.getFilenames();
for (let i = 0, n = fileNames.length; i < n; i++) {
diagnostics.push.apply(this.getDiagnostics(fileNames[i]));
for (const fileName of this.languageServiceAdapterHost.getFilenames()) {
diagnostics.push.apply(this.getDiagnostics(fileName));
}

return diagnostics;
Expand Down Expand Up @@ -580,12 +579,12 @@ namespace FourSlash {
this.raiseError(`goToDefinitions failed - expected to find ${endMarkers.length} definitions but got ${definitions.length}`);
}

for (let i = 0; i < endMarkers.length; i++) {
const marker = this.getMarkerByName(endMarkers[i]), definition = definitions[i];
ts.zip(endMarkers, definitions, (endMarker, definition, i) => {
const marker = this.getMarkerByName(endMarker);
if (marker.fileName !== definition.fileName || marker.position !== definition.textSpan.start) {
this.raiseError(`goToDefinition failed for definition ${i}: expected ${marker.fileName} at ${marker.position}, got ${definition.fileName} at ${definition.textSpan.start}`);
}
}
});
}

public verifyGetEmitOutputForCurrentFile(expected: string): void {
Expand All @@ -602,10 +601,10 @@ namespace FourSlash {
public verifyGetEmitOutputContentsForCurrentFile(expected: ts.OutputFile[]): void {
const emit = this.languageService.getEmitOutput(this.activeFile.fileName);
assert.equal(emit.outputFiles.length, expected.length, "Number of emit output files");
for (let i = 0; i < emit.outputFiles.length; i++) {
assert.equal(emit.outputFiles[i].name, expected[i].name, "FileName");
assert.equal(emit.outputFiles[i].text, expected[i].text, "Content");
}
ts.zip(emit.outputFiles, expected, (outputFile, expected) => {
assert.equal(outputFile.name, expected.name, "FileName");
assert.equal(outputFile.text, expected.text, "Content");
});
}

public verifyMemberListContains(symbol: string, text?: string, documentation?: string, kind?: string) {
Expand Down Expand Up @@ -668,9 +667,9 @@ namespace FourSlash {

const entries = this.getCompletionListAtCaret().entries;
assert.isTrue(items.length <= entries.length, `Amount of expected items in completion list [ ${items.length} ] is greater than actual number of items in list [ ${entries.length} ]`);
for (let i = 0; i < items.length; i++) {
assert.equal(entries[i].name, items[i], `Unexpected item in completion list`);
}
ts.zip(entries, items, (entry, item) => {
assert.equal(entry.name, item, `Unexpected item in completion list`);
});
}

public noItemsWithSameNameButDifferentKind(): void {
Expand All @@ -692,15 +691,7 @@ namespace FourSlash {
this.raiseError("Member list is empty at Caret");
}
else if ((members && members.entries.length !== 0) && !negative) {

let errorMsg = "\n" + "Member List contains: [" + members.entries[0].name;
for (let i = 1; i < members.entries.length; i++) {
errorMsg += ", " + members.entries[i].name;
}
errorMsg += "]\n";

this.raiseError("Member list is not empty at Caret: " + errorMsg);

this.raiseError(`Member list is not empty at Caret:\nMember List contains: ${stringify(members.entries.map(e => e.name))}`);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are we using ES5's Array.map now?

}
}

Expand All @@ -710,13 +701,8 @@ namespace FourSlash {
this.raiseError("Completion list is empty at caret at position " + this.activeFile.fileName + " " + this.currentCaretPosition);
}
else if (completions && completions.entries.length !== 0 && !negative) {
let errorMsg = "\n" + "Completion List contains: [" + completions.entries[0].name;
for (let i = 1; i < completions.entries.length; i++) {
errorMsg += ", " + completions.entries[i].name;
}
errorMsg += "]\n";

this.raiseError("Completion list is not empty at caret at position " + this.activeFile.fileName + " " + this.currentCaretPosition + errorMsg);
this.raiseError(`Completion list is not empty at caret at position ${this.activeFile.fileName} ${this.currentCaretPosition}\n` +
`Completion List contains: ${stringify(completions.entries.map(e => e.name))}`);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here. Are we using Array.map now?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be fine since fourslash only runs when testing the compiler, not when actually compiling.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use Array.map now. We haven't been ES3-compatible for a while.

}
}

Expand Down Expand Up @@ -890,8 +876,7 @@ namespace FourSlash {
}

private verifyReferencesWorker(references: ts.ReferenceEntry[], fileName: string, start: number, end: number, isWriteAccess?: boolean, isDefinition?: boolean) {
for (let i = 0; i < references.length; i++) {
const reference = references[i];
for (const reference of references) {
if (reference && reference.fileName === fileName && reference.textSpan.start === start && ts.textSpanEnd(reference.textSpan) === end) {
if (typeof isWriteAccess !== "undefined" && reference.isWriteAccess !== isWriteAccess) {
this.raiseError(`verifyReferencesAtPositionListContains failed - item isWriteAccess value does not match, actual: ${reference.isWriteAccess}, expected: ${isWriteAccess}.`);
Expand Down Expand Up @@ -1008,16 +993,11 @@ namespace FourSlash {
ranges = ranges.sort((r1, r2) => r1.start - r2.start);
references = references.sort((r1, r2) => r1.textSpan.start - r2.textSpan.start);

for (let i = 0, n = ranges.length; i < n; i++) {
const reference = references[i];
const range = ranges[i];

if (reference.textSpan.start !== range.start ||
ts.textSpanEnd(reference.textSpan) !== range.end) {

ts.zip(references, ranges, (reference, range) => {
if (reference.textSpan.start !== range.start || ts.textSpanEnd(reference.textSpan) !== range.end) {
this.raiseError("Rename location results do not match.\n\nExpected: " + stringify(ranges) + "\n\nActual:" + JSON.stringify(references));
}
}
});
}
else {
this.raiseError("Expected rename to succeed, but it actually failed.");
Expand Down Expand Up @@ -1247,8 +1227,7 @@ namespace FourSlash {
const emitFiles: FourSlashFile[] = []; // List of FourSlashFile that has emitThisFile flag on

const allFourSlashFiles = this.testData.files;
for (let idx = 0; idx < allFourSlashFiles.length; idx++) {
const file = allFourSlashFiles[idx];
for (const file of allFourSlashFiles) {
if (file.fileOptions[metadataOptionNames.emitThisFile] === "true") {
// Find a file with the flag emitThisFile turned on
emitFiles.push(file);
Expand All @@ -1273,8 +1252,8 @@ namespace FourSlash {
if (emitOutput.emitSkipped) {
resultString += "Diagnostics:" + Harness.IO.newLine();
const diagnostics = ts.getPreEmitDiagnostics(this.languageService.getProgram());
for (let i = 0, n = diagnostics.length; i < n; i++) {
resultString += " " + diagnostics[0].messageText + Harness.IO.newLine();
for (const diagnostic of diagnostics) {
resultString += " " + diagnostic.messageText + Harness.IO.newLine();
}
}

Expand Down Expand Up @@ -1340,8 +1319,7 @@ namespace FourSlash {
}

public printCurrentFileState(makeWhitespaceVisible = false, makeCaretVisible = true) {
for (let i = 0; i < this.testData.files.length; i++) {
const file = this.testData.files[i];
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);
Expand Down Expand Up @@ -1576,10 +1554,10 @@ namespace FourSlash {
edits = edits.sort((a, b) => a.span.start - b.span.start);
// Get a snapshot of the content of the file so we can make sure any formatting edits didn't destroy non-whitespace characters
const oldContent = this.getFileContent(this.activeFile.fileName);
for (let j = 0; j < edits.length; j++) {
this.languageServiceAdapterHost.editScript(fileName, edits[j].span.start + runningOffset, ts.textSpanEnd(edits[j].span) + runningOffset, edits[j].newText);
this.updateMarkersForEdit(fileName, edits[j].span.start + runningOffset, ts.textSpanEnd(edits[j].span) + runningOffset, edits[j].newText);
const change = (edits[j].span.start - ts.textSpanEnd(edits[j].span)) + edits[j].newText.length;
for (const edit of edits) {
this.languageServiceAdapterHost.editScript(fileName, edit.span.start + runningOffset, ts.textSpanEnd(edit.span) + runningOffset, edit.newText);
this.updateMarkersForEdit(fileName, edit.span.start + runningOffset, ts.textSpanEnd(edit.span) + runningOffset, edit.newText);
const change = (edit.span.start - ts.textSpanEnd(edit.span)) + edit.newText.length;
runningOffset += change;
// TODO: Consider doing this at least some of the time for higher fidelity. Currently causes a failure (bug 707150)
// this.languageService.getScriptLexicalStructure(fileName);
Expand Down Expand Up @@ -1913,10 +1891,7 @@ namespace FourSlash {
jsonMismatchString());
}

for (let i = 0; i < expected.length; i++) {
const expectedClassification = expected[i];
const actualClassification = actual[i];

ts.zip(expected, actual, (expectedClassification, actualClassification) => {
const expectedType: string = (<any>ts.ClassificationTypeNames)[expectedClassification.classificationType];
if (expectedType !== actualClassification.classificationType) {
this.raiseError("verifyClassifications failed - expected classifications type to be " +
Expand Down Expand Up @@ -1946,7 +1921,7 @@ namespace FourSlash {
actualText +
jsonMismatchString());
}
}
});

function jsonMismatchString() {
return Harness.IO.newLine() +
Expand Down Expand Up @@ -1991,13 +1966,11 @@ namespace FourSlash {
this.raiseError(`verifyOutliningSpans failed - expected total spans to be ${spans.length}, but was ${actual.length}`);
}

for (let i = 0; i < spans.length; i++) {
const expectedSpan = spans[i];
const actualSpan = actual[i];
ts.zip(spans, actual, (expectedSpan, actualSpan, i) => {
if (expectedSpan.start !== actualSpan.textSpan.start || expectedSpan.end !== ts.textSpanEnd(actualSpan.textSpan)) {
this.raiseError(`verifyOutliningSpans failed - span ${(i + 1)} expected: (${expectedSpan.start},${expectedSpan.end}), actual: (${actualSpan.textSpan.start},${ts.textSpanEnd(actualSpan.textSpan)})`);
}
}
});
}

public verifyTodoComments(descriptors: string[], spans: TextSpan[]) {
Expand All @@ -2008,15 +1981,13 @@ namespace FourSlash {
this.raiseError(`verifyTodoComments failed - expected total spans to be ${spans.length}, but was ${actual.length}`);
}

for (let i = 0; i < spans.length; i++) {
const expectedSpan = spans[i];
const actualComment = actual[i];
ts.zip(spans, actual, (expectedSpan, actualComment, i) => {
const actualCommentSpan = ts.createTextSpan(actualComment.position, actualComment.message.length);

if (expectedSpan.start !== actualCommentSpan.start || expectedSpan.end !== ts.textSpanEnd(actualCommentSpan)) {
this.raiseError(`verifyOutliningSpans failed - span ${(i + 1)} expected: (${expectedSpan.start},${expectedSpan.end}), actual: (${actualCommentSpan.start},${ts.textSpanEnd(actualCommentSpan)})`);
}
}
});
}

private getCodeFixes(errorCode?: number) {
Expand Down Expand Up @@ -2163,11 +2134,9 @@ namespace FourSlash {
public verifyNavigationItemsCount(expected: number, searchValue: string, matchKind?: string, fileName?: string) {
const items = this.languageService.getNavigateToItems(searchValue, /*maxResultCount*/ undefined, fileName);
let actual = 0;
let item: ts.NavigateToItem;

// Count only the match that match the same MatchKind
for (let i = 0; i < items.length; i++) {
item = items[i];
for (const item of items) {
if (!matchKind || item.matchKind === matchKind) {
actual++;
}
Expand Down Expand Up @@ -2195,8 +2164,7 @@ namespace FourSlash {
this.raiseError("verifyNavigationItemsListContains failed - found 0 navigation items, expected at least one.");
}

for (let i = 0; i < items.length; i++) {
const item = items[i];
for (const item of items) {
if (item && item.name === name && item.kind === kind &&
(matchKind === undefined || item.matchKind === matchKind) &&
(fileName === undefined || item.fileName === fileName) &&
Expand Down Expand Up @@ -2247,24 +2215,16 @@ namespace FourSlash {

public printNavigationItems(searchValue: string) {
const items = this.languageService.getNavigateToItems(searchValue);
const length = items && items.length;

Harness.IO.log(`NavigationItems list (${length} items)`);

for (let i = 0; i < length; i++) {
const item = items[i];
Harness.IO.log(`NavigationItems list (${items.length} items)`);
for (const item of items) {
Harness.IO.log(`name: ${item.name}, kind: ${item.kind}, parentName: ${item.containerName}, fileName: ${item.fileName}`);
}
}

public printNavigationBar() {
const items = this.languageService.getNavigationBarItems(this.activeFile.fileName);
const length = items && items.length;

Harness.IO.log(`Navigation bar (${length} items)`);

for (let i = 0; i < length; i++) {
const item = items[i];
Harness.IO.log(`Navigation bar (${items.length} items)`);
for (const item of items) {
Harness.IO.log(`${repeatString(item.indent, " ")}name: ${item.text}, kind: ${item.kind}, childItems: ${item.childItems.map(child => child.text)}`);
}
}
Expand Down Expand Up @@ -2385,8 +2345,7 @@ namespace FourSlash {
}

private assertItemInCompletionList(items: ts.CompletionEntry[], name: string, text?: string, documentation?: string, kind?: string, spanIndex?: number) {
for (let i = 0; i < items.length; i++) {
const item = items[i];
for (const item of items) {
if (item.name === name) {
if (documentation != undefined || text !== undefined) {
const details = this.getCompletionEntryDetails(item.name);
Expand Down Expand Up @@ -2435,20 +2394,17 @@ namespace FourSlash {
name = name.indexOf("/") === -1 ? (this.basePath + "/" + name) : name;

const availableNames: string[] = [];
let foundIt = false;
for (let i = 0; i < this.testData.files.length; i++) {
const fn = this.testData.files[i].fileName;
result = ts.forEach(this.testData.files, file => {
const fn = file.fileName;
if (fn) {
if (fn === name) {
result = this.testData.files[i];
foundIt = true;
break;
return file;
}
availableNames.push(fn);
}
}
});

if (!foundIt) {
if (!result) {
throw new Error(`No test file named "${name}" exists. Available file names are: ${availableNames.join(", ")}`);
}
}
Expand Down Expand Up @@ -2549,8 +2505,8 @@ ${code}

function chompLeadingSpace(content: string) {
const lines = content.split("\n");
for (let i = 0; i < lines.length; i++) {
if ((lines[i].length !== 0) && (lines[i].charAt(0) !== " ")) {
for (const line of lines) {
if ((line.length !== 0) && (line.charAt(0) !== " ")) {
return content;
}
}
Expand Down Expand Up @@ -2588,8 +2544,7 @@ ${code}
currentFileName = fileName;
}

for (let i = 0; i < lines.length; i++) {
let line = lines[i];
for (let line of lines) {
const lineLength = line.length;

if (lineLength > 0 && line.charAt(lineLength - 1) === "\r") {
Expand Down