Skip to content

Include declarationSpan as relevant declaration span when defintion or other places are declaration name #31587

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
merged 39 commits into from
Jun 18, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
fd86f40
Include declarationSpan as relevant declaration span when defintion o…
sheetalkamat May 23, 2019
1d830ff
Start fixing fourslash tests
sheetalkamat May 24, 2019
6a961b5
More tests
sheetalkamat May 28, 2019
8948fe4
Rename declarationRange to declarationRangeIndex
sheetalkamat May 30, 2019
f37ae23
More test fixes
sheetalkamat May 30, 2019
bbfbf8f
Use import export sepcifier as declaration for the property name of i…
sheetalkamat May 30, 2019
2fc1143
More tests
sheetalkamat May 31, 2019
15ce996
Export assignment identifier use ExportAssigment as declaration
sheetalkamat May 31, 2019
bbd2d00
More tests
sheetalkamat May 31, 2019
6c04a0d
For property name of binding element use binding element as preview node
sheetalkamat May 31, 2019
bcf7752
More tests
sheetalkamat May 31, 2019
9703b3d
Show property assignment for special property assignments in js files
sheetalkamat May 31, 2019
b61813e
More test cases
sheetalkamat May 31, 2019
dfb613c
Use for-of declaration list + expression as span for preview
sheetalkamat May 31, 2019
c0537d9
More tests
sheetalkamat May 31, 2019
0fee3b0
Handle destructuring assignments
sheetalkamat Jun 3, 2019
01bbc4d
More tests
sheetalkamat Jun 3, 2019
e41533a
Handle computed property names
sheetalkamat Jun 3, 2019
34624a0
More Tests
sheetalkamat Jun 3, 2019
424f2c9
More tests
sheetalkamat Jun 4, 2019
eaeeb06
Handle when declarationSpan from declarationNode is undefined
sheetalkamat Jun 4, 2019
cc1cb54
More tests
sheetalkamat Jun 4, 2019
004488c
Set declaration span only if its not same as own span
sheetalkamat Jun 4, 2019
35c0499
More tests
sheetalkamat Jun 5, 2019
edffcce
Take optional texts to verify parameter for rangesWithSameTextAreRena…
sheetalkamat Jun 5, 2019
6dc2ba7
Take optional string of range text for singleReferenceGroup
sheetalkamat Jun 5, 2019
d1dc837
Cache ranges by text
sheetalkamat Jun 5, 2019
e1e1603
More tests
sheetalkamat Jun 5, 2019
1163a93
Handle default keyword of default export
sheetalkamat Jun 5, 2019
64de998
More tests
sheetalkamat Jun 6, 2019
e4a2dd5
Handle export keyword of export assignment
sheetalkamat Jun 6, 2019
018026a
More tests
sheetalkamat Jun 6, 2019
edad317
Fourslash server tests
sheetalkamat Jun 6, 2019
768c9ed
Handle jsx Opening, Closing and Self closing tags
sheetalkamat Jun 6, 2019
a120c59
Handle Lable
sheetalkamat Jun 6, 2019
a67b375
Handle module specifiers
sheetalkamat Jun 6, 2019
a84ed93
Merge branch 'master' into definitionSpan
sheetalkamat Jun 6, 2019
da2aa97
Revert to using spread instead of mutating value later
sheetalkamat Jun 12, 2019
73bf268
Rename to use contextSpan
sheetalkamat Jun 13, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
11 changes: 9 additions & 2 deletions src/harness/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -395,8 +395,15 @@ namespace ts.server {
const locations: RenameLocation[] = [];
for (const entry of body.locs) {
const fileName = entry.file;
for (const { start, end, ...prefixSuffixText } of entry.locs) {
locations.push({ textSpan: this.decodeSpan({ start, end }, fileName), fileName, ...prefixSuffixText });
for (const { start, end, contextStart, contextEnd, ...prefixSuffixText } of entry.locs) {
locations.push({
textSpan: this.decodeSpan({ start, end }, fileName),
fileName,
...(contextStart !== undefined ?
{ contextSpan: this.decodeSpan({ start: contextStart, end: contextEnd! }, fileName) } :
undefined),
...prefixSuffixText
});
}
}

Expand Down
50 changes: 35 additions & 15 deletions src/harness/fourslash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ namespace FourSlash {
* is a range with `text in range` "selected".
*/
ranges: Range[];
rangesByText?: ts.MultiMap<Range>;
}

export interface Marker {
Expand Down Expand Up @@ -955,12 +956,15 @@ namespace FourSlash {
const fullExpected = ts.map<FourSlashInterface.ReferenceGroup, ReferenceGroupJson>(parts, ({ definition, ranges }) => ({
definition: typeof definition === "string" ? definition : { ...definition, range: ts.createTextSpanFromRange(definition.range) },
references: ranges.map<ts.ReferenceEntry>(r => {
const { isWriteAccess = false, isDefinition = false, isInString } = (r.marker && r.marker.data || {}) as { isWriteAccess?: boolean, isDefinition?: boolean, isInString?: true };
const { isWriteAccess = false, isDefinition = false, isInString, contextRangeIndex } = (r.marker && r.marker.data || {}) as { isWriteAccess?: boolean, isDefinition?: boolean, isInString?: true, contextRangeIndex?: number };
return {
fileName: r.fileName,
textSpan: ts.createTextSpanFromRange(r),
isWriteAccess,
isDefinition,
...(contextRangeIndex !== undefined ?
{ contextSpan: ts.createTextSpanFromRange(this.getRanges()[contextRangeIndex]) } :
undefined),
...(isInString ? { isInString: true } : undefined),
};
}),
Expand Down Expand Up @@ -997,8 +1001,8 @@ namespace FourSlash {
assert.deepEqual<ReadonlyArray<ts.ReferenceEntry> | undefined>(refs, expected);
}

public verifySingleReferenceGroup(definition: FourSlashInterface.ReferenceGroupDefinition, ranges?: Range[]) {
ranges = ranges || this.getRanges();
public verifySingleReferenceGroup(definition: FourSlashInterface.ReferenceGroupDefinition, ranges?: Range[] | string) {
ranges = ts.isString(ranges) ? this.rangesByText().get(ranges)! : ranges || this.getRanges();
this.verifyReferenceGroups(ranges, [{ definition, ranges }]);
}

Expand All @@ -1011,7 +1015,7 @@ Actual: ${stringify(fullActual)}`);
};

if ((actual === undefined) !== (expected === undefined)) {
fail(`Expected ${expected}, got ${actual}`);
fail(`Expected ${stringify(expected)}, got ${stringify(actual)}`);
}

for (const key in actual) {
Expand All @@ -1021,7 +1025,7 @@ Actual: ${stringify(fullActual)}`);
recur(ak, ek, path ? path + "." + key : key);
}
else if (ak !== ek) {
fail(`Expected '${key}' to be '${ek}', got '${ak}'`);
fail(`Expected '${key}' to be '${stringify(ek)}', got '${stringify(ak)}'`);
}
}
}
Expand Down Expand Up @@ -1189,7 +1193,15 @@ Actual: ${stringify(fullActual)}`);
locations && ts.sort(locations, (r1, r2) => ts.compareStringsCaseSensitive(r1.fileName, r2.fileName) || r1.textSpan.start - r2.textSpan.start);
assert.deepEqual(sort(references), sort(ranges.map((rangeOrOptions): ts.RenameLocation => {
const { range, ...prefixSuffixText } = "range" in rangeOrOptions ? rangeOrOptions : { range: rangeOrOptions };
return { fileName: range.fileName, textSpan: ts.createTextSpanFromRange(range), ...prefixSuffixText };
const { contextRangeIndex } = (range.marker && range.marker.data || {}) as { contextRangeIndex?: number; };
return {
fileName: range.fileName,
textSpan: ts.createTextSpanFromRange(range),
...(contextRangeIndex !== undefined ?
{ contextSpan: ts.createTextSpanFromRange(this.getRanges()[contextRangeIndex]) } :
undefined),
...prefixSuffixText
};
})));
}
}
Expand Down Expand Up @@ -1844,6 +1856,7 @@ Actual: ${stringify(fullActual)}`);
range.end = updatePosition(range.end, editStart, editEnd, newText);
}
}
this.testData.rangesByText = undefined;
}

