Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
531f3aa
Nodes -> Node
DanielRosenwasser May 7, 2015
1a17aba
Added tests.
DanielRosenwasser May 16, 2015
9907d62
Added rename tests.
DanielRosenwasser May 19, 2015
693c587
Removed unused code, use valueDeclaration for symbol.
DanielRosenwasser May 19, 2015
24c5044
Error smarter.
DanielRosenwasser May 19, 2015
70911a3
Adjust tests.
DanielRosenwasser May 20, 2015
cf193a4
Renamed tests.
DanielRosenwasser May 20, 2015
2539ebe
Verify references count.
DanielRosenwasser May 20, 2015
b9ddfcc
Spit out JSON when expected =/= actual.
DanielRosenwasser May 21, 2015
ddb67e4
Fixed tests.
DanielRosenwasser May 21, 2015
2c4bb57
Find the references themselves.
DanielRosenwasser May 21, 2015
33693e8
Adjust expectations.
DanielRosenwasser May 21, 2015
6f925ac
Add rename tests for function expressions.
DanielRosenwasser May 21, 2015
934a3bf
Fixed 'displayName'.
DanielRosenwasser May 21, 2015
035962b
Added test for find all refs of function expressions.
DanielRosenwasser May 28, 2015
bca3d01
Limit scope of function expressions to themselves.
DanielRosenwasser May 28, 2015
6a7ce44
a deer, a female deer -> does
DanielRosenwasser May 29, 2015
abd7db7
Merge branch 'master' into fixDeFaultOfFindAllRefsToMaster
DanielRosenwasser Jun 23, 2015
eeec05d
Merge branch 'master' into fixDeFaultOfFindAllRefsToMaster
DanielRosenwasser Jun 26, 2015
8316369
Added tests.
DanielRosenwasser Jun 29, 2015
4143d1d
Addressed CR feedback.
DanielRosenwasser Jun 29, 2015
cdc8c3b
Use full display name in tests.
DanielRosenwasser Jun 29, 2015
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
16 changes: 11 additions & 5 deletions src/harness/fourslash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -743,7 +743,7 @@ module FourSlash {
var reference = references[i];
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 doe not match, actual: ' + reference.isWriteAccess + ', expected: ' + isWriteAccess + '.');
this.raiseError('verifyReferencesAtPositionListContains failed - item isWriteAccess value does not match, actual: ' + reference.isWriteAccess + ', expected: ' + isWriteAccess + '.');
}
return;
}
Expand Down Expand Up @@ -884,8 +884,16 @@ module FourSlash {
this.activeFile.fileName, this.currentCaretPosition, findInStrings, findInComments);

var ranges = this.getRanges();

if (!references) {
if (ranges.length !== 0) {
this.raiseError(`Expected ${ranges.length} rename locations; got none.`);
}
return;
}

if (ranges.length !== references.length) {
this.raiseError(this.assertionMessage("Rename locations", references.length, ranges.length));
this.raiseError("Rename location count does not match result.\n\nExpected: " + JSON.stringify(ranges) + "\n\nActual:" + JSON.stringify(references));
}

ranges = ranges.sort((r1, r2) => r1.start - r2.start);
Expand All @@ -898,9 +906,7 @@ module FourSlash {
if (reference.textSpan.start !== range.start ||
ts.textSpanEnd(reference.textSpan) !== range.end) {

this.raiseError(this.assertionMessage("Rename location",
"[" + reference.textSpan.start + "," + ts.textSpanEnd(reference.textSpan) + ")",
"[" + range.start + "," + range.end + ")"));
this.raiseError("Rename location results do not match.\n\nExpected: " + JSON.stringify(ranges) + "\n\nActual:" + JSON.stringify(references));
}
}
}
Expand Down
93 changes: 31 additions & 62 deletions src/services/services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4265,7 +4265,7 @@ namespace ts {
isLiteralNameOfPropertyDeclarationOrIndexAccess(node) ||
isNameOfExternalModuleImportOrDeclaration(node)) {

let referencedSymbols = getReferencedSymbolsForNodes(node, sourceFilesToSearch, /*findInStrings:*/ false, /*findInComments:*/ false);
let referencedSymbols = getReferencedSymbolsForNode(node, sourceFilesToSearch, /*findInStrings:*/ false, /*findInComments:*/ false);
return convertReferencedSymbols(referencedSymbols);
}

