Unreachable loops#2141
Conversation
|
I realized I forgot |
See the original #1347. The idea is that we don't want to lint as "unreachable comment" some code that only exists to terminate a In principle a |
Codecov Report
@@ Coverage Diff @@
## main #2141 +/- ##
=======================================
Coverage 99.65% 99.65%
=======================================
Files 114 114
Lines 5176 5198 +22
=======================================
+ Hits 5158 5180 +22
Misses 18 18
|
| linter <- unreachable_code_linter() | ||
| msg <- rex::rex("Code and comments coming after a return() or stop()") | ||
|
|
||
| lines <- trim_some(" |
There was a problem hiding this comment.
long tests like this are good for robustness, but best to start with very succinct tests of only one aspect of the lint logic.
it means more lines of code which can be tedious, but is much preferable for test readability & ease of maintenance
There was a problem hiding this comment.
it may also help to either annotate the test with a comment on what all is being tested (it looks like "various types of if/else cases"), or to split into different test_that() and record that in the test's name
|
Hi @MEO265, we're closing in on sealing up a release (lintr 3.1.1). If you'd like this PR included, please take a chance to wrap up. If you'd like it included and won't have time to work on it in the next few weeks, let us know and we can try taking over from here. Thanks! |
Unfortunately, I have absolutely no idea whether I will find time this week or next. I would fix everything the following Monday at the latest. |
|
Hi @MEO265, friendly ping :) |
|
Fixed the more trivial requests. I'll let the long tests slide for now, @MEO265 a follow-up improving the tests would be appreciated if you find the time! Thanks for the PR. |
Closes #2105
It would also be possible to put the two cases
stop/returnandnext/breaktogether. Then there would be no separate message and more than the sensible situations would be tested, e.g.nextin functions, but it would be faster.I would be grateful for a hint as to why it is only necessary to filter out the end of "nolint" blocks but not the beginning