-
Notifications
You must be signed in to change notification settings - Fork 13.1k
Issue an error when a derived class precedes its base class #8636
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
Conversation
|
Why are we only checking between base/derived? Basically any value-side reference to a class should be an error before its declaration. |
|
|
||
|
|
||
| ==== tests/cases/compiler/baseTypeWrappingInstantiationChain.ts (2 errors) ==== | ||
| class C<T1> extends CBase<T1> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you reorder these classes to fix the error? I'd rather keep types and symbols around for tests that are not meant to exercise the new error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And probably some of the other tests too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
|
👍 |
|
Does this cover the ordering across files when used with -outFile? That is, Derived and Base are in different files, but when compiled into the single output the order can go a wrong way — is that also reported? Also what about this case, will it be (incorrectly) reported as an error? function some() {
class Derived extends Base {
}
return new Derived();
}
class Base { }Textually Derived is preceding the Base here, but it won't be cause any problems during execution. My logic suggests ordering warnings should only apply if Derived and Base are top-level or namespace-level classes. |
|
Yep, this fix uses the function |
Fixes #8633 and dozens of other reportings of this easily-detected error case