Skip to content

Commit 325a4ca

Browse files
committed
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
1 parent 0b97d60 commit 325a4ca

File tree

3 files changed

+90
-1
lines changed

3 files changed

+90
-1
lines changed
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ class SquishedSQLHeredocs < Base
4848
SQL = 'SQL'
4949
SQUISH = '.squish'
5050
MSG = 'Use `%<expect>s` instead of `%<current>s`.'
51+
SQL_IDENTIFIER_MARKERS = /(".+?")|('.+?')|(\[.+?\])/.freeze
5152

5253
def on_heredoc(node)
5354
return unless offense_detected?(node)
@@ -60,7 +61,7 @@ def on_heredoc(node)
6061
private
6162

6263
def offense_detected?(node)
63-
sql_heredoc?(node) && !using_squish?(node)
64+
sql_heredoc?(node) && !using_squish?(node) && !singleline_comments_present?(node)
6465
end
6566

6667
def sql_heredoc?(node)
@@ -71,6 +72,12 @@ def using_squish?(node)
7172
node.parent&.send_type? && node.parent&.method?(:squish)
7273
end
7374

75+
def singleline_comments_present?(node)
76+
sql = node.children.map { |c| c.is_a?(String) ? c : c.source }.join('\n')
77+
78+
sql.gsub(SQL_IDENTIFIER_MARKERS, '').include?('--')
79+
end
80+
7481
def message(node)
7582
format(MSG, expect: "#{node.source}#{SQUISH}", current: node.source)
7683
end

spec/rubocop/cop/rails/squished_sql_heredocs_spec.rb

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,25 @@
1919
RUBY
2020
end
2121

22+
it 'registers an offense and corrects it with comment-like string and identifiers' do
23+
expect_offense(<<~RUBY)
24+
<<~SQL
25+
^^^^^^ Use `<<~SQL.squish` instead of `<<~SQL`.
26+
SELECT * FROM posts
27+
WHERE [--another identifier!] = 1
28+
AND "-- this is a name, not a comment" = '-- this is a string, not a comment'
29+
SQL
30+
RUBY
31+
32+
expect_correction(<<~RUBY)
33+
<<~SQL.squish
34+
SELECT * FROM posts
35+
WHERE [--another identifier!] = 1
36+
AND "-- this is a name, not a comment" = '-- this is a string, not a comment'
37+
SQL
38+
RUBY
39+
end
40+
2241
it 'does not register an offense' do
2342
expect_no_offenses(<<~RUBY)
2443
<<-SQL.squish
@@ -27,6 +46,16 @@
2746
SQL
2847
RUBY
2948
end
49+
50+
it 'does not register an offense due to comments' do
51+
expect_no_offenses(<<~RUBY)
52+
<<-SQL
53+
SELECT * FROM posts
54+
-- This is a comment, so squish can't be used
55+
WHERE id = 1
56+
SQL
57+
RUBY
58+
end
3059
end
3160

3261
context 'with single line heredoc' do
@@ -45,13 +74,36 @@
4574
RUBY
4675
end
4776

77+
it 'registers an offense and corrects it with comment-like string and identifiers' do
78+
expect_offense(<<~RUBY)
79+
<<~SQL
80+
^^^^^^ Use `<<~SQL.squish` instead of `<<~SQL`.
81+
SELECT * FROM posts WHERE [--another identifier!] = 1 AND "-- this is a name, not a comment" = '-- this is a string, not a comment';
82+
SQL
83+
RUBY
84+
85+
expect_correction(<<~RUBY)
86+
<<~SQL.squish
87+
SELECT * FROM posts WHERE [--another identifier!] = 1 AND "-- this is a name, not a comment" = '-- this is a string, not a comment';
88+
SQL
89+
RUBY
90+
end
91+
4892
it 'does not register an offense' do
4993
expect_no_offenses(<<~RUBY)
5094
<<-SQL.squish
5195
SELECT * FROM posts;
5296
SQL
5397
RUBY
5498
end
99+
100+
it 'does not register an offense due to comments' do
101+
expect_no_offenses(<<~RUBY)
102+
<<-SQL
103+
-- This is a comment, so squish can't be used
104+
SQL
105+
RUBY
106+
end
55107
end
56108

57109
context 'with heredocs as method parameters' do
@@ -72,6 +124,25 @@
72124
RUBY
73125
end
74126

127+
it 'registers an offense and corrects it with comment-like string and identifiers' do
128+
expect_offense(<<~RUBY)
129+
execute(<<~SQL, "Post Load")
130+
^^^^^^ Use `<<~SQL.squish` instead of `<<~SQL`.
131+
SELECT * FROM posts
132+
WHERE [--another identifier!] = 1
133+
AND "-- this is a name, not a comment" = '-- this is a string, not a comment'
134+
SQL
135+
RUBY
136+
137+
expect_correction(<<~RUBY)
138+
execute(<<~SQL.squish, "Post Load")
139+
SELECT * FROM posts
140+
WHERE [--another identifier!] = 1
141+
AND "-- this is a name, not a comment" = '-- this is a string, not a comment'
142+
SQL
143+
RUBY
144+
end
145+
75146
it 'does not register an offense' do
76147
expect_no_offenses(<<~RUBY)
77148
execute(<<~SQL.squish, "Post Load")
@@ -80,5 +151,15 @@
80151
SQL
81152
RUBY
82153
end
154+
155+
it 'does not register an offense due to comments' do
156+
expect_no_offenses(<<~RUBY)
157+
execute(<<-SQL, "Post Load")
158+
SELECT * FROM posts
159+
-- This is a comment, so squish can't be used
160+
WHERE post_id = 1
161+
SQL
162+
RUBY
163+
end
83164
end
84165
end

0 commit comments

Comments
 (0)