New unreachable_code_linter #1003
Conversation
| //FUNCTION | ||
| /following-sibling::expr | ||
| /*[ | ||
| self::expr |
There was a problem hiding this comment.
This is equivalent to using /expr instead of /*
There was a problem hiding this comment.
not quite, because position() needs to include all nodes, not just expr. I'll add a comment.
| /*[ | ||
| self::expr | ||
| and expr[SYMBOL_FUNCTION_CALL[text() = 'return' or text() = 'stop']] | ||
| and (position() != last() - 1 or not(following-sibling::OP-RIGHT-BRACE)) |
There was a problem hiding this comment.
Curious: Does this cope properly with return in switch?
There was a problem hiding this comment.
Indeed it doesn't -- just added an example test (commented out) to make sure I captured what you had in mind.
I think tryCatch() is equally affected.
I recommend covering all the new linters together. FWIW, the linters have all been vetted on our internal code base (O(10k) files) so I'm reasonably confident about false positives. False negatives are something we can always improve iteratively. |
Part of #962
The current version of this works best alongside our
ExplicitReturnLinter(), where terminal statements will already bereturn(). The logic of nailing down the last expression in xpath is a bit of a mess so we avoided replicating it inside this linter. Now I'm wondering how useful this linter is without that added logic 🤔