Check component templates with Herb in herb:check

docs/CHANGELOG.md

@@ -10,6 +10,10 @@ nav_order: 6
 
 ## main
 
+* Add `bin/rails view_component:herb:check`, which compiles component HTML+ERB templates through Herb, and report component failures from `bin/rails herb:check` on Rails 8.2, as sidecar and inline templates aren't in the view paths that task reads.
+
+    *Clément Joubert*
+
 ## 4.23.0
 
 * Restore request-dependent URL generation when using `render_preview` inside `with_request_url`.

docs/compatibility.md

@@ -14,6 +14,12 @@ ViewComponent supports all actively supported versions of [Ruby](https://endofli
 
 ViewComponent is tested against ERB, Haml, and Slim, but it should support most Rails template handlers.
 
+### Herb
+
+Rails 8.2 can compile HTML+ERB templates through [Herb](https://github.com/marcoroth/herb) (`config.action_view.erb_implementation = :herb`), and ships `bin/rails herb:check` to find templates that don't compile with it before adopting it. Component templates, sidecar and inline, compile through Herb too, but aren't in the view paths `herb:check` reads.
+
+`bin/rails view_component:herb:check` compiles the HTML+ERB templates of every component through Herb and reports the ones that fail. `bin/rails herb:check` reports component failures first, then checks the views, and fails if either does, so one command covers both.
+
 ## Bridgetown (Static Site Generator)
 
 [Bridgetown](https://www.bridgetownrb.com/) supports ViewComponent via an experimental shim provided by the [bridgetown-view-component gem](https://github.com/bridgetownrb/bridgetown-view-component). More information available [here](https://www.bridgetownrb.com/docs/components/ruby#need-compatibility-with-rails-try-viewcomponent-experimental).

lib/view_component.rb

@@ -14,6 +14,7 @@ module ViewComponent
   autoload :CompileCache
   autoload :Config
   autoload :Deprecation
+  autoload :HerbChecker
   autoload :InlineTemplate
   autoload :Instrumentation
   autoload :Preview

lib/view_component/compiler.rb

@@ -88,6 +88,14 @@ def template_formats
       gather_templates.map { |template| template.format || :html }.uniq
     end
 
+    # Templates the component compiles from source: sidecar files and inline
+    # templates, but not `#call` methods or templates inherited from a parent.
+    #
+    # @private
+    def source_templates
+      gather_templates.reject(&:inline_call?)
+    end
+
     # @private
     def templates_for_call(method_name)
       gather_templates.select { |template| template.call_method_name == method_name }

lib/view_component/engine.rb

@@ -110,6 +110,10 @@ class Engine < Rails::Engine # :nodoc:
       end
     end
 
+    rake_tasks do
+      load "view_component/tasks/herb.rake" if ViewComponent::HerbChecker.supported?
+    end
+
     initializer "compiler mode" do |_app|
       ViewComponent::Compiler.__vc_development_mode = (Rails.env.development? || Rails.env.test?)
     end

lib/view_component/herb_checker.rb

@@ -0,0 +1,63 @@
+# frozen_string_literal: true
+
+module ViewComponent
+  # Compiles the HTML+ERB templates of components through Herb, the way
+  # `ActionView::HerbChecker` does for the templates in the view paths.
+  #
+  # Sidecar templates aren't in the view paths and inline templates aren't
+  # files, so `bin/rails herb:check` doesn't reach them on its own.
+  #
+  # @private
+  class HerbChecker
+    Failure = Data.define(:component, :template, :error) do
+      # @return [String]
+      def identifier
+        location = template.handler_template.short_identifier
+        # Herb counts lines from the start of the template, not of the Ruby file.
+        location = "#{location}:#{component.__vc_inline_template.lineno}" if template.type == :inline
+
+        "#{location} (#{component.name || component.inspect})"
+      end
+    end
+
+    # Whether this version of Action View compiles ERB through Herb.
+    #
+    # @return [Boolean]
+    def self.supported?
+      ActionView::Template::Handlers::ERB.const_defined?(:Herb, false)
+    end
+
+    # @param components [Array<Class>] components whose templates to check
+    #
+    # @return [Array<Failure>]
+    def self.check(components = ViewComponent::Base.descendants.reject(&:anonymous?))
+      # Required here rather than at load time, so `supported?` doesn't need
+      # the herb gem.
+      require "action_view/template/handlers/erb/herb"
+      require "herb/engine/validators/security_validator"
+
+      handler = ActionView::Template::Handlers::ERB.new
+
+      # Share directory listings and file reads across components, as eager
+      # loading does.
+      CompileCache.with_sidecar_files_cache do
+        components.flat_map do |component|
+          component.__vc_compiler.source_templates.filter_map do |template|
+            next unless template.html? && template.details.handler == :erb
+
+            handler.call(
+              template.handler_template,
+              template.handler_source,
+              implementation: ActionView::Template::Handlers::ERB::Herb,
+              validate_ruby: true,
+              visitors: [::Herb::Engine::Validators::SecurityValidator.new]
+            )
+            nil
+          rescue ::Herb::Engine::CompilationError, ::Herb::Engine::SecurityError => error
+            Failure.new(component, template, error)
+          end
+        end
+      end
+    end
+  end
+end

lib/view_component/tasks/herb.rake

@@ -0,0 +1,45 @@
+# frozen_string_literal: true
+
+namespace :view_component do
+  namespace :herb do
+    # Prints the component templates Herb fails to compile and returns them.
+    check_components = lambda do
+      # Components are only known once loaded.
+      Rails.application.eager_load!
+
+      failures = ViewComponent::HerbChecker.check
+
+      if failures.empty?
+        puts "All component HTML+ERB templates compile through Herb."
+      else
+        failures.each do |failure|
+          puts "#{failure.identifier}:"
+          failure.error.message.strip.lines.first(10).each { |line| puts "  #{line}" }
+          puts
+        end
+      end
+
+      failures
+    end
+
+    failure_message = lambda do |failures|
+      "#{failures.size} component #{"HTML+ERB template".pluralize(failures.size)} failed to compile through Herb."
+    end
+
+    desc "Check that component HTML+ERB templates compile through Herb"
+    task check: :environment do
+      failures = check_components.call
+      abort failure_message.call(failures) if failures.any?
+    end
+
+    # Runs before `herb:check`, and fails only once it has finished, so the
+    # views are checked even when a component template fails.
+    task report: :environment do
+      failures = check_components.call
+      at_exit { abort failure_message.call(failures) } if failures.any?
+    end
+  end
+end
+
+# `herb:check` reads the view paths, which don't hold component templates.
+task "herb:check" => "view_component:herb:report" unless Rails.application.config.api_only

lib/view_component/template.rb

@@ -204,6 +204,24 @@ def normalized_variant_name
       variant.to_s.gsub("-", "__")
     end
 
+    # The template object passed to the template handler.
+    #
+    # @private
+    def handler_template
+      short_identifier = defined?(Rails.root) ? @path.sub("#{Rails.root}/", "") : @path
+      format = self.format || DEFAULT_FORMAT
+      type = ActionView::Template::Types[format]
+
+      DataWithSource.new(format:, identifier: @path, short_identifier:, type:)
+    end
+
+    # The source passed to the template handler.
+    #
+    # @private
+    def handler_source(original_source = source)
+      @component.strip_trailing_whitespace? ? original_source.rstrip : original_source
+    end
+
     private
 
     attr_reader :lineno
@@ -225,13 +243,8 @@ def annotated?
     def compiled_source
       handler = details.handler_class
       original_source = source
-      this_source = @component.strip_trailing_whitespace? ? original_source.rstrip : original_source
-
-      short_identifier = defined?(Rails.root) ? @path.sub("#{Rails.root}/", "") : @path
-      format = self.format || DEFAULT_FORMAT
-      type = ActionView::Template::Types[format]
 
-      handler.call(DataWithSource.new(format:, identifier: @path, short_identifier:, type:), this_source).tap do |compiled|
+      handler.call(handler_template, handler_source(original_source)).tap do |compiled|
         CacheDigest.compiled(handler, original_source, compiled)
       end
     end

test/sandbox/test/herb_checker_test.rb

@@ -0,0 +1,122 @@
+# frozen_string_literal: true
+
+require "test_helper"
+require "rake"
+
+class HerbCheckerTest < ActiveSupport::TestCase
+  def test_source_templates_include_sidecar_and_inline_templates_only
+    assert_equal(
+      ["#{Rails.root}/app/components/my_component.html.erb"],
+      MyComponent.__vc_compiler.source_templates.map { |template| File.expand_path(template.path) }
+    )
+
+    inline = Class.new(ViewComponent::Base) { erb_template "<div></div>" }
+    assert_equal [:inline], inline.__vc_compiler.source_templates.map(&:type)
+
+    call = Class.new(ViewComponent::Base) do
+      def call
+        "<div></div>".html_safe
+      end
+    end
+    assert_empty call.__vc_compiler.source_templates
+  end
+
+  def test_handler_template_and_source_match_what_the_compiler_passes_the_handler
+    component = Class.new(ViewComponent::Base) do
+      strip_trailing_whitespace
+
+      erb_template "<div></div>\n\n"
+    end
+    template = component.__vc_compiler.source_templates.first
+
+    assert_equal :html, template.handler_template.format
+    assert_equal "text/html", template.handler_template.type.to_s
+    assert_equal File.basename(__FILE__), File.basename(template.handler_template.identifier)
+    assert_equal "<div></div>", template.handler_source
+  end
+
+  def test_reports_sidecar_template_that_herb_cannot_compile
+    skip_unless_herb_supported
+
+    Dir.mktmpdir do |directory|
+      component = component_in(directory, "BrokenSidecarComponent")
+      File.write(File.join(directory, "broken_sidecar_component.html.erb"), "<div><%= 1 %>\n")
+
+      failures = ViewComponent::HerbChecker.check([component])
+
+      assert_equal 1, failures.size
+      assert_equal component, failures.first.component
+      assert_equal File.join(directory, "broken_sidecar_component.html.erb"), failures.first.template.path
+      assert_kind_of ::Herb::Engine::CompilationError, failures.first.error
+      assert_includes failures.first.identifier, "(BrokenSidecarComponent)"
+    end
+  end
+
+  def test_reports_inline_template_that_herb_cannot_compile
+    skip_unless_herb_supported
+
+    component = Class.new(ViewComponent::Base) { erb_template "<div><%= 1 %>" }
+
+    failures = ViewComponent::HerbChecker.check([component])
+
+    assert_equal 1, failures.size
+    assert_equal :inline, failures.first.template.type
+    assert_match(/#{File.basename(__FILE__)}:\d+ \(/, failures.first.identifier)
+  end
+
+  def test_reports_erb_output_in_attribute_names
+    skip_unless_herb_supported
+
+    component = Class.new(ViewComponent::Base) { erb_template "<div <%= 'class' %>=\"a\"></div>" }
+
+    assert_equal 1, ViewComponent::HerbChecker.check([component]).size
+  end
+
+  def test_accepts_templates_herb_compiles
+    skip_unless_herb_supported
+
+    Dir.mktmpdir do |directory|
+      component = component_in(directory, "ValidSidecarComponent")
+      File.write(File.join(directory, "valid_sidecar_component.html.erb"), "<div><%= 1 %></div>\n")
+      inline = Class.new(ViewComponent::Base) { erb_template "<p><%= 1 %></p>" }
+
+      assert_empty ViewComponent::HerbChecker.check([component, inline])
+    end
+  end
+
+  def test_skips_templates_that_are_not_html_erb
+    skip_unless_herb_supported
+
+    Dir.mktmpdir do |directory|
+      component = component_in(directory, "OtherFormatsComponent")
+      File.write(File.join(directory, "other_formats_component.html.haml"), "%div\n")
+      File.write(File.join(directory, "other_formats_component.text.erb"), "<div><%= 1 %>\n")
+
+      assert_empty ViewComponent::HerbChecker.check([component])
+    end
+  end
+
+  def test_herb_check_task_runs_the_component_check
+    application = Rake::Application.new
+    Rake.with_application(application) do
+      load File.expand_path("../../../lib/view_component/tasks/herb.rake", __dir__)
+    end
+
+    assert_equal ["environment"], application["view_component:herb:check"].prerequisites
+    assert_equal ["environment"], application["view_component:herb:report"].prerequisites
+    assert_equal ["view_component:herb:report"], application["herb:check"].prerequisites
+  end
+
+  private
+
+  def skip_unless_herb_supported
+    skip "Action View doesn't compile ERB through Herb" unless ViewComponent::HerbChecker.supported?
+  end
+
+  def component_in(directory, name)
+    Class.new(ViewComponent::Base).tap do |component|
+      component.identifier = File.join(directory, "#{name.underscore}.rb")
+      component.define_singleton_method(:name) { name }
+    end
+  end
+end