Skip to content

Commit

Permalink
Lint/FormatParameterMismatch ignores % if LHS or RHS is not a literal
Browse files Browse the repository at this point in the history
If the LHS is not a literal, it could evaluate to a format string with any
number of arguments, so we can't count an offense for that. On the other hand,
if the RHS is not a literal, it could evaluate to an array with any number
of arguments, so we can't count an offense for that either.

Fixes rubocop#2307.
  • Loading branch information
alexdowad committed Oct 27, 2015
1 parent eb0d726 commit cbb5019
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 6 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
* [#2331](https://github.com/bbatsov/rubocop/issues/2331): Do not register an offense in `Performance/Size` for `count` with an argument. ([@rrosenblum][])
* Handle a backslash at the end of a line in `Style/SpaceAroundOperators`. ([@lumeet][])
* Don't warn about lack of "leading space" in a =begin/=end comment. ([@alexdowad][])
* [#2307](https://github.com/bbatsov/rubocop/issues/2307): In `Lint/FormatParameterMismatch`, don't register an offense if either argument to % is not a literal. ([@alexdowad][])

## 0.34.2 (21/09/2015)

Expand Down
30 changes: 24 additions & 6 deletions lib/rubocop/cop/lint/format_parameter_mismatch.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,10 @@ def offending_node?(node)
false
else
num_of_format_args, num_of_expected_fields = count_matches(node)
num_of_expected_fields != num_of_format_args

num_of_format_args != :unknown &&
num_of_expected_fields != :unknown &&
num_of_expected_fields != num_of_format_args
end
else
false
Expand Down Expand Up @@ -79,22 +82,36 @@ def heredoc?(node)
args.loc.expression.source[0, 2] == SHOVEL
end

def literal?(node)
node.int_type? ||
node.str_type? ||
node.sym_type? ||
node.float_type? ||
node.dstr_type? ||
node.dsym_type?
end

def count_matches(node)
receiver_node, _method_name, *args = *node

if (sprintf?(node) || format?(node)) && !heredoc?(node)
number_of_args_for_format = (args.size - 1)
number_of_expected_fields = expected_fields_count(args.first)
elsif percent?(node)
first_child_argument = args.first
elsif percent?(node) && args.first.array_type?
number_of_expected_fields = expected_fields_count(receiver_node)

first_child_argument = args.first
if first_child_argument.array_type?
number_of_args_for_format = args.first.child_nodes.size
number_of_expected_fields = expected_fields_count(receiver_node)
else
elsif literal?(first_child_argument)
number_of_args_for_format = 1
number_of_expected_fields = expected_fields_count(receiver_node)
else
# non-literals might evaluate to an Array, or they might not
# so we can't tell just how many format args there will be
number_of_args_for_format = :unknown
end
else
number_of_args_for_format = number_of_expected_fields = :unknown
end

[number_of_args_for_format, number_of_expected_fields]
Expand All @@ -113,6 +130,7 @@ def format_method?(name, node)
end

def expected_fields_count(node)
return :unknown unless node.str_type?
node
.loc
.expression
Expand Down
24 changes: 24 additions & 0 deletions spec/rubocop/cop/lint/format_parameter_mismatch_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,30 @@
expect(cop.offenses).to be_empty
end

context 'when multiple arguments are called for' do
context 'and a single variable argument is passed' do
it 'does not register an offense' do
# the variable could evaluate to an array
inspect_source(cop, 'puts "%s %s" % var')
expect(cop.offenses).to be_empty
end
end

context 'and a single send node is passed' do
it 'does not register an offense' do
inspect_source(cop, 'puts "%s %s" % ("ab".chars)')
expect(cop.offenses).to be_empty
end
end
end

context 'when format is not a string literal' do
it 'does not register an offense' do
inspect_source(cop, 'puts str % [1, 2]')
expect(cop.offenses).to be_empty
end
end

it 'ignores percent right next to format string' do
inspect_source(cop, 'format("%0.1f%% percent", 22.5)')
expect(cop.offenses).to be_empty
Expand Down

0 comments on commit cbb5019

Please sign in to comment.