Style/RescueModifier doesn't detect offence in single-line method #2052
Conversation
urbanautomaton
changed the title
|
If this AST is the same, this is likely a |
|
Nope, not a bug. Parser ASTs are semantic, and these constructs are semantically equivalent. |
|
One thing you can do is look at the token stream, where block rescue would be |
|
Yep, perhaps this would be best. |
urbanautomaton
force-pushed
the
hotfix/modifier-rescue-in-method
branch
from
4443763
to
baa7770
Compare
|
@whitequark: Thanks, that did the trick nicely. @bbatsov: I've implemented the fix and all the tests now pass. I had to update the HTML formatter output fixture because of a minor order change, I presume because the offence is now reported on the |
|
@urbanautomaton Looks good. Just add some test about the position highlighting and squash the commits together. |
|
Add a changelog entry as well. |
urbanautomaton
force-pushed
the
hotfix/modifier-rescue-in-method
branch
3 times, most recently
from
3e5e6bf
to
6ee269e
Compare
|
Done, done and done. I assume since I've not introduced any new uncovered lines of code that the coverage decrease is either a) noise or b) because I've slightly reduced the number of covered lines. |
| inspect_source(cop, | ||
| 'method rescue handle') | ||
| expect(cop.offenses.size).to eq(1) | ||
| expect(cop.offenses.first.location.source).to eq('rescue') |
There was a problem hiding this comment.
Checkout cop.highlights. It's the preferred way to check for offence highlights.
There was a problem hiding this comment.
Cool, changed.
Because they are semantically identical, it's not possible to use the AST to
distinguish the following methods:
def a_method
test rescue nil
end
def another_method
test
rescue
nil
end
Instead, scan the token stream for `:kRESCUE_MOD` to detect rescue modifier
use.
urbanautomaton
force-pushed
the
hotfix/modifier-rescue-in-method
branch
from
6ee269e
to
e2275ae
Compare
…n-method Style/RescueModifier doesn't detect offence in single-line method
|
👍 Thanks! |
Currently the cop Style/RescueModifier does not detect an offence for the following, which I would expect to fail:
Here is output from an example file.
The issue seems to be that the method body is detected as a node of type
:rescue, causing it to be ignored by this line. It's not clear to me how to fix this without breaking other test cases, though, asparserreturns an identical AST for the following code, which should not register an offence:In the absence of any better ideas, I've added a failing test case. Any suggestions on how to make it pass would be gratefully received. 😄