Skip to content

Commit

Permalink
Check for local key literals in controllers
Browse files Browse the repository at this point in the history
  • Loading branch information
jhawthorn committed Jan 14, 2021
1 parent 6c39739 commit 54f3e11
Show file tree
Hide file tree
Showing 4 changed files with 94 additions and 11 deletions.
26 changes: 25 additions & 1 deletion lib/rubocop/cop/github/rails_controller_render_literal.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,9 @@ class RailsControllerRenderLiteral < Cop
def on_send(node)
return unless render?(node)

if render_literal?(node) || render_view_component?(node) || render_const?(node)
return if render_view_component?(node) || render_const?(node)

if render_literal?(node)
elsif option_pairs = render_with_options?(node)
option_pairs = option_pairs.reject { |pair| options_key?(pair) }

Expand All @@ -65,18 +67,40 @@ def on_send(node)
if template_node = option_pairs.map { |pair| template_key?(pair) }.compact.first
if !literal?(template_node)
add_offense(node, location: :expression)
return
end
else
add_offense(node, location: :expression)
return
end

if layout_node = option_pairs.map { |pair| layout_key?(pair) }.compact.first
if !literal?(layout_node)
add_offense(node, location: :expression)
return
end
end
else
add_offense(node, location: :expression)
return
end

if render_literal?(node)
option_hash = node.arguments[1]
if option_hash && !option_hash.hash_type?
add_offense(node, location: :expression)
return
end
option_pairs = option_hash && option_hash.pairs
else
option_pairs = node.arguments[0].pairs
end

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, location: :expression)
end
end
end
end
Expand Down
10 changes: 0 additions & 10 deletions lib/rubocop/cop/github/rails_view_render_literal.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,6 @@ class RailsViewRenderLiteral < Cop
}) $_)
PATTERN

def_node_matcher :locals_key?, <<-PATTERN
(pair (sym {
:locals
}) $_)
PATTERN

def_node_matcher :partial_key?, <<-PATTERN
(pair (sym {
:file
Expand All @@ -32,10 +26,6 @@ class RailsViewRenderLiteral < Cop
}) $_)
PATTERN

def hash_with_literal_keys?(hash)
hash.pairs.all? { |pair| literal?(pair.key) }
end

def on_send(node)
return unless render?(node)

Expand Down
8 changes: 8 additions & 0 deletions lib/rubocop/cop/github/render_literal_helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,14 @@ module RenderLiteralHelpers
(send nil? {:render :render_to_string} (send _ :with_collection ...) ...)
PATTERN

def_node_matcher :locals_key?, <<-PATTERN
(pair (sym :locals) $_)
PATTERN

def hash_with_literal_keys?(hash)
hash.pairs.all? { |pair| literal?(pair.key) }
end

def render_view_component?(node)
render_view_component_instance?(node) ||
render_view_component_collection?(node)
Expand Down
61 changes: 61 additions & 0 deletions test/test_rails_controller_render_literal.rb
Original file line number Diff line number Diff line change
Expand Up @@ -393,4 +393,65 @@ def index
assert_equal 1, cop.offenses.count
assert_equal "render must be used with a string literal or an instance of a Class", cop.offenses[0].message
end

def test_render_shorthand_static_locals_no_offsense
investigate cop, <<-RUBY, "app/controllers/products_controller.rb"
class ProductsController < ActionController::Base
def index
render "products/index", locals: { product: product }
end
end
RUBY

assert_equal 0, cop.offenses.count
end

def test_render_partial_static_locals_no_offsense
investigate cop, <<-RUBY, "app/controllers/products_controller.rb"
class ProductsController < ActionController::Base
def index
render partial: "products/index", locals: { product: product }
end
end
RUBY

assert_equal 0, cop.offenses.count
end

def test_render_literal_dynamic_options_offense
investigate cop, <<-RUBY, "app/controllers/products_controller.rb"
class ProductsController < ActionController::Base
def index
render "products/product", options
end
end
RUBY

assert_equal 1, cop.offenses.count
end

def test_render_literal_dynamic_locals_offense
investigate cop, <<-RUBY, "app/controllers/products_controller.rb"
class ProductsController < ActionController::Base
def index
render "products/product", locals: locals
end
end
RUBY

assert_equal 1, cop.offenses.count
end


def test_render_literal_dynamic_local_key_offense
investigate cop, <<-RUBY, "app/controllers/products_controller.rb"
class ProductsController < ActionController::Base
def index
render "products/product", locals: { product_key => product }
end
end
RUBY

assert_equal 1, cop.offenses.count
end
end

0 comments on commit 54f3e11

Please sign in to comment.