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

Methods with an instantation expression can lead to a circular type annotation #57346

Closed
DavidArchibald opened this issue Feb 8, 2024 · 7 comments
Labels
Not a Defect This behavior is one of several equally-correct options

Comments

@DavidArchibald
Copy link

🔎 Search Terms

Instantiation expression, referenced directly or indirectly in its own type annotation, circular type annotation, this, parent class base constraint.

🕗 Version & Regression Information

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about referenced directly or directly in its own type annotation

⏯ Playground Link

https://www.typescriptlang.org/play?#code/MYGwhgzhAEDqCWAXAdgUygSQgBTAJ1WUQGFwoAeYAe2QkWioFd6BlKgW1VMhlQA9EhACYxcBItygA+aAG8AvgFgAUCtA9oYwiTIxZK6IegQOqBCnQQAFAHckaKAC5oiAJ4AHVFQBmce5awtCV1yRAALeAgpAEpnADcqeCE5JWVUtV1oFkYAI0leAWFRfG18uQMjE05zB2s7CycXDy9fGoCcEuCeUIio2OgEpLloVKMAejGjKamAPWgAcnra+ehI6AJvVHFgVGSheAJgRBBXBjxV5H3D49P4ZFXEGCobe7dPaDBkZCpEMER4GgAOisACYAKwABhB0RUqSAA

💻 Code

class WitnessIsParentClass<const out SomeClass extends ParentClass> {}

class ParentClass {
    someWitness(witness: typeof WitnessIsParentClass<this>): void {}
}

class SubClass extends ParentClass {
    someWitness(witness: typeof WitnessIsParentClass<this>): void { }
   //           ^ 'witness' is referenced directly or indirectly in its own type annotation.(2502)
}

🙁 Actual behavior

This errors with 'witness' is referenced directly or indirectly in its own type annotation.(2502)

🙂 Expected behavior

No error.

Additional information about the issue

If you change from typeof WitnessIsParentClass<this> to WitnessIsParentClass<this> the error goes away so this seems to be specific to instantiation expressions. I was hoping the variance annotation would give it enough information to resolve this problem but it didn't. I suppose that's probably because the variance annotations are only for the instance but not for the class as a whole.

This code example is obviously contrived but I ran into this while writing a complex mixin. I've actually ran into this style of error; 'x' is referenced directly or indirectly in its own type annotation.(2502) many times but this was the first time it was so easy to create a minimal example. I think the root cause in those cases may have been different in those cases however, so I should probably find a way to simplify them enough to share.

Interestingly if you add a level of indirection this works fine:

class WitnessIsParentClass<SomeClass extends ParentClass> {}

type WitnessHelper<SomeClass extends ParentClass> = typeof WitnessIsParentClass<SomeClass>

class ParentClass {
    someWitness(witness: WitnessHelper<this>): void {}
}

class SubClass extends ParentClass {
    someWitness(witness: WitnessHelper<this>): void { }
}

I assume this is because it essentially hoists the variance check and breaks the loop or something.

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Feb 8, 2024

This is a true circularity:

  • SubClass has to be assignable to ParentClass for the declaration to be legal
  • Is SubClass assignable to ParentClass?
  • That depends on whether SubClass#someWitness is a subtype of ParentClass#someWitness
  • That depends on the type of the witness parameter
  • That depends on whether a SubClass-restricted instantiation of WitnessHelper<this> is assignable to a ParentClass-restricted instantiation of WitnessHelper<this>
  • Compute whether WitnessHelper<subtype of SubClass> is assignable to WitnessHelper<subtype of ParentClass>
  • This is a legal instantiation only if SubClass is assignable to ParentClass
  • Is SubClass assignable to ParentClass?
  • That's a question we're already in the process of asking

@DavidArchibald
Copy link
Author

DavidArchibald commented Feb 8, 2024

@RyanCavanaugh, thank you for the explanation, however I'm confused because you mention WitnessHelper in your explanation. However in the code example where I used WitnessHelper it compiles and functions seemingly perfectly, that's what I meant by the line right before the code snippet that brought in WitnessHelper:

Interestingly if you add a level of indirection this works fine

