Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TypeScript language service cannot find subclass references/implementation of mixin methods #58307

Open
boconnell opened this issue Apr 24, 2024 · 0 comments
Labels
Experience Enhancement Noncontroversial enhancements Help Wanted You can do this Suggestion An idea for TypeScript
Milestone

Comments

@boconnell
Copy link

boconnell commented Apr 24, 2024

🔎 Search Terms

mixin mixins reference references implementation implementations find go to goto language service server protocol lsp

🕗 Version & Regression Information

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about _________

⏯ Playground Link

https://www.typescriptlang.org/play?#code/C4TwDgpgBAwg9gOwM7AE4FcDGw6oCrjQC8UAhgEYqqnZQIQDuUAFAHTtinUC2SAXGQQgA2gF0AlFCIA+QSABQAM3QJsAS0RQAYnDhaAPHigQAHsAgIAJkliIqWHPkLTmAIVJIIAvJIDe8qECySjQaYChMABsPGx04YzMLayh3Tyh-IMzgqjCoRV1mcQEANzg1SwBuAKCAX2rA1AhgdFQEbV0quvkomJSuBPMrWN0tZgB5cgArCGw-eryCvzqaoA

💻 Code

type ConstructorType = abstract new (...params: any[]) => any
function FooF<T extends ConstructorType>(Base: T) {
    abstract class Foo extends Base {
        abstract foo(): void;
    }
    return Foo;
}

class Bar extends FooF(Object) {
    foo() {}
}

🙁 Actual behavior

Note: I noticed this in my editor and not using the TS language service API directly, but I'm assuming the problem is at the Language Service layer

In the TS playground, if you try to "Find All / Go to References", "Rename symbol", or "Find All / Go to Implementation" (in VS Code, for example) for Foo.foo, it won't find/update Bar.foo.

🙂 Expected behavior

I would expect TS to recognize that Bar.foo is a reference/implementation of Foo.foo. Note that TS does correctly require Bar to implement foo, so the compiler already has some idea that they are connected.

I know it's not within the scope of this repo but I'll mention it in case it helps with anything, but FWIW while IntelliJ also fails at finding references/implementations (probably because it defers to the TS Language service), IntelliJ also provides a "Go to Super method" function that does correctly jump from Bar.foo to Foo.foo.

Additional information about the issue

After snooping around the source a little, my guess is that either:

  • something around how TS represents mixin classes, as kind of un-named class expressions e.g. ((abstract new (...params: any[]) => Foo) & { prototype: FooF<any>.Foo; }) & T instead of typeof Foo & T in the above example (visible if you hover over FooF), and/or
  • something around how TS searches for whether a symbol inherits from another symbol so explicitly checks the extends clause (
    /**
    * Determines if the parent symbol occurs somewhere in the child's ancestry. If the parent symbol
    * is an interface, determines if some ancestor of the child symbol extends or inherits from it.
    * Also takes in a cache of previous results which makes this slightly more efficient and is
    * necessary to avoid potential loops like so:
    * class A extends B { }
    * class B extends A { }
    *
    * We traverse the AST rather than using the type checker because users are typically only interested
    * in explicit implementations of an interface/class when calling "Go to Implementation". Sibling
    * implementations of types that share a common ancestor with the type whose implementation we are
    * searching for need to be filtered out of the results. The type checker doesn't let us make the
    * distinction between structurally compatible implementations and explicit implementations, so we
    * must use the AST.
    *
    * @param symbol A class or interface Symbol
    * @param parent Another class or interface Symbol
    * @param cachedResults A map of symbol id pairs (i.e. "child,parent") to booleans indicating previous results
    */
    function explicitlyInheritsFrom(symbol: Symbol, parent: Symbol, cachedResults: Map<string, boolean>, checker: TypeChecker): boolean {
    ) could be improved
@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript Help Wanted You can do this Experience Enhancement Noncontroversial enhancements labels Apr 25, 2024
@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Experience Enhancement Noncontroversial enhancements Help Wanted You can do this Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

2 participants