Fix Lint::FormatParameterMismatch cop

CHANGELOG.md

@@ -8,6 +8,7 @@
 * [#8081](https://github.com/rubocop-hq/rubocop/issues/8081): Fix a false positive for `Lint/SuppressedException` when empty rescue block in `do` block. ([@koic][])
 * [#8096](https://github.com/rubocop-hq/rubocop/issues/8096): Fix a false positive for `Lint/SuppressedException` when empty rescue block in defs. ([@koic][])
 * [#8108](https://github.com/rubocop-hq/rubocop/issues/8108): Fix infinite loop in `Layout/HeredocIndentation` auto-correct. ([@jonas054][])
+* [#8042](https://github.com/rubocop-hq/rubocop/pull/8042): Fix raising error in `Lint::FormatParameterMismatch` when it handles invalid format strings and add new offense. ([@andrykonchin][])
 
 ## 0.85.0 (2020-06-01)
 
@@ -4571,3 +4572,4 @@
 [@jschneid]: https://github.com/jschneid
 [@ric2b]: https://github.com/ric2b
 [@burnettk]: https://github.com/burnettk
+[@andrykonchin]: https://github.com/andrykonchin

docs/modules/ROOT/pages/cops_lint.adoc

@@ -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]
@@ -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
 
 |===

lib/rubocop/cop/lint/format_parameter_mismatch.rb

@@ -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
@@ -18,26 +22,53 @@ 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)

lib/rubocop/cop/utils/format_string.rb

@@ -97,6 +97,10 @@ def format_sequences
           @format_sequences ||= parse
         end
 
+        def valid?
+          !mixed_formats?
+        end
+
         def named_interpolation?
           format_sequences.any?(&:name)
         end
@@ -114,6 +118,20 @@ def parse
             )
           end
         end
+
+        def mixed_formats?
+          formats = format_sequences.map do |seq|
+            if seq.name
+              :named
+            elsif seq.max_digit_dollar_num
+              :numbered
+            else
+              :unnumbered
+            end
+          end
+
+          formats.uniq.size > 1
+        end
       end
     end
   end

spec/rubocop/cop/lint/format_parameter_mismatch_spec.rb

@@ -171,6 +171,15 @@
     end
   end
 
+  context 'when format is invalid' do
+    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
+
   # Regression: https://github.com/rubocop-hq/rubocop/issues/3869
   context 'when passed an empty array' do
     it 'does not register an offense' do

spec/rubocop/cop/utils/format_string_spec.rb

@@ -80,4 +80,36 @@ def format_sequences(string)
     it_behaves_like 'named format sequence', '%+0<num>8.2f'
     it_behaves_like 'named format sequence', '%+08<num>.2f'
   end
+
+  describe '#valid?' do
+    it 'returns true when there are only unnumbered formats' do
+      fs = described_class.new('%s %d')
+      expect(fs.valid?).to eq true
+    end
+
+    it 'returns true when there are only numbered formats' do
+      fs = described_class.new('%1$s %2$d')
+      expect(fs.valid?).to eq true
+    end
+
+    it 'returns true when there are only named formats' do
+      fs = described_class.new('%{foo}s')
+      expect(fs.valid?).to eq true
+    end
+
+    it 'returns false when there are unnumbered and numbered formats' do
+      fs = described_class.new('%s %1$d')
+      expect(fs.valid?).to eq false
+    end
+
+    it 'returns false when there are unnumbered and named formats' do
+      fs = described_class.new('%s %{foo}d')
+      expect(fs.valid?).to eq false
+    end
+
+    it 'returns false when there are numbered and named formats' do
+      fs = described_class.new('%1$s %{foo}d')
+      expect(fs.valid?).to eq false
+    end
+  end
 end