Skip to content

Commit

Permalink
More PR feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
riknoll committed Sep 14, 2016
1 parent f91a123 commit 4a37fd7
Show file tree
Hide file tree
Showing 51 changed files with 192 additions and 225 deletions.
32 changes: 14 additions & 18 deletions src/harness/fourslash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1680,13 +1680,13 @@ namespace FourSlash {
assertFn(actualCount, expectedCount, this.messageAtLastKnownMarker("Type definitions Count"));
}

public verifyImplementationsCount(negative: boolean, expectedCount: number) {
public verifyImplementationListIsEmpty(negative: boolean) {
const assertFn = negative ? assert.notEqual : assert.equal;

const implementations = this.languageService.getImplementationAtPosition(this.activeFile.fileName, this.currentCaretPosition);
const actualCount = implementations && implementations.length || 0;

This comment has been minimized.

Copy link
@riknoll

riknoll Sep 14, 2016

Author Member

Actually, I think right now it is possible for getImplementationsAtPosition to return an empty array. I'll fix it so that it always returns undefined if no implementations are found and update this function.


assertFn(actualCount, expectedCount, this.messageAtLastKnownMarker("Implementations Count"));
assertFn(actualCount, 0, this.messageAtLastKnownMarker("Implementations Count"));
}

public verifyGoToDefinitionName(expectedName: string, expectedContainerName: string) {
Expand All @@ -1697,26 +1697,22 @@ namespace FourSlash {
assert.equal(actualDefinitionContainerName, expectedContainerName, this.messageAtLastKnownMarker("Definition Info Container Name"));
}

public goToImplementation(implIndex?: number) {
public goToImplementation() {
const implementations = this.languageService.getImplementationAtPosition(this.activeFile.fileName, this.currentCaretPosition);
if (!implementations || !implementations.length) {
this.raiseError("goToImplementation failed - expected to find at least one implementation location but got 0");
}

if (implIndex === undefined && implementations.length > 1) {
this.raiseError(`goToImplementation failed - no index given but more than 1 implementation returned (${implementations.length})`);
}

if (implIndex >= implementations.length) {
this.raiseError(`goToImplementation failed - implIndex value (${implIndex}) exceeds implementation list size (${implementations.length})`);
if (implementations.length > 1) {
this.raiseError(`goToImplementation failed - more than 1 implementation returned (${implementations.length})`);
}

const implementation = implementations[implIndex || 0];
const implementation = implementations[0];
this.openFile(implementation.fileName);
this.currentCaretPosition = implementation.textSpan.start;
}

public verifyRangesInImplementationList() {
public verifyRangesInImplementationList(markerName: string) {
this.goToMarker(markerName);
const implementations: ImplementationLocationInformation[] = this.languageService.getImplementationAtPosition(this.activeFile.fileName, this.currentCaretPosition);
if (!implementations || !implementations.length) {
this.raiseError("verifyRangesInImplementationList failed - expected to find at least one implementation location but got 0");
Expand Down Expand Up @@ -2954,8 +2950,8 @@ namespace FourSlashInterface {
this.state.goToTypeDefinition(definitionIndex);
}

public implementation(implementationIndex?: number) {
this.state.goToImplementation(implementationIndex);
public implementation() {
this.state.goToImplementation();
}

public position(position: number, fileIndex?: number): void;
Expand Down Expand Up @@ -3062,8 +3058,8 @@ namespace FourSlashInterface {
this.state.verifyTypeDefinitionsCount(this.negative, expectedCount);
}

public implementationCountIs(expectedCount: number) {
this.state.verifyImplementationsCount(this.negative, expectedCount);
public implementationListIsEmpty() {
this.state.verifyImplementationListIsEmpty(this.negative);
}

public isValidBraceCompletionAtPosition(openingBrace: string) {
Expand Down Expand Up @@ -3319,8 +3315,8 @@ namespace FourSlashInterface {
this.state.verifyProjectInfo(expected);
}

public allRangesAppearInImplementationList() {
this.state.verifyRangesInImplementationList();
public allRangesAppearInImplementationList(markerName: string) {
this.state.verifyRangesInImplementationList(markerName);
}
}

Expand Down
169 changes: 84 additions & 85 deletions src/services/services.ts

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/services/shims.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ namespace ts {

/**
* Returns a JSON-encoded value of the type:
* { fileName: string; textSpan: { start: number; length: number}; isWriteAccess: boolean, isDefinition?: boolean }[]
* { fileName: string; textSpan: { start: number; length: number}; }[]
*/
getImplementationAtPosition(fileName: string, position: number): string;

Expand Down
6 changes: 3 additions & 3 deletions tests/cases/fourslash/fourslash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ declare namespace FourSlashInterface {
bof(): void;
eof(): void;
type(definitionIndex?: number): void;
implementation(implementationIndex?: number): void;
implementation(): void;
position(position: number, fileIndex?: number): any;
position(position: number, fileName?: string): any;
file(index: number, content?: string, scriptKindName?: string): any;
Expand All @@ -134,7 +134,7 @@ declare namespace FourSlashInterface {
quickInfoIs(expectedText?: string, expectedDocumentation?: string): void;
quickInfoExists(): void;
typeDefinitionCountIs(expectedCount: number): void;
implementationCountIs(expectedCount: number): void;
implementationListIsEmpty(): void;
isValidBraceCompletionAtPosition(openingBrace?: string): void;
}
class verify extends verifyNegatable {
Expand Down Expand Up @@ -242,7 +242,7 @@ declare namespace FourSlashInterface {
getSyntacticDiagnostics(expected: string): void;
getSemanticDiagnostics(expected: string): void;
ProjectInfo(expected: string[]): void;
allRangesAppearInImplementationList(): void;
allRangesAppearInImplementationList(markerName: string): void;
}
class edit {
backspace(count?: number): void;
Expand Down
3 changes: 1 addition & 2 deletions tests/cases/fourslash/goToImplementationClassMethod_00.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,4 @@
////
//// new Bar().hel/*reference*/lo;

goTo.marker("reference");
verify.allRangesAppearInImplementationList();
verify.allRangesAppearInImplementationList("reference");
7 changes: 2 additions & 5 deletions tests/cases/fourslash/goToImplementationClassMethod_01.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,5 @@
//// x.he/*reference*/llo();
//// }

goTo.marker("reference");
verify.allRangesAppearInImplementationList();

goTo.marker("declaration");
verify.allRangesAppearInImplementationList();
verify.allRangesAppearInImplementationList("reference");
verify.allRangesAppearInImplementationList("declaration");
3 changes: 1 addition & 2 deletions tests/cases/fourslash/goToImplementationEnum_00.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,4 @@
////
//// Foo.Fo/*reference*/o1;

goTo.marker("reference");
verify.allRangesAppearInImplementationList();
verify.allRangesAppearInImplementationList("reference");
3 changes: 1 addition & 2 deletions tests/cases/fourslash/goToImplementationEnum_01.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,4 @@
////
//// Fo/*reference*/o;

goTo.marker("reference");
verify.allRangesAppearInImplementationList();
verify.allRangesAppearInImplementationList("reference");
7 changes: 2 additions & 5 deletions tests/cases/fourslash/goToImplementationInterfaceMethod_00.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,5 @@
//// constructor(public f: Foo = { [|hello() {/**3*/}|] } ) {}
//// }

goTo.marker("function_call");
verify.allRangesAppearInImplementationList();

goTo.marker("declaration");
verify.allRangesAppearInImplementationList();
verify.allRangesAppearInImplementationList("function_call");
verify.allRangesAppearInImplementationList("declaration");
7 changes: 2 additions & 5 deletions tests/cases/fourslash/goToImplementationInterfaceMethod_01.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,5 @@
////
//// whatever(new Bar());

goTo.marker("function_call");
verify.allRangesAppearInImplementationList();

goTo.marker("declaration");
verify.allRangesAppearInImplementationList();
verify.allRangesAppearInImplementationList("function_call");
verify.allRangesAppearInImplementationList("declaration");
7 changes: 2 additions & 5 deletions tests/cases/fourslash/goToImplementationInterfaceMethod_02.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,5 @@
//// a.he/*function_call*/llo();
//// }

goTo.marker("function_call");
verify.allRangesAppearInImplementationList();

goTo.marker("declaration");
verify.allRangesAppearInImplementationList();
verify.allRangesAppearInImplementationList("function_call");
verify.allRangesAppearInImplementationList("declaration");
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,4 @@
//// new Bar().hel/*function_call*/lo();
//// new Bar()["hello"]();

goTo.marker("function_call");
verify.allRangesAppearInImplementationList();
verify.allRangesAppearInImplementationList("function_call");
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,4 @@
//// x.he/*function_call*/llo()
//// }

goTo.marker("function_call");
verify.allRangesAppearInImplementationList();
verify.allRangesAppearInImplementationList("function_call");
Original file line number Diff line number Diff line change
Expand Up @@ -34,5 +34,4 @@
//// x.he/*function_call*/llo()
//// }

goTo.marker("function_call");
verify.allRangesAppearInImplementationList();
verify.allRangesAppearInImplementationList("function_call");
Original file line number Diff line number Diff line change
Expand Up @@ -45,5 +45,4 @@
//// x.he/*function_call*/llo()
//// }

goTo.marker("function_call");
verify.allRangesAppearInImplementationList();
verify.allRangesAppearInImplementationList("function_call");
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,4 @@
//// }


goTo.marker("function_call");
verify.allRangesAppearInImplementationList();
verify.allRangesAppearInImplementationList("function_call");
7 changes: 2 additions & 5 deletions tests/cases/fourslash/goToImplementationInterfaceMethod_09.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,5 @@
//// hello() {}
//// }

goTo.marker("function_call");
verify.allRangesAppearInImplementationList();

goTo.marker("element_access");
verify.allRangesAppearInImplementationList();
verify.allRangesAppearInImplementationList("function_call");
verify.allRangesAppearInImplementationList("element_access");
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,5 @@
//// }

for (var i = 0; i < 2; i++) {
goTo.marker("function_call" + i);
verify.allRangesAppearInImplementationList();
verify.allRangesAppearInImplementationList("function_call" + i);
}
4 changes: 2 additions & 2 deletions tests/cases/fourslash/goToImplementationInterfaceMethod_11.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,5 @@
////
//// var x = <Foo> { [|hello: () => {}|] };
//// var y = <Foo> (((({ [|hello: () => {}|] }))));
goTo.marker("reference");
verify.allRangesAppearInImplementationList();

verify.allRangesAppearInImplementationList("reference");
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,4 @@
//// constructor(public f: Foo = { [|hello: 7|] } ) {}
//// }

goTo.marker("reference");
verify.allRangesAppearInImplementationList();
verify.allRangesAppearInImplementationList("reference");
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,4 @@
//// foo.he/*reference*/llo;
//// }

goTo.marker("reference");
verify.allRangesAppearInImplementationList();
verify.allRangesAppearInImplementationList("reference");
4 changes: 1 addition & 3 deletions tests/cases/fourslash/goToImplementationInterface_00.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,4 @@
//// constructor(public f: Foo = [|{ hello() {/**3*/} }|] ) {}
//// }


goTo.marker("interface_definition");
verify.allRangesAppearInImplementationList();
verify.allRangesAppearInImplementationList("interface_definition");
3 changes: 1 addition & 2 deletions tests/cases/fourslash/goToImplementationInterface_01.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,4 @@
//// var y: SuperBar = new SuperBar();
//// var z: AbstractBar = new NotAbstractBar();

goTo.marker("interface_definition");
verify.allRangesAppearInImplementationList();
verify.allRangesAppearInImplementationList("interface_definition");
5 changes: 3 additions & 2 deletions tests/cases/fourslash/goToImplementationInterface_02.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,12 @@
//// }|];
//// }
////
//// let createFoo2 = (): Foo => [|({hello() {}})|];
////
//// function createFooLike() {
//// return {
//// hello() {}
//// };
//// }

goTo.marker("interface_definition");
verify.allRangesAppearInImplementationList();
verify.allRangesAppearInImplementationList("interface_definition");
3 changes: 1 addition & 2 deletions tests/cases/fourslash/goToImplementationInterface_03.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,4 @@
////
//// var x = <Foo> [|{ hello: () => {} }|];

goTo.marker("interface_definition");
verify.allRangesAppearInImplementationList();
verify.allRangesAppearInImplementationList("interface_definition");
4 changes: 1 addition & 3 deletions tests/cases/fourslash/goToImplementationInterface_04.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,4 @@
//// constructor(public f: Foo = [|function(a) {}|] ) {}
//// }


goTo.marker("interface_definition");
verify.allRangesAppearInImplementationList();
verify.allRangesAppearInImplementationList("interface_definition");
4 changes: 1 addition & 3 deletions tests/cases/fourslash/goToImplementationInterface_05.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,4 @@
//// let bar2 = <Foo> [|function(a) {}|];
////


goTo.marker("interface_definition");
verify.allRangesAppearInImplementationList();
verify.allRangesAppearInImplementationList("interface_definition");
4 changes: 1 addition & 3 deletions tests/cases/fourslash/goToImplementationInterface_06.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,4 @@
//// let x: Foo = [|class { constructor (a: number) {} }|];
//// let y = <Foo> [|class { constructor (a: number) {} }|];


goTo.marker("interface_definition");
verify.allRangesAppearInImplementationList();
verify.allRangesAppearInImplementationList("interface_definition");
3 changes: 1 addition & 2 deletions tests/cases/fourslash/goToImplementationInterface_07.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,4 @@
//// return true;
//// }

goTo.marker("interface_definition");
verify.allRangesAppearInImplementationList();
verify.allRangesAppearInImplementationList("interface_definition");
21 changes: 21 additions & 0 deletions tests/cases/fourslash/goToImplementationInterface_08.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/// <reference path='fourslash.ts'/>

// Should not hang on inheritance loops

//// interface Base {
//// hello (): void;
//// }
////
//// interface A extends Base {}
//// interface B extends C, A {}
//// interface C extends B, A {}
////
//// class X implements B {
//// [|hello() {}|]
//// }
////
//// function someFunction(d : A) {
//// d.he/*function_call*/llo();
//// }

verify.allRangesAppearInImplementationList("function_call");
Loading

0 comments on commit 4a37fd7

Please sign in to comment.