Skip to content

Commit f90f8e8

Browse files
CyrusNajmabadiCyrusNajmabadi
committed
CR feedback.
1 parent 441735a commit f90f8e8

File tree

3 files changed

+100
-29
lines changed

3 files changed

+100
-29
lines changed

‎src/services/navigateTo.ts‎

Lines changed: 93 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,12 @@ module ts.NavigateTo {
2626
// It was a match! If the pattern has dots in it, then also see if hte
2727
// declaration container matches as well.
2828
if (patternMatcher.patternContainsDots) {
29-
var containerName = getContainerName(getContainerNode(declaration));
30-
matches = patternMatcher.getMatches(name, containerName);
29+
var containers = getContainers(declaration);
30+
if (!containers) {
31+
return undefined;
32+
}
33+
34+
matches = patternMatcher.getMatches(containers, name);
3135

3236
if (!matches) {
3337
continue;
@@ -36,7 +40,7 @@ module ts.NavigateTo {
3640

3741
var fileName = sourceFile.fileName;
3842
var matchKind = bestMatchKind(matches);
39-
rawItems.push({ name, fileName, matchKind, isCaseSensitive: isCaseSensitive(matches), declaration });
43+
rawItems.push({ name, fileName, matchKind, isCaseSensitive: allMatchesAreCaseSensitive(matches), declaration });
4044
}
4145
}
4246
});
@@ -50,7 +54,7 @@ module ts.NavigateTo {
5054

5155
return items;
5256

53-
function isCaseSensitive(matches: PatternMatch[]): boolean {
57+
function allMatchesAreCaseSensitive(matches: PatternMatch[]): boolean {
5458
Debug.assert(matches.length > 0);
5559

5660
// This is a case sensitive match, only if all the submatches were case sensitive.
@@ -64,26 +68,99 @@ module ts.NavigateTo {
6468
}
6569

6670
function getDeclarationName(declaration: Declaration): string {
67-
if (declaration.name.kind === SyntaxKind.Identifier ||
68-
declaration.name.kind === SyntaxKind.StringLiteral ||
69-
declaration.name.kind === SyntaxKind.NumericLiteral) {
71+
var result = getTextOfIdentifierOrLiteral(declaration.name);
72+
if (result !== undefined) {
73+
return result;
74+
}
75+
76+
if (declaration.name.kind === SyntaxKind.ComputedPropertyName) {
77+
var expr = (<ComputedPropertyName>declaration.name).expression;
78+
if (expr.kind === SyntaxKind.PropertyAccessExpression) {
79+
return (<PropertyAccessExpression>expr).name.text;
80+
}
81+
82+
return getTextOfIdentifierOrLiteral(expr);
83+
}
84+
85+
return undefined;
86+
}
87+
88+
function getTextOfIdentifierOrLiteral(node: Node) {
89+
if (node.kind === SyntaxKind.Identifier ||
90+
node.kind === SyntaxKind.StringLiteral ||
91+
node.kind === SyntaxKind.NumericLiteral) {
7092

71-
return (<Identifier>declaration.name).text;
93+
return (<Identifier | LiteralExpression>node).text;
7294
}
7395

7496
return undefined;
7597
}
7698

77-
function getContainerName(declaration: Declaration): string {
78-
var name = getDeclarationName(declaration);
79-
if (name === undefined) {
80-
return undefined;
99+
function tryAddSingleDeclarationName(declaration: Declaration, containers: string[]) {
100+
if (declaration && declaration.name) {
101+
var text = getTextOfIdentifierOrLiteral(declaration.name);
102+
if (text !== undefined) {
103+
containers.unshift(text);
104+
}
105+
else if (declaration.name.kind === SyntaxKind.ComputedPropertyName) {
106+
return tryAddComputedPropertyName((<ComputedPropertyName>declaration.name).expression, containers, /*includeLastPortion:*/ true);
107+
}
108+
else {
109+
// Don't know how to add this.
110+
return false
111+
}
112+
}
113+
114+
return true;
115+
}
116+
117+
// Only added the names of computed properties if they're simple dotted expressions, like:
118+
//
119+
// [X.Y.Z]() { }
120+
function tryAddComputedPropertyName(expression: Expression, containers: string[], includeLastPortion: boolean): boolean {
121+
var text = getTextOfIdentifierOrLiteral(expression);
122+
if (text !== undefined) {
123+
if (includeLastPortion) {
124+
containers.unshift(text);
125+
}
126+
return true;
127+
}
128+
129+
if (expression.kind === SyntaxKind.PropertyAccessExpression) {
130+
var propertyAccess = <PropertyAccessExpression>expression;
131+
if (includeLastPortion) {
132+
containers.unshift(propertyAccess.name.text);
133+
}
134+
135+
return tryAddComputedPropertyName(propertyAccess.expression, containers, /*includeLastPortion:*/ true);
136+
}
137+
138+
return false;
139+
}
140+
141+
function getContainers(declaration: Declaration) {
142+
var containers: string[] = [];
143+
144+
// First, if we started with a computed property name, then add all but the last
145+
// portion into the container array.
146+
if (declaration.name.kind === SyntaxKind.ComputedPropertyName) {
147+
if (!tryAddComputedPropertyName((<ComputedPropertyName>declaration.name).expression, containers, /*includeLastPortion:*/ false)) {
148+
return undefined;
149+
}
150+
}
151+
152+
// Now, walk up our containers, adding all their names to the container array.
153+
declaration = getContainerNode(declaration);
154+
155+
while (declaration) {
156+
if (!tryAddSingleDeclarationName(declaration, containers)) {
157+
return undefined;
158+
}
159+
160+
declaration = getContainerNode(declaration);
81161
}
82162

83-
var container = getContainerNode(declaration);
84-
return container && container.name
85-
? getContainerName(container) + "." + name
86-
: name;
163+
return containers;
87164
}
88165

89166
function bestMatchKind(matches: PatternMatch[]) {

‎src/services/patternMatcher.ts‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ module ts {
4646
// Fully checks a candidate, with an dotted container, against the search pattern.
4747
// The candidate must match the last part of the search pattern, and the dotted container
4848
// must match the preceding segments of the pattern.
49-
getMatches(candidate: string, dottedContainer: string): PatternMatch[];
49+
getMatches(candidateContainers: string[], candidate: string): PatternMatch[];
5050

5151
// Whether or not the pattern contained dots or not. Clients can use this to determine
5252
// If they should call getMatches, or if getMatchesForLastSegmentOfPattern is sufficient.
@@ -139,7 +139,7 @@ module ts {
139139
return matchSegment(candidate, lastOrUndefined(dotSeparatedSegments));
140140
}
141141

142-
function getMatches(candidate: string, dottedContainer: string): PatternMatch[] {
142+
function getMatches(candidateContainers: string[], candidate: string): PatternMatch[] {
143143
if (skipMatch(candidate)) {
144144
return undefined;
145145
}
@@ -152,12 +152,11 @@ module ts {
152152
return undefined;
153153
}
154154

155-
dottedContainer = dottedContainer || "";
156-
var containerParts = dottedContainer.split(".");
155+
candidateContainers = candidateContainers || [];
157156

158157
// -1 because the last part was checked against the name, and only the rest
159158
// of the parts are checked against the container.
160-
if (dotSeparatedSegments.length - 1 > containerParts.length) {
159+
if (dotSeparatedSegments.length - 1 > candidateContainers.length) {
161160
// There weren't enough container parts to match against the pattern parts.
162161
// So this definitely doesn't match.
163162
return undefined;
@@ -167,12 +166,12 @@ module ts {
167166
// the dotted parts match up correctly.
168167
var totalMatch = candidateMatch;
169168

170-
for (var i = dotSeparatedSegments.length - 2, j = containerParts.length - 1;
169+
for (var i = dotSeparatedSegments.length - 2, j = candidateContainers.length - 1;
171170
i >= 0;
172171
i--, j--) {
173172

174173
var segment = dotSeparatedSegments[i];
175-
var containerName = containerParts[j];
174+
var containerName = candidateContainers[j];
176175

177176
var containerMatch = matchSegment(containerName, segment);
178177
if (!containerMatch) {

‎tests/cases/unittests/services/patternMatcher.ts‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,6 @@ describe('PatternMatcher', function () {
9696

9797
describe("SingleWordPattern", () => {
9898
it("PreferCaseSensitiveExact", () => {
99-
debugger;
10099
var match = getFirstMatch("Foo", "Foo");
101100

102101
assert.equal(ts.PatternMatchKind.exact, match.kind);
@@ -125,7 +124,6 @@ describe('PatternMatcher', function () {
125124
});
126125

127126
it("PreferCaseSensitiveCamelCaseMatchSimple", () => {
128-
debugger;
129127
var match = getFirstMatch("FogBar", "FB");
130128

131129
assert.equal(ts.PatternMatchKind.camelCase, match.kind);
@@ -172,7 +170,6 @@ describe('PatternMatcher', function () {
172170
});
173171

174172
it("PreferCaseSensitiveLowercasePattern", () => {
175-
debugger;
176173
var match = getFirstMatch("FogBar", "b");
177174

178175
assert.equal(ts.PatternMatchKind.substring, match.kind);
@@ -266,7 +263,6 @@ describe('PatternMatcher', function () {
266263
});
267264

268265
it("AllLowerPattern1", () => {
269-
debugger;
270266
var match = getFirstMatch("FogBarChangedEventArgs", "changedeventargs");
271267

272268
assert.isTrue(undefined !== match);
@@ -473,7 +469,6 @@ describe('PatternMatcher', function () {
473469
});
474470

475471
it("DottedPattern7", () => {
476-
debugger;
477472
var match = getFirstMatch("UIElement", "UIElement");
478473
var match = getFirstMatch("GetKeyword", "UIElement");
479474
assert.isTrue(match === undefined);
@@ -490,7 +485,7 @@ describe('PatternMatcher', function () {
490485
}
491486

492487
function getFirstMatchForDottedPattern(dottedContainer: string, candidate: string, pattern: string): ts.PatternMatch {
493-
var matches = ts.createPatternMatcher(pattern).getMatches(candidate, dottedContainer);
488+
var matches = ts.createPatternMatcher(pattern).getMatches(dottedContainer.split("."), candidate);
494489
return matches ? matches[0] : undefined;
495490
}
496491

0 commit comments

Comments
 (0)