From fd1b5875e233203ccdc749b090c4e218e4d8acad Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sun, 22 Feb 2015 21:25:43 -0800 Subject: [PATCH 1/3] Move NavigateTo over to using the new pattern matcher. --- src/server/client.ts | 1 + src/server/protocol.d.ts | 5 + src/services/navigateTo.ts | 145 ++++++++++-------- src/services/patternMatcher.ts | 24 +-- src/services/services.ts | 5 +- .../baselines/reference/APISample_compile.js | 1 + .../reference/APISample_compile.types | 3 + tests/baselines/reference/APISample_linter.js | 1 + .../reference/APISample_linter.types | 3 + .../reference/APISample_transform.js | 1 + .../reference/APISample_transform.types | 3 + .../baselines/reference/APISample_watcher.js | 1 + .../reference/APISample_watcher.types | 3 + tests/cases/fourslash/declareFunction.ts | 2 +- .../navigationItemsSubStringMatch.ts | 16 +- .../navigationItemsSubStringMatch2.ts | 4 +- tests/cases/fourslash/server/navto.ts | 10 +- .../unittests/services/patternMatcher.ts | 86 +++++------ 18 files changed, 175 insertions(+), 139 deletions(-) diff --git a/src/server/client.ts b/src/server/client.ts index 60486e8037..de6ab1e773 100644 --- a/src/server/client.ts +++ b/src/server/client.ts @@ -229,6 +229,7 @@ module ts.server { kind: entry.kind, kindModifiers: entry.kindModifiers, matchKind: entry.matchKind, + isCaseSensitive: entry.isCaseSensitive, fileName: fileName, textSpan: ts.createTextSpanFromBounds(start, end) }; diff --git a/src/server/protocol.d.ts b/src/server/protocol.d.ts index b07fb20d48..ccfddf8a54 100644 --- a/src/server/protocol.d.ts +++ b/src/server/protocol.d.ts @@ -707,6 +707,11 @@ declare module ts.server.protocol { * exact, substring, or prefix. */ matchKind?: string; + + /** + * If this was a case sensitive or insensitive match. + */ + isCaseSensitive?: boolean; /** * Optional modifiers for the kind (such as 'public'). diff --git a/src/services/navigateTo.ts b/src/services/navigateTo.ts index d008cac80f..11edbe9592 100644 --- a/src/services/navigateTo.ts +++ b/src/services/navigateTo.ts @@ -1,35 +1,42 @@ module ts.NavigateTo { - type RawNavigateToItem = { name: string; fileName: string; matchKind: MatchKind; declaration: Declaration }; - - enum MatchKind { - none = 0, - exact = 1, - substring = 2, - prefix = 3 - } - - export function getNavigateToItems(program: Program, cancellationToken: CancellationTokenObject, searchValue: string, maxResultCount: number): NavigateToItem[]{ - // Split search value in terms array - var terms = searchValue.split(" "); - - // default NavigateTo approach: if search term contains only lower-case chars - use case-insensitive search, otherwise switch to case-sensitive version - var searchTerms = map(terms, t => ({ caseSensitive: hasAnyUpperCaseCharacter(t), term: t })); + type RawNavigateToItem = { name: string; fileName: string; matchKind: PatternMatchKind; isCaseSensitive: boolean; declaration: Declaration }; + export function getNavigateToItems(program: Program, cancellationToken: CancellationTokenObject, searchValue: string, maxResultCount: number): NavigateToItem[] { + var patternMatcher = createPatternMatcher(searchValue); var rawItems: RawNavigateToItem[] = []; // Search the declarations in all files and output matched NavigateToItem into array of NavigateToItem[] forEach(program.getSourceFiles(), sourceFile => { cancellationToken.throwIfCancellationRequested(); - var fileName = sourceFile.fileName; var declarations = sourceFile.getNamedDeclarations(); for (var i = 0, n = declarations.length; i < n; i++) { var declaration = declarations[i]; - // TODO(jfreeman): Skip this declaration if it has a computed name - var name = (declaration.name).text; - var matchKind = getMatchKind(searchTerms, name); - if (matchKind !== MatchKind.none) { - rawItems.push({ name, fileName, matchKind, declaration }); + var name = getDeclarationName(declaration); + if (name !== undefined) { + + // First do a quick check to see if the name of the declaration matches the + // last portion of the (possibly) dotted name they're searching for. + var matches = patternMatcher.getMatchesForLastSegmentOfPattern(name); + + if (!matches) { + continue; + } + + // It was a match! If the pattern has dots in it, then also see if hte + // declaration container matches as well. + if (patternMatcher.patternContainsDots) { + var containerName = getContainerName(getContainerNode(declaration)); + matches = patternMatcher.getMatches(name, containerName); + + if (!matches) { + continue; + } + } + + var fileName = sourceFile.fileName; + var matchKind = bestMatchKind(matches); + rawItems.push({ name, fileName, matchKind, isCaseSensitive: isCaseSensitive(matches), declaration }); } } }); @@ -43,6 +50,56 @@ module ts.NavigateTo { return items; + function isCaseSensitive(matches: PatternMatch[]): boolean { + Debug.assert(matches.length > 0); + + // This is a case sensitive match, only if all the submatches were case sensitive. + for (var i = 0, n = matches.length; i < n; i++) { + if (!matches[i].isCaseSensitive) { + return false; + } + } + + return true; + } + + function getDeclarationName(declaration: Declaration): string { + if (declaration.name.kind === SyntaxKind.Identifier || + declaration.name.kind === SyntaxKind.StringLiteral || + declaration.name.kind === SyntaxKind.NumericLiteral) { + + return (declaration.name).text; + } + + return undefined; + } + + function getContainerName(declaration: Declaration): string { + var name = getDeclarationName(declaration); + if (name === undefined) { + return undefined; + } + + var container = getContainerNode(declaration); + return container && container.name + ? getContainerName(container) + "." + name + : name; + } + + function bestMatchKind(matches: PatternMatch[]) { + Debug.assert(matches.length > 0); + var bestMatchKind = PatternMatchKind.camelCase; + + for (var i = 0, n = matches.length; i < n; i++) { + var kind = matches[i].kind; + if (kind < bestMatchKind) { + bestMatchKind = kind; + } + } + + return bestMatchKind; + } + // This means "compare in a case insensitive manner." var baseSensitivity: Intl.CollatorOptions = { sensitivity: "base" }; function compareNavigateToItems(i1: RawNavigateToItem, i2: RawNavigateToItem) { @@ -62,7 +119,8 @@ module ts.NavigateTo { name: rawItem.name, kind: getNodeKind(declaration), kindModifiers: getNodeModifiers(declaration), - matchKind: MatchKind[rawItem.matchKind], + matchKind: PatternMatchKind[rawItem.matchKind], + isCaseSensitive: rawItem.isCaseSensitive, fileName: rawItem.fileName, textSpan: createTextSpanFromBounds(declaration.getStart(), declaration.getEnd()), // TODO(jfreeman): What should be the containerName when the container has a computed name? @@ -70,48 +128,5 @@ module ts.NavigateTo { containerKind: container && container.name ? getNodeKind(container) : "" }; } - - function hasAnyUpperCaseCharacter(s: string): boolean { - for (var i = 0, n = s.length; i < n; i++) { - var c = s.charCodeAt(i); - if ((CharacterCodes.A <= c && c <= CharacterCodes.Z) || - (c >= CharacterCodes.maxAsciiCharacter && s.charAt(i).toLocaleLowerCase() !== s.charAt(i))) { - return true; - } - } - - return false; - } - - function getMatchKind(searchTerms: { caseSensitive: boolean; term: string }[], name: string): MatchKind { - var matchKind = MatchKind.none; - - if (name) { - for (var j = 0, n = searchTerms.length; j < n; j++) { - var searchTerm = searchTerms[j]; - var nameToSearch = searchTerm.caseSensitive ? name : name.toLocaleLowerCase(); - // in case of case-insensitive search searchTerm.term will already be lower-cased - var index = nameToSearch.indexOf(searchTerm.term); - if (index < 0) { - // Didn't match. - return MatchKind.none; - } - - var termKind = MatchKind.substring; - if (index === 0) { - // here we know that match occur at the beginning of the string. - // if search term and declName has the same length - we have an exact match, otherwise declName have longer length and this will be prefix match - termKind = name.length === searchTerm.term.length ? MatchKind.exact : MatchKind.prefix; - } - - // Update our match kind if we don't have one, or if this match is better. - if (matchKind === MatchKind.none || termKind < matchKind) { - matchKind = termKind; - } - } - } - - return matchKind; - } } } \ No newline at end of file diff --git a/src/services/patternMatcher.ts b/src/services/patternMatcher.ts index da07c4400b..462dcbc578 100644 --- a/src/services/patternMatcher.ts +++ b/src/services/patternMatcher.ts @@ -1,10 +1,10 @@ module ts { // Note(cyrusn): this enum is ordered from strongest match type to weakest match type. export enum PatternMatchKind { - Exact, - Prefix, - Substring, - CamelCase + exact, + prefix, + substring, + camelCase } // Information about a match made by the pattern matcher between a candidate and the @@ -202,12 +202,12 @@ module ts { if (chunk.text.length === candidate.length) { // a) Check if the part matches the candidate entirely, in an case insensitive or // sensitive manner. If it does, return that there was an exact match. - return createPatternMatch(PatternMatchKind.Exact, punctuationStripped, /*isCaseSensitive:*/ candidate === chunk.text); + return createPatternMatch(PatternMatchKind.exact, punctuationStripped, /*isCaseSensitive:*/ candidate === chunk.text); } else { // b) Check if the part is a prefix of the candidate, in a case insensitive or sensitive // manner. If it does, return that there was a prefix match. - return createPatternMatch(PatternMatchKind.Prefix, punctuationStripped, /*isCaseSensitive:*/ startsWith(candidate, chunk.text)); + return createPatternMatch(PatternMatchKind.prefix, punctuationStripped, /*isCaseSensitive:*/ startsWith(candidate, chunk.text)); } } @@ -225,7 +225,7 @@ module ts { for (var i = 0, n = wordSpans.length; i < n; i++) { var span = wordSpans[i] if (partStartsWith(candidate, span, chunk.text, /*ignoreCase:*/ true)) { - return createPatternMatch(PatternMatchKind.Substring, punctuationStripped, + return createPatternMatch(PatternMatchKind.substring, punctuationStripped, /*isCaseSensitive:*/ partStartsWith(candidate, span, chunk.text, /*ignoreCase:*/ false)); } } @@ -236,7 +236,7 @@ module ts { // candidate in a case *sensitive* manner. If so, return that there was a substring // match. if (candidate.indexOf(chunk.text) > 0) { - return createPatternMatch(PatternMatchKind.Substring, punctuationStripped, /*isCaseSensitive:*/ true); + return createPatternMatch(PatternMatchKind.substring, punctuationStripped, /*isCaseSensitive:*/ true); } } @@ -246,12 +246,12 @@ module ts { var candidateParts = getWordSpans(candidate); var camelCaseWeight = tryCamelCaseMatch(candidate, candidateParts, chunk, /*ignoreCase:*/ false); if (camelCaseWeight !== undefined) { - return createPatternMatch(PatternMatchKind.CamelCase, punctuationStripped, /*isCaseSensitive:*/ true, /*camelCaseWeight:*/ camelCaseWeight); + return createPatternMatch(PatternMatchKind.camelCase, punctuationStripped, /*isCaseSensitive:*/ true, /*camelCaseWeight:*/ camelCaseWeight); } camelCaseWeight = tryCamelCaseMatch(candidate, candidateParts, chunk, /*ignoreCase:*/ true); if (camelCaseWeight !== undefined) { - return createPatternMatch(PatternMatchKind.CamelCase, punctuationStripped, /*isCaseSensitive:*/ false, /*camelCaseWeight:*/ camelCaseWeight); + return createPatternMatch(PatternMatchKind.camelCase, punctuationStripped, /*isCaseSensitive:*/ false, /*camelCaseWeight:*/ camelCaseWeight); } } } @@ -266,7 +266,7 @@ module ts { // (Pattern: fogbar, Candidate: quuxfogbarFogBar). if (chunk.text.length < candidate.length) { if (index > 0 && isUpperCaseLetter(candidate.charCodeAt(index))) { - return createPatternMatch(PatternMatchKind.Substring, punctuationStripped, /*isCaseSensitive:*/ false); + return createPatternMatch(PatternMatchKind.substring, punctuationStripped, /*isCaseSensitive:*/ false); } } } @@ -508,7 +508,7 @@ module ts { } function compareCamelCase(result1: PatternMatch, result2: PatternMatch) { - if (result1.kind === PatternMatchKind.CamelCase && result2.kind === PatternMatchKind.CamelCase) { + if (result1.kind === PatternMatchKind.camelCase && result2.kind === PatternMatchKind.camelCase) { // Swap the values here. If result1 has a higher weight, then we want it to come // first. return result2.camelCaseWeight - result1.camelCaseWeight; diff --git a/src/services/services.ts b/src/services/services.ts index 7af0b52d3c..475b720db6 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -970,6 +970,7 @@ module ts { kind: string; kindModifiers: string; matchKind: string; + isCaseSensitive: boolean; fileName: string; textSpan: TextSpan; containerName: string; @@ -1986,7 +1987,7 @@ module ts { }); } - /* @internal */ export function getContainerNode(node: Node): Node { + /* @internal */ export function getContainerNode(node: Node): Declaration { while (true) { node = node.parent; if (!node) { @@ -2004,7 +2005,7 @@ module ts { case SyntaxKind.InterfaceDeclaration: case SyntaxKind.EnumDeclaration: case SyntaxKind.ModuleDeclaration: - return node; + return node; } } } diff --git a/tests/baselines/reference/APISample_compile.js b/tests/baselines/reference/APISample_compile.js index b7ae84a231..f05f4cfd6c 100644 --- a/tests/baselines/reference/APISample_compile.js +++ b/tests/baselines/reference/APISample_compile.js @@ -1597,6 +1597,7 @@ declare module "typescript" { kind: string; kindModifiers: string; matchKind: string; + isCaseSensitive: boolean; fileName: string; textSpan: TextSpan; containerName: string; diff --git a/tests/baselines/reference/APISample_compile.types b/tests/baselines/reference/APISample_compile.types index 9a31d48980..f7d8c733dd 100644 --- a/tests/baselines/reference/APISample_compile.types +++ b/tests/baselines/reference/APISample_compile.types @@ -5178,6 +5178,9 @@ declare module "typescript" { matchKind: string; >matchKind : string + isCaseSensitive: boolean; +>isCaseSensitive : boolean + fileName: string; >fileName : string diff --git a/tests/baselines/reference/APISample_linter.js b/tests/baselines/reference/APISample_linter.js index 50ed483b4a..f30086b0f2 100644 --- a/tests/baselines/reference/APISample_linter.js +++ b/tests/baselines/reference/APISample_linter.js @@ -1628,6 +1628,7 @@ declare module "typescript" { kind: string; kindModifiers: string; matchKind: string; + isCaseSensitive: boolean; fileName: string; textSpan: TextSpan; containerName: string; diff --git a/tests/baselines/reference/APISample_linter.types b/tests/baselines/reference/APISample_linter.types index 512cd68930..97b0f03a36 100644 --- a/tests/baselines/reference/APISample_linter.types +++ b/tests/baselines/reference/APISample_linter.types @@ -5324,6 +5324,9 @@ declare module "typescript" { matchKind: string; >matchKind : string + isCaseSensitive: boolean; +>isCaseSensitive : boolean + fileName: string; >fileName : string diff --git a/tests/baselines/reference/APISample_transform.js b/tests/baselines/reference/APISample_transform.js index 8b3c64099c..b24bb19329 100644 --- a/tests/baselines/reference/APISample_transform.js +++ b/tests/baselines/reference/APISample_transform.js @@ -1629,6 +1629,7 @@ declare module "typescript" { kind: string; kindModifiers: string; matchKind: string; + isCaseSensitive: boolean; fileName: string; textSpan: TextSpan; containerName: string; diff --git a/tests/baselines/reference/APISample_transform.types b/tests/baselines/reference/APISample_transform.types index 83ef90f726..ae38d8379d 100644 --- a/tests/baselines/reference/APISample_transform.types +++ b/tests/baselines/reference/APISample_transform.types @@ -5274,6 +5274,9 @@ declare module "typescript" { matchKind: string; >matchKind : string + isCaseSensitive: boolean; +>isCaseSensitive : boolean + fileName: string; >fileName : string diff --git a/tests/baselines/reference/APISample_watcher.js b/tests/baselines/reference/APISample_watcher.js index d9488db8bc..1de7ca5c6e 100644 --- a/tests/baselines/reference/APISample_watcher.js +++ b/tests/baselines/reference/APISample_watcher.js @@ -1666,6 +1666,7 @@ declare module "typescript" { kind: string; kindModifiers: string; matchKind: string; + isCaseSensitive: boolean; fileName: string; textSpan: TextSpan; containerName: string; diff --git a/tests/baselines/reference/APISample_watcher.types b/tests/baselines/reference/APISample_watcher.types index 11a726ea39..33ec6d1e26 100644 --- a/tests/baselines/reference/APISample_watcher.types +++ b/tests/baselines/reference/APISample_watcher.types @@ -5447,6 +5447,9 @@ declare module "typescript" { matchKind: string; >matchKind : string + isCaseSensitive: boolean; +>isCaseSensitive : boolean + fileName: string; >fileName : string diff --git a/tests/cases/fourslash/declareFunction.ts b/tests/cases/fourslash/declareFunction.ts index 171596f3ea..8806bc6d75 100644 --- a/tests/cases/fourslash/declareFunction.ts +++ b/tests/cases/fourslash/declareFunction.ts @@ -4,4 +4,4 @@ goTo.marker(); //verify there is no empty navigation item. -verify.navigationItemsListCount(0, "^$"/*empty string*/) +verify.navigationItemsListCount(0, "") diff --git a/tests/cases/fourslash/navigationItemsSubStringMatch.ts b/tests/cases/fourslash/navigationItemsSubStringMatch.ts index 8dc42a2365..b90a2a2f33 100644 --- a/tests/cases/fourslash/navigationItemsSubStringMatch.ts +++ b/tests/cases/fourslash/navigationItemsSubStringMatch.ts @@ -1,28 +1,26 @@ /// /////// Module -////{| "itemName": "Shapes", "kind": "module", "parentName": "", "matchKind": "substring" |}module Shapes { +////{| "itemName": "MyShapes", "kind": "module", "parentName": "", "matchKind": "substring" |}module MyShapes { //// //// // Class -//// {| "itemName": "Point", "kind": "class", "parentName": "Shapes", "matchKind": "substring" |}export class Point { +//// {| "itemName": "MyPoint", "kind": "class", "parentName": "MyShapes", "matchKind": "substring" |}export class MyPoint { //// // Instance member -//// {| "itemName": "originPointAttheHorizon", "kind": "property", "parentName": "Point", "matchKind": "substring"|}private originPointAttheHorizon = 0.0; +//// {| "itemName": "MyoriginPointAttheHorizon", "kind": "property", "parentName": "MyPoint", "matchKind": "substring"|}private MyoriginPointAttheHorizon = 0.0; //// //// // Getter -//// {| "itemName": "distanceFromOrigin", "kind": "getter", "parentName": "Point", "matchKind": "substring" |}get distanceFromOrigin(): number { return 0; } +//// {| "itemName": "MydistanceFromOrigin", "kind": "getter", "parentName": "MyPoint", "matchKind": "substring" |}get MydistanceFromOrigin(): number { return 0; } //// //// } ////} //// ////// Local variables -////{| "itemName": "myPointThatIJustInitiated", "kind": "var", "parentName": "", "matchKind": "substring"|}var myPointThatIJustInitiated = new Shapes.Point(); - -//// Testing for substring matching of navigationItems -//var searchValue = "FromOrigin horizon INITIATED Shape Point"; +////{| "itemName": "MymyPointThatIJustInitiated", "kind": "var", "parentName": "", "matchKind": "substring"|}var MymyPointThatIJustInitiated = new Shapes.Point(); +debugger; test.markers().forEach((marker) => { if (marker.data) { var name = marker.data.itemName; - verify.navigationItemsListContains(name, marker.data.kind, name.substr(1), marker.data.matchKind, marker.fileName, marker.data.parentName); + verify.navigationItemsListContains(name, marker.data.kind, name.substr(2), marker.data.matchKind, marker.fileName, marker.data.parentName); } }); \ No newline at end of file diff --git a/tests/cases/fourslash/navigationItemsSubStringMatch2.ts b/tests/cases/fourslash/navigationItemsSubStringMatch2.ts index c1afd7326c..edce5ad761 100644 --- a/tests/cases/fourslash/navigationItemsSubStringMatch2.ts +++ b/tests/cases/fourslash/navigationItemsSubStringMatch2.ts @@ -17,12 +17,12 @@ //// INITIATED123; //// public horizon(): void; ////} - +debugger; var notFoundSearchValue = "mPointThatIJustInitiated wrongKeyWord"; goTo.marker("file1"); // case sensitive matching for 'Horizon' will fail -verify.navigationItemsListCount(0, "Horizon", "exact"); +verify.navigationItemsListCount(1, "Horizon", "exact"); // case insensitive matching will find 'horizon' verify.navigationItemsListCount(1, "horizon", "exact"); // case sensitive matching will find 'Distance' and INITIATED diff --git a/tests/cases/fourslash/server/navto.ts b/tests/cases/fourslash/server/navto.ts index 8dc42a2365..0179c16855 100644 --- a/tests/cases/fourslash/server/navto.ts +++ b/tests/cases/fourslash/server/navto.ts @@ -1,15 +1,15 @@ /// /////// Module -////{| "itemName": "Shapes", "kind": "module", "parentName": "", "matchKind": "substring" |}module Shapes { +////{| "itemName": "MyShapes", "kind": "module", "parentName": "", "matchKind": "substring" |}module MyShapes { //// //// // Class -//// {| "itemName": "Point", "kind": "class", "parentName": "Shapes", "matchKind": "substring" |}export class Point { +//// {| "itemName": "MyPoint", "kind": "class", "parentName": "MyShapes", "matchKind": "substring" |}export class MyPoint { //// // Instance member -//// {| "itemName": "originPointAttheHorizon", "kind": "property", "parentName": "Point", "matchKind": "substring"|}private originPointAttheHorizon = 0.0; +//// {| "itemName": "MyoriginPointAttheHorizon", "kind": "property", "parentName": "MyPoint", "matchKind": "substring"|}private MyoriginPointAttheHorizon = 0.0; //// //// // Getter -//// {| "itemName": "distanceFromOrigin", "kind": "getter", "parentName": "Point", "matchKind": "substring" |}get distanceFromOrigin(): number { return 0; } +//// {| "itemName": "MydistanceFromOrigin", "kind": "getter", "parentName": "MyPoint", "matchKind": "substring" |}get MydistanceFromOrigin(): number { return 0; } //// //// } ////} @@ -23,6 +23,6 @@ test.markers().forEach((marker) => { if (marker.data) { var name = marker.data.itemName; - verify.navigationItemsListContains(name, marker.data.kind, name.substr(1), marker.data.matchKind, marker.fileName, marker.data.parentName); + verify.navigationItemsListContains(name, marker.data.kind, name.substr(2), marker.data.matchKind, marker.fileName, marker.data.parentName); } }); \ No newline at end of file diff --git a/tests/cases/unittests/services/patternMatcher.ts b/tests/cases/unittests/services/patternMatcher.ts index ddf1f9fe34..1af64545db 100644 --- a/tests/cases/unittests/services/patternMatcher.ts +++ b/tests/cases/unittests/services/patternMatcher.ts @@ -99,28 +99,28 @@ describe('PatternMatcher', function () { debugger; var match = getFirstMatch("Foo", "Foo"); - assert.equal(ts.PatternMatchKind.Exact, match.kind); + assert.equal(ts.PatternMatchKind.exact, match.kind); assert.equal(true, match.isCaseSensitive); }); it("PreferCaseSensitiveExactInsensitive", () => { var match = getFirstMatch("foo", "Foo"); - assert.equal(ts.PatternMatchKind.Exact, match.kind); + assert.equal(ts.PatternMatchKind.exact, match.kind); assert.equal(false, match.isCaseSensitive); }); it("PreferCaseSensitivePrefix", () => { var match = getFirstMatch("Foo", "Fo"); - assert.equal(ts.PatternMatchKind.Prefix, match.kind); + assert.equal(ts.PatternMatchKind.prefix, match.kind); assert.equal(true, match.isCaseSensitive); }); it("PreferCaseSensitivePrefixCaseInsensitive", () => { var match = getFirstMatch("Foo", "fo"); - assert.equal(ts.PatternMatchKind.Prefix, match.kind); + assert.equal(ts.PatternMatchKind.prefix, match.kind); assert.equal(false, match.isCaseSensitive); }); @@ -128,7 +128,7 @@ describe('PatternMatcher', function () { debugger; var match = getFirstMatch("FogBar", "FB"); - assert.equal(ts.PatternMatchKind.CamelCase, match.kind); + assert.equal(ts.PatternMatchKind.camelCase, match.kind); assert.equal(true, match.isCaseSensitive); assertInRange(match.camelCaseWeight, 1, 1 << 30); }); @@ -136,7 +136,7 @@ describe('PatternMatcher', function () { it("PreferCaseSensitiveCamelCaseMatchPartialPattern", () => { var match = getFirstMatch("FogBar", "FoB"); - assert.equal(ts.PatternMatchKind.CamelCase, match.kind); + assert.equal(ts.PatternMatchKind.camelCase, match.kind); assert.equal(true, match.isCaseSensitive); }); @@ -167,7 +167,7 @@ describe('PatternMatcher', function () { it("TwoUppercaseCharacters", () => { var match = getFirstMatch("SimpleUIElement", "SiUI"); - assert.equal(ts.PatternMatchKind.CamelCase, match.kind); + assert.equal(ts.PatternMatchKind.camelCase, match.kind); assert.equal(true, match.isCaseSensitive); }); @@ -175,49 +175,49 @@ describe('PatternMatcher', function () { debugger; var match = getFirstMatch("FogBar", "b"); - assert.equal(ts.PatternMatchKind.Substring, match.kind); + assert.equal(ts.PatternMatchKind.substring, match.kind); assert.equal(false, match.isCaseSensitive); }); it("PreferCaseSensitiveLowercasePattern2", () => { var match = getFirstMatch("FogBar", "fB"); - assert.equal(ts.PatternMatchKind.CamelCase, match.kind); + assert.equal(ts.PatternMatchKind.camelCase, match.kind); assert.equal(false, match.isCaseSensitive); }); it("PreferCaseSensitiveTryUnderscoredName", () => { var match = getFirstMatch("_fogBar", "_fB"); - assert.equal(ts.PatternMatchKind.CamelCase, match.kind); + assert.equal(ts.PatternMatchKind.camelCase, match.kind); assert.equal(true, match.isCaseSensitive); }); it("PreferCaseSensitiveTryUnderscoredName2", () => { var match = getFirstMatch("_fogBar", "fB"); - assert.equal(ts.PatternMatchKind.CamelCase, match.kind); + assert.equal(ts.PatternMatchKind.camelCase, match.kind); assert.equal(true, match.isCaseSensitive); }); it("PreferCaseSensitiveTryUnderscoredNameInsensitive", () => { var match = getFirstMatch("_FogBar", "_fB"); - assert.equal(ts.PatternMatchKind.CamelCase, match.kind); + assert.equal(ts.PatternMatchKind.camelCase, match.kind); assert.equal(false, match.isCaseSensitive); }); it("PreferCaseSensitiveMiddleUnderscore", () => { var match = getFirstMatch("Fog_Bar", "FB"); - assert.equal(ts.PatternMatchKind.CamelCase, match.kind); + assert.equal(ts.PatternMatchKind.camelCase, match.kind); assert.equal(true, match.isCaseSensitive); }); it("PreferCaseSensitiveMiddleUnderscore2", () => { var match = getFirstMatch("Fog_Bar", "F_B"); - assert.equal(ts.PatternMatchKind.CamelCase, match.kind); + assert.equal(ts.PatternMatchKind.camelCase, match.kind); assert.equal(true, match.isCaseSensitive); }); @@ -230,14 +230,14 @@ describe('PatternMatcher', function () { it("PreferCaseSensitiveMiddleUnderscore4", () => { var match = getFirstMatch("Fog_Bar", "f_B"); - assert.equal(ts.PatternMatchKind.CamelCase, match.kind); + assert.equal(ts.PatternMatchKind.camelCase, match.kind); assert.equal(false, match.isCaseSensitive); }); it("PreferCaseSensitiveMiddleUnderscore5", () => { var match = getFirstMatch("Fog_Bar", "F_b"); - assert.equal(ts.PatternMatchKind.CamelCase, match.kind); + assert.equal(ts.PatternMatchKind.camelCase, match.kind); assert.equal(false, match.isCaseSensitive); }); @@ -295,81 +295,81 @@ describe('PatternMatcher', function () { it("ExactWithLowercase", () => { var matches = getAllMatches("AddMetadataReference", "addmetadatareference"); - assertContainsKind(ts.PatternMatchKind.Exact, matches); + assertContainsKind(ts.PatternMatchKind.exact, matches); }); it("SingleLowercasedSearchWord1", () => { var matches = getAllMatches("AddMetadataReference", "add"); - assertContainsKind(ts.PatternMatchKind.Prefix, matches); + assertContainsKind(ts.PatternMatchKind.prefix, matches); }); it("SingleLowercasedSearchWord2", () => { var matches = getAllMatches("AddMetadataReference", "metadata"); - assertContainsKind(ts.PatternMatchKind.Substring, matches); + assertContainsKind(ts.PatternMatchKind.substring, matches); }); it("SingleUppercaseSearchWord1", () => { var matches = getAllMatches("AddMetadataReference", "Add"); - assertContainsKind(ts.PatternMatchKind.Prefix, matches); + assertContainsKind(ts.PatternMatchKind.prefix, matches); }); it("SingleUppercaseSearchWord2", () => { var matches = getAllMatches("AddMetadataReference", "Metadata"); - assertContainsKind(ts.PatternMatchKind.Substring, matches); + assertContainsKind(ts.PatternMatchKind.substring, matches); }); it("SingleUppercaseSearchLetter1", () => { var matches = getAllMatches("AddMetadataReference", "A"); - assertContainsKind(ts.PatternMatchKind.Prefix, matches); + assertContainsKind(ts.PatternMatchKind.prefix, matches); }); it("SingleUppercaseSearchLetter2", () => { var matches = getAllMatches("AddMetadataReference", "M"); - assertContainsKind(ts.PatternMatchKind.Substring, matches); + assertContainsKind(ts.PatternMatchKind.substring, matches); }); it("TwoLowercaseWords", () => { var matches = getAllMatches("AddMetadataReference", "add metadata"); - assertContainsKind(ts.PatternMatchKind.Prefix, matches); - assertContainsKind(ts.PatternMatchKind.Substring, matches); + assertContainsKind(ts.PatternMatchKind.prefix, matches); + assertContainsKind(ts.PatternMatchKind.substring, matches); }); it("TwoLowercaseWords", () => { var matches = getAllMatches("AddMetadataReference", "A M"); - assertContainsKind(ts.PatternMatchKind.Prefix, matches); - assertContainsKind(ts.PatternMatchKind.Substring, matches); + assertContainsKind(ts.PatternMatchKind.prefix, matches); + assertContainsKind(ts.PatternMatchKind.substring, matches); }); it("TwoLowercaseWords", () => { var matches = getAllMatches("AddMetadataReference", "AM"); - assertContainsKind(ts.PatternMatchKind.CamelCase, matches); + assertContainsKind(ts.PatternMatchKind.camelCase, matches); }); it("TwoLowercaseWords", () => { var matches = getAllMatches("AddMetadataReference", "ref Metadata") - assertArrayEquals(ts.map(matches, m => m.kind), [ts.PatternMatchKind.Substring, ts.PatternMatchKind.Substring]); + assertArrayEquals(ts.map(matches, m => m.kind), [ts.PatternMatchKind.substring, ts.PatternMatchKind.substring]); }); it("TwoLowercaseWords", () => { var matches = getAllMatches("AddMetadataReference", "ref M") - assertArrayEquals(ts.map(matches, m => m.kind), [ts.PatternMatchKind.Substring, ts.PatternMatchKind.Substring]); + assertArrayEquals(ts.map(matches, m => m.kind), [ts.PatternMatchKind.substring, ts.PatternMatchKind.substring]); }); it("MixedCamelCase", () => { var matches = getAllMatches("AddMetadataReference", "AMRe"); - assertContainsKind(ts.PatternMatchKind.CamelCase, matches); + assertContainsKind(ts.PatternMatchKind.camelCase, matches); }); it("BlankPattern", () => { @@ -387,22 +387,22 @@ describe('PatternMatcher', function () { it("EachWordSeparately1", () => { var matches = getAllMatches("AddMetadataReference", "add Meta"); - assertContainsKind(ts.PatternMatchKind.Prefix, matches); - assertContainsKind(ts.PatternMatchKind.Substring, matches); + assertContainsKind(ts.PatternMatchKind.prefix, matches); + assertContainsKind(ts.PatternMatchKind.substring, matches); }); it("EachWordSeparately2", () => { var matches = getAllMatches("AddMetadataReference", "Add meta"); - assertContainsKind(ts.PatternMatchKind.Prefix, matches); - assertContainsKind(ts.PatternMatchKind.Substring, matches); + assertContainsKind(ts.PatternMatchKind.prefix, matches); + assertContainsKind(ts.PatternMatchKind.substring, matches); }); it("EachWordSeparately3", () => { var matches = getAllMatches("AddMetadataReference", "Add Meta"); - assertContainsKind(ts.PatternMatchKind.Prefix, matches); - assertContainsKind(ts.PatternMatchKind.Substring, matches); + assertContainsKind(ts.PatternMatchKind.prefix, matches); + assertContainsKind(ts.PatternMatchKind.substring, matches); }); it("MixedCasing", () => { @@ -420,7 +420,7 @@ describe('PatternMatcher', function () { it("AsteriskSplit", () => { var matches = getAllMatches("GetKeyWord", "K*W"); - assertArrayEquals(ts.map(matches, m => m.kind), [ts.PatternMatchKind.Substring, ts.PatternMatchKind.Substring]); + assertArrayEquals(ts.map(matches, m => m.kind), [ts.PatternMatchKind.substring, ts.PatternMatchKind.substring]); }); it("LowercaseSubstring1", () => { @@ -431,7 +431,7 @@ describe('PatternMatcher', function () { it("LowercaseSubstring2", () => { var matches = getAllMatches("FooAttribute", "a"); - assertContainsKind(ts.PatternMatchKind.Substring, matches); + assertContainsKind(ts.PatternMatchKind.substring, matches); assert.isFalse(matches[0].isCaseSensitive); }); }); @@ -440,7 +440,7 @@ describe('PatternMatcher', function () { it("DottedPattern1", () => { var match = getFirstMatchForDottedPattern("Foo.Bar.Baz", "Quux", "B.Q"); - assert.equal(ts.PatternMatchKind.Prefix, match.kind); + assert.equal(ts.PatternMatchKind.prefix, match.kind); assert.equal(true, match.isCaseSensitive); }); @@ -451,19 +451,19 @@ describe('PatternMatcher', function () { it("DottedPattern3", () => { var match = getFirstMatchForDottedPattern("Foo.Bar.Baz", "Quux", "B.B.Q"); - assert.equal(ts.PatternMatchKind.Prefix, match.kind); + assert.equal(ts.PatternMatchKind.prefix, match.kind); assert.equal(true, match.isCaseSensitive); }); it("DottedPattern4", () => { var match = getFirstMatchForDottedPattern("Foo.Bar.Baz", "Quux", "Baz.Quux"); - assert.equal(ts.PatternMatchKind.Exact, match.kind); + assert.equal(ts.PatternMatchKind.exact, match.kind); assert.equal(true, match.isCaseSensitive); }); it("DottedPattern5", () => { var match = getFirstMatchForDottedPattern("Foo.Bar.Baz", "Quux", "F.B.B.Quux"); - assert.equal(ts.PatternMatchKind.Exact, match.kind); + assert.equal(ts.PatternMatchKind.exact, match.kind); assert.equal(true, match.isCaseSensitive); }); From 441735a9d349dad08da5c4953d41636b6410633b Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sun, 22 Feb 2015 21:56:10 -0800 Subject: [PATCH 2/3] CR feedback --- src/services/patternMatcher.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/patternMatcher.ts b/src/services/patternMatcher.ts index 462dcbc578..b19ee8fcd2 100644 --- a/src/services/patternMatcher.ts +++ b/src/services/patternMatcher.ts @@ -160,7 +160,7 @@ module ts { if (dotSeparatedSegments.length - 1 > containerParts.length) { // There weren't enough container parts to match against the pattern parts. // So this definitely doesn't match. - return null; + return undefined; } // So far so good. Now break up the container for the candidate and check if all From f90f8e80615fe301bb7cd0de784f53b86a205243 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 23 Feb 2015 15:41:43 -0800 Subject: [PATCH 3/3] CR feedback. --- src/services/navigateTo.ts | 109 +++++++++++++++--- src/services/patternMatcher.ts | 13 +-- .../unittests/services/patternMatcher.ts | 7 +- 3 files changed, 100 insertions(+), 29 deletions(-) diff --git a/src/services/navigateTo.ts b/src/services/navigateTo.ts index 11edbe9592..b3f72d83cb 100644 --- a/src/services/navigateTo.ts +++ b/src/services/navigateTo.ts @@ -26,8 +26,12 @@ module ts.NavigateTo { // It was a match! If the pattern has dots in it, then also see if hte // declaration container matches as well. if (patternMatcher.patternContainsDots) { - var containerName = getContainerName(getContainerNode(declaration)); - matches = patternMatcher.getMatches(name, containerName); + var containers = getContainers(declaration); + if (!containers) { + return undefined; + } + + matches = patternMatcher.getMatches(containers, name); if (!matches) { continue; @@ -36,7 +40,7 @@ module ts.NavigateTo { var fileName = sourceFile.fileName; var matchKind = bestMatchKind(matches); - rawItems.push({ name, fileName, matchKind, isCaseSensitive: isCaseSensitive(matches), declaration }); + rawItems.push({ name, fileName, matchKind, isCaseSensitive: allMatchesAreCaseSensitive(matches), declaration }); } } }); @@ -50,7 +54,7 @@ module ts.NavigateTo { return items; - function isCaseSensitive(matches: PatternMatch[]): boolean { + function allMatchesAreCaseSensitive(matches: PatternMatch[]): boolean { Debug.assert(matches.length > 0); // This is a case sensitive match, only if all the submatches were case sensitive. @@ -64,26 +68,99 @@ module ts.NavigateTo { } function getDeclarationName(declaration: Declaration): string { - if (declaration.name.kind === SyntaxKind.Identifier || - declaration.name.kind === SyntaxKind.StringLiteral || - declaration.name.kind === SyntaxKind.NumericLiteral) { + var result = getTextOfIdentifierOrLiteral(declaration.name); + if (result !== undefined) { + return result; + } - return (declaration.name).text; + if (declaration.name.kind === SyntaxKind.ComputedPropertyName) { + var expr = (declaration.name).expression; + if (expr.kind === SyntaxKind.PropertyAccessExpression) { + return (expr).name.text; + } + + return getTextOfIdentifierOrLiteral(expr); } return undefined; } - function getContainerName(declaration: Declaration): string { - var name = getDeclarationName(declaration); - if (name === undefined) { - return undefined; + function getTextOfIdentifierOrLiteral(node: Node) { + if (node.kind === SyntaxKind.Identifier || + node.kind === SyntaxKind.StringLiteral || + node.kind === SyntaxKind.NumericLiteral) { + + return (node).text; } - var container = getContainerNode(declaration); - return container && container.name - ? getContainerName(container) + "." + name - : name; + return undefined; + } + + function tryAddSingleDeclarationName(declaration: Declaration, containers: string[]) { + if (declaration && declaration.name) { + var text = getTextOfIdentifierOrLiteral(declaration.name); + if (text !== undefined) { + containers.unshift(text); + } + else if (declaration.name.kind === SyntaxKind.ComputedPropertyName) { + return tryAddComputedPropertyName((declaration.name).expression, containers, /*includeLastPortion:*/ true); + } + else { + // Don't know how to add this. + return false + } + } + + return true; + } + + // Only added the names of computed properties if they're simple dotted expressions, like: + // + // [X.Y.Z]() { } + function tryAddComputedPropertyName(expression: Expression, containers: string[], includeLastPortion: boolean): boolean { + var text = getTextOfIdentifierOrLiteral(expression); + if (text !== undefined) { + if (includeLastPortion) { + containers.unshift(text); + } + return true; + } + + if (expression.kind === SyntaxKind.PropertyAccessExpression) { + var propertyAccess = expression; + if (includeLastPortion) { + containers.unshift(propertyAccess.name.text); + } + + return tryAddComputedPropertyName(propertyAccess.expression, containers, /*includeLastPortion:*/ true); + } + + return false; + } + + function getContainers(declaration: Declaration) { + var containers: string[] = []; + + // First, if we started with a computed property name, then add all but the last + // portion into the container array. + if (declaration.name.kind === SyntaxKind.ComputedPropertyName) { + if (!tryAddComputedPropertyName((declaration.name).expression, containers, /*includeLastPortion:*/ false)) { + return undefined; + } + } + + // Now, walk up our containers, adding all their names to the container array. + declaration = getContainerNode(declaration); + + while (declaration) { + if (!tryAddSingleDeclarationName(declaration, containers)) { + return undefined; + } + + declaration = getContainerNode(declaration); + } + + return containers; } function bestMatchKind(matches: PatternMatch[]) { diff --git a/src/services/patternMatcher.ts b/src/services/patternMatcher.ts index b19ee8fcd2..9bcd3e1d00 100644 --- a/src/services/patternMatcher.ts +++ b/src/services/patternMatcher.ts @@ -46,7 +46,7 @@ module ts { // Fully checks a candidate, with an dotted container, against the search pattern. // The candidate must match the last part of the search pattern, and the dotted container // must match the preceding segments of the pattern. - getMatches(candidate: string, dottedContainer: string): PatternMatch[]; + getMatches(candidateContainers: string[], candidate: string): PatternMatch[]; // Whether or not the pattern contained dots or not. Clients can use this to determine // If they should call getMatches, or if getMatchesForLastSegmentOfPattern is sufficient. @@ -139,7 +139,7 @@ module ts { return matchSegment(candidate, lastOrUndefined(dotSeparatedSegments)); } - function getMatches(candidate: string, dottedContainer: string): PatternMatch[] { + function getMatches(candidateContainers: string[], candidate: string): PatternMatch[] { if (skipMatch(candidate)) { return undefined; } @@ -152,12 +152,11 @@ module ts { return undefined; } - dottedContainer = dottedContainer || ""; - var containerParts = dottedContainer.split("."); + candidateContainers = candidateContainers || []; // -1 because the last part was checked against the name, and only the rest // of the parts are checked against the container. - if (dotSeparatedSegments.length - 1 > containerParts.length) { + if (dotSeparatedSegments.length - 1 > candidateContainers.length) { // There weren't enough container parts to match against the pattern parts. // So this definitely doesn't match. return undefined; @@ -167,12 +166,12 @@ module ts { // the dotted parts match up correctly. var totalMatch = candidateMatch; - for (var i = dotSeparatedSegments.length - 2, j = containerParts.length - 1; + for (var i = dotSeparatedSegments.length - 2, j = candidateContainers.length - 1; i >= 0; i--, j--) { var segment = dotSeparatedSegments[i]; - var containerName = containerParts[j]; + var containerName = candidateContainers[j]; var containerMatch = matchSegment(containerName, segment); if (!containerMatch) { diff --git a/tests/cases/unittests/services/patternMatcher.ts b/tests/cases/unittests/services/patternMatcher.ts index 1af64545db..21f62507f4 100644 --- a/tests/cases/unittests/services/patternMatcher.ts +++ b/tests/cases/unittests/services/patternMatcher.ts @@ -96,7 +96,6 @@ describe('PatternMatcher', function () { describe("SingleWordPattern", () => { it("PreferCaseSensitiveExact", () => { - debugger; var match = getFirstMatch("Foo", "Foo"); assert.equal(ts.PatternMatchKind.exact, match.kind); @@ -125,7 +124,6 @@ describe('PatternMatcher', function () { }); it("PreferCaseSensitiveCamelCaseMatchSimple", () => { - debugger; var match = getFirstMatch("FogBar", "FB"); assert.equal(ts.PatternMatchKind.camelCase, match.kind); @@ -172,7 +170,6 @@ describe('PatternMatcher', function () { }); it("PreferCaseSensitiveLowercasePattern", () => { - debugger; var match = getFirstMatch("FogBar", "b"); assert.equal(ts.PatternMatchKind.substring, match.kind); @@ -266,7 +263,6 @@ describe('PatternMatcher', function () { }); it("AllLowerPattern1", () => { - debugger; var match = getFirstMatch("FogBarChangedEventArgs", "changedeventargs"); assert.isTrue(undefined !== match); @@ -473,7 +469,6 @@ describe('PatternMatcher', function () { }); it("DottedPattern7", () => { - debugger; var match = getFirstMatch("UIElement", "UIElement"); var match = getFirstMatch("GetKeyword", "UIElement"); assert.isTrue(match === undefined); @@ -490,7 +485,7 @@ describe('PatternMatcher', function () { } function getFirstMatchForDottedPattern(dottedContainer: string, candidate: string, pattern: string): ts.PatternMatch { - var matches = ts.createPatternMatcher(pattern).getMatches(candidate, dottedContainer); + var matches = ts.createPatternMatcher(pattern).getMatches(dottedContainer.split("."), candidate); return matches ? matches[0] : undefined; }