From bf1ea6508b672e43513bcc4f30423c8c400d7cfa Mon Sep 17 00:00:00 2001 From: ShuiRuTian <158983297@qq.com> Date: Fri, 10 Jul 2020 03:21:12 +0800 Subject: [PATCH] fix static method reference non-static (#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 Co-authored-by: Sheetal Nandi --- src/services/findAllReferences.ts | 46 ++++++++++++++++--- ...lRefsForStaticInstanceMethodInheritance.ts | 37 +++++++++++++++ ...efsForStaticInstancePropertyInheritance.ts | 37 +++++++++++++++ 3 files changed, 113 insertions(+), 7 deletions(-) create mode 100644 tests/cases/fourslash/findAllRefsForStaticInstanceMethodInheritance.ts create mode 100644 tests/cases/fourslash/findAllRefsForStaticInstancePropertyInheritance.ts diff --git a/src/services/findAllReferences.ts b/src/services/findAllReferences.ts index 96d3ccf8992..a8a809008ab 100644 --- a/src/services/findAllReferences.ts +++ b/src/services/findAllReferences.ts @@ -1906,13 +1906,28 @@ namespace ts.FindAllReferences { function populateSearchSymbolSet(symbol: Symbol, location: Node, checker: TypeChecker, isForRename: boolean, providePrefixAndSuffixText: boolean, implementations: boolean): Symbol[] { const result: Symbol[] = []; forEachRelatedSymbol(symbol, location, checker, isForRename, !(isForRename && providePrefixAndSuffixText), - (sym, root, base) => { result.push(base || root || sym); }, - /*allowBaseTypes*/ () => !implementations); + (sym, root, base) => { + // static method/property and instance method/property might have the same name. Only include static or only include instance. + if (base) { + if (isStatic(symbol) !== isStatic(base)) { + base = undefined; + } + } + result.push(base || root || sym); + }, + // when try to find implementation, implementations is true, and not allowed to find base class + /*allowBaseTypes*/() => !implementations); return result; } + /** + * @param allowBaseTypes return true means it would try to find in base class or interface. + */ function forEachRelatedSymbol( symbol: Symbol, location: Node, checker: TypeChecker, isForRenamePopulateSearchSymbolSet: boolean, onlyIncludeBindingElementAtReferenceLocation: boolean, + /** + * @param baseSymbol This symbol means one property/mehtod from base class or interface when it is not null or undefined, + */ cbSymbol: (symbol: Symbol, rootSymbol?: Symbol, baseSymbol?: Symbol, kind?: NodeEntryKind) => T | undefined, allowBaseTypes: (rootSymbol: Symbol) => boolean, ): T | undefined { @@ -2031,16 +2046,33 @@ namespace ts.FindAllReferences { readonly symbol: Symbol; readonly kind: NodeEntryKind | undefined; } + + function isStatic(symbol: Symbol): boolean { + if (!symbol.valueDeclaration) { return false; } + const modifierFlags = getEffectiveModifierFlags(symbol.valueDeclaration); + return !!(modifierFlags & ModifierFlags.Static); + } + function getRelatedSymbol(search: Search, referenceSymbol: Symbol, referenceLocation: Node, state: State): RelatedSymbol | undefined { const { checker } = state; return forEachRelatedSymbol(referenceSymbol, referenceLocation, checker, /*isForRenamePopulateSearchSymbolSet*/ false, /*onlyIncludeBindingElementAtReferenceLocation*/ state.options.use !== FindReferencesUse.Rename || !!state.options.providePrefixAndSuffixTextForRename, - (sym, rootSymbol, baseSymbol, kind): RelatedSymbol | undefined => search.includes(baseSymbol || rootSymbol || sym) - // For a base type, use the symbol for the derived type. For a synthetic (e.g. union) property, use the union symbol. - ? { symbol: rootSymbol && !(getCheckFlags(sym) & CheckFlags.Synthetic) ? rootSymbol : sym, kind } - : undefined, + (sym, rootSymbol, baseSymbol, kind): RelatedSymbol | undefined => { + // check whether the symbol used to search itself is just the searched one. + if (baseSymbol) { + // static method/property and instance method/property might have the same name. Only check static or only check instance. + if (isStatic(referenceSymbol) !== isStatic(baseSymbol)) { + baseSymbol = undefined; + } + } + return search.includes(baseSymbol || rootSymbol || sym) + // For a base type, use the symbol for the derived type. For a synthetic (e.g. union) property, use the union symbol. + ? { symbol: rootSymbol && !(getCheckFlags(sym) & CheckFlags.Synthetic) ? rootSymbol : sym, kind } + : undefined; + }, /*allowBaseTypes*/ rootSymbol => - !(search.parents && !search.parents.some(parent => explicitlyInheritsFrom(rootSymbol.parent!, parent, state.inheritsFromCache, checker)))); + !(search.parents && !search.parents.some(parent => explicitlyInheritsFrom(rootSymbol.parent!, parent, state.inheritsFromCache, checker))) + ); } /** diff --git a/tests/cases/fourslash/findAllRefsForStaticInstanceMethodInheritance.ts b/tests/cases/fourslash/findAllRefsForStaticInstanceMethodInheritance.ts new file mode 100644 index 00000000000..6e63cc63aad --- /dev/null +++ b/tests/cases/fourslash/findAllRefsForStaticInstanceMethodInheritance.ts @@ -0,0 +1,37 @@ +/// + +////class X{ +//// [|[|{| "isDefinition": true, "contextRangeIndex": 0, "isWriteAccess": true |}foo|](): void{}|] +////} +//// +////class Y extends X{ +//// [|static [|{| "isDefinition": true, "contextRangeIndex": 2, "isWriteAccess": true |}foo|](): void{}|] +////} +//// +////class Z extends Y{ +//// [|static [|{| "isDefinition": true, "contextRangeIndex": 4, "isWriteAccess": true |}foo|](): void{}|] +//// [|[|{| "isDefinition": true, "contextRangeIndex": 6, "isWriteAccess": true |}foo|](): void{}|] +////} +//// +////const x = new X(); +////const y = new Y(); +////const z = new Z(); +////x.[|foo|](); +////y.[|foo|](); +////z.[|foo|](); +////Y.[|foo|](); +////Z.[|foo|](); + +const [r0Def, r0, r1Def, r1, r2Def, r2, r3Def, r3, r4, r5, r6, r7, r8] = test.ranges(); +verify.referenceGroups([r0, r3, r4, r5, r6], [ + { definition: { text: '(method) X.foo(): void', range: r0 }, ranges: [r0, r4, r5] }, + { definition: { text: '(method) Z.foo(): void', range: r3 }, ranges: [r3, r6] }, +]); + +verify.referenceGroups([r1, r7], [ + { definition: { text: '(method) Y.foo(): void', range: r1 }, ranges: [r1, r7] }, +]); + +verify.referenceGroups([r2, r8], [ + { definition: { text: '(method) Z.foo(): void', range: r2 }, ranges: [r2, r8] }, +]); diff --git a/tests/cases/fourslash/findAllRefsForStaticInstancePropertyInheritance.ts b/tests/cases/fourslash/findAllRefsForStaticInstancePropertyInheritance.ts new file mode 100644 index 00000000000..62f2066ed98 --- /dev/null +++ b/tests/cases/fourslash/findAllRefsForStaticInstancePropertyInheritance.ts @@ -0,0 +1,37 @@ +/// + +////class X{ +//// [|[|{| "isDefinition": true, "contextRangeIndex": 0|}foo|]:any|] +////} +//// +////class Y extends X{ +//// [|static [|{| "isDefinition": true, "contextRangeIndex": 2|}foo|]:any|] +////} +//// +////class Z extends Y{ +//// [|static [|{| "isDefinition": true, "contextRangeIndex": 4|}foo|]:any|] +//// [|[|{| "isDefinition": true, "contextRangeIndex": 6|}foo|]:any|] +////} +//// +////const x = new X(); +////const y = new Y(); +////const z = new Z(); +////x.[|foo|]; +////y.[|foo|]; +////z.[|foo|]; +////Y.[|foo|]; +////Z.[|foo|]; + +const [r0Def, r0, r1Def, r1, r2Def, r2, r3Def, r3, r4, r5, r6, r7, r8] = test.ranges(); +verify.referenceGroups([r0, r3, r4, r5, r6], [ + { definition: { text: '(property) X.foo: any', range: r0 }, ranges: [r0, r4, r5] }, + { definition: { text: '(property) Z.foo: any', range: r3 }, ranges: [r3, r6] }, +]); + +verify.referenceGroups([r1, r7], [ + { definition: { text: '(property) Y.foo: any', range: r1 }, ranges: [r1, r7] }, +]); + +verify.referenceGroups([r2, r8], [ + { definition: { text: '(property) Z.foo: any', range: r2 }, ranges: [r2, r8] }, +]);