Skip to content

Commit bf1ea65

Browse files
ShuiRuTianSong Gaosheetalkamat
authored
fix static method reference non-static (microsoft#38730)
* fix static method reference non-static * fix contextRangeIndex * fix lint. * fix style. * update according to suggestion. * continue fix. * use every rather than foreach * format * fix * add comment. * fix according to review suggestions. * Update src/services/findAllReferences.ts * Update src/services/findAllReferences.ts * Update src/services/findAllReferences.ts * Update src/services/findAllReferences.ts * Update src/services/findAllReferences.ts Co-authored-by: Song Gao <song.gao@laserfiche.com> Co-authored-by: Sheetal Nandi <shkamat@microsoft.com>
1 parent 8e9de9b commit bf1ea65

3 files changed

Lines changed: 113 additions & 7 deletions

File tree

‎src/services/findAllReferences.ts‎

Lines changed: 39 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1906,13 +1906,28 @@ namespace ts.FindAllReferences {
19061906
function populateSearchSymbolSet(symbol: Symbol, location: Node, checker: TypeChecker, isForRename: boolean, providePrefixAndSuffixText: boolean, implementations: boolean): Symbol[] {
19071907
const result: Symbol[] = [];
19081908
forEachRelatedSymbol<void>(symbol, location, checker, isForRename, !(isForRename && providePrefixAndSuffixText),
1909-
(sym, root, base) => { result.push(base || root || sym); },
1910-
/*allowBaseTypes*/ () => !implementations);
1909+
(sym, root, base) => {
1910+
// static method/property and instance method/property might have the same name. Only include static or only include instance.
1911+
if (base) {
1912+
if (isStatic(symbol) !== isStatic(base)) {
1913+
base = undefined;
1914+
}
1915+
}
1916+
result.push(base || root || sym);
1917+
},
1918+
// when try to find implementation, implementations is true, and not allowed to find base class
1919+
/*allowBaseTypes*/() => !implementations);
19111920
return result;
19121921
}
19131922

1923+
/**
1924+
* @param allowBaseTypes return true means it would try to find in base class or interface.
1925+
*/
19141926
function forEachRelatedSymbol<T>(
19151927
symbol: Symbol, location: Node, checker: TypeChecker, isForRenamePopulateSearchSymbolSet: boolean, onlyIncludeBindingElementAtReferenceLocation: boolean,
1928+
/**
1929+
* @param baseSymbol This symbol means one property/mehtod from base class or interface when it is not null or undefined,
1930+
*/
19161931
cbSymbol: (symbol: Symbol, rootSymbol?: Symbol, baseSymbol?: Symbol, kind?: NodeEntryKind) => T | undefined,
19171932
allowBaseTypes: (rootSymbol: Symbol) => boolean,
19181933
): T | undefined {
@@ -2031,16 +2046,33 @@ namespace ts.FindAllReferences {
20312046
readonly symbol: Symbol;
20322047
readonly kind: NodeEntryKind | undefined;
20332048
}
2049+
2050+
function isStatic(symbol: Symbol): boolean {
2051+
if (!symbol.valueDeclaration) { return false; }
2052+
const modifierFlags = getEffectiveModifierFlags(symbol.valueDeclaration);
2053+
return !!(modifierFlags & ModifierFlags.Static);
2054+
}
2055+
20342056
function getRelatedSymbol(search: Search, referenceSymbol: Symbol, referenceLocation: Node, state: State): RelatedSymbol | undefined {
20352057
const { checker } = state;
20362058
return forEachRelatedSymbol(referenceSymbol, referenceLocation, checker, /*isForRenamePopulateSearchSymbolSet*/ false,
20372059
/*onlyIncludeBindingElementAtReferenceLocation*/ state.options.use !== FindReferencesUse.Rename || !!state.options.providePrefixAndSuffixTextForRename,
2038-
(sym, rootSymbol, baseSymbol, kind): RelatedSymbol | undefined => search.includes(baseSymbol || rootSymbol || sym)
2039-
// For a base type, use the symbol for the derived type. For a synthetic (e.g. union) property, use the union symbol.
2040-
? { symbol: rootSymbol && !(getCheckFlags(sym) & CheckFlags.Synthetic) ? rootSymbol : sym, kind }
2041-
: undefined,
2060+
(sym, rootSymbol, baseSymbol, kind): RelatedSymbol | undefined => {
2061+
// check whether the symbol used to search itself is just the searched one.
2062+
if (baseSymbol) {
2063+
// static method/property and instance method/property might have the same name. Only check static or only check instance.
2064+
if (isStatic(referenceSymbol) !== isStatic(baseSymbol)) {
2065+
baseSymbol = undefined;
2066+
}
2067+
}
2068+
return search.includes(baseSymbol || rootSymbol || sym)
2069+
// For a base type, use the symbol for the derived type. For a synthetic (e.g. union) property, use the union symbol.
2070+
? { symbol: rootSymbol && !(getCheckFlags(sym) & CheckFlags.Synthetic) ? rootSymbol : sym, kind }
2071+
: undefined;
2072+
},
20422073
/*allowBaseTypes*/ rootSymbol =>
2043-
!(search.parents && !search.parents.some(parent => explicitlyInheritsFrom(rootSymbol.parent!, parent, state.inheritsFromCache, checker))));
2074+
!(search.parents && !search.parents.some(parent => explicitlyInheritsFrom(rootSymbol.parent!, parent, state.inheritsFromCache, checker)))
2075+
);
20442076
}
20452077

20462078
/**

‎tests/cases/fourslash/findAllRefsForStaticInstanceMethodInheritance.ts‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
/// <reference path='fourslash.ts'/>
2+
3+
////class X{
4+
//// [|[|{| "isDefinition": true, "contextRangeIndex": 0, "isWriteAccess": true |}foo|](): void{}|]
5+
////}
6+
////
7+
////class Y extends X{
8+
//// [|static [|{| "isDefinition": true, "contextRangeIndex": 2, "isWriteAccess": true |}foo|](): void{}|]
9+
////}
10+
////
11+
////class Z extends Y{
12+
//// [|static [|{| "isDefinition": true, "contextRangeIndex": 4, "isWriteAccess": true |}foo|](): void{}|]
13+
//// [|[|{| "isDefinition": true, "contextRangeIndex": 6, "isWriteAccess": true |}foo|](): void{}|]
14+
////}
15+
////
16+
////const x = new X();
17+
////const y = new Y();
18+
////const z = new Z();
19+
////x.[|foo|]();
20+
////y.[|foo|]();
21+
////z.[|foo|]();
22+
////Y.[|foo|]();
23+
////Z.[|foo|]();
24+
25+
const [r0Def, r0, r1Def, r1, r2Def, r2, r3Def, r3, r4, r5, r6, r7, r8] = test.ranges();
26+
verify.referenceGroups([r0, r3, r4, r5, r6], [
27+
{ definition: { text: '(method) X.foo(): void', range: r0 }, ranges: [r0, r4, r5] },
28+
{ definition: { text: '(method) Z.foo(): void', range: r3 }, ranges: [r3, r6] },
29+
]);
30+
31+
verify.referenceGroups([r1, r7], [
32+
{ definition: { text: '(method) Y.foo(): void', range: r1 }, ranges: [r1, r7] },
33+
]);
34+
35+
verify.referenceGroups([r2, r8], [
36+
{ definition: { text: '(method) Z.foo(): void', range: r2 }, ranges: [r2, r8] },
37+
]);

‎tests/cases/fourslash/findAllRefsForStaticInstancePropertyInheritance.ts‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
/// <reference path='fourslash.ts'/>
2+
3+
////class X{
4+
//// [|[|{| "isDefinition": true, "contextRangeIndex": 0|}foo|]:any|]
5+
////}
6+
////
7+
////class Y extends X{
8+
//// [|static [|{| "isDefinition": true, "contextRangeIndex": 2|}foo|]:any|]
9+
////}
10+
////
11+
////class Z extends Y{
12+
//// [|static [|{| "isDefinition": true, "contextRangeIndex": 4|}foo|]:any|]
13+
//// [|[|{| "isDefinition": true, "contextRangeIndex": 6|}foo|]:any|]
14+
////}
15+
////
16+
////const x = new X();
17+
////const y = new Y();
18+
////const z = new Z();
19+
////x.[|foo|];
20+
////y.[|foo|];
21+
////z.[|foo|];
22+
////Y.[|foo|];
23+
////Z.[|foo|];
24+
25+
const [r0Def, r0, r1Def, r1, r2Def, r2, r3Def, r3, r4, r5, r6, r7, r8] = test.ranges();
26+
verify.referenceGroups([r0, r3, r4, r5, r6], [
27+
{ definition: { text: '(property) X.foo: any', range: r0 }, ranges: [r0, r4, r5] },
28+
{ definition: { text: '(property) Z.foo: any', range: r3 }, ranges: [r3, r6] },
29+
]);
30+
31+
verify.referenceGroups([r1, r7], [
32+
{ definition: { text: '(property) Y.foo: any', range: r1 }, ranges: [r1, r7] },
33+
]);
34+
35+
verify.referenceGroups([r2, r8], [
36+
{ definition: { text: '(property) Z.foo: any', range: r2 }, ranges: [r2, r8] },
37+
]);

0 commit comments

Comments
 (0)