fixAddMissingMember: Support interface and don't crash on type parameter #25995
Conversation
| } | ||
| return res; | ||
| } | ||
|
|
||
| type ClassOrInterface = ClassLikeDeclaration | InterfaceDeclaration; | ||
| interface InfoBase { |
There was a problem hiding this comment.
I'm not sure I understand the way the Info types are broken down. Why isn't there a separate InfoKind for interface? Is that uninteresting? What's the advantage of having a base type if the members are redeclared in the subtypes? Is there an advantage/disadvantage compared to just unioning the two subtypes? Why does EnumInfo have fewer fields?
There was a problem hiding this comment.
Oh, I see that Info is declared as a union type. Why does InfoBase exist at all then?
There was a problem hiding this comment.
Having fewer cases in a union is better since each union type needs its own code to handle. EnumInfo has fewer fields because the code handling it is a completely separate path that does different things from the ClassOrInterfaceInfo case. That's necessary because enums work much differently than classes/interfaces, but for class/interface the handling is mostly the same and it's better to just test isInterfaceDeclaration when necessary.
| interface InfoBase { | ||
| readonly kind: InfoKind; | ||
| readonly token: Identifier; | ||
| readonly parentDeclaration: EnumDeclaration | ClassLikeDeclaration; | ||
| readonly parentDeclaration: EnumDeclaration | ClassOrInterface; | ||
| } | ||
| enum InfoKind { enum, class } |
There was a problem hiding this comment.
Do you really need this enum since you can do this check through parentDeclaration.kind ?
There was a problem hiding this comment.
That currently wouldn't work to discriminate the union. See #18758.
ghost
force-pushed
the
codeFixAddMissingMember_interface
branch
from
1270cac to
c3e765d
Compare
ghost
force-pushed
the
codeFixAddMissingMember_interface
branch
from
c3e765d to
3166ef3
Compare
|
@amcasey @sheetalkamat Please re-review |
* origin/master: (283 commits) Don't error on destructure of private property with computed property syntax (microsoft#26360) getDefaultExportInfo: Use `getImmediateAliasedSymbol` instead of `getAliasedSymbol` (microsoft#26364) review comments restore old algorithm Dont use baseURL relative absolute paths in declaration emit, use absolute paths in bundle emit (microsoft#26341) Update user baselines (microsoft#26358) Don't store @template constraint in a TypeParameterDeclaration node (microsoft#26283) fixAddMissingMember: Support interface and don't crash on type parameter (microsoft#25995) Don't include class getter in spread type (microsoft#26287) Don't crash on computed property in destructure (microsoft#26334) Check the ambientness of a symbol name before attempting to trim it (microsoft#26312) Still generate signatures in SkipContextSensitive mode just to match on return types (microsoft#25937) fix handling if there is no commonPrefix Actually add sorting of elaboration text to user baselines Ping ryan instead of mohammed for user PRs now handle failed lookups make it work for root directory really, really fix test(?) add test fix commonPrefix handling ...
Fixes #25725