Clone signature return type nodes if they are present and yield the same type as the signature return type #23296
Conversation
weswigham
changed the title
|
@weswigham can you look at the failing test in the build? |
| @@ -2797,7 +2797,7 @@ namespace ts { | |||
| } | |||
| } | |||
|
|
|||
| function isEntityNameVisible(entityName: EntityNameOrEntityNameExpression, enclosingDeclaration: Node): SymbolVisibilityResult { | |||
| function resolveNameFromEntityName(entityName: EntityNameOrEntityNameExpression, enclosingDeclaration: Node) { | |||
There was a problem hiding this comment.
Don't we already have a resolveEntityName function?
There was a problem hiding this comment.
This skips a lot of the work resolveEntityName does to verify namespaceyness and also generates an appropriate meaning from the entityName's parent. So they're a bit different, unfortunately.
There was a problem hiding this comment.
We either should have only one function with options to control behavior (preferred), or should clearly document the difference between the two.
| function getDeepSynthesizedCloneAndPaintVisibility<T extends Node>(node: T | undefined): T | undefined { | ||
| if (isEntityName(node) || isEntityNameExpression(node)) { | ||
| // Paint visible aliases required for copied nodes (so required imports are not elided) | ||
| isEntityNameVisible(node, context.enclosingDeclaration); |
There was a problem hiding this comment.
Calling isEntityNameVisible without using the result is odd. I understand what this is doing, but perhaps its better to split isEntityNameVisible into two functions: one that tracks visibility and one that uses the result.
There was a problem hiding this comment.
That'd just be hasVisibleDeclarations(resolveNameFromEntityName(entityName, enclosingDeclaration), /*shouldComputeAliasToMakeVisible*/ true), I suppose?
|
Thanks for your contribution. This PR has not been updated in a while and cannot be automatically merged at the time being. For housekeeping purposes we are closing stale PRs. If you'd still like to continue working on this PR, please leave a message and one of the maintainers can reopen it. |
In #23280 @Andy-MS brought up that we never preserve aliases used to create inferred types (which makes sense, given that we immediately collapse type aliases down to their target types). With this PR, we preserve and clone return types of signatures (and thereby their aliases) provided they still yield the same type in the new context. This won't solve the issue in the OP of #23280 (as the alias is annotated within the function body and not as the signature return type node itself), but does make @Andy-MS's simplified example work as one may hope.
I'm not 100% sold on this always being a good thing (checkout the type baselines for all the places where
anybecame a copy of the erroneous declaration that generatedany), but it does produce nice results in many places (including preserving input style).