Skip to content

Commit

Permalink
Add offense to Lint::FormatParameterMismatch about invalid format string
Browse files Browse the repository at this point in the history
  • Loading branch information
andrykonchin authored and bbatsov committed Jun 7, 2020
1 parent 937f549 commit acdab2e
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 7 deletions.
18 changes: 18 additions & 0 deletions docs/modules/ROOT/pages/cops_lint.adoc
Expand Up @@ -1100,6 +1100,10 @@ This lint sees if there is a mismatch between the number of
expected fields for format/sprintf/#% and what is actually
passed as arguments.

In addition it checks whether different formats are used in the same
format string. Do not mix numbered, unnumbered, and named formats in
the same format string.

=== Examples

[source,ruby]
Expand All @@ -1116,6 +1120,20 @@ format('A value: %s and another: %i', a_value)
format('A value: %s and another: %i', a_value, another)
----

[source,ruby]
----
# bad
format('Unnumbered format: %s and numbered: %2$s', a_value, another)
----

[source,ruby]
----
# good
format('Numbered format: %1$s and numbered %2$s', a_value, another)
----

== Lint/HeredocMethodCallPosition

|===
Expand Down
38 changes: 33 additions & 5 deletions lib/rubocop/cop/lint/format_parameter_mismatch.rb
Expand Up @@ -7,6 +7,10 @@ module Lint
# expected fields for format/sprintf/#% and what is actually
# passed as arguments.
#
# In addition it checks whether different formats are used in the same
# format string. Do not mix numbered, unnumbered, and named formats in
# the same format string.
#
# @example
#
# # bad
Expand All @@ -18,32 +22,58 @@ module Lint
# # good
#
# format('A value: %s and another: %i', a_value, another)
#
# @example
#
# # bad
#
# format('Unnumbered format: %s and numbered: %2$s', a_value, another)
#
# @example
#
# # good
#
# format('Numbered format: %1$s and numbered %2$s', a_value, another)
class FormatParameterMismatch < Cop
# http://rubular.com/r/CvpbxkcTzy
MSG = "Number of arguments (%<arg_num>i) to `%<method>s` doesn't " \
'match the number of fields (%<field_num>i).'
MSG_INVALID = 'Format string is invalid because formatting sequence types ' \
'(numbered, named or unnumbered) are mixed.'

KERNEL = 'Kernel'
SHOVEL = '<<'
STRING_TYPES = %i[str dstr].freeze

def on_send(node)
return unless format_string?(node)

if invalid_format_string?(node)
add_offense(node, location: :selector, message: MSG_INVALID)
return
end

return unless offending_node?(node)

add_offense(node, location: :selector)
end

private

def format_string?(node)
called_on_string?(node) && method_with_format_args?(node)
end

def invalid_format_string?(node)
!RuboCop::Cop::Utils::FormatString.new(node.source).valid?
end

def offending_node?(node)
return false unless called_on_string?(node)
return false unless method_with_format_args?(node)
return false if splat_args?(node)

num_of_format_args, num_of_expected_fields = count_matches(node)

return false if num_of_format_args == :unknown
return false if num_of_expected_fields == :unknown

matched_arguments_count?(num_of_expected_fields, num_of_format_args)
end
Expand Down Expand Up @@ -114,8 +144,6 @@ def expected_fields_count(node)
return :unknown unless node.str_type?

format_string = RuboCop::Cop::Utils::FormatString.new(node.source)

return :unknown unless format_string.valid?
return 1 if format_string.named_interpolation?

max_digit_dollar_num = format_string.max_digit_dollar_num
Expand Down
7 changes: 5 additions & 2 deletions spec/rubocop/cop/lint/format_parameter_mismatch_spec.rb
Expand Up @@ -172,8 +172,11 @@
end

context 'when format is invalid' do
it 'does not register an offense' do
expect_no_offenses("format('%s %2$s', 'foo', 'bar')")
it 'registers an offense' do
expect_offense(<<~RUBY)
format('%s %2$s', 'foo', 'bar')
^^^^^^ Format string is invalid because formatting sequence types (numbered, named or unnumbered) are mixed.
RUBY
end
end

Expand Down

0 comments on commit acdab2e

Please sign in to comment.