From 12ca67210ba924d247768a087f6a1c9a80cdf4fe Mon Sep 17 00:00:00 2001
From: John Kelly <johnharrykelly@gmail.com>
Date: Thu, 26 Jan 2023 17:47:17 +0000
Subject: [PATCH] Only apply SquishSQLHeredoc when the SQL has no comments

Removes all strings and identifiers from the SQL and then checks it for single-comment markers, and does not apply the cop if they are present
---
 .../cop/rails/squished_sql_heredocs.rb        | 11 ++-
 .../cop/rails/squished_sql_heredocs_spec.rb   | 81 +++++++++++++++++++
 2 files changed, 91 insertions(+), 1 deletion(-)

diff --git a/lib/rubocop/cop/rails/squished_sql_heredocs.rb b/lib/rubocop/cop/rails/squished_sql_heredocs.rb
index 55f5654baf..d75c657e31 100644
--- a/lib/rubocop/cop/rails/squished_sql_heredocs.rb
+++ b/lib/rubocop/cop/rails/squished_sql_heredocs.rb
@@ -48,6 +48,7 @@ class SquishedSQLHeredocs < Base
         SQL = 'SQL'
         SQUISH = '.squish'
         MSG = 'Use `%<expect>s` instead of `%<current>s`.'
+        SQL_IDENTIFIER_MARKERS = [/".+?"/.freeze, /'.+?'/.freeze, /\[.+?\]/.freeze].freeze
 
         def on_heredoc(node)
           return unless offense_detected?(node)
@@ -60,7 +61,7 @@ def on_heredoc(node)
         private
 
         def offense_detected?(node)
-          sql_heredoc?(node) && !using_squish?(node)
+          sql_heredoc?(node) && !using_squish?(node) && !singleline_comments_present?(node)
         end
 
         def sql_heredoc?(node)
@@ -71,6 +72,14 @@ def using_squish?(node)
           node.parent&.send_type? && node.parent&.method?(:squish)
         end
 
+        def singleline_comments_present?(node)
+          text = node.children&.map { |c| c.is_a?(String) ? c : c.value }.join('\n')
+
+          SQL_IDENTIFIER_MARKERS.each { |m| text = text.sub(m, '') }
+          
+          text.include?("--")
+        end
+
         def message(node)
           format(MSG, expect: "#{node.source}#{SQUISH}", current: node.source)
         end
diff --git a/spec/rubocop/cop/rails/squished_sql_heredocs_spec.rb b/spec/rubocop/cop/rails/squished_sql_heredocs_spec.rb
index 0baf039957..cdeb265a6f 100644
--- a/spec/rubocop/cop/rails/squished_sql_heredocs_spec.rb
+++ b/spec/rubocop/cop/rails/squished_sql_heredocs_spec.rb
@@ -19,6 +19,25 @@
       RUBY
     end
 
+    it 'registers an offense and corrects it with comment-like string and identifiers' do
+      expect_offense(<<~RUBY)
+        <<~SQL
+        ^^^^^^ Use `<<~SQL.squish` instead of `<<~SQL`.
+          SELECT * FROM posts
+            WHERE [--another identifier!] = 1
+            AND "-- this is a name, not a comment" = '-- this is a string, not a comment'
+        SQL
+      RUBY
+
+      expect_correction(<<~RUBY)
+        <<~SQL.squish
+          SELECT * FROM posts
+            WHERE [--another identifier!] = 1
+            AND "-- this is a name, not a comment" = '-- this is a string, not a comment'
+        SQL
+      RUBY
+    end
+
     it 'does not register an offense' do
       expect_no_offenses(<<~RUBY)
         <<-SQL.squish
@@ -27,6 +46,16 @@
         SQL
       RUBY
     end
+
+    it 'does not register an offense due to comments' do
+      expect_no_offenses(<<~RUBY)
+        <<-SQL
+          SELECT * FROM posts
+            -- This is a comment, so squish can't be used
+            WHERE id = 1
+        SQL
+      RUBY
+    end
   end
 
   context 'with single line heredoc' do
@@ -45,6 +74,21 @@
       RUBY
     end
 
+    it 'registers an offense and corrects it with comment-like string and identifiers' do
+      expect_offense(<<~RUBY)
+        <<~SQL
+        ^^^^^^ Use `<<~SQL.squish` instead of `<<~SQL`.
+          SELECT * FROM posts WHERE [--another identifier!] = 1 AND "-- this is a name, not a comment" = '-- this is a string, not a comment';
+        SQL
+      RUBY
+
+      expect_correction(<<~RUBY)
+        <<~SQL.squish
+          SELECT * FROM posts WHERE [--another identifier!] = 1 AND "-- this is a name, not a comment" = '-- this is a string, not a comment';
+        SQL
+      RUBY
+    end
+
     it 'does not register an offense' do
       expect_no_offenses(<<~RUBY)
         <<-SQL.squish
@@ -52,6 +96,14 @@
         SQL
       RUBY
     end
+
+    it 'does not register an offense due to comments' do
+      expect_no_offenses(<<~RUBY)
+        <<-SQL
+          -- This is a comment, so squish can't be used
+        SQL
+      RUBY
+    end
   end
 
   context 'with heredocs as method parameters' do
@@ -72,6 +124,25 @@
       RUBY
     end
 
+    it 'registers an offense and corrects it with comment-like string and identifiers' do
+      expect_offense(<<~RUBY)
+        execute(<<~SQL, "Post Load")
+                ^^^^^^ Use `<<~SQL.squish` instead of `<<~SQL`.
+          SELECT * FROM posts
+            WHERE [--another identifier!] = 1
+            AND "-- this is a name, not a comment" = '-- this is a string, not a comment'
+        SQL
+      RUBY
+
+      expect_correction(<<~RUBY)
+        execute(<<~SQL.squish, "Post Load")
+          SELECT * FROM posts
+            WHERE [--another identifier!] = 1
+            AND "-- this is a name, not a comment" = '-- this is a string, not a comment'
+        SQL
+      RUBY
+    end
+
     it 'does not register an offense' do
       expect_no_offenses(<<~RUBY)
         execute(<<~SQL.squish, "Post Load")
@@ -80,5 +151,15 @@
         SQL
       RUBY
     end
+
+    it 'does not register an offense due to comments' do
+      expect_no_offenses(<<~RUBY)
+        execute(<<-SQL, "Post Load")
+          SELECT * FROM posts
+          -- This is a comment, so squish can't be used
+            WHERE post_id = 1
+        SQL
+      RUBY
+    end
   end
 end