Merge pull request rubocop#929 from john-h-k/fix/sql-squish-comments

Only apply SquishSQLHeredoc when the SQL has no comments

Author: koic

changelog/fix_invalid_squish_sql_heredoc.md

@@ -0,0 +1 @@
+* [#929](https://github.com/rubocop/rubocop-rails/issues/929): Prevent `Rails/SquishedSQLHeredocs` applying when single-line comments are present. ([@john-h-k][])

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
 
         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,12 @@ def using_squish?(node)
           node.parent&.send_type? && node.parent&.method?(:squish)
         end
 
+        def singleline_comments_present?(node)
+          sql = node.children.map { |c| c.is_a?(String) ? c : c.source }.join('\n')
+
+          sql.gsub(SQL_IDENTIFIER_MARKERS, '').include?('--')
+        end
+
         def message(node)
           format(MSG, expect: "#{node.source}#{SQUISH}", current: node.source)
         end

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,13 +74,36 @@
       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
           SELECT * FROM posts;
         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