Don't issue a use-before-declared error for a property that exists in a superclass #17910
Conversation
ghost
requested a review
from 
src/compiler/checker.ts
Outdated
| if (isInPropertyInitializer(node) && | ||
| !isBlockScopedNameDeclaredBeforeUse(valueDeclaration, right) | ||
| && !isPropertyDeclaredInAncestorClass(prop)) { | ||
| error(right, Diagnostics.Block_scoped_variable_0_used_before_its_declaration, unescapeLeadingUnderscores(right.escapedText)); |
There was a problem hiding this comment.
Should we change this to say "property" instead of "block-scoped variable"?
src/compiler/checker.ts
Outdated
|
|
||
| // TODO: there must be a better way of doing this | ||
| function getSuperClass(classSymbol: Symbol): Symbol | undefined { | ||
| const classDecl = classSymbol.valueDeclaration; |
There was a problem hiding this comment.
return getBaseTypes(getTypeOfSymbol(classSymbol)).map(t => t.symbol); but this makes the return type Symbol[] not Symbol | undefined.
There was a problem hiding this comment.
Oh, maybe this only applies to base types that are classes, in which case you can filter for that and assert that there's only one.
There was a problem hiding this comment.
Or maybe assume that there's only one; I believe that the code for getBaseTypes is pretty clear that there's only one.
src/compiler/checker.ts
Outdated
| } | ||
|
|
||
| function isPropertyDeclaredInAncestorClass(prop: Symbol): boolean { | ||
| // It's possible that "prop.valueDeclaration" is a local declaration, but the property was also declared in a superclass. |
There was a problem hiding this comment.
use jsdoc syntax for this comment
src/compiler/checker.ts
Outdated
| // In that case we won't consider it used before its declaration, because it gets its value from the superclass' declaration. | ||
| let classSymbol = prop.parent; | ||
| while (true) { | ||
| classSymbol = getSuperClass(classSymbol); |
There was a problem hiding this comment.
I would change getSuperClass to return Type, rename classSymbol to classType, and then change to const superProperty = getPropertyOfType(classType)
There was a problem hiding this comment.
Is it not safe to access members directly? We should document that if so.
There was a problem hiding this comment.
No, the main point is to switch to retrieving the super Type instead of the Symbol. After that, the correct/easiest way to retrieve the property symbol is getPropertyOfType not type.symbol.members.get
|
@sandersn Any more comments? |
| */ | ||
| function isPropertyDeclaredInAncestorClass(prop: Symbol): boolean { | ||
| let classType = getTypeOfSymbol(prop.parent) as InterfaceType; | ||
| while (true) { |
There was a problem hiding this comment.
It sure would be convenient to say while (classType = getSuperClass(classType)) here, but we only use this idiom one other place in checker, so perhaps not good to start now.
ghost
mentioned this pull request
Fixes #14822
@sandersn What's the proper way to get a superclass symbol from a class symbol?