Skip to content

Commit

Permalink
[Fix rubocop#7130] Skip autocorrect in Style/FormatString if second…
Browse files Browse the repository at this point in the history
… argument is variable

For `String#%` is second argument is variable, flag offense but skip
autocorrect. It is not possible for rubocop to know if variable is
array in which case the autocorrect leads to invalid code.

Closes rubocop#7130.
  • Loading branch information
tejasbubane committed Jun 25, 2019
1 parent efe9ab4 commit c2a568f
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 3 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
* [#7151](https://github.com/rubocop-hq/rubocop/issues/7151): Fix `Style/WordArray` to also consider words containing hyphens. ([@fwitzke][])
* [#6893](https://github.com/rubocop-hq/rubocop/issues/6893): Handle implicit rescue correctly in `Naming/RescuedExceptionsVariableName`. ([@pocke][], [@anthony-robin][])
* [#7165](https://github.com/rubocop-hq/rubocop/issues/7165): Fix an auto-correct error for `Style/ConditionalAssignment` when without `else` branch'. ([@koic][])
* [#7113](https://github.com/rubocop-hq/rubocop/pull/7113): This PR renames `EnforcedStyle: rails` to `EnabledStyle: outdented_access_modifiers` for `Layout/IndentationConsistency`. ([@koic][])
* [#7130](https://github.com/rubocop-hq/rubocop/pull/7130): Skip autocorrect in `Style/FormatString` if second argument to `String#%` is a variable. ([@tejasbubane][])

### Changes

Expand Down
10 changes: 7 additions & 3 deletions lib/rubocop/cop/style/format_string.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ class FormatString < Cop
}
PATTERN

def_node_matcher :variable_argument?, <<-PATTERN
(send {str dstr} :% {send_type? lvar_type?})
PATTERN

def on_send(node)
formatter(node) do |selector|
detected_style = selector == :% ? :percent : selector
Expand All @@ -70,10 +74,10 @@ def method_name(style_name)
end

def autocorrect(node)
lambda do |corrector|
detected_method = node.method_name
return if variable_argument?(node)

case detected_method
lambda do |corrector|
case node.method_name
when :%
autocorrect_from_percent(corrector, node)
when :format, :sprintf
Expand Down
44 changes: 44 additions & 0 deletions spec/rubocop/cop/style/format_string_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,13 @@
RUBY
end

it 'registers an offense for variable argument' do
expect_offense(<<~RUBY)
puts "%f" % a
^ Favor `sprintf` over `String#%`.
RUBY
end

it 'does not register an offense for numbers' do
expect_no_offenses('puts 10 % 4')
end
Expand Down Expand Up @@ -68,6 +75,21 @@
corrected = autocorrect_source('puts x % { a: 10, b: 11 }')
expect(corrected).to eq 'puts sprintf(x, a: 10, b: 11)'
end

it 'does not auto-correct String#% with variable argument' do
source = <<~RUBY
puts "%d" % a
RUBY
expect(autocorrect_source(source)).to eq(source)
end

it 'does not auto-correct String#% with variable argument and assignment' do
source = <<~RUBY
a = something()
puts "%d" % a
RUBY
expect(autocorrect_source(source)).to eq(source)
end
end

context 'when enforced style is format' do
Expand All @@ -94,6 +116,13 @@
RUBY
end

it 'registers an offense for variable argument' do
expect_offense(<<~RUBY)
puts "%f" % a
^ Favor `format` over `String#%`.
RUBY
end

it 'does not register an offense for numbers' do
expect_no_offenses('puts 10 % 4')
end
Expand Down Expand Up @@ -142,6 +171,21 @@
corrected = autocorrect_source('puts x % { a: 10, b: 11 }')
expect(corrected).to eq 'puts format(x, a: 10, b: 11)'
end

it 'does not auto-correct String#% with variable argument' do
source = <<~RUBY
puts "%d" % a
RUBY
expect(autocorrect_source(source)).to eq(source)
end

it 'does not auto-correct String#% with variable argument and assignment' do
source = <<~RUBY
a = something()
puts "%d" % a
RUBY
expect(autocorrect_source(source)).to eq(source)
end
end

context 'when enforced style is percent' do
Expand Down

0 comments on commit c2a568f

Please sign in to comment.