I guess maybe in that case it's truely circular in a broken way too but there's simply no compile error? That actually seems worse though and should be fixed.

To clarify the original code snippet is the only code snippet I shared that doesn't compile. That'd be the one under "Playground Link" and "Code" but the one I mention in "Additional information about the issue" compiles which was meant as a clarification about how it doesn't seem impossible to solve.

Here's some more code snippets that also compiles:

// Implementation option 1
class WitnessIsParentClass<const out SomeClass extends ParentClass> {};

// Implementation option 2
type WitnessIsParentClass<SomeClass extends ParentClass> = SomeClass;

// Implementation option 3
interface WitnessIsParentClass<out SomeClass extends ParentClass> {
    prop: SomeClass
};

class ParentClass {
    someWitness(witness: WitnessIsParentClass<this>): void {}
}

class SubClass extends ParentClass {
    someWitness(witness: WitnessIsParentClass<this>): void { }
}

(The implementation options are mutually exclusive)

And in general patterns like this compile:

class ParentClass {
    someWitness(witness: ParentClass): void {}
}

class SubClass<T> extends ParentClass {
    someWitness<T>(witness: SubClass<T>): void { }
}

So I'm unfortunately not following the explanation because my gut instinct is to say that the same about it truly being explanation applies. Especially since WitnessHelper only introduces a bit of indirection and then even typeof WitnessIsParentClass<SomeClass> where the inlined version doesn't.

@RyanCavanaugh
Copy link
Member

I guess maybe in that case it's truely circular in a broken way too but there's simply no compile error? That actually seems worse though and should be fixed.

It's not broken. Basically, there are places where deferrals can occur, so if you introduce a deferral at one of the steps in the process, then the question at the bottom of the list gets asked at a later time where the question at the top of the list isn't one under consideration, removing the circularity.

But not every operation is deferred (this is bad for both performance and, in some cases, semantics), so two programs that appear equivalent actually have different evaluation semantics in terms of when certain types get resolved. We're always working to try to reduce the number of circularity errors you encounter (e.g. #57293 this week) but it's not feasible to reduce them all the way to zero, nor is it desirable to error more than needed in order to achieve greater consistency.

@DavidArchibald
Copy link
Author

Thank you for elaborating! I've been interested in the codebase so I could be willing to contribute a fix... if I eventually get the know how. Unfortunately I've found running through the core logic of the checker to be a large hurdle and even just opening the TypeScript repo can slow down VSCode quite a bit so it's been difficult.

Besides that, I would say one major thing; the error message 'witness' is referenced directly or indirectly in its own type annotation.(2502) is rather undescriptive of the circular error at hand.

I'm sure there's some root reasons why it'd be tricky to improve but if you'd indulge me, I'd suggest something a bit like 'witness''s type annotation leads to a circular loop. I've also seen existing errors that would be better to have been emitted in the ideal case. Because it's not like I wrote witness: typeof witness (a case where this emitted case makes a lot of sense).

Given that I know how to fix this (by deferring with WitnessHelper) and since I don't really personally need action so I can close this if you'd like but I'm leaving this issue under the assumption someone else might land here and find the same issue.

@RyanCavanaugh RyanCavanaugh added the Not a Defect This behavior is one of several equally-correct options label Feb 9, 2024
@rotu
Copy link

rotu commented Feb 10, 2024

BTW, you can also short-circuit the circularity with an intersection, which I think is the most elegant workaround:

class SubClass extends ParentClass {
    someWitness(witness: typeof WitnessIsParentClass<ParentClass & this>): void { }
}

Also, in exploring this, I noticed that WitnessIsParentClass<SubClass>.prototype is typed as WitnessIsParentClass<any>. Not sure the implications or if that any might cause trouble with this approach down the line.

@typescript-bot
Copy link
Collaborator

This issue has been marked as "Not a Defect" and has seen no recent activity. It has been automatically closed for house-keeping purposes.

@typescript-bot typescript-bot closed this as not planned Won't fix, can't repro, duplicate, stale Feb 13, 2024
@DavidArchibald
Copy link
Author

@rotu That would be related to my other issue, #57317.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Not a Defect This behavior is one of several equally-correct options
Projects
None yet
Development

No branches or pull requests

4 participants