Skip to content

Commit

Permalink
Only apply SquishSQLHeredoc when the SQL has no comments
Browse files Browse the repository at this point in the history
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
  • Loading branch information
john-h-k committed Jan 27, 2023
1 parent 0b97d60 commit 325a4ca
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 1 deletion.
1 change: 1 addition & 0 deletions 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][])
9 changes: 8 additions & 1 deletion lib/rubocop/cop/rails/squished_sql_heredocs.rb
Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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
Expand Down
81 changes: 81 additions & 0 deletions spec/rubocop/cop/rails/squished_sql_heredocs_spec.rb
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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")
Expand All @@ -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

0 comments on commit 325a4ca

Please sign in to comment.