From a09eaddcaa024bee84fc5c20243165fb0b63f519 Mon Sep 17 00:00:00 2001 From: James Dennes <jdennes@gmail.com> Date: Wed, 17 Jul 2024 16:48:39 +0200 Subject: [PATCH 1/4] Use meaningful message for locals offense --- lib/rubocop/cop/github/rails_controller_render_literal.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/rubocop/cop/github/rails_controller_render_literal.rb b/lib/rubocop/cop/github/rails_controller_render_literal.rb index 0f2efd39..44c82bd5 100644 --- a/lib/rubocop/cop/github/rails_controller_render_literal.rb +++ b/lib/rubocop/cop/github/rails_controller_render_literal.rb @@ -11,6 +11,8 @@ class RailsControllerRenderLiteral < Base MSG = "render must be used with a string literal or an instance of a Class" + LOCALS_MSG = "render must be used with a hash literal for the locals keyword argument" + def_node_matcher :ignore_key?, <<-PATTERN (pair (sym { :body @@ -99,7 +101,7 @@ def on_send(node) if option_pairs locals = option_pairs.map { |pair| locals_key?(pair) }.compact.first if locals && (!locals.hash_type? || !hash_with_literal_keys?(locals)) - add_offense(node) + add_offense(node, message: LOCALS_MSG) end end end From 9cfbd5915f2cf1f8ac3745dddbb8341a46e8b1d2 Mon Sep 17 00:00:00 2001 From: James Dennes <jdennes@gmail.com> Date: Wed, 17 Jul 2024 16:54:09 +0200 Subject: [PATCH 2/4] Just use a single message --- lib/rubocop/cop/github/rails_controller_render_literal.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/rubocop/cop/github/rails_controller_render_literal.rb b/lib/rubocop/cop/github/rails_controller_render_literal.rb index 44c82bd5..4ed294e1 100644 --- a/lib/rubocop/cop/github/rails_controller_render_literal.rb +++ b/lib/rubocop/cop/github/rails_controller_render_literal.rb @@ -9,9 +9,7 @@ module GitHub class RailsControllerRenderLiteral < Base include RenderLiteralHelpers - MSG = "render must be used with a string literal or an instance of a Class" - - LOCALS_MSG = "render must be used with a hash literal for the locals keyword argument" + MSG = "render must be used with a string literal or an instance of a Class, and use literals for locals keys" def_node_matcher :ignore_key?, <<-PATTERN (pair (sym { @@ -101,7 +99,7 @@ def on_send(node) if option_pairs locals = option_pairs.map { |pair| locals_key?(pair) }.compact.first if locals && (!locals.hash_type? || !hash_with_literal_keys?(locals)) - add_offense(node, message: LOCALS_MSG) + add_offense(node) end end end From 0aed77073ef16108388ec01fea97d1ccbbf3723e Mon Sep 17 00:00:00 2001 From: James Dennes <jdennes@gmail.com> Date: Wed, 17 Jul 2024 16:54:15 +0200 Subject: [PATCH 3/4] Update tests --- test/test_rails_controller_render_literal.rb | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/test/test_rails_controller_render_literal.rb b/test/test_rails_controller_render_literal.rb index 5acd5c26..753399a9 100644 --- a/test/test_rails_controller_render_literal.rb +++ b/test/test_rails_controller_render_literal.rb @@ -274,7 +274,7 @@ def index RUBY assert_equal 1, offenses.count - assert_equal "#{cop.name}: render must be used with a string literal or an instance of a Class", offenses[0].message + assert_equal "#{cop.name}: render must be used with a string literal or an instance of a Class, and use literals for locals keys", offenses[0].message end def test_render_implicit_with_layout_offense @@ -287,7 +287,7 @@ def index RUBY assert_equal 1, offenses.count - assert_equal "#{cop.name}: render must be used with a string literal or an instance of a Class", offenses[0].message + assert_equal "#{cop.name}: render must be used with a string literal or an instance of a Class, and use literals for locals keys", offenses[0].message end def test_render_implicit_with_status_offense @@ -300,7 +300,7 @@ def index RUBY assert_equal 1, offenses.count - assert_equal "#{cop.name}: render must be used with a string literal or an instance of a Class", offenses[0].message + assert_equal "#{cop.name}: render must be used with a string literal or an instance of a Class, and use literals for locals keys", offenses[0].message end def test_render_variable_offense @@ -313,7 +313,7 @@ def index RUBY assert_equal 1, offenses.count - assert_equal "#{cop.name}: render must be used with a string literal or an instance of a Class", offenses[0].message + assert_equal "#{cop.name}: render must be used with a string literal or an instance of a Class, and use literals for locals keys", offenses[0].message end def test_render_to_string_variable_offense @@ -326,7 +326,7 @@ def index RUBY assert_equal 1, offenses.count - assert_equal "#{cop.name}: render must be used with a string literal or an instance of a Class", offenses[0].message + assert_equal "#{cop.name}: render must be used with a string literal or an instance of a Class, and use literals for locals keys", offenses[0].message end def test_render_action_variable_offense @@ -339,7 +339,7 @@ def index RUBY assert_equal 1, offenses.count - assert_equal "#{cop.name}: render must be used with a string literal or an instance of a Class", offenses[0].message + assert_equal "#{cop.name}: render must be used with a string literal or an instance of a Class, and use literals for locals keys", offenses[0].message end def test_render_template_variable_offense @@ -352,7 +352,7 @@ def index RUBY assert_equal 1, offenses.count - assert_equal "#{cop.name}: render must be used with a string literal or an instance of a Class", offenses[0].message + assert_equal "#{cop.name}: render must be used with a string literal or an instance of a Class, and use literals for locals keys", offenses[0].message end def test_render_partial_variable_offense @@ -365,7 +365,7 @@ def index RUBY assert_equal 1, offenses.count - assert_equal "#{cop.name}: render must be used with a string literal or an instance of a Class", offenses[0].message + assert_equal "#{cop.name}: render must be used with a string literal or an instance of a Class, and use literals for locals keys", offenses[0].message end def test_render_template_with_layout_variable_offense @@ -378,7 +378,7 @@ def index RUBY assert_equal 1, offenses.count - assert_equal "#{cop.name}: render must be used with a string literal or an instance of a Class", offenses[0].message + assert_equal "#{cop.name}: render must be used with a string literal or an instance of a Class, and use literals for locals keys", offenses[0].message end def test_render_template_variable_with_layout_offense @@ -391,7 +391,7 @@ def index RUBY assert_equal 1, offenses.count - assert_equal "#{cop.name}: render must be used with a string literal or an instance of a Class", offenses[0].message + assert_equal "#{cop.name}: render must be used with a string literal or an instance of a Class, and use literals for locals keys", offenses[0].message end def test_render_shorthand_static_locals_no_offsense From cec95e7afbc1926b49b1b653461ae9cd4d67812a Mon Sep 17 00:00:00 2001 From: James Dennes <jdennes@gmail.com> Date: Wed, 17 Jul 2024 16:58:00 +0200 Subject: [PATCH 4/4] Make line length reasonable --- test/test_rails_controller_render_literal.rb | 40 +++++++++++++++----- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/test/test_rails_controller_render_literal.rb b/test/test_rails_controller_render_literal.rb index 753399a9..eb13928f 100644 --- a/test/test_rails_controller_render_literal.rb +++ b/test/test_rails_controller_render_literal.rb @@ -274,7 +274,9 @@ def index RUBY assert_equal 1, offenses.count - assert_equal "#{cop.name}: render must be used with a string literal or an instance of a Class, and use literals for locals keys", offenses[0].message + assert_equal \ + "#{cop.name}: render must be used with a string literal or an instance of a Class, and use literals for locals keys", + offenses[0].message end def test_render_implicit_with_layout_offense @@ -287,7 +289,9 @@ def index RUBY assert_equal 1, offenses.count - assert_equal "#{cop.name}: render must be used with a string literal or an instance of a Class, and use literals for locals keys", offenses[0].message + assert_equal \ + "#{cop.name}: render must be used with a string literal or an instance of a Class, and use literals for locals keys", + offenses[0].message end def test_render_implicit_with_status_offense @@ -300,7 +304,9 @@ def index RUBY assert_equal 1, offenses.count - assert_equal "#{cop.name}: render must be used with a string literal or an instance of a Class, and use literals for locals keys", offenses[0].message + assert_equal \ + "#{cop.name}: render must be used with a string literal or an instance of a Class, and use literals for locals keys", + offenses[0].message end def test_render_variable_offense @@ -313,7 +319,9 @@ def index RUBY assert_equal 1, offenses.count - assert_equal "#{cop.name}: render must be used with a string literal or an instance of a Class, and use literals for locals keys", offenses[0].message + assert_equal \ + "#{cop.name}: render must be used with a string literal or an instance of a Class, and use literals for locals keys", + offenses[0].message end def test_render_to_string_variable_offense @@ -326,7 +334,9 @@ def index RUBY assert_equal 1, offenses.count - assert_equal "#{cop.name}: render must be used with a string literal or an instance of a Class, and use literals for locals keys", offenses[0].message + assert_equal \ + "#{cop.name}: render must be used with a string literal or an instance of a Class, and use literals for locals keys", + offenses[0].message end def test_render_action_variable_offense @@ -339,7 +349,9 @@ def index RUBY assert_equal 1, offenses.count - assert_equal "#{cop.name}: render must be used with a string literal or an instance of a Class, and use literals for locals keys", offenses[0].message + assert_equal \ + "#{cop.name}: render must be used with a string literal or an instance of a Class, and use literals for locals keys", + offenses[0].message end def test_render_template_variable_offense @@ -352,7 +364,9 @@ def index RUBY assert_equal 1, offenses.count - assert_equal "#{cop.name}: render must be used with a string literal or an instance of a Class, and use literals for locals keys", offenses[0].message + assert_equal \ + "#{cop.name}: render must be used with a string literal or an instance of a Class, and use literals for locals keys", + offenses[0].message end def test_render_partial_variable_offense @@ -365,7 +379,9 @@ def index RUBY assert_equal 1, offenses.count - assert_equal "#{cop.name}: render must be used with a string literal or an instance of a Class, and use literals for locals keys", offenses[0].message + assert_equal \ + "#{cop.name}: render must be used with a string literal or an instance of a Class, and use literals for locals keys", + offenses[0].message end def test_render_template_with_layout_variable_offense @@ -378,7 +394,9 @@ def index RUBY assert_equal 1, offenses.count - assert_equal "#{cop.name}: render must be used with a string literal or an instance of a Class, and use literals for locals keys", offenses[0].message + assert_equal \ + "#{cop.name}: render must be used with a string literal or an instance of a Class, and use literals for locals keys", + offenses[0].message end def test_render_template_variable_with_layout_offense @@ -391,7 +409,9 @@ def index RUBY assert_equal 1, offenses.count - assert_equal "#{cop.name}: render must be used with a string literal or an instance of a Class, and use literals for locals keys", offenses[0].message + assert_equal \ + "#{cop.name}: render must be used with a string literal or an instance of a Class, and use literals for locals keys", + offenses[0].message end def test_render_shorthand_static_locals_no_offsense