From 8eb4f1b99ac2c4d28a7aa22ad0e953544f082916 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?fn=20=E2=8C=83=20=E2=8C=A5?=
 <70830482+FnControlOption@users.noreply.github.com>
Date: Sun, 3 Oct 2021 12:18:26 -0700
Subject: [PATCH] [Fix #7900] Fix `Style/FormatStringToken` false positive with
 formatted input and `template` style enforced, and add autocorrection

---
 ...tyle_format_string_token_false_positive.md |   1 +
 lib/rubocop/cop/style/format_string_token.rb  |  65 +++++--
 .../cop/style/format_string_token_spec.rb     | 177 ++++++++++++------
 3 files changed, 171 insertions(+), 72 deletions(-)
 create mode 100644 changelog/fix_fix_style_format_string_token_false_positive.md

diff --git a/changelog/fix_fix_style_format_string_token_false_positive.md b/changelog/fix_fix_style_format_string_token_false_positive.md
new file mode 100644
index 00000000000..36eb675ed99
--- /dev/null
+++ b/changelog/fix_fix_style_format_string_token_false_positive.md
@@ -0,0 +1 @@
+* [#7900](https://github.com/rubocop/rubocop/issues/7900): Fix `Style/FormatStringToken` false positive with formatted input and `template` style enforced, and add autocorrection. ([@FnControlOption][])
diff --git a/lib/rubocop/cop/style/format_string_token.rb b/lib/rubocop/cop/style/format_string_token.rb
index 5d2256ed644..62231058f3b 100644
--- a/lib/rubocop/cop/style/format_string_token.rb
+++ b/lib/rubocop/cop/style/format_string_token.rb
@@ -69,6 +69,7 @@ module Style
       class FormatStringToken < Base
         include ConfigurableEnforcedStyle
         include IgnoredMethods
+        extend AutoCorrector
 
         def on_str(node)
           return if format_string_token?(node) || use_ignored_method?(node)
@@ -77,13 +78,8 @@ def on_str(node)
           return if detections.empty?
           return if allowed_unannotated?(detections)
 
-          detections.each do |detected_style, token_range|
-            if detected_style == style
-              correct_style_detected
-            else
-              style_detected(detected_style)
-              add_offense(token_range, message: message(detected_style))
-            end
+          detections.each do |detected_sequence, token_range|
+            check_sequence(detected_sequence, token_range)
           end
         end
 
@@ -106,6 +102,38 @@ def use_ignored_method?(node)
           send_parent && ignored_method?(send_parent.method_name)
         end
 
+        def check_sequence(detected_sequence, token_range)
+          if detected_sequence.style == style
+            correct_style_detected
+          elsif correctable_sequence?(detected_sequence.type)
+            style_detected(detected_sequence.style)
+            add_offense(token_range, message: message(detected_sequence.style)) do |corrector|
+              autocorrect_sequence(corrector, detected_sequence, token_range)
+            end
+          end
+        end
+
+        def correctable_sequence?(detected_type)
+          detected_type == 's' || style == :annotated || style == :unannotated
+        end
+
+        def autocorrect_sequence(corrector, detected_sequence, token_range)
+          return if style == :unannotated
+
+          name = detected_sequence.name
+          return if name.nil?
+
+          flags = detected_sequence.flags
+          width = detected_sequence.width
+          precision = detected_sequence.precision
+          type = detected_sequence.style == :template ? 's' : detected_sequence.type
+          correction = case style
+                       when :annotated then "%<#{name}>#{flags}#{width}#{precision}#{type}"
+                       when :template then "%#{flags}#{width}#{precision}{#{name}}"
+                       end
+          corrector.replace(token_range, correction)
+        end
+
         def unannotated_format?(node, detected_style)
           detected_style == :unannotated && !format_string_in_typical_context?(node)
         end
@@ -143,30 +171,33 @@ def str_contents(source_map)
         def token_ranges(contents)
           format_string = RuboCop::Cop::Utils::FormatString.new(contents.source)
 
-          format_string.format_sequences.each do |seq|
-            next if seq.percent?
+          format_string.format_sequences.each do |detected_sequence|
+            next if detected_sequence.percent?
 
-            detected_style = seq.style
-            token = contents.begin.adjust(begin_pos: seq.begin_pos, end_pos: seq.end_pos)
+            token = contents.begin.adjust(begin_pos: detected_sequence.begin_pos,
+                                          end_pos: detected_sequence.end_pos)
 
-            yield(detected_style, token)
+            yield(detected_sequence, token)
           end
         end
 
         def collect_detections(node)
           detections = []
-          tokens(node) do |detected_style, token_range|
-            unless unannotated_format?(node, detected_style)
-              detections << [detected_style, token_range]
+          tokens(node) do |detected_sequence, token_range|
+            unless unannotated_format?(node, detected_sequence.style)
+              detections << [detected_sequence, token_range]
             end
           end
           detections
         end
 
         def allowed_unannotated?(detections)
-          return false if detections.size > max_unannotated_placeholders_allowed
+          return false unless detections.all? do |detected_sequence,|
+                                detected_sequence.style == :unannotated
+                              end
+          return true if detections.size <= max_unannotated_placeholders_allowed
 
-          detections.all? { |detected_style,| detected_style == :unannotated }
+          detections.any? { |detected_sequence,| !correctable_sequence?(detected_sequence.type) }
         end
 
         def max_unannotated_placeholders_allowed
diff --git a/spec/rubocop/cop/style/format_string_token_spec.rb b/spec/rubocop/cop/style/format_string_token_spec.rb
index 2e1fe1d036a..a298763a1be 100644
--- a/spec/rubocop/cop/style/format_string_token_spec.rb
+++ b/spec/rubocop/cop/style/format_string_token_spec.rb
@@ -13,7 +13,7 @@
     }
   end
 
-  shared_examples 'maximum allowed unannotated' do |token|
+  shared_examples 'maximum allowed unannotated' do |token, correctable_sequence:|
     context 'when MaxUnannotatedPlaceholdersAllowed is 1' do
       before { cop_config['MaxUnannotatedPlaceholdersAllowed'] = 1 }
 
@@ -21,12 +21,20 @@
         expect_no_offenses("format('%#{token}', foo)")
       end
 
-      it 'registers offense for dual unannotated' do
-        expect_offense(<<~RUBY)
-          format('%#{token} %s', foo, bar)
-                  ^^ Prefer [...]
-                     ^^ Prefer [...]
-        RUBY
+      if correctable_sequence
+        it 'registers offense for dual unannotated' do
+          expect_offense(<<~RUBY)
+            format('%#{token} %s', foo, bar)
+                    ^^ Prefer [...]
+                       ^^ Prefer [...]
+          RUBY
+        end
+      else
+        it 'does not register offenses for dual unannotated' do
+          expect_no_offenses(<<~RUBY)
+            format('%#{token} %s', foo, bar)
+          RUBY
+        end
       end
     end
 
@@ -43,13 +51,28 @@
     end
   end
 
-  shared_examples 'enforced styles for format string tokens' do |token|
+  shared_examples 'enforced styles for format string tokens' do |token, template_correction:|
     template  = '%{template}'
     annotated = "%<named>#{token}"
 
+    template_to_annotated = '%<template>s'
+    annotated_to_template = '%{named}'
+
+    context 'when enforced style is unannotated' do
+      let(:enforced_style) { :unannotated }
+
+      specify '#correctable_sequence?' do
+        expect(cop.send(:correctable_sequence?, token)).to be true
+      end
+    end
+
     context 'when enforced style is annotated' do
       let(:enforced_style) { :annotated }
 
+      specify '#correctable_sequence?' do
+        expect(cop.send(:correctable_sequence?, token)).to be true
+      end
+
       it 'registers offenses for template style' do
         expect_offense(<<~RUBY, annotated: annotated, template: template)
           <<-HEREDOC
@@ -58,7 +81,11 @@
           HEREDOC
         RUBY
 
-        expect_no_corrections
+        expect_correction(<<~RUBY)
+          <<-HEREDOC
+          foo #{annotated} + bar #{template_to_annotated}
+          HEREDOC
+        RUBY
       end
 
       it 'supports dynamic string with interpolation' do
@@ -67,7 +94,9 @@
                 _{annotated}      ^{template} Prefer annotated tokens [...]
         RUBY
 
-        expect_no_corrections
+        expect_correction(<<~RUBY)
+          "a\#{b}#{annotated} c\#{d}#{template_to_annotated} e\#{f}"
+        RUBY
       end
 
       it 'sets the enforced style to annotated after inspecting "%<a>s"' do
@@ -76,52 +105,82 @@
         expect(cop.config_to_allow_offenses).to eq('EnforcedStyle' => 'annotated')
       end
 
-      it 'configures the enforced style to template after inspecting "%{a}"' do
+      it 'detects when the cop must be disabled to avoid offenses' do
         expect_offense(<<~RUBY)
           "%{a}"
            ^^^^ Prefer annotated tokens [...]
         RUBY
 
-        expect_no_corrections
+        expect_correction(<<~RUBY)
+          "%<a>s"
+        RUBY
 
-        expect(cop.config_to_allow_offenses).to eq('EnforcedStyle' => 'template')
+        expect(cop.config_to_allow_offenses).to eq('Enabled' => false)
       end
 
-      it_behaves_like 'maximum allowed unannotated', token
+      it_behaves_like 'maximum allowed unannotated', token, correctable_sequence: true
     end
 
     context 'when enforced style is template' do
       let(:enforced_style) { :template }
 
-      it 'registers offenses for annotated style' do
-        expect_offense(<<~RUBY, annotated: annotated, template: template)
-          <<-HEREDOC
-          foo %{template} + bar %{annotated}
-              _{template}       ^{annotated} Prefer template tokens [...]
-          HEREDOC
-        RUBY
+      specify '#correctable_sequence?' do
+        expect(cop.send(:correctable_sequence?, token)).to be template_correction
+      end
 
-        expect_no_corrections
+      if template_correction
+        it 'registers offenses for annotated style' do
+          expect_offense(<<~RUBY, annotated: annotated, template: template)
+            <<-HEREDOC
+            foo %{template} + bar %{annotated}
+                _{template}       ^{annotated} Prefer template tokens [...]
+            HEREDOC
+          RUBY
+
+          expect_correction(<<~RUBY)
+            <<-HEREDOC
+            foo #{template} + bar #{annotated_to_template}
+            HEREDOC
+          RUBY
+        end
+      else
+        it 'does not register offenses for annotated style' do
+          expect_no_offenses(<<~RUBY, annotated: annotated, template: template)
+            <<-HEREDOC
+            foo %{template} + bar %{annotated}
+            HEREDOC
+          RUBY
+        end
       end
 
       it 'supports dynamic string with interpolation' do
-        expect_offense(<<~'RUBY', annotated: annotated, template: template)
-          "a#{b}%{template} c#{d}%{annotated} e#{f}"
-                _{template}      ^{annotated} Prefer template tokens [...]
-        RUBY
-
-        expect_no_corrections
+        if template_correction
+          expect_offense(<<~'RUBY', annotated: annotated, template: template)
+            "a#{b}%{template} c#{d}%{annotated} e#{f}"
+                  _{template}      ^{annotated} Prefer template tokens [...]
+          RUBY
+
+          expect_correction(<<~RUBY)
+            "a\#{b}#{template} c\#{d}#{annotated_to_template} e\#{f}"
+          RUBY
+        else
+          expect_no_offenses(<<~'RUBY', annotated: annotated, template: template)
+            "a#{b}%{template} c#{d}%{annotated} e#{f}"
+          RUBY
+        end
       end
 
-      it 'sets the enforced style to annotated after inspecting "%<a>s"' do
+      it 'detects when the cop must be disabled to avoid offenses' do
         expect_offense(<<~RUBY)
           "%<a>s"
            ^^^^^ Prefer template tokens [...]
         RUBY
 
-        expect_no_corrections
+        expect_correction(<<~RUBY)
+          "%{a}"
+        RUBY
 
-        expect(cop.config_to_allow_offenses).to eq('EnforcedStyle' => 'annotated')
+        expect(cop.config_to_allow_offenses).to eq('Enabled' => false)
       end
 
       it 'configures the enforced style to template after inspecting "%{a}"' do
@@ -130,7 +189,7 @@
         expect(cop.config_to_allow_offenses).to eq('EnforcedStyle' => 'template')
       end
 
-      it_behaves_like 'maximum allowed unannotated', token
+      it_behaves_like 'maximum allowed unannotated', token, correctable_sequence: template_correction
     end
   end
 
@@ -178,7 +237,9 @@
             ^^^^^^^^^^^ Prefer annotated tokens (like `%<foo>s`) over template tokens (like `%{foo}`).
     RUBY
 
-    expect_no_corrections
+    expect_correction(<<~'RUBY')
+      "c#{b}%<template>s"
+    RUBY
   end
 
   it 'ignores http links' do
@@ -205,7 +266,9 @@
                      ^^^^^^ Prefer annotated tokens (like `%<foo>s`) over template tokens (like `%{foo}`).
     RUBY
 
-    expect_no_corrections
+    expect_correction(<<~RUBY)
+      { bar: format('%<foo>s', foo: 'foo') }
+    RUBY
   end
 
   it 'supports flags and modifiers' do
@@ -222,24 +285,24 @@
     expect_no_offenses('__FILE__')
   end
 
-  it_behaves_like 'enforced styles for format string tokens', 'A'
-  it_behaves_like 'enforced styles for format string tokens', 'B'
-  it_behaves_like 'enforced styles for format string tokens', 'E'
-  it_behaves_like 'enforced styles for format string tokens', 'G'
-  it_behaves_like 'enforced styles for format string tokens', 'X'
-  it_behaves_like 'enforced styles for format string tokens', 'a'
-  it_behaves_like 'enforced styles for format string tokens', 'b'
-  it_behaves_like 'enforced styles for format string tokens', 'c'
-  it_behaves_like 'enforced styles for format string tokens', 'd'
-  it_behaves_like 'enforced styles for format string tokens', 'e'
-  it_behaves_like 'enforced styles for format string tokens', 'f'
-  it_behaves_like 'enforced styles for format string tokens', 'g'
-  it_behaves_like 'enforced styles for format string tokens', 'i'
-  it_behaves_like 'enforced styles for format string tokens', 'o'
-  it_behaves_like 'enforced styles for format string tokens', 'p'
-  it_behaves_like 'enforced styles for format string tokens', 's'
-  it_behaves_like 'enforced styles for format string tokens', 'u'
-  it_behaves_like 'enforced styles for format string tokens', 'x'
+  it_behaves_like 'enforced styles for format string tokens', 'A', template_correction: false
+  it_behaves_like 'enforced styles for format string tokens', 'B', template_correction: false
+  it_behaves_like 'enforced styles for format string tokens', 'E', template_correction: false
+  it_behaves_like 'enforced styles for format string tokens', 'G', template_correction: false
+  it_behaves_like 'enforced styles for format string tokens', 'X', template_correction: false
+  it_behaves_like 'enforced styles for format string tokens', 'a', template_correction: false
+  it_behaves_like 'enforced styles for format string tokens', 'b', template_correction: false
+  it_behaves_like 'enforced styles for format string tokens', 'c', template_correction: false
+  it_behaves_like 'enforced styles for format string tokens', 'd', template_correction: false
+  it_behaves_like 'enforced styles for format string tokens', 'e', template_correction: false
+  it_behaves_like 'enforced styles for format string tokens', 'f', template_correction: false
+  it_behaves_like 'enforced styles for format string tokens', 'g', template_correction: false
+  it_behaves_like 'enforced styles for format string tokens', 'i', template_correction: false
+  it_behaves_like 'enforced styles for format string tokens', 'o', template_correction: false
+  it_behaves_like 'enforced styles for format string tokens', 'p', template_correction: false
+  it_behaves_like 'enforced styles for format string tokens', 's', template_correction: true
+  it_behaves_like 'enforced styles for format string tokens', 'u', template_correction: false
+  it_behaves_like 'enforced styles for format string tokens', 'x', template_correction: false
 
   context 'when enforced style is annotated' do
     let(:enforced_style) { :annotated }
@@ -250,7 +313,9 @@
          ^^^^^^ Prefer annotated tokens (like `%<foo>s`) over template tokens (like `%{foo}`).
       RUBY
 
-      expect_no_corrections
+      expect_correction(<<~RUBY)
+        "%<foo>s"
+      RUBY
     end
 
     context 'when `IgnoredMethods: redirect`' do
@@ -293,11 +358,13 @@
 
     it 'gives a helpful error message' do
       expect_offense(<<~RUBY)
-        "%<foo>d"
+        "%<foo>s"
          ^^^^^^^ Prefer template tokens (like `%{foo}`) over annotated tokens (like `%<foo>s`).
       RUBY
 
-      expect_no_corrections
+      expect_correction(<<~RUBY)
+        "%{foo}"
+      RUBY
     end
   end