Simplify and correct PermittedJumps computation #18165
Conversation
|
@amcasey, |
|
|
||
| return errors; | ||
|
|
||
| function visit(node: Node) { | ||
| function visit(node: Node, permittedJumps: PermittedJumps) { |
There was a problem hiding this comment.
this is rather a style comment. we have used passing functions as arguments to forEach, forEachChild, map, etc..
There was a problem hiding this comment.
Sorry, I'm not sure I understand this comment. Were you saying that it was more conventional before, when we just captured a variable and reset it after each recursive call?
There was a problem hiding this comment.
Fair enough. I'll just revert 57906fe.
When the return type is void, there's no `returnValueProperty`, but that doesn't mean we don't need a `return` at the call site. Fixes microsoft#18140.
1. It was looking at the parent which wasn't guaranteed to be in the extracted range. 2. It was checking direct, rather than indirect containment - apparently to avoid applying the rules to certain expressions (which can't contain jumps anyway, unless they're in anonymous functions, in which case they're fine). Fixes microsoft#18144
…tern" This reverts commit 57906fe.
Simplify and correct PermittedJumps computation (cherry picked from commit deefb01)
Simplify and correct PermittedJumps computation (cherry picked from commit deefb01)
| break; | ||
| } | ||
|
|
||
| switch (node.kind) { |
There was a problem hiding this comment.
This function now switches on node.kind four different times, is there any way it could be simplified or broken up?
There was a problem hiding this comment.
I'm not sure I understand your concern. The switches don't appear to overlap (I didn't check exhaustively) and different kinds are interesting at different times. I imagine that parts of the method could be split out into helpers but I don't see much advantage to doing so.
Simplify and correct PermittedJumps computation (cherry picked from commit deefb01)
Fixes #18144
Note that this is built on top of #18164. They're independent, but git would report a merge conflict between their newly introduced tests. Reviewers can ignore amcasey@e8ff205.