private removeWhitespace(text: string): string {
Expand Down Expand Up @@ -2026,7 +2039,9 @@ Actual: ${stringify(fullActual)}`);
}

public rangesByText(): ts.Map<Range[]> {
if (this.testData.rangesByText) return this.testData.rangesByText;
const result = ts.createMultiMap<Range>();
this.testData.rangesByText = result;
for (const range of this.getRanges()) {
const text = this.rangeText(range);
result.add(text, range);
Expand Down Expand Up @@ -2700,8 +2715,8 @@ Actual: ${stringify(fullActual)}`);
return this.languageService.getDocumentHighlights(this.activeFile.fileName, this.currentCaretPosition, filesToSearch);
}

public verifyRangesAreOccurrences(isWriteAccess?: boolean) {
const ranges = this.getRanges();
public verifyRangesAreOccurrences(isWriteAccess?: boolean, ranges?: Range[]) {
ranges = ranges || this.getRanges();
for (const r of ranges) {
this.goToRangeStart(r);
this.verifyOccurrencesAtPositionListCount(ranges.length);
Expand All @@ -2711,8 +2726,13 @@ Actual: ${stringify(fullActual)}`);
}
}

public verifyRangesWithSameTextAreRenameLocations() {
this.rangesByText().forEach(ranges => this.verifyRangesAreRenameLocations(ranges));
public verifyRangesWithSameTextAreRenameLocations(...texts: string[]) {
if (texts.length) {
texts.forEach(text => this.verifyRangesAreRenameLocations(this.rangesByText().get(text)!));
}
else {
this.rangesByText().forEach(ranges => this.verifyRangesAreRenameLocations(ranges));
}
}

public verifyRangesWithSameTextAreDocumentHighlights() {
Expand Down Expand Up @@ -3957,7 +3977,7 @@ namespace FourSlashInterface {
this.state.verifyGetReferencesForServerTest(expected);
}

public singleReferenceGroup(definition: ReferenceGroupDefinition, ranges?: FourSlash.Range[]) {
public singleReferenceGroup(definition: ReferenceGroupDefinition, ranges?: FourSlash.Range[] | string) {
this.state.verifySingleReferenceGroup(definition, ranges);
}

Expand Down Expand Up @@ -4075,12 +4095,12 @@ namespace FourSlashInterface {
this.state.verifyOccurrencesAtPositionListCount(expectedCount);
}

public rangesAreOccurrences(isWriteAccess?: boolean) {
this.state.verifyRangesAreOccurrences(isWriteAccess);
public rangesAreOccurrences(isWriteAccess?: boolean, ranges?: FourSlash.Range[]) {
this.state.verifyRangesAreOccurrences(isWriteAccess, ranges);
}

public rangesWithSameTextAreRenameLocations() {
this.state.verifyRangesWithSameTextAreRenameLocations();
public rangesWithSameTextAreRenameLocations(...texts: string[]) {
this.state.verifyRangesWithSameTextAreRenameLocations(...texts);
}

public rangesAreRenameLocations(options?: FourSlash.Range[] | { findInStrings?: boolean, findInComments?: boolean, ranges?: FourSlash.Range[] }) {
Expand Down
24 changes: 16 additions & 8 deletions src/server/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -872,16 +872,24 @@ namespace ts.server.protocol {
file: string;
}

export interface TextSpanWithContext extends TextSpan {
contextStart?: Location;
contextEnd?: Location;
}

export interface FileSpanWithContext extends FileSpan, TextSpanWithContext {
}

export interface DefinitionInfoAndBoundSpan {
definitions: ReadonlyArray<FileSpan>;
definitions: ReadonlyArray<FileSpanWithContext>;
textSpan: TextSpan;
}

/**
* Definition response message. Gives text range for definition.
*/
export interface DefinitionResponse extends Response {
body?: FileSpan[];
body?: FileSpanWithContext[];
}

export interface DefinitionInfoAndBoundSpanReponse extends Response {
Expand All @@ -892,14 +900,14 @@ namespace ts.server.protocol {
* Definition response message. Gives text range for definition.
*/
export interface TypeDefinitionResponse extends Response {
body?: FileSpan[];
body?: FileSpanWithContext[];
}

/**
* Implementation response message. Gives text range for implementations.
*/
export interface ImplementationResponse extends Response {
body?: FileSpan[];
body?: FileSpanWithContext[];
}

/**
Expand Down Expand Up @@ -942,7 +950,7 @@ namespace ts.server.protocol {
}

/** @deprecated */
export interface OccurrencesResponseItem extends FileSpan {
export interface OccurrencesResponseItem extends FileSpanWithContext {
/**
* True if the occurrence is a write location, false otherwise.
*/
Expand Down Expand Up @@ -972,7 +980,7 @@ namespace ts.server.protocol {
/**
* Span augmented with extra information that denotes the kind of the highlighting to be used for span.
*/
export interface HighlightSpan extends TextSpan {
export interface HighlightSpan extends TextSpanWithContext {
kind: HighlightSpanKind;
}

Expand Down Expand Up @@ -1007,7 +1015,7 @@ namespace ts.server.protocol {
command: CommandTypes.References;
}

export interface ReferencesResponseItem extends FileSpan {
export interface ReferencesResponseItem extends FileSpanWithContext {
/** Text of line containing the reference. Including this
* with the response avoids latency of editor loading files
* to show text of reference line (the server already has
Expand Down Expand Up @@ -1150,7 +1158,7 @@ namespace ts.server.protocol {
locs: RenameTextSpan[];
}

export interface RenameTextSpan extends TextSpan {
export interface RenameTextSpan extends TextSpanWithContext {
readonly prefixText?: string;
readonly suffixText?: string;
}
Expand Down
Loading