labml icon indicating copy to clipboard operation
labml copied to clipboard

with_content for ViewComponent flagged as dynamic render path

Open gavingmiller opened this issue 1 year ago • 4 comments

Background

Brakeman version: 5.4.1 Rails version: 6.1.7.6 Ruby version: 2.7.7p221

Link to Rails application code: N/A

Issue

When rendering a ViewComponent via the with_content method a Dynamic Render Path warning will be raised. However, using the block version, the warning is not raised. Example using the basic implementation for a view component:

<!-- No error raised -->
<%= render ExampleComponent.new(title: "my title") %>

<!-- Error raised -->
<%= render ExampleComponent.new(title: "my title").with_content("string") %>

<!-- No error raised -->
<%= render ExampleComponent.new(title: "my title") do %>
  string
<% end %>

<!-- No error raised -->
<%= render ExampleComponent.new(title: "my title") do %>
  <%= "string" %>
<% end %>

My debugging has lead me to the renderable? method, and it appears like it's missing logic to detect the with_content usage of this pattern.

gavingmiller avatar Mar 02 '24 22:03 gavingmiller

I've managed to put together what's potentially a patch for this. Code below:

def renderable? exp
  return false unless call?(exp)

  if exp.method == :with_content
    exp = exp.target
  end

  return false unless constant?(exp.target)   
  target_class_name = class_name(exp.target)
  known_renderable_class?(target_class_name) or tracker.find_method(:render_in, target_class_name)
end

This looks to handle the cases of with_content in our codebase, and I'm curious if this would be more broadly applicable?

gavingmiller avatar Mar 09 '24 21:03 gavingmiller

That works and would probably be how I'd do it, too.

presidentbeef avatar Mar 11 '24 07:03 presidentbeef

And maybe one day make this check optional.

presidentbeef avatar Mar 11 '24 07:03 presidentbeef

@presidentbeef awesome, I'll PR it in.

gavingmiller avatar Mar 11 '24 14:03 gavingmiller