Expand Down Expand Up @@ -4917,10 +4917,10 @@ namespace ts {
}

Debug.assert(node.kind === SyntaxKind.Identifier || node.kind === SyntaxKind.NumericLiteral || node.kind === SyntaxKind.StringLiteral);
return getReferencedSymbolsForNodes(node, program.getSourceFiles(), findInStrings, findInComments);
return getReferencedSymbolsForNode(node, program.getSourceFiles(), findInStrings, findInComments);
}

function getReferencedSymbolsForNodes(node: Node, sourceFiles: SourceFile[], findInStrings: boolean, findInComments: boolean): ReferencedSymbol[] {
function getReferencedSymbolsForNode(node: Node, sourceFiles: SourceFile[], findInStrings: boolean, findInComments: boolean): ReferencedSymbol[] {
let typeChecker = program.getTypeChecker();

// Labels
Expand Down Expand Up @@ -4955,7 +4955,7 @@ namespace ts {

let declarations = symbol.declarations;

// The symbol was an internal symbol and does not have a declaration e.g.undefined symbol
// The symbol was an internal symbol and does not have a declaration e.g. undefined symbol
if (!declarations || !declarations.length) {
return undefined;
}
Expand All @@ -4965,8 +4965,9 @@ namespace ts {
// Compute the meaning from the location and the symbol it references
let searchMeaning = getIntersectingMeaningFromDeclarations(getMeaningFromLocation(node), declarations);

// Get the text to search for, we need to normalize it as external module names will have quote
let declaredName = getDeclaredName(symbol, node);
// Get the text to search for.
// Note: if this is an external module symbol, the name doesn't include quotes.
let declaredName = getDeclaredName(typeChecker, symbol, node);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What you mean by normalization? stripping quotes in getDeclaredName? if so could you mention this in the comment


// Try to get the smallest valid scope that we can limit our search to;
// otherwise we'll need to search globally (i.e. include each file).
Expand Down Expand Up @@ -5013,76 +5014,43 @@ namespace ts {
};
}

function isImportOrExportSpecifierName(location: Node): boolean {
return location.parent &&
(location.parent.kind === SyntaxKind.ImportSpecifier || location.parent.kind === SyntaxKind.ExportSpecifier) &&
(<ImportOrExportSpecifier>location.parent).propertyName === location;
}

function isImportOrExportSpecifierImportSymbol(symbol: Symbol) {
return (symbol.flags & SymbolFlags.Alias) && forEach(symbol.declarations, declaration => {
return declaration.kind === SyntaxKind.ImportSpecifier || declaration.kind === SyntaxKind.ExportSpecifier;
});
}

function getDeclaredName(symbol: Symbol, location: Node) {
// Special case for function expressions, whose names are solely local to their bodies.
let functionExpression = forEach(symbol.declarations, d => d.kind === SyntaxKind.FunctionExpression ? <FunctionExpression>d : undefined);

// When a name gets interned into a SourceFile's 'identifiers' Map,
// its name is escaped and stored in the same way its symbol name/identifier
// name should be stored. Function expressions, however, are a special case,
// because despite sometimes having a name, the binder unconditionally binds them
// to a symbol with the name "__function".
let name: string;
if (functionExpression && functionExpression.name) {
name = functionExpression.name.text;
}

// If this is an export or import specifier it could have been renamed using the as syntax.
// if so we want to search for whatever under the cursor, the symbol is pointing to the alias (name)
// so check for the propertyName.
if (isImportOrExportSpecifierName(location)) {
return location.getText();
}

name = typeChecker.symbolToString(symbol);

return stripQuotes(name);
}

function getInternedName(symbol: Symbol, location: Node, declarations: Declaration[]): string {
// If this is an export or import specifier it could have been renamed using the as syntax.
// if so we want to search for whatever under the cursor, the symbol is pointing to the alias (name)
// so check for the propertyName.
// If this is an export or import specifier it could have been renamed using the 'as' syntax.
// If so we want to search for whatever under the cursor.
if (isImportOrExportSpecifierName(location)) {
return location.getText();
}

// Special case for function expressions, whose names are solely local to their bodies.
let functionExpression = forEach(declarations, d => d.kind === SyntaxKind.FunctionExpression ? <FunctionExpression>d : undefined);

// When a name gets interned into a SourceFile's 'identifiers' Map,
// its name is escaped and stored in the same way its symbol name/identifier
// name should be stored. Function expressions, however, are a special case,
// because despite sometimes having a name, the binder unconditionally binds them
// to a symbol with the name "__function".
let name = functionExpression && functionExpression.name
? functionExpression.name.text
: symbol.name;

return stripQuotes(name);
}
// Try to get the local symbol if we're dealing with an 'export default'
// since that symbol has the "true" name.
let localExportDefaultSymbol = getLocalSymbolForExportDefault(symbol);
symbol = localExportDefaultSymbol || symbol;

function stripQuotes(name: string) {
let length = name.length;
if (length >= 2 && name.charCodeAt(0) === CharacterCodes.doubleQuote && name.charCodeAt(length - 1) === CharacterCodes.doubleQuote) {
return name.substring(1, length - 1);
};
return name;
return stripQuotes(symbol.name);
}

/**
* Determines the smallest scope in which a symbol may have named references.
* Note that not every construct has been accounted for. This function can
* probably be improved.
*
* @returns undefined if the scope cannot be determined, implying that
* a reference to a symbol can occur anywhere.
*/
function getSymbolScope(symbol: Symbol): Node {
// If this is the symbol of a function expression, then named references
// are limited to its own scope.
let valueDeclaration = symbol.valueDeclaration;
if (valueDeclaration && valueDeclaration.kind === SyntaxKind.FunctionExpression) {
return valueDeclaration;
}

// If this is private property or method, the scope is the containing class
if (symbol.flags & (SymbolFlags.Property | SymbolFlags.Method)) {
let privateDeclaration = forEach(symbol.getDeclarations(), d => (d.flags & NodeFlags.Private) ? d : undefined);
Expand Down Expand Up @@ -6724,12 +6692,13 @@ namespace ts {
}
}

let displayName = getDeclaredName(typeChecker, symbol, node);
let kind = getSymbolKind(symbol, node);
if (kind) {
return {
canRename: true,
localizedErrorMessage: undefined,
displayName: symbol.name,
displayName,
fullDisplayName: typeChecker.getFullyQualifiedName(symbol),
kind: kind,
kindModifiers: getSymbolModifiers(symbol),
Expand Down
30 changes: 30 additions & 0 deletions src/services/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -652,4 +652,34 @@ namespace ts {
typechecker.getSymbolDisplayBuilder().buildSignatureDisplay(signature, writer, enclosingDeclaration, flags);
});
}

export function getDeclaredName(typeChecker: TypeChecker, symbol: Symbol, location: Node): string {
// If this is an export or import specifier it could have been renamed using the 'as' syntax.
// If so we want to search for whatever is under the cursor.
if (isImportOrExportSpecifierName(location)) {
return location.getText();
}

// Try to get the local symbol if we're dealing with an 'export default'
// since that symbol has the "true" name.
let localExportDefaultSymbol = getLocalSymbolForExportDefault(symbol);

let name = typeChecker.symbolToString(localExportDefaultSymbol || symbol);

return stripQuotes(name);
}

export function isImportOrExportSpecifierName(location: Node): boolean {
return location.parent &&
(location.parent.kind === SyntaxKind.ImportSpecifier || location.parent.kind === SyntaxKind.ExportSpecifier) &&
(<ImportOrExportSpecifier>location.parent).propertyName === location;
}

export function stripQuotes(name: string) {
let length = name.length;
if (length >= 2 && name.charCodeAt(0) === CharacterCodes.doubleQuote && name.charCodeAt(length - 1) === CharacterCodes.doubleQuote) {
return name.substring(1, length - 1);
};
return name;
}
}
18 changes: 18 additions & 0 deletions tests/cases/fourslash/findAllRefsForDefaultExport01.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/// <reference path='fourslash.ts'/>

////export default class [|DefaultExportedClass|] {
////}
////
////var x: [|DefaultExportedClass|];
////
////var y = new [|DefaultExportedClass|];

let ranges = test.ranges()
for (let range of ranges) {
goTo.position(range.start);

verify.referencesCountIs(ranges.length);
for (let expectedReference of ranges) {
verify.referencesAtPositionContains(expectedReference);
}
}
19 changes: 19 additions & 0 deletions tests/cases/fourslash/findAllRefsForDefaultExport02.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/// <reference path='fourslash.ts'/>

////export default function [|DefaultExportedFunction|]() {
//// return [|DefaultExportedFunction|]
////}
////
////var x: typeof [|DefaultExportedFunction|];
////
////var y = [|DefaultExportedFunction|]();

let ranges = test.ranges()
for (let range of ranges) {
goTo.position(range.start);

verify.referencesCountIs(ranges.length);
for (let expectedReference of ranges) {
verify.referencesAtPositionContains(expectedReference);
}
}
25 changes: 25 additions & 0 deletions tests/cases/fourslash/findAllRefsForDefaultExport03.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/// <reference path='fourslash.ts'/>

////function [|f|]() {
//// return 100;
////}
////
////export default [|f|];
////
////var x: typeof [|f|];
////
////var y = [|f|]();
////
////namespace [|f|] {
//// var local = 100;
////}

let ranges = test.ranges()
for (let range of ranges) {
goTo.position(range.start);

verify.referencesCountIs(ranges.length);
for (let expectedReference of ranges) {
verify.referencesAtPositionContains(expectedReference);
}
}
28 changes: 28 additions & 0 deletions tests/cases/fourslash/findAllRefsForDefaultExport04.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/// <reference path='fourslash.ts'/>

////function f() {
//// return 100;
////}
////
////export default [|f|];
////
////var x: typeof f;
////
////var y = f();
////
////namespace /**/[|f|] {
////}

// The function 'f' and the namespace 'f' don't get merged,
// but the 'export default' site, includes both meanings.

// Here we are testing whether the 'export default'
// site is included in the references to the namespace.

goTo.marker();
let ranges = test.ranges();
verify.referencesCountIs(ranges.length);

for (let expectedReference of ranges) {
verify.referencesAtPositionContains(expectedReference);
}
28 changes: 28 additions & 0 deletions tests/cases/fourslash/findAllRefsForDefaultExport05.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/// <reference path='fourslash.ts'/>

////function /**/[|f|]() {
//// return 100;
////}
////
////export default [|f|];
////
////var x: typeof [|f|];
////
////var y = [|f|]();
////
////namespace f {
////}

// The function 'f' and the namespace 'f' don't get merged,
// but the 'export default' site, includes both meanings.

// Here we are testing whether the 'export default' site
// and all value-uses of 'f' are included in the references to the function.

goTo.marker();
let ranges = test.ranges();
verify.referencesCountIs(ranges.length);

for (let expectedReference of ranges) {
verify.referencesAtPositionContains(expectedReference);
}
28 changes: 28 additions & 0 deletions tests/cases/fourslash/findAllRefsForDefaultExport06.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/// <reference path='fourslash.ts'/>

////function [|f|]() {
//// return 100;
////}
////
////export default /**/[|f|];
////
////var x: typeof [|f|];
////
////var y = [|f|]();
////
////namespace [|f|] {
////}

// The function 'f' and the namespace 'f' don't get merged,
// but the 'export default' site, includes both meanings.

// Here we are testing whether the 'export default' site
// and all value-uses of 'f' are included in the references to the function.

goTo.marker();
let ranges = test.ranges();
verify.referencesCountIs(ranges.length);

for (let expectedReference of ranges) {
verify.referencesAtPositionContains(expectedReference);
}
Loading