Add partial autocorrect support for Lint/LiteralAsCondition
#13117
Conversation
|
Tests do not cover the post-loop while case as I was unable to prevent it from also checking post-loop until's, which do not autocorrect as all the literals tested by the spec are truthy literals. |
|
Good point-- I've added that. |
|
The current autocorrection for code which has a redundant conditional check is just to remove the check-- right now nothing is being done to code which has a conditional check that will never pass (i.e. Thinking about it, that is unreachable code, and so the autocorrection would be just to delete the code entirely. Or maybe it should fall under the purview of |
09c01ff to
24b445a
Compare
I'd say - |
|
Ok, I'll handle that cop in a second PR. |
|
Although, implementing this further, it does seem to make more sense to have the autocorrect logic in the same place-- right now I'm just raising an offense to maintain behaviour, but the testing and the logic setup are much better suited here than they are in As it stands I'll keep things as they are, but unless anyone has strong objections I'll probably add autocorrect for |
ab2cabc to
21cc43e
Compare
6e04a68 to
89f0ad6
Compare
| %w(1 2.0 [1] {} :sym :"#{a}").each do |lit| | ||
| it "registers an offense for literal #{lit} in if" do | ||
| it "registers an offense for truthy literal #{lit} in if" do | ||
| expect_offense(<<~RUBY, lit: lit) | ||
| if %{lit} | ||
| ^{lit} Literal `#{lit}` appeared as a condition. | ||
| top | ||
| end | ||
| RUBY | ||
|
|
||
| expect_correction(<<~RUBY) | ||
| top | ||
| RUBY | ||
| end |
There was a problem hiding this comment.
I think that "#{a}" should not be included in the correctable cases because unlike the other literals, removing it will potentially stop calling a method which could have side effects.
| expect_correction(<<~RUBY) | ||
| while true | ||
| top | ||
| end | ||
| RUBY |
There was a problem hiding this comment.
Should the form of loop change here? I think that maybe should be the domain of a separate Style cop.
ie. I think this should remain as a post-while loop:
begin
top
end while true
Makes progress on #12840.
Before submitting the PR make sure the following are checked:
[Fix #issue-number](if the related issue exists).master(if not - rebase it).bundle exec rake default. It executes all tests and runs RuboCop on its own code.{change_type}_{change_description}.mdif the new code introduces user-observable changes. See changelog entry format for details.