From 2cf1d083ca860a7a6a37b66890cc5830bdabf6bd Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Thu, 1 Oct 2026 13:36:45 -0600 Subject: [PATCH 01/18] Use Herb binding for RuboCop extraction Replace the duplicate Ruby source builder with Herb.extract_ruby while retaining range collection for safe autocorrection. Normalize masked multibyte characters to RuboCop's character offsets. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .github/labeler.yml | 1 + .github/workflows/build-gems.yml | 5 + .github/workflows/build.yml | 2 +- .rubocop.yml | 1 + Gemfile | 2 + Gemfile.lock | 9 + README.md | 21 ++ Rakefile | 6 + gems/rubocop-herb/README.md | 26 +++ gems/rubocop-herb/config/default.yml | 115 ++++++++++ gems/rubocop-herb/lib/rubocop/herb.rb | 16 ++ gems/rubocop-herb/lib/rubocop/herb/plugin.rb | 38 ++++ .../rubocop/herb/processed_source_builder.rb | 33 +++ .../herb/range_restricted_autocorrect.rb | 34 +++ .../herb/range_restricted_corrector.rb | 31 +++ .../lib/rubocop/herb/ruby_extractor.rb | 59 +++++ .../lib/rubocop/herb/ruby_range_collector.rb | 52 +++++ gems/rubocop-herb/lib/rubocop/herb/version.rb | 9 + gems/rubocop-herb/rubocop-herb.gemspec | 36 +++ gems/rubocop-herb/test/integration_test.rb | 211 ++++++++++++++++++ gems/rubocop-herb/test/plugin_test.rb | 30 +++ gems/rubocop-herb/test/ruby_extractor_test.rb | 90 ++++++++ gems/rubocop-herb/test/test_helper.rb | 32 +++ 23 files changed, 858 insertions(+), 1 deletion(-) create mode 100644 gems/rubocop-herb/README.md create mode 100644 gems/rubocop-herb/config/default.yml create mode 100644 gems/rubocop-herb/lib/rubocop/herb.rb create mode 100644 gems/rubocop-herb/lib/rubocop/herb/plugin.rb create mode 100644 gems/rubocop-herb/lib/rubocop/herb/processed_source_builder.rb create mode 100644 gems/rubocop-herb/lib/rubocop/herb/range_restricted_autocorrect.rb create mode 100644 gems/rubocop-herb/lib/rubocop/herb/range_restricted_corrector.rb create mode 100644 gems/rubocop-herb/lib/rubocop/herb/ruby_extractor.rb create mode 100644 gems/rubocop-herb/lib/rubocop/herb/ruby_range_collector.rb create mode 100644 gems/rubocop-herb/lib/rubocop/herb/version.rb create mode 100644 gems/rubocop-herb/rubocop-herb.gemspec create mode 100644 gems/rubocop-herb/test/integration_test.rb create mode 100644 gems/rubocop-herb/test/plugin_test.rb create mode 100644 gems/rubocop-herb/test/ruby_extractor_test.rb create mode 100644 gems/rubocop-herb/test/test_helper.rb diff --git a/.github/labeler.yml b/.github/labeler.yml index 4da50d1f4..cac25a8b9 100644 --- a/.github/labeler.yml +++ b/.github/labeler.yml @@ -266,6 +266,7 @@ rubygem: - changed-files: - any-glob-to-any-file: - 'herb.gemspec' + - 'gems/**/*' - 'lib/**/*' - 'templates/lib/**/*' diff --git a/.github/workflows/build-gems.yml b/.github/workflows/build-gems.yml index 546389a9b..7511c82f1 100644 --- a/.github/workflows/build-gems.yml +++ b/.github/workflows/build-gems.yml @@ -10,6 +10,7 @@ on: paths: - "src/**" - "lib/**" + - "gems/**" - "ext/**" - "Gemfile" - "Gemfile.lock" @@ -22,6 +23,7 @@ on: paths: - "src/**" - "lib/**" + - "gems/**" - "ext/**" - "Gemfile" - "Gemfile.lock" @@ -89,6 +91,9 @@ jobs: run: | if [ "${{ matrix.target }}" = "default" ]; then bundle exec rake build + mkdir -p pkg + cd gems/rubocop-herb + gem build rubocop-herb.gemspec --output ../../pkg/rubocop-herb.gem else bundle exec rake gem:${{ matrix.target }} fi diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index f41d75a42..861fed9d8 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -83,7 +83,7 @@ jobs: run: bundle exec rake compile - name: Run Ruby Tests - run: bundle exec rake test:all + run: bundle exec rake test:all test:rubocop_herb - name: Run C tests run: ./run_herb_tests diff --git a/.rubocop.yml b/.rubocop.yml index d7f35d1f0..085f37aea 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -158,6 +158,7 @@ Metrics/ClassLength: - lib/herb/visitor/stack.rb - templates/template.rb - test/**/*_test.rb + - gems/**/test/**/*_test.rb Metrics/ModuleLength: Exclude: diff --git a/Gemfile b/Gemfile index 19724cc0b..bbdda992b 100644 --- a/Gemfile +++ b/Gemfile @@ -4,6 +4,8 @@ source "https://rubygems.org" gemspec +gem "rubocop-herb", path: "gems/rubocop-herb" + gem "prism", github: "ruby/prism", tag: "v1.9.0" gem "actionview", "~> 8.1", require: "action_view" diff --git a/Gemfile.lock b/Gemfile.lock index 120ea2e15..fecc81b22 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -21,6 +21,14 @@ PATH specs: herb (0.11.0) +PATH + remote: gems/rubocop-herb + specs: + rubocop-herb (0.10.3) + herb (= 0.10.3) + lint_roller (~> 1.1) + rubocop (>= 1.72.1, < 2) + GEM remote: https://rubygems.org/ specs: @@ -369,6 +377,7 @@ DEPENDENCIES rolldown (~> 1.2) rubocop (~> 1.72) rubocop-performance (~> 1.26.0) + rubocop-herb! sorbet steep (~> 2.1) turbo-rails (~> 2.0) diff --git a/README.md b/README.md index a7796a04e..a9501bd56 100644 --- a/README.md +++ b/README.md @@ -53,6 +53,27 @@ Run the linter and the formatter: ```sh npx @herb-tools/linter +``` + +Run your project's configured RuboCop rules against Ruby in HTML+ERB (`.html.erb`) templates with the `rubocop-herb` plugin: + +```sh +bundle add rubocop-herb +``` + +```yaml +# .rubocop.yml +plugins: + - rubocop-herb +``` + +```sh +bundle exec rubocop app/views +``` + +Format your HTML+ERB templates: + +```sh npx @herb-tools/formatter ``` diff --git a/Rakefile b/Rakefile index ab561137a..78f74dcbd 100644 --- a/Rakefile +++ b/Rakefile @@ -28,6 +28,12 @@ Rake::TestTask.new("test:all") do |t| t.test_files = FileList["test/**/*_test.rb"] end +Rake::TestTask.new("test:rubocop_herb") do |t| + t.libs << "gems/rubocop-herb/test" + t.libs << "gems/rubocop-herb/lib" + t.test_files = FileList["gems/rubocop-herb/test/**/*_test.rb"] +end + task "make" do puts "Running make..." IO.popen("make") do |output| diff --git a/gems/rubocop-herb/README.md b/gems/rubocop-herb/README.md new file mode 100644 index 000000000..aa2980a15 --- /dev/null +++ b/gems/rubocop-herb/README.md @@ -0,0 +1,26 @@ +# rubocop-herb + +`rubocop-herb` runs a project's configured RuboCop rules against Ruby embedded in HTML+ERB (`.html.erb`) templates. Herb builds a position-preserving Ruby representation of the entire template so RuboCop can understand control flow and variable usage across ERB tags while reporting offenses at their original template locations. + +Add the gem to your bundle: + +```sh +bundle add rubocop-herb +``` + +Then enable the plugin in `.rubocop.yml`: + +```yaml +plugins: + - rubocop-herb +``` + +Then use RuboCop normally: + +```sh +bundle exec rubocop app/views +bundle exec rubocop -a app/views +bundle exec rubocop -A app/views +``` + +The plugin honors the project's RuboCop configuration, including custom cops, inherited configuration, target Ruby versions, and file exclusions. Autocorrections are applied only when the complete edit falls within one original Ruby region, so corrections cannot modify HTML or ERB delimiters. Cops that depend on physical Ruby file contents or layout are [excluded for HTML+ERB templates by default](config/default.yml). diff --git a/gems/rubocop-herb/config/default.yml b/gems/rubocop-herb/config/default.yml new file mode 100644 index 000000000..520dfb7d5 --- /dev/null +++ b/gems/rubocop-herb/config/default.yml @@ -0,0 +1,115 @@ +inherit_mode: + merge: + - Exclude + - Include + +AllCops: + Include: + - '**/*.html.erb' + +# These cops depend on physical Ruby file contents or layout that is obscured +# by the position-preserving template representation. +Layout/ArgumentAlignment: + Exclude: ['**/*.html.erb'] +Layout/ArrayAlignment: + Exclude: ['**/*.html.erb'] +Layout/BlockAlignment: + Exclude: ['**/*.html.erb'] +Layout/ClosingParenthesisIndentation: + Exclude: ['**/*.html.erb'] +Layout/CommentIndentation: + Exclude: ['**/*.html.erb'] +Layout/InitialIndentation: + Exclude: ['**/*.html.erb'] +Layout/EmptyLineAfterGuardClause: + Exclude: ['**/*.html.erb'] +Layout/EndAlignment: + Exclude: ['**/*.html.erb'] +Layout/FirstArgumentIndentation: + Exclude: ['**/*.html.erb'] +Layout/FirstArrayElementIndentation: + Exclude: ['**/*.html.erb'] +Layout/FirstArrayElementLineBreak: + Exclude: ['**/*.html.erb'] +Layout/FirstHashElementIndentation: + Exclude: ['**/*.html.erb'] +Layout/FirstHashElementLineBreak: + Exclude: ['**/*.html.erb'] +Layout/FirstMethodArgumentLineBreak: + Exclude: ['**/*.html.erb'] +Layout/FirstParameterIndentation: + Exclude: ['**/*.html.erb'] +Layout/HashAlignment: + Exclude: ['**/*.html.erb'] +Layout/IndentationConsistency: + Exclude: ['**/*.html.erb'] +Layout/IndentationWidth: + Exclude: ['**/*.html.erb'] +Layout/LeadingEmptyLine: + Exclude: ['**/*.html.erb'] +Layout/LeadingEmptyLines: + Exclude: ['**/*.html.erb'] +Layout/LineEndStringConcatenationIndentation: + Exclude: ['**/*.html.erb'] +Layout/LineLength: + Exclude: ['**/*.html.erb'] +Layout/MultilineArrayBraceLayout: + Exclude: ['**/*.html.erb'] +Layout/MultilineAssignmentLayout: + Exclude: ['**/*.html.erb'] +Layout/MultilineHashBraceLayout: + Exclude: ['**/*.html.erb'] +Layout/MultilineMethodCallBraceLayout: + Exclude: ['**/*.html.erb'] +Layout/MultilineMethodCallIndentation: + Exclude: ['**/*.html.erb'] +Layout/MultilineMethodDefinitionBraceLayout: + Exclude: ['**/*.html.erb'] +Layout/MultilineOperationIndentation: + Exclude: ['**/*.html.erb'] +Layout/ParameterAlignment: + Exclude: ['**/*.html.erb'] +Layout/SpaceBeforeSemicolon: + Exclude: ['**/*.html.erb'] +Layout/TrailingEmptyLines: + Exclude: ['**/*.html.erb'] +Layout/TrailingWhitespace: + Exclude: ['**/*.html.erb'] +Lint/EmptyFile: + Exclude: ['**/*.html.erb'] +Lint/EmptyConditionalBody: + Exclude: ['**/*.html.erb'] +Lint/OutOfRangeRegexpRef: + Exclude: ['**/*.html.erb'] +Lint/Syntax: + Exclude: ['**/*.html.erb'] +Lint/Void: + Exclude: ['**/*.html.erb'] +Metrics/BlockLength: + Exclude: ['**/*.html.erb'] +Naming/FileName: + Exclude: ['**/*.html.erb'] +Rails/Presence: + Exclude: ['**/*.html.erb'] +Style/FrozenStringLiteralComment: + Exclude: ['**/*.html.erb'] +Style/EmptyElse: + Exclude: ['**/*.html.erb'] +Style/IdenticalConditionalBranches: + Exclude: ['**/*.html.erb'] +Style/IfWithSemicolon: + Exclude: ['**/*.html.erb'] +Style/IfUnlessModifier: + Exclude: ['**/*.html.erb'] +Style/MultilineTernaryOperator: + Exclude: ['**/*.html.erb'] +Style/NestedTernaryOperator: + Exclude: ['**/*.html.erb'] +Style/Next: + Exclude: ['**/*.html.erb'] +Style/Semicolon: + Exclude: ['**/*.html.erb'] +Style/WhileUntilDo: + Exclude: ['**/*.html.erb'] +Style/WhileUntilModifier: + Exclude: ['**/*.html.erb'] diff --git a/gems/rubocop-herb/lib/rubocop/herb.rb b/gems/rubocop-herb/lib/rubocop/herb.rb new file mode 100644 index 000000000..1a1d60d3d --- /dev/null +++ b/gems/rubocop-herb/lib/rubocop/herb.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +require "rubocop" + +module RuboCop + module Herb + autoload :ProcessedSourceBuilder, "rubocop/herb/processed_source_builder" + autoload :RangeRestrictedAutocorrect, "rubocop/herb/range_restricted_autocorrect" + autoload :RangeRestrictedCorrector, "rubocop/herb/range_restricted_corrector" + autoload :RubyExtractor, "rubocop/herb/ruby_extractor" + autoload :RubyRangeCollector, "rubocop/herb/ruby_range_collector" + end +end + +require_relative "herb/plugin" +require_relative "herb/version" diff --git a/gems/rubocop-herb/lib/rubocop/herb/plugin.rb b/gems/rubocop-herb/lib/rubocop/herb/plugin.rb new file mode 100644 index 000000000..6feaa6dcb --- /dev/null +++ b/gems/rubocop-herb/lib/rubocop/herb/plugin.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +require "lint_roller" +require "pathname" + +module RuboCop + module Herb + class Plugin < LintRoller::Plugin + def about + LintRoller::About.new( + description: "Run configured RuboCop rules against Ruby in ERB templates.", + homepage: "https://github.com/marcoroth/herb", + name: "rubocop-herb", + version: VERSION + ) + end + + def rules(_context) + extractors = RuboCop::Runner.ruby_extractors + extractors.unshift(RubyExtractor) unless extractors.include?(RubyExtractor) + team = RuboCop::Cop::Team + team.prepend(RangeRestrictedAutocorrect) unless team < RangeRestrictedAutocorrect + corrector = RuboCop::Cop::Corrector + corrector.prepend(RangeRestrictedCorrector) unless corrector < RangeRestrictedCorrector + + LintRoller::Rules.new( + config_format: :rubocop, + type: :path, + value: Pathname.new(__dir__).join("../../../config/default.yml") + ) + end + + def supported?(context) + context.engine == :rubocop + end + end + end +end diff --git a/gems/rubocop-herb/lib/rubocop/herb/processed_source_builder.rb b/gems/rubocop-herb/lib/rubocop/herb/processed_source_builder.rb new file mode 100644 index 000000000..00ace4e21 --- /dev/null +++ b/gems/rubocop-herb/lib/rubocop/herb/processed_source_builder.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +module RuboCop + module Herb + class ProcessedSourceBuilder + def self.call(code:, processed_source:, ruby_ranges:) + new(code, processed_source, ruby_ranges).call + end + + def initialize(code, processed_source, ruby_ranges) + @code = code + @processed_source = processed_source + @ruby_ranges = ruby_ranges + end + + def call + source = ::RuboCop::ProcessedSource.new( + @code, + @processed_source.ruby_version, + @processed_source.path, + parser_engine: @processed_source.parser_engine + ) + source.config = @processed_source.config + source.registry = @processed_source.registry + source.define_singleton_method(:herb_ruby_ranges) { @ruby_ranges } + source.instance_variable_set(:@ruby_ranges, @ruby_ranges) + source.buffer.define_singleton_method(:herb_ruby_ranges) { @ruby_ranges } + source.buffer.instance_variable_set(:@ruby_ranges, @ruby_ranges) + source + end + end + end +end diff --git a/gems/rubocop-herb/lib/rubocop/herb/range_restricted_autocorrect.rb b/gems/rubocop-herb/lib/rubocop/herb/range_restricted_autocorrect.rb new file mode 100644 index 000000000..7527491c8 --- /dev/null +++ b/gems/rubocop-herb/lib/rubocop/herb/range_restricted_autocorrect.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +module RuboCop + module Herb + module RangeRestrictedAutocorrect + private + + def autocorrect_report(report, offset:, original:) + ranges = herb_ruby_ranges(report) + return super unless ranges + + corrector = collate_corrections(report, offset:, original:) + restricted = RuboCop::Cop::Corrector.new(original) + + corrector.as_replacements.each do |range, replacement| + restricted.replace(range, replacement) if herb_correction_allowed?(range, ranges) + end + + restricted.rewrite unless restricted.empty? + end + + def herb_ruby_ranges(report) + source = report.processed_source + source.herb_ruby_ranges if source.respond_to?(:herb_ruby_ranges) + end + + def herb_correction_allowed?(correction, ranges) + ranges.any? do |range| + range.from <= correction.begin_pos && correction.end_pos <= range.to + end + end + end + end +end diff --git a/gems/rubocop-herb/lib/rubocop/herb/range_restricted_corrector.rb b/gems/rubocop-herb/lib/rubocop/herb/range_restricted_corrector.rb new file mode 100644 index 000000000..94aa1eb74 --- /dev/null +++ b/gems/rubocop-herb/lib/rubocop/herb/range_restricted_corrector.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +module RuboCop + module Herb + module RangeRestrictedCorrector + def replace(node_or_range, content) + return unless herb_range_allowed?(node_or_range) + + super + end + + def wrap(node_or_range, insert_before, insert_after) + return unless herb_range_allowed?(node_or_range) + + super + end + + private + + def herb_range_allowed?(node_or_range) + ranges = source_buffer.herb_ruby_ranges if source_buffer.respond_to?(:herb_ruby_ranges) + return true unless ranges + + correction = to_range(node_or_range) + ranges.any? do |range| + range.from <= correction.begin_pos && correction.end_pos <= range.to + end + end + end + end +end diff --git a/gems/rubocop-herb/lib/rubocop/herb/ruby_extractor.rb b/gems/rubocop-herb/lib/rubocop/herb/ruby_extractor.rb new file mode 100644 index 000000000..04c553190 --- /dev/null +++ b/gems/rubocop-herb/lib/rubocop/herb/ruby_extractor.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +require "herb" +require "rubocop" + +module RuboCop + module Herb + class RubyExtractor + def self.call(processed_source) + new(processed_source).call + end + + def initialize(processed_source) + @processed_source = processed_source + end + + def call + return unless erb_file? + return [] unless html_erb_file? + + template = @processed_source.raw_source + ruby_ranges = RubyRangeCollector.call(template) + return [] if ruby_ranges.empty? + + source = ProcessedSourceBuilder.call( + code: ruby_source(template), + processed_source: @processed_source, + ruby_ranges: + ) + return [] unless source.valid_syntax? + + [{ offset: 0, processed_source: source }] + end + + private + + def erb_file? + @processed_source.path&.end_with?(".erb") + end + + def html_erb_file? + @processed_source.path&.end_with?(".html.erb") + end + + def ruby_source(template) + extracted = ::Herb.extract_ruby(template) + return extracted if extracted.length == template.length + + byte_offset = 0 + template.each_char.map do |character| + extracted_character = extracted.byteslice(byte_offset, character.bytesize) + byte_offset += character.bytesize + + extracted_character == character ? character : extracted_character.each_char.first + end.join + end + end + end +end diff --git a/gems/rubocop-herb/lib/rubocop/herb/ruby_range_collector.rb b/gems/rubocop-herb/lib/rubocop/herb/ruby_range_collector.rb new file mode 100644 index 000000000..8fd945122 --- /dev/null +++ b/gems/rubocop-herb/lib/rubocop/herb/ruby_range_collector.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +module RuboCop + module Herb + class RubyRangeCollector + def self.call(template) + new(template).call + end + + def initialize(template) + @template = template + end + + def call + visitor = ErbNodeVisitor.new + visitor.visit(::Herb.parse(@template).value) + visitor.ranges.map { |range| character_range(range) } + end + + private + + def character_range(byte_range) + ::Herb::Range.new( + @template.byteslice(0, byte_range.from).length, + @template.byteslice(0, byte_range.to).length + ) + end + + class ErbNodeVisitor < ::Herb::Visitor + attr_reader :ranges + + def initialize + super + @ranges = [] + end + + def visit_erb_node(node) + @ranges << node.content.range if inspectable?(node) + end + + private + + def inspectable?(node) + return false unless node.respond_to?(:content) && node.content + + opening = node.tag_opening&.value + opening && opening != "<%#" && opening != "<%%" && opening != "<%graphql" + end + end + end + end +end diff --git a/gems/rubocop-herb/lib/rubocop/herb/version.rb b/gems/rubocop-herb/lib/rubocop/herb/version.rb new file mode 100644 index 000000000..ff4a8dceb --- /dev/null +++ b/gems/rubocop-herb/lib/rubocop/herb/version.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +require "herb/version" + +module RuboCop + module Herb + VERSION = ::Herb::VERSION + end +end diff --git a/gems/rubocop-herb/rubocop-herb.gemspec b/gems/rubocop-herb/rubocop-herb.gemspec new file mode 100644 index 000000000..99466b6c6 --- /dev/null +++ b/gems/rubocop-herb/rubocop-herb.gemspec @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require_relative "../../lib/herb/version" + +Gem::Specification.new do |spec| + spec.name = "rubocop-herb" + spec.version = Herb::VERSION + spec.authors = ["Marco Roth"] + spec.email = ["marco.roth@intergga.ch"] + + spec.summary = "Run RuboCop against Ruby embedded in Herb templates" + spec.description = "A RuboCop plugin that uses Herb to extract Ruby from ERB templates." + spec.homepage = "https://herb-tools.dev" + spec.license = "MIT" + + spec.required_ruby_version = ">= 3.2.0" + spec.require_paths = ["lib"] + spec.files = Dir.chdir(__dir__) do + Dir[ + "README.md", + "config/**/*.yml", + "lib/**/*.rb" + ] + end + + spec.metadata["allowed_push_host"] = "https://rubygems.org" + spec.metadata["rubygems_mfa_required"] = "true" + spec.metadata["default_lint_roller_plugin"] = "RuboCop::Herb::Plugin" + spec.metadata["homepage_uri"] = "https://herb-tools.dev" + spec.metadata["source_code_uri"] = "https://github.com/marcoroth/herb" + spec.metadata["bug_tracker_uri"] = "https://github.com/marcoroth/herb/issues" + + spec.add_dependency "herb", "= #{Herb::VERSION}" + spec.add_dependency "lint_roller", "~> 1.1" + spec.add_dependency "rubocop", ">= 1.72.1", "< 2" +end diff --git a/gems/rubocop-herb/test/integration_test.rb b/gems/rubocop-herb/test/integration_test.rb new file mode 100644 index 000000000..fe340e20e --- /dev/null +++ b/gems/rubocop-herb/test/integration_test.rb @@ -0,0 +1,211 @@ +# frozen_string_literal: true + +require_relative "test_helper" + +class RuboCopHerbIntegrationTest < Minitest::Spec + include RuboCopHerbTestHelpers + + def setup + @directory = Dir.mktmpdir("rubocop-herb") + end + + def teardown + FileUtils.remove_entry(@directory) + end + + test "runs configured built-in and custom cops on ERB Ruby" do + write("custom_cop.rb", <<~RUBY) + module RuboCop + module Cop + module HerbTest + class NoFoo < Base + MSG = "Do not call foo." + + def on_send(node) + add_offense(node) if node.method_name == :foo + end + end + end + end + end + RUBY + write_config(<<~YAML) + require: + - ./custom_cop + Layout/SpaceAroundOperators: + Enabled: true + HerbTest/NoFoo: + Enabled: true + YAML + write("example.html.erb", "

Café <%= foo(x=1) %>

\n") + + output, error, status = run_rubocop("example.html.erb", chdir: @directory) + + refute status.success? + assert_empty error + assert_includes output, "Layout/SpaceAroundOperators" + assert_includes output, "HerbTest/NoFoo" + assert_includes output, "example.html.erb:1:13" + end + + test "autocorrects only the embedded Ruby" do + write_config(<<~YAML) + Layout/SpaceAroundOperators: + Enabled: true + YAML + write("example.html.erb", "
Café <%= x=1 %>
\n") + + _output, error, status = run_rubocop("-a", "example.html.erb", chdir: @directory) + + assert status.success? + assert_empty error + assert_equal "
Café <%= x = 1 %>
\n", read("example.html.erb") + end + + test "does not run cops excluded for ERB fragments" do + write_config(<<~YAML) + Style/FrozenStringLiteralComment: + Enabled: true + YAML + write("example.html.erb", "<%= user.name %>\n") + + output, error, status = run_rubocop("example.html.erb", chdir: @directory) + + assert status.success?, output + assert_empty error + refute_includes output, "Style/FrozenStringLiteralComment" + end + + test "honors project exclusions for template filenames" do + write_config(<<~YAML) + Layout/SpaceAroundOperators: + Enabled: true + Exclude: + - example.html.erb + YAML + write("example.html.erb", "<%= x=1 %>\n") + + output, error, status = run_rubocop("example.html.erb", chdir: @directory) + + assert status.success?, output + assert_empty error + refute_includes output, "Layout/SpaceAroundOperators" + end + + test "tracks variable usage across ERB tags" do + write_config(<<~YAML) + Lint/UselessAssignment: + Enabled: true + YAML + write("example.html.erb", "<% value = compute %>\n

<%= value %>

\n") + + output, error, status = run_rubocop("example.html.erb", chdir: @directory) + + assert status.success?, output + assert_empty error + refute_includes output, "Lint/UselessAssignment" + end + + test "runs only on HTML ERB files" do + write_config(<<~YAML) + Layout/SpaceAroundOperators: + Enabled: true + YAML + write("example.html.erb", "<%= html=1 %>\n") + write("example.rss.erb", "<%= rss=1 %>\n") + write("example.erb", "<%= generic=1 %>\n") + + output, error, status = run_rubocop(".", chdir: @directory) + + refute status.success? + assert_empty error + assert_includes output, "example.html.erb" + refute_includes output, "example.rss.erb" + refute_includes output, "example.erb" + end + + test "does not inspect explicitly passed non-HTML ERB files" do + write_config(<<~YAML) + Layout/SpaceAroundOperators: + Enabled: true + YAML + write("example.rss.erb", "<%= rss=1 %>\n") + + output, error, status = run_rubocop("example.rss.erb", chdir: @directory) + + assert status.success?, output + assert_empty error + refute_includes output, "Layout/SpaceAroundOperators" + end + + test "rejects autocorrections spanning non-Ruby template content" do + write("custom_cop.rb", <<~RUBY) + module RuboCop + module Cop + module HerbTest + class ReplaceConditional < Base + extend AutoCorrector + + MSG = "Do not use conditionals." + + def on_if(node) + add_offense(node) { |corrector| corrector.replace(node, "replacement") } + end + end + end + end + end + RUBY + write_config(<<~YAML) + require: + - ./custom_cop + HerbTest/ReplaceConditional: + Enabled: true + YAML + template = "<% if condition %><%= value %><% end %>\n" + write("example.html.erb", template) + + output, error, status = run_rubocop("-A", "example.html.erb", chdir: @directory) + + refute status.success? + assert_empty error + assert_includes output, "HerbTest/ReplaceConditional" + refute_includes output, "[Corrected]" + assert_equal template, read("example.html.erb") + end + + test "leaves ordinary Ruby inspection unchanged" do + write_config(<<~YAML) + Layout/SpaceAroundOperators: + Enabled: true + YAML + write("example.rb", "x=1\n") + + output, error, status = run_rubocop("example.rb", chdir: @directory) + + refute status.success? + assert_empty error + assert_includes output, "Layout/SpaceAroundOperators" + end + + private + + def write_config(cops) + write(".rubocop.yml", <<~YAML) + plugins: + - rubocop-herb + AllCops: + DisabledByDefault: true + SuggestExtensions: false + #{cops} + YAML + end + + def write(path, contents) + File.write(File.join(@directory, path), contents) + end + + def read(path) + File.read(File.join(@directory, path)) + end +end diff --git a/gems/rubocop-herb/test/plugin_test.rb b/gems/rubocop-herb/test/plugin_test.rb new file mode 100644 index 000000000..ed78e4e21 --- /dev/null +++ b/gems/rubocop-herb/test/plugin_test.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +require_relative "test_helper" +require "yaml" + +module RuboCop + module Herb + class PluginTest < Minitest::Spec + test "registers the extractor only once" do + extractors = ::RuboCop::Runner.ruby_extractors + extractors.delete(RubyExtractor) + plugin = Plugin.new + context = LintRoller::Context.new(engine: :rubocop, engine_version: "1.84.0") + + 2.times { plugin.rules(context) } + + assert_equal 1, extractors.count(RubyExtractor) + ensure + extractors&.delete(RubyExtractor) + end + + test "excludes cops that misinterpret HTML conditional bodies" do + config = YAML.load_file(File.expand_path("../config/default.yml", __dir__)) + + assert_equal ["**/*.html.erb"], config.dig("Lint/EmptyConditionalBody", "Exclude") + assert_equal ["**/*.html.erb"], config.dig("Rails/Presence", "Exclude") + end + end + end +end diff --git a/gems/rubocop-herb/test/ruby_extractor_test.rb b/gems/rubocop-herb/test/ruby_extractor_test.rb new file mode 100644 index 000000000..12f3dc7d5 --- /dev/null +++ b/gems/rubocop-herb/test/ruby_extractor_test.rb @@ -0,0 +1,90 @@ +# frozen_string_literal: true + +require_relative "test_helper" + +module RuboCop + module Herb + class RubyExtractorTest < Minitest::Spec + test "builds one position-preserving Ruby source" do + template = "

Café <%= x=1 %>

\n" + fragment = extract(template).first + source = fragment[:processed_source].raw_source + + assert_equal 0, fragment[:offset] + assert_equal template.length, source.length + assert_equal "x=1", source[template.index("x=1"), 3] + assert fragment[:processed_source].valid_syntax? + end + + test "preserves Ruby control flow across ERB tags" do + fragment = extract(<<~ERB).first + <% case status %> + <% when "open", "closed" %> + <% if user.admin? %> + <%= users.each do |user| %> + <%= user.name %> + <% end %> + <% end %> + <% end %> + ERB + + source = fragment[:processed_source] + + assert source.valid_syntax? + assert_equal :case, source.ast.type + assert_includes source.raw_source, "when \"open\", \"closed\"" + assert_includes source.raw_source, "users.each do |user|" + end + + test "separates adjacent Ruby tags on the same line" do + source = extract("<%= first %><%= second %>\n").first[:processed_source] + + assert source.valid_syntax? + assert_equal 2, source.ast.each_node(:send).count + end + + test "uses Herb's Ruby extraction binding" do + template = "<%= first %><%= second %>\n" + source = extract(template).first[:processed_source] + + assert_equal ::Herb.extract_ruby(template), source.raw_source + end + + test "preserves multibyte characters in Ruby" do + template = "<%= name == \"José\" %>\n" + source = extract(template).first[:processed_source] + + assert_includes source.raw_source, "name == \"José\"" + end + + test "skips comments, escaped ERB, and GraphQL tags" do + fragments = extract(<<~ERB) + <%# x=1 %> + <%% x=1 %%> + <%graphql query Example { viewer { login } } %> + ERB + + assert_empty fragments + end + + test "returns nil for non-ERB files" do + assert_nil RubyExtractor.call(processed_source("x=1\n", "example.rb")) + end + + test "does not extract Ruby from non-HTML ERB files" do + assert_empty RubyExtractor.call(processed_source("<%= x=1 %>\n", "example.rss.erb")) + assert_empty RubyExtractor.call(processed_source("<%= x=1 %>\n", "example.erb")) + end + + private + + def extract(source) + RubyExtractor.call(processed_source(source, "example.html.erb")) + end + + def processed_source(source, path) + ::RuboCop::ProcessedSource.new(source, 3.2, path) + end + end + end +end diff --git a/gems/rubocop-herb/test/test_helper.rb b/gems/rubocop-herb/test/test_helper.rb new file mode 100644 index 000000000..ab33de74a --- /dev/null +++ b/gems/rubocop-herb/test/test_helper.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +root = File.expand_path("../../..", __dir__) +$LOAD_PATH.unshift File.join(root, "lib") +$LOAD_PATH.unshift File.expand_path("../lib", __dir__) + +require "herb" +require "fileutils" +require "maxitest/autorun" +require "minitest/spec" +require "open3" +require "rubocop/herb" +require "tmpdir" + +Minitest::Spec::DSL.send(:alias_method, :test, :it) + +module RuboCopHerbTestHelpers + ROOT = File.expand_path("../../..", __dir__) + + def run_rubocop(*, chdir:) + Open3.capture3( + { "BUNDLE_GEMFILE" => File.join(ROOT, "Gemfile") }, + RbConfig.ruby, + "-S", + "bundle", + "exec", + "rubocop", + *, + chdir: + ) + end +end From e8b8f7bf06a88bb154c11199d480f78c0d5cfa58 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Thu, 1 Oct 2026 13:42:14 -0600 Subject: [PATCH 02/18] Simplify RuboCop Herb extraction Inline the minimal RuboCop extraction hook around Herb.extract_ruby and remove the standalone extractor, range collector, and autocorrection restriction patches. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- gems/rubocop-herb/README.md | 2 +- gems/rubocop-herb/lib/rubocop/herb.rb | 5 +- gems/rubocop-herb/lib/rubocop/herb/plugin.rb | 38 ++++++++++-- .../rubocop/herb/processed_source_builder.rb | 11 +--- .../herb/range_restricted_autocorrect.rb | 34 ----------- .../herb/range_restricted_corrector.rb | 31 ---------- .../lib/rubocop/herb/ruby_extractor.rb | 59 ------------------- .../lib/rubocop/herb/ruby_range_collector.rb | 52 ---------------- gems/rubocop-herb/test/integration_test.rb | 36 ----------- ...ctor_test.rb => plugin_extraction_test.rb} | 10 ++-- gems/rubocop-herb/test/plugin_test.rb | 7 ++- 11 files changed, 47 insertions(+), 238 deletions(-) delete mode 100644 gems/rubocop-herb/lib/rubocop/herb/range_restricted_autocorrect.rb delete mode 100644 gems/rubocop-herb/lib/rubocop/herb/range_restricted_corrector.rb delete mode 100644 gems/rubocop-herb/lib/rubocop/herb/ruby_extractor.rb delete mode 100644 gems/rubocop-herb/lib/rubocop/herb/ruby_range_collector.rb rename gems/rubocop-herb/test/{ruby_extractor_test.rb => plugin_extraction_test.rb} (86%) diff --git a/gems/rubocop-herb/README.md b/gems/rubocop-herb/README.md index aa2980a15..76820700d 100644 --- a/gems/rubocop-herb/README.md +++ b/gems/rubocop-herb/README.md @@ -23,4 +23,4 @@ bundle exec rubocop -a app/views bundle exec rubocop -A app/views ``` -The plugin honors the project's RuboCop configuration, including custom cops, inherited configuration, target Ruby versions, and file exclusions. Autocorrections are applied only when the complete edit falls within one original Ruby region, so corrections cannot modify HTML or ERB delimiters. Cops that depend on physical Ruby file contents or layout are [excluded for HTML+ERB templates by default](config/default.yml). +The plugin honors the project's RuboCop configuration, including custom cops, inherited configuration, target Ruby versions, file exclusions, and autocorrection. Cops that depend on physical Ruby file contents or layout are [excluded for HTML+ERB templates by default](config/default.yml). diff --git a/gems/rubocop-herb/lib/rubocop/herb.rb b/gems/rubocop-herb/lib/rubocop/herb.rb index 1a1d60d3d..984024b39 100644 --- a/gems/rubocop-herb/lib/rubocop/herb.rb +++ b/gems/rubocop-herb/lib/rubocop/herb.rb @@ -1,14 +1,11 @@ # frozen_string_literal: true +require "herb" require "rubocop" module RuboCop module Herb autoload :ProcessedSourceBuilder, "rubocop/herb/processed_source_builder" - autoload :RangeRestrictedAutocorrect, "rubocop/herb/range_restricted_autocorrect" - autoload :RangeRestrictedCorrector, "rubocop/herb/range_restricted_corrector" - autoload :RubyExtractor, "rubocop/herb/ruby_extractor" - autoload :RubyRangeCollector, "rubocop/herb/ruby_range_collector" end end diff --git a/gems/rubocop-herb/lib/rubocop/herb/plugin.rb b/gems/rubocop-herb/lib/rubocop/herb/plugin.rb index 6feaa6dcb..e68b5f830 100644 --- a/gems/rubocop-herb/lib/rubocop/herb/plugin.rb +++ b/gems/rubocop-herb/lib/rubocop/herb/plugin.rb @@ -17,11 +17,7 @@ def about def rules(_context) extractors = RuboCop::Runner.ruby_extractors - extractors.unshift(RubyExtractor) unless extractors.include?(RubyExtractor) - team = RuboCop::Cop::Team - team.prepend(RangeRestrictedAutocorrect) unless team < RangeRestrictedAutocorrect - corrector = RuboCop::Cop::Corrector - corrector.prepend(RangeRestrictedCorrector) unless corrector < RangeRestrictedCorrector + extractors.unshift(EXTRACT_RUBY) unless extractors.include?(EXTRACT_RUBY) LintRoller::Rules.new( config_format: :rubocop, @@ -33,6 +29,38 @@ def rules(_context) def supported?(context) context.engine == :rubocop end + + def self.extract_ruby(processed_source) + path = processed_source.path + return unless path&.end_with?(".erb") + return [] unless path.end_with?(".html.erb") + + template = processed_source.raw_source + code = position_preserving_ruby(template) + return [] if code.strip.empty? + + source = ProcessedSourceBuilder.call(code:, processed_source:) + return [] unless source.valid_syntax? && source.ast + + [{ offset: 0, processed_source: source }] + end + + def self.position_preserving_ruby(template) + extracted = ::Herb.extract_ruby(template) + return extracted if extracted.length == template.length + + byte_offset = 0 + template.each_char.map do |character| + extracted_character = extracted.byteslice(byte_offset, character.bytesize) + byte_offset += character.bytesize + + extracted_character == character ? character : extracted_character.each_char.first + end.join + end + + private_class_method :position_preserving_ruby + + EXTRACT_RUBY = method(:extract_ruby).to_proc end end end diff --git a/gems/rubocop-herb/lib/rubocop/herb/processed_source_builder.rb b/gems/rubocop-herb/lib/rubocop/herb/processed_source_builder.rb index 00ace4e21..d92777d72 100644 --- a/gems/rubocop-herb/lib/rubocop/herb/processed_source_builder.rb +++ b/gems/rubocop-herb/lib/rubocop/herb/processed_source_builder.rb @@ -3,14 +3,13 @@ module RuboCop module Herb class ProcessedSourceBuilder - def self.call(code:, processed_source:, ruby_ranges:) - new(code, processed_source, ruby_ranges).call + def self.call(code:, processed_source:) + new(code, processed_source).call end - def initialize(code, processed_source, ruby_ranges) + def initialize(code, processed_source) @code = code @processed_source = processed_source - @ruby_ranges = ruby_ranges end def call @@ -22,10 +21,6 @@ def call ) source.config = @processed_source.config source.registry = @processed_source.registry - source.define_singleton_method(:herb_ruby_ranges) { @ruby_ranges } - source.instance_variable_set(:@ruby_ranges, @ruby_ranges) - source.buffer.define_singleton_method(:herb_ruby_ranges) { @ruby_ranges } - source.buffer.instance_variable_set(:@ruby_ranges, @ruby_ranges) source end end diff --git a/gems/rubocop-herb/lib/rubocop/herb/range_restricted_autocorrect.rb b/gems/rubocop-herb/lib/rubocop/herb/range_restricted_autocorrect.rb deleted file mode 100644 index 7527491c8..000000000 --- a/gems/rubocop-herb/lib/rubocop/herb/range_restricted_autocorrect.rb +++ /dev/null @@ -1,34 +0,0 @@ -# frozen_string_literal: true - -module RuboCop - module Herb - module RangeRestrictedAutocorrect - private - - def autocorrect_report(report, offset:, original:) - ranges = herb_ruby_ranges(report) - return super unless ranges - - corrector = collate_corrections(report, offset:, original:) - restricted = RuboCop::Cop::Corrector.new(original) - - corrector.as_replacements.each do |range, replacement| - restricted.replace(range, replacement) if herb_correction_allowed?(range, ranges) - end - - restricted.rewrite unless restricted.empty? - end - - def herb_ruby_ranges(report) - source = report.processed_source - source.herb_ruby_ranges if source.respond_to?(:herb_ruby_ranges) - end - - def herb_correction_allowed?(correction, ranges) - ranges.any? do |range| - range.from <= correction.begin_pos && correction.end_pos <= range.to - end - end - end - end -end diff --git a/gems/rubocop-herb/lib/rubocop/herb/range_restricted_corrector.rb b/gems/rubocop-herb/lib/rubocop/herb/range_restricted_corrector.rb deleted file mode 100644 index 94aa1eb74..000000000 --- a/gems/rubocop-herb/lib/rubocop/herb/range_restricted_corrector.rb +++ /dev/null @@ -1,31 +0,0 @@ -# frozen_string_literal: true - -module RuboCop - module Herb - module RangeRestrictedCorrector - def replace(node_or_range, content) - return unless herb_range_allowed?(node_or_range) - - super - end - - def wrap(node_or_range, insert_before, insert_after) - return unless herb_range_allowed?(node_or_range) - - super - end - - private - - def herb_range_allowed?(node_or_range) - ranges = source_buffer.herb_ruby_ranges if source_buffer.respond_to?(:herb_ruby_ranges) - return true unless ranges - - correction = to_range(node_or_range) - ranges.any? do |range| - range.from <= correction.begin_pos && correction.end_pos <= range.to - end - end - end - end -end diff --git a/gems/rubocop-herb/lib/rubocop/herb/ruby_extractor.rb b/gems/rubocop-herb/lib/rubocop/herb/ruby_extractor.rb deleted file mode 100644 index 04c553190..000000000 --- a/gems/rubocop-herb/lib/rubocop/herb/ruby_extractor.rb +++ /dev/null @@ -1,59 +0,0 @@ -# frozen_string_literal: true - -require "herb" -require "rubocop" - -module RuboCop - module Herb - class RubyExtractor - def self.call(processed_source) - new(processed_source).call - end - - def initialize(processed_source) - @processed_source = processed_source - end - - def call - return unless erb_file? - return [] unless html_erb_file? - - template = @processed_source.raw_source - ruby_ranges = RubyRangeCollector.call(template) - return [] if ruby_ranges.empty? - - source = ProcessedSourceBuilder.call( - code: ruby_source(template), - processed_source: @processed_source, - ruby_ranges: - ) - return [] unless source.valid_syntax? - - [{ offset: 0, processed_source: source }] - end - - private - - def erb_file? - @processed_source.path&.end_with?(".erb") - end - - def html_erb_file? - @processed_source.path&.end_with?(".html.erb") - end - - def ruby_source(template) - extracted = ::Herb.extract_ruby(template) - return extracted if extracted.length == template.length - - byte_offset = 0 - template.each_char.map do |character| - extracted_character = extracted.byteslice(byte_offset, character.bytesize) - byte_offset += character.bytesize - - extracted_character == character ? character : extracted_character.each_char.first - end.join - end - end - end -end diff --git a/gems/rubocop-herb/lib/rubocop/herb/ruby_range_collector.rb b/gems/rubocop-herb/lib/rubocop/herb/ruby_range_collector.rb deleted file mode 100644 index 8fd945122..000000000 --- a/gems/rubocop-herb/lib/rubocop/herb/ruby_range_collector.rb +++ /dev/null @@ -1,52 +0,0 @@ -# frozen_string_literal: true - -module RuboCop - module Herb - class RubyRangeCollector - def self.call(template) - new(template).call - end - - def initialize(template) - @template = template - end - - def call - visitor = ErbNodeVisitor.new - visitor.visit(::Herb.parse(@template).value) - visitor.ranges.map { |range| character_range(range) } - end - - private - - def character_range(byte_range) - ::Herb::Range.new( - @template.byteslice(0, byte_range.from).length, - @template.byteslice(0, byte_range.to).length - ) - end - - class ErbNodeVisitor < ::Herb::Visitor - attr_reader :ranges - - def initialize - super - @ranges = [] - end - - def visit_erb_node(node) - @ranges << node.content.range if inspectable?(node) - end - - private - - def inspectable?(node) - return false unless node.respond_to?(:content) && node.content - - opening = node.tag_opening&.value - opening && opening != "<%#" && opening != "<%%" && opening != "<%graphql" - end - end - end - end -end diff --git a/gems/rubocop-herb/test/integration_test.rb b/gems/rubocop-herb/test/integration_test.rb index fe340e20e..1cab2082e 100644 --- a/gems/rubocop-herb/test/integration_test.rb +++ b/gems/rubocop-herb/test/integration_test.rb @@ -138,42 +138,6 @@ def on_send(node) refute_includes output, "Layout/SpaceAroundOperators" end - test "rejects autocorrections spanning non-Ruby template content" do - write("custom_cop.rb", <<~RUBY) - module RuboCop - module Cop - module HerbTest - class ReplaceConditional < Base - extend AutoCorrector - - MSG = "Do not use conditionals." - - def on_if(node) - add_offense(node) { |corrector| corrector.replace(node, "replacement") } - end - end - end - end - end - RUBY - write_config(<<~YAML) - require: - - ./custom_cop - HerbTest/ReplaceConditional: - Enabled: true - YAML - template = "<% if condition %><%= value %><% end %>\n" - write("example.html.erb", template) - - output, error, status = run_rubocop("-A", "example.html.erb", chdir: @directory) - - refute status.success? - assert_empty error - assert_includes output, "HerbTest/ReplaceConditional" - refute_includes output, "[Corrected]" - assert_equal template, read("example.html.erb") - end - test "leaves ordinary Ruby inspection unchanged" do write_config(<<~YAML) Layout/SpaceAroundOperators: diff --git a/gems/rubocop-herb/test/ruby_extractor_test.rb b/gems/rubocop-herb/test/plugin_extraction_test.rb similarity index 86% rename from gems/rubocop-herb/test/ruby_extractor_test.rb rename to gems/rubocop-herb/test/plugin_extraction_test.rb index 12f3dc7d5..c52448f5a 100644 --- a/gems/rubocop-herb/test/ruby_extractor_test.rb +++ b/gems/rubocop-herb/test/plugin_extraction_test.rb @@ -4,7 +4,7 @@ module RuboCop module Herb - class RubyExtractorTest < Minitest::Spec + class PluginExtractionTest < Minitest::Spec test "builds one position-preserving Ruby source" do template = "

Café <%= x=1 %>

\n" fragment = extract(template).first @@ -68,18 +68,18 @@ class RubyExtractorTest < Minitest::Spec end test "returns nil for non-ERB files" do - assert_nil RubyExtractor.call(processed_source("x=1\n", "example.rb")) + assert_nil Plugin.extract_ruby(processed_source("x=1\n", "example.rb")) end test "does not extract Ruby from non-HTML ERB files" do - assert_empty RubyExtractor.call(processed_source("<%= x=1 %>\n", "example.rss.erb")) - assert_empty RubyExtractor.call(processed_source("<%= x=1 %>\n", "example.erb")) + assert_empty Plugin.extract_ruby(processed_source("<%= x=1 %>\n", "example.rss.erb")) + assert_empty Plugin.extract_ruby(processed_source("<%= x=1 %>\n", "example.erb")) end private def extract(source) - RubyExtractor.call(processed_source(source, "example.html.erb")) + Plugin.extract_ruby(processed_source(source, "example.html.erb")) end def processed_source(source, path) diff --git a/gems/rubocop-herb/test/plugin_test.rb b/gems/rubocop-herb/test/plugin_test.rb index ed78e4e21..d717003fc 100644 --- a/gems/rubocop-herb/test/plugin_test.rb +++ b/gems/rubocop-herb/test/plugin_test.rb @@ -8,15 +8,16 @@ module Herb class PluginTest < Minitest::Spec test "registers the extractor only once" do extractors = ::RuboCop::Runner.ruby_extractors - extractors.delete(RubyExtractor) + extractor = Plugin::EXTRACT_RUBY + extractors.delete(extractor) plugin = Plugin.new context = LintRoller::Context.new(engine: :rubocop, engine_version: "1.84.0") 2.times { plugin.rules(context) } - assert_equal 1, extractors.count(RubyExtractor) + assert_equal 1, extractors.count(extractor) ensure - extractors&.delete(RubyExtractor) + extractors&.delete(extractor) end test "excludes cops that misinterpret HTML conditional bodies" do From 75e2ebed456636e56bdae3ceaa004621911d0aa8 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Thu, 1 Oct 2026 15:06:46 -0600 Subject: [PATCH 03/18] Resolve upstream RuboCop Herb changes Refresh the path gem lock entry for Herb 0.11.0 and align extraction tests with the current Herb binding behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- Gemfile.lock | 6 +++--- gems/rubocop-herb/test/plugin_extraction_test.rb | 3 +-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index fecc81b22..e94fb9041 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -24,8 +24,8 @@ PATH PATH remote: gems/rubocop-herb specs: - rubocop-herb (0.10.3) - herb (= 0.10.3) + rubocop-herb (0.11.0) + herb (= 0.11.0) lint_roller (~> 1.1) rubocop (>= 1.72.1, < 2) @@ -376,8 +376,8 @@ DEPENDENCIES reline (~> 0.7) rolldown (~> 1.2) rubocop (~> 1.72) - rubocop-performance (~> 1.26.0) rubocop-herb! + rubocop-performance (~> 1.26.0) sorbet steep (~> 2.1) turbo-rails (~> 2.0) diff --git a/gems/rubocop-herb/test/plugin_extraction_test.rb b/gems/rubocop-herb/test/plugin_extraction_test.rb index c52448f5a..fb994ece3 100644 --- a/gems/rubocop-herb/test/plugin_extraction_test.rb +++ b/gems/rubocop-herb/test/plugin_extraction_test.rb @@ -57,11 +57,10 @@ class PluginExtractionTest < Minitest::Spec assert_includes source.raw_source, "name == \"José\"" end - test "skips comments, escaped ERB, and GraphQL tags" do + test "skips comments and escaped ERB" do fragments = extract(<<~ERB) <%# x=1 %> <%% x=1 %%> - <%graphql query Example { viewer { login } } %> ERB assert_empty fragments From 4240cab6f9ff7814ed31e04cd530b7dbf221760c Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 2 Oct 2026 10:39:55 -0600 Subject: [PATCH 04/18] Apply suggestions from code review Signed-off-by: Joel Hawksley --- gems/rubocop-herb/config/default.yml | 8 -------- 1 file changed, 8 deletions(-) diff --git a/gems/rubocop-herb/config/default.yml b/gems/rubocop-herb/config/default.yml index 520dfb7d5..0b0123770 100644 --- a/gems/rubocop-herb/config/default.yml +++ b/gems/rubocop-herb/config/default.yml @@ -79,12 +79,6 @@ Lint/EmptyFile: Exclude: ['**/*.html.erb'] Lint/EmptyConditionalBody: Exclude: ['**/*.html.erb'] -Lint/OutOfRangeRegexpRef: - Exclude: ['**/*.html.erb'] -Lint/Syntax: - Exclude: ['**/*.html.erb'] -Lint/Void: - Exclude: ['**/*.html.erb'] Metrics/BlockLength: Exclude: ['**/*.html.erb'] Naming/FileName: @@ -109,7 +103,5 @@ Style/Next: Exclude: ['**/*.html.erb'] Style/Semicolon: Exclude: ['**/*.html.erb'] -Style/WhileUntilDo: - Exclude: ['**/*.html.erb'] Style/WhileUntilModifier: Exclude: ['**/*.html.erb'] From a886d5de5945c097806ab55f1dbb01ea4f304052 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 2 Oct 2026 10:47:15 -0600 Subject: [PATCH 05/18] Scan all ERB and Herb templates Register every .erb and .herb file for RuboCop extraction, apply template-specific exclusions consistently, and cover non-HTML ERB and Herb files in integration tests. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- README.md | 2 +- gems/rubocop-herb/README.md | 4 +- gems/rubocop-herb/config/default.yml | 101 +++++++++--------- gems/rubocop-herb/lib/rubocop/herb/plugin.rb | 3 +- gems/rubocop-herb/test/integration_test.rb | 19 ++-- .../test/plugin_extraction_test.rb | 10 +- gems/rubocop-herb/test/plugin_test.rb | 7 +- 7 files changed, 75 insertions(+), 71 deletions(-) diff --git a/README.md b/README.md index a9501bd56..bb0b83712 100644 --- a/README.md +++ b/README.md @@ -55,7 +55,7 @@ Run the linter and the formatter: npx @herb-tools/linter ``` -Run your project's configured RuboCop rules against Ruby in HTML+ERB (`.html.erb`) templates with the `rubocop-herb` plugin: +Run your project's configured RuboCop rules against Ruby in `.erb` and `.herb` templates with the `rubocop-herb` plugin: ```sh bundle add rubocop-herb diff --git a/gems/rubocop-herb/README.md b/gems/rubocop-herb/README.md index 76820700d..812350d60 100644 --- a/gems/rubocop-herb/README.md +++ b/gems/rubocop-herb/README.md @@ -1,6 +1,6 @@ # rubocop-herb -`rubocop-herb` runs a project's configured RuboCop rules against Ruby embedded in HTML+ERB (`.html.erb`) templates. Herb builds a position-preserving Ruby representation of the entire template so RuboCop can understand control flow and variable usage across ERB tags while reporting offenses at their original template locations. +`rubocop-herb` runs a project's configured RuboCop rules against Ruby embedded in `.erb` and `.herb` templates. Herb builds a position-preserving Ruby representation of the entire template so RuboCop can understand control flow and variable usage across ERB tags while reporting offenses at their original template locations. Add the gem to your bundle: @@ -23,4 +23,4 @@ bundle exec rubocop -a app/views bundle exec rubocop -A app/views ``` -The plugin honors the project's RuboCop configuration, including custom cops, inherited configuration, target Ruby versions, file exclusions, and autocorrection. Cops that depend on physical Ruby file contents or layout are [excluded for HTML+ERB templates by default](config/default.yml). +The plugin honors the project's RuboCop configuration, including custom cops, inherited configuration, target Ruby versions, file exclusions, and autocorrection. Cops that depend on physical Ruby file contents or layout are [excluded for template files by default](config/default.yml). diff --git a/gems/rubocop-herb/config/default.yml b/gems/rubocop-herb/config/default.yml index 0b0123770..d2ee85c74 100644 --- a/gems/rubocop-herb/config/default.yml +++ b/gems/rubocop-herb/config/default.yml @@ -4,104 +4,105 @@ inherit_mode: - Include AllCops: - Include: - - '**/*.html.erb' + Include: &template_files + - '**/*.erb' + - '**/*.herb' # These cops depend on physical Ruby file contents or layout that is obscured # by the position-preserving template representation. Layout/ArgumentAlignment: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Layout/ArrayAlignment: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Layout/BlockAlignment: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Layout/ClosingParenthesisIndentation: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Layout/CommentIndentation: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Layout/InitialIndentation: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Layout/EmptyLineAfterGuardClause: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Layout/EndAlignment: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Layout/FirstArgumentIndentation: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Layout/FirstArrayElementIndentation: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Layout/FirstArrayElementLineBreak: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Layout/FirstHashElementIndentation: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Layout/FirstHashElementLineBreak: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Layout/FirstMethodArgumentLineBreak: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Layout/FirstParameterIndentation: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Layout/HashAlignment: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Layout/IndentationConsistency: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Layout/IndentationWidth: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Layout/LeadingEmptyLine: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Layout/LeadingEmptyLines: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Layout/LineEndStringConcatenationIndentation: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Layout/LineLength: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Layout/MultilineArrayBraceLayout: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Layout/MultilineAssignmentLayout: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Layout/MultilineHashBraceLayout: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Layout/MultilineMethodCallBraceLayout: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Layout/MultilineMethodCallIndentation: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Layout/MultilineMethodDefinitionBraceLayout: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Layout/MultilineOperationIndentation: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Layout/ParameterAlignment: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Layout/SpaceBeforeSemicolon: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Layout/TrailingEmptyLines: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Layout/TrailingWhitespace: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Lint/EmptyFile: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Lint/EmptyConditionalBody: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Metrics/BlockLength: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Naming/FileName: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Rails/Presence: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Style/FrozenStringLiteralComment: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Style/EmptyElse: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Style/IdenticalConditionalBranches: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Style/IfWithSemicolon: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Style/IfUnlessModifier: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Style/MultilineTernaryOperator: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Style/NestedTernaryOperator: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Style/Next: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Style/Semicolon: - Exclude: ['**/*.html.erb'] + Exclude: *template_files Style/WhileUntilModifier: - Exclude: ['**/*.html.erb'] + Exclude: *template_files diff --git a/gems/rubocop-herb/lib/rubocop/herb/plugin.rb b/gems/rubocop-herb/lib/rubocop/herb/plugin.rb index e68b5f830..4f1a9aee6 100644 --- a/gems/rubocop-herb/lib/rubocop/herb/plugin.rb +++ b/gems/rubocop-herb/lib/rubocop/herb/plugin.rb @@ -32,8 +32,7 @@ def supported?(context) def self.extract_ruby(processed_source) path = processed_source.path - return unless path&.end_with?(".erb") - return [] unless path.end_with?(".html.erb") + return unless path&.end_with?(".erb", ".herb") template = processed_source.raw_source code = position_preserving_ruby(template) diff --git a/gems/rubocop-herb/test/integration_test.rb b/gems/rubocop-herb/test/integration_test.rb index 1cab2082e..a0c750256 100644 --- a/gems/rubocop-herb/test/integration_test.rb +++ b/gems/rubocop-herb/test/integration_test.rb @@ -106,12 +106,11 @@ def on_send(node) refute_includes output, "Lint/UselessAssignment" end - test "runs only on HTML ERB files" do + test "runs on non-HTML ERB files" do write_config(<<~YAML) Layout/SpaceAroundOperators: Enabled: true YAML - write("example.html.erb", "<%= html=1 %>\n") write("example.rss.erb", "<%= rss=1 %>\n") write("example.erb", "<%= generic=1 %>\n") @@ -119,23 +118,23 @@ def on_send(node) refute status.success? assert_empty error - assert_includes output, "example.html.erb" - refute_includes output, "example.rss.erb" - refute_includes output, "example.erb" + assert_includes output, "example.rss.erb" + assert_includes output, "example.erb" end - test "does not inspect explicitly passed non-HTML ERB files" do + test "runs on Herb files" do write_config(<<~YAML) Layout/SpaceAroundOperators: Enabled: true YAML - write("example.rss.erb", "<%= rss=1 %>\n") + write("example.herb", "<%= herb=1 %>\n") - output, error, status = run_rubocop("example.rss.erb", chdir: @directory) + output, error, status = run_rubocop("example.herb", chdir: @directory) - assert status.success?, output + refute status.success? assert_empty error - refute_includes output, "Layout/SpaceAroundOperators" + assert_includes output, "example.herb" + assert_includes output, "Layout/SpaceAroundOperators" end test "leaves ordinary Ruby inspection unchanged" do diff --git a/gems/rubocop-herb/test/plugin_extraction_test.rb b/gems/rubocop-herb/test/plugin_extraction_test.rb index fb994ece3..f5d0d77a8 100644 --- a/gems/rubocop-herb/test/plugin_extraction_test.rb +++ b/gems/rubocop-herb/test/plugin_extraction_test.rb @@ -70,9 +70,13 @@ class PluginExtractionTest < Minitest::Spec assert_nil Plugin.extract_ruby(processed_source("x=1\n", "example.rb")) end - test "does not extract Ruby from non-HTML ERB files" do - assert_empty Plugin.extract_ruby(processed_source("<%= x=1 %>\n", "example.rss.erb")) - assert_empty Plugin.extract_ruby(processed_source("<%= x=1 %>\n", "example.erb")) + test "extracts Ruby from non-HTML ERB files" do + refute_empty Plugin.extract_ruby(processed_source("<%= x=1 %>\n", "example.rss.erb")) + refute_empty Plugin.extract_ruby(processed_source("<%= x=1 %>\n", "example.erb")) + end + + test "extracts Ruby from Herb files" do + refute_empty Plugin.extract_ruby(processed_source("<%= x=1 %>\n", "example.herb")) end private diff --git a/gems/rubocop-herb/test/plugin_test.rb b/gems/rubocop-herb/test/plugin_test.rb index d717003fc..fd0b9bcbb 100644 --- a/gems/rubocop-herb/test/plugin_test.rb +++ b/gems/rubocop-herb/test/plugin_test.rb @@ -21,10 +21,11 @@ class PluginTest < Minitest::Spec end test "excludes cops that misinterpret HTML conditional bodies" do - config = YAML.load_file(File.expand_path("../config/default.yml", __dir__)) + config = YAML.load_file(File.expand_path("../config/default.yml", __dir__), aliases: true) + template_files = ["**/*.erb", "**/*.herb"] - assert_equal ["**/*.html.erb"], config.dig("Lint/EmptyConditionalBody", "Exclude") - assert_equal ["**/*.html.erb"], config.dig("Rails/Presence", "Exclude") + assert_equal template_files, config.dig("Lint/EmptyConditionalBody", "Exclude") + assert_equal template_files, config.dig("Rails/Presence", "Exclude") end end end From cfcc57d7ac2285c2b5d517914004ed44ffacbdab Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 2 Oct 2026 10:50:25 -0600 Subject: [PATCH 06/18] Use canonical extractor registration Register the Herb extractor directly through RuboCop::Runner.ruby_extractors, matching RuboCop's established template plugins. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- gems/rubocop-herb/lib/rubocop/herb/plugin.rb | 3 +-- gems/rubocop-herb/test/plugin_test.rb | 6 +++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/gems/rubocop-herb/lib/rubocop/herb/plugin.rb b/gems/rubocop-herb/lib/rubocop/herb/plugin.rb index 4f1a9aee6..c704371e3 100644 --- a/gems/rubocop-herb/lib/rubocop/herb/plugin.rb +++ b/gems/rubocop-herb/lib/rubocop/herb/plugin.rb @@ -16,8 +16,7 @@ def about end def rules(_context) - extractors = RuboCop::Runner.ruby_extractors - extractors.unshift(EXTRACT_RUBY) unless extractors.include?(EXTRACT_RUBY) + RuboCop::Runner.ruby_extractors.unshift(EXTRACT_RUBY) LintRoller::Rules.new( config_format: :rubocop, diff --git a/gems/rubocop-herb/test/plugin_test.rb b/gems/rubocop-herb/test/plugin_test.rb index fd0b9bcbb..79fd27b7e 100644 --- a/gems/rubocop-herb/test/plugin_test.rb +++ b/gems/rubocop-herb/test/plugin_test.rb @@ -6,16 +6,16 @@ module RuboCop module Herb class PluginTest < Minitest::Spec - test "registers the extractor only once" do + test "registers the extractor" do extractors = ::RuboCop::Runner.ruby_extractors extractor = Plugin::EXTRACT_RUBY extractors.delete(extractor) plugin = Plugin.new context = LintRoller::Context.new(engine: :rubocop, engine_version: "1.84.0") - 2.times { plugin.rules(context) } + plugin.rules(context) - assert_equal 1, extractors.count(extractor) + assert_equal extractor, extractors.first ensure extractors&.delete(extractor) end From 46d795dbd8a584e64f9e12492c2db099e2506c49 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Tue, 6 Oct 2026 08:58:18 -0600 Subject: [PATCH 07/18] Update gems/rubocop-herb/rubocop-herb.gemspec Co-authored-by: Marco Roth Signed-off-by: Joel Hawksley --- gems/rubocop-herb/rubocop-herb.gemspec | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/gems/rubocop-herb/rubocop-herb.gemspec b/gems/rubocop-herb/rubocop-herb.gemspec index 99466b6c6..94f86a3f5 100644 --- a/gems/rubocop-herb/rubocop-herb.gemspec +++ b/gems/rubocop-herb/rubocop-herb.gemspec @@ -15,13 +15,11 @@ Gem::Specification.new do |spec| spec.required_ruby_version = ">= 3.2.0" spec.require_paths = ["lib"] - spec.files = Dir.chdir(__dir__) do - Dir[ - "README.md", - "config/**/*.yml", - "lib/**/*.rb" - ] - end + spec.files = Dir[ + "README.md", + "config/**/*.yml", + "lib/**/*.rb" + ] spec.metadata["allowed_push_host"] = "https://rubygems.org" spec.metadata["rubygems_mfa_required"] = "true" From b04c6e8e28e2701f165d6ee59c5f5fdb43e3966b Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Tue, 6 Oct 2026 09:04:37 -0600 Subject: [PATCH 08/18] Use ProcessedSource directly Remove the single-use ProcessedSourceBuilder class and construct extracted RuboCop sources directly in the plugin while preserving parser and configuration metadata. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- gems/rubocop-herb/lib/rubocop/herb.rb | 6 ---- gems/rubocop-herb/lib/rubocop/herb/plugin.rb | 19 +++++++++++-- .../rubocop/herb/processed_source_builder.rb | 28 ------------------- .../test/plugin_extraction_test.rb | 14 ++++++++++ 4 files changed, 31 insertions(+), 36 deletions(-) delete mode 100644 gems/rubocop-herb/lib/rubocop/herb/processed_source_builder.rb diff --git a/gems/rubocop-herb/lib/rubocop/herb.rb b/gems/rubocop-herb/lib/rubocop/herb.rb index 984024b39..a1fb07272 100644 --- a/gems/rubocop-herb/lib/rubocop/herb.rb +++ b/gems/rubocop-herb/lib/rubocop/herb.rb @@ -3,11 +3,5 @@ require "herb" require "rubocop" -module RuboCop - module Herb - autoload :ProcessedSourceBuilder, "rubocop/herb/processed_source_builder" - end -end - require_relative "herb/plugin" require_relative "herb/version" diff --git a/gems/rubocop-herb/lib/rubocop/herb/plugin.rb b/gems/rubocop-herb/lib/rubocop/herb/plugin.rb index c704371e3..56c60ba35 100644 --- a/gems/rubocop-herb/lib/rubocop/herb/plugin.rb +++ b/gems/rubocop-herb/lib/rubocop/herb/plugin.rb @@ -37,12 +37,27 @@ def self.extract_ruby(processed_source) code = position_preserving_ruby(template) return [] if code.strip.empty? - source = ProcessedSourceBuilder.call(code:, processed_source:) + source = processed_source( + code, + original: processed_source + ) return [] unless source.valid_syntax? && source.ast [{ offset: 0, processed_source: source }] end + def self.processed_source(code, original:) + ::RuboCop::ProcessedSource.new( + code, + original.ruby_version, + original.path, + parser_engine: original.parser_engine + ).tap do |source| + source.config = original.config + source.registry = original.registry + end + end + def self.position_preserving_ruby(template) extracted = ::Herb.extract_ruby(template) return extracted if extracted.length == template.length @@ -56,7 +71,7 @@ def self.position_preserving_ruby(template) end.join end - private_class_method :position_preserving_ruby + private_class_method :position_preserving_ruby, :processed_source EXTRACT_RUBY = method(:extract_ruby).to_proc end diff --git a/gems/rubocop-herb/lib/rubocop/herb/processed_source_builder.rb b/gems/rubocop-herb/lib/rubocop/herb/processed_source_builder.rb deleted file mode 100644 index d92777d72..000000000 --- a/gems/rubocop-herb/lib/rubocop/herb/processed_source_builder.rb +++ /dev/null @@ -1,28 +0,0 @@ -# frozen_string_literal: true - -module RuboCop - module Herb - class ProcessedSourceBuilder - def self.call(code:, processed_source:) - new(code, processed_source).call - end - - def initialize(code, processed_source) - @code = code - @processed_source = processed_source - end - - def call - source = ::RuboCop::ProcessedSource.new( - @code, - @processed_source.ruby_version, - @processed_source.path, - parser_engine: @processed_source.parser_engine - ) - source.config = @processed_source.config - source.registry = @processed_source.registry - source - end - end - end -end diff --git a/gems/rubocop-herb/test/plugin_extraction_test.rb b/gems/rubocop-herb/test/plugin_extraction_test.rb index f5d0d77a8..483561e55 100644 --- a/gems/rubocop-herb/test/plugin_extraction_test.rb +++ b/gems/rubocop-herb/test/plugin_extraction_test.rb @@ -79,6 +79,20 @@ class PluginExtractionTest < Minitest::Spec refute_empty Plugin.extract_ruby(processed_source("<%= x=1 %>\n", "example.herb")) end + test "preserves processed source metadata" do + original = processed_source("<%= x=1 %>\n", "example.herb") + original.config = Object.new + original.registry = Object.new + + extracted = Plugin.extract_ruby(original).first[:processed_source] + + assert_equal original.path, extracted.path + assert_equal original.ruby_version, extracted.ruby_version + assert_equal original.parser_engine, extracted.parser_engine + assert_same original.config, extracted.config + assert_same original.registry, extracted.registry + end + private def extract(source) From c1d679c5c54370caca56897956b2c76b73ce216e Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Tue, 6 Oct 2026 11:14:29 -0600 Subject: [PATCH 09/18] Simplify RuboCop Herb plugin Use Herb's version directly, replace Pathname with File.expand_path, keep the extractor as a Method, and streamline source construction naming and calls. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- gems/rubocop-herb/lib/rubocop/herb.rb | 1 - gems/rubocop-herb/lib/rubocop/herb/plugin.rb | 19 +++++++------------ gems/rubocop-herb/lib/rubocop/herb/version.rb | 9 --------- 3 files changed, 7 insertions(+), 22 deletions(-) delete mode 100644 gems/rubocop-herb/lib/rubocop/herb/version.rb diff --git a/gems/rubocop-herb/lib/rubocop/herb.rb b/gems/rubocop-herb/lib/rubocop/herb.rb index a1fb07272..073986b9e 100644 --- a/gems/rubocop-herb/lib/rubocop/herb.rb +++ b/gems/rubocop-herb/lib/rubocop/herb.rb @@ -4,4 +4,3 @@ require "rubocop" require_relative "herb/plugin" -require_relative "herb/version" diff --git a/gems/rubocop-herb/lib/rubocop/herb/plugin.rb b/gems/rubocop-herb/lib/rubocop/herb/plugin.rb index 56c60ba35..1d6b74b3d 100644 --- a/gems/rubocop-herb/lib/rubocop/herb/plugin.rb +++ b/gems/rubocop-herb/lib/rubocop/herb/plugin.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require "lint_roller" -require "pathname" module RuboCop module Herb @@ -11,7 +10,7 @@ def about description: "Run configured RuboCop rules against Ruby in ERB templates.", homepage: "https://github.com/marcoroth/herb", name: "rubocop-herb", - version: VERSION + version: ::Herb::VERSION ) end @@ -21,7 +20,7 @@ def rules(_context) LintRoller::Rules.new( config_format: :rubocop, type: :path, - value: Pathname.new(__dir__).join("../../../config/default.yml") + value: File.expand_path("../../../config/default.yml", __dir__) ) end @@ -30,23 +29,19 @@ def supported?(context) end def self.extract_ruby(processed_source) - path = processed_source.path - return unless path&.end_with?(".erb", ".herb") + return unless processed_source.path&.end_with?(".erb", ".herb") template = processed_source.raw_source code = position_preserving_ruby(template) return [] if code.strip.empty? - source = processed_source( - code, - original: processed_source - ) + source = build_processed_source(code, original: processed_source) return [] unless source.valid_syntax? && source.ast [{ offset: 0, processed_source: source }] end - def self.processed_source(code, original:) + def self.build_processed_source(code, original:) ::RuboCop::ProcessedSource.new( code, original.ruby_version, @@ -71,9 +66,9 @@ def self.position_preserving_ruby(template) end.join end - private_class_method :position_preserving_ruby, :processed_source + private_class_method :build_processed_source, :position_preserving_ruby - EXTRACT_RUBY = method(:extract_ruby).to_proc + EXTRACT_RUBY = method(:extract_ruby) end end end diff --git a/gems/rubocop-herb/lib/rubocop/herb/version.rb b/gems/rubocop-herb/lib/rubocop/herb/version.rb deleted file mode 100644 index ff4a8dceb..000000000 --- a/gems/rubocop-herb/lib/rubocop/herb/version.rb +++ /dev/null @@ -1,9 +0,0 @@ -# frozen_string_literal: true - -require "herb/version" - -module RuboCop - module Herb - VERSION = ::Herb::VERSION - end -end From d0323df360553502e31b14ff570990f907f46b2a Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Tue, 6 Oct 2026 11:15:47 -0600 Subject: [PATCH 10/18] Report embedded Ruby syntax errors Keep invalid extracted sources in RuboCop's pipeline so Lint/Syntax can report malformed Ruby, while continuing to skip valid template fragments with no AST. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- gems/rubocop-herb/lib/rubocop/herb/plugin.rb | 2 +- gems/rubocop-herb/test/integration_test.rb | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/gems/rubocop-herb/lib/rubocop/herb/plugin.rb b/gems/rubocop-herb/lib/rubocop/herb/plugin.rb index 1d6b74b3d..cf0cbd281 100644 --- a/gems/rubocop-herb/lib/rubocop/herb/plugin.rb +++ b/gems/rubocop-herb/lib/rubocop/herb/plugin.rb @@ -36,7 +36,7 @@ def self.extract_ruby(processed_source) return [] if code.strip.empty? source = build_processed_source(code, original: processed_source) - return [] unless source.valid_syntax? && source.ast + return [] if source.valid_syntax? && !source.ast [{ offset: 0, processed_source: source }] end diff --git a/gems/rubocop-herb/test/integration_test.rb b/gems/rubocop-herb/test/integration_test.rb index a0c750256..b75c39bbb 100644 --- a/gems/rubocop-herb/test/integration_test.rb +++ b/gems/rubocop-herb/test/integration_test.rb @@ -106,6 +106,21 @@ def on_send(node) refute_includes output, "Lint/UselessAssignment" end + test "reports Ruby syntax errors" do + write_config(<<~YAML) + Lint/Syntax: + Enabled: true + YAML + write("example.html.erb", "<%= user. %>\n") + + output, error, status = run_rubocop("example.html.erb", chdir: @directory) + + refute status.success? + assert_empty error + assert_includes output, "Lint/Syntax" + assert_includes output, "example.html.erb:1" + end + test "runs on non-HTML ERB files" do write_config(<<~YAML) Layout/SpaceAroundOperators: From a28592f7687ccf1be8ab9f9621e866bc500946da Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Tue, 6 Oct 2026 11:39:58 -0600 Subject: [PATCH 11/18] Preserve Unicode character positions in Ruby extraction Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- docs/docs/bindings/extracting.md | 4 +- java/org/herb/HerbTest.java | 13 +++ .../packages/node-wasm/test/node-wasm.test.ts | 10 ++ javascript/packages/node/test/node.test.ts | 10 ++ lib/herb/ruby_program.rb | 4 +- rust/tests/extract_ruby_options_test.rs | 18 ++++ src/analyze/analyze.c | 3 +- src/analyze/parse_errors.c | 22 +++- src/analyze/prism_annotate.c | 3 +- src/extract.c | 102 ++++++++++++------ src/include/extract.h | 1 + src/include/extract_internal.h | 13 +++ test/c/test_extract.c | 30 ++++++ test/extractor/extract_ruby_test.rb | 10 ++ test/ruby_program_test.rb | 9 ++ 15 files changed, 206 insertions(+), 46 deletions(-) create mode 100644 src/include/extract_internal.h diff --git a/docs/docs/bindings/extracting.md b/docs/docs/bindings/extracting.md index 4e9eb097a..e93e4fdbc 100644 --- a/docs/docs/bindings/extracting.md +++ b/docs/docs/bindings/extracting.md @@ -38,7 +38,7 @@ let ruby = extract_ruby("

Hello <%= user.name %>

").unwrap(); ``` ::: -By default every character keeps the position it had in the template, with the HTML replaced by spaces. A Ruby error on line 12 column 30 of the extracted output is on line 12 column 30 of the template, so positions need no translation. +By default every Unicode character keeps the position it had in the template, with non-Ruby content replaced by one space per character. Newlines and Ruby content are retained. A Ruby error on line 12 column 30 of the extracted output is on line 12 column 30 of the template, so character-based positions need no translation. Because UTF-8 characters can contain multiple bytes, the extracted output can have a different byte length than the template. ## Options @@ -46,7 +46,7 @@ By default every character keeps the position it had in the template, with the H |---|---|---|---| | `semicolons` | Boolean | `true` | Add ` ;` at the end of each ERB tag to separate statements | | `comments` | Boolean | `false` | Include ERB comments (`<%# %>`) in the output | -| `preserve_positions` | Boolean | `true` | Maintain character positions by padding with whitespace | +| `preserve_positions` | Boolean | `true` | Preserve Unicode character positions, newlines, and Ruby content | ::: code-group ```ruby [Ruby] diff --git a/java/org/herb/HerbTest.java b/java/org/herb/HerbTest.java index 05d5b92c6..e45a994fd 100644 --- a/java/org/herb/HerbTest.java +++ b/java/org/herb/HerbTest.java @@ -79,6 +79,19 @@ void testExtractRuby() { assertTrue(ruby.contains("foo")); } + @Test + void testExtractRubyPreservesUnicodeCharacterPositions() { + String source = "

Café <%= x=1 %>

"; + String ruby = Herb.extractRuby(source); + + assertEquals(" x=1 ; ", ruby); + assertEquals(source.codePointCount(0, source.length()), ruby.codePointCount(0, ruby.length())); + assertEquals(source.indexOf("x"), ruby.indexOf("x")); + + String compact = Herb.extractRuby(source, ExtractRubyOptions.create().preservePositions(false)); + assertEquals(" x=1 ", compact); + } + @Test void testExtractHTML() { String html = Herb.extractHTML("
<%= foo %>
"); diff --git a/javascript/packages/node-wasm/test/node-wasm.test.ts b/javascript/packages/node-wasm/test/node-wasm.test.ts index 801c4d406..24d66665f 100644 --- a/javascript/packages/node-wasm/test/node-wasm.test.ts +++ b/javascript/packages/node-wasm/test/node-wasm.test.ts @@ -40,6 +40,16 @@ describe("@herb-tools/node-wasm", () => { expect(ruby).toBe(' "Hello World" ; ') }) + test("extractRuby() preserves Unicode character positions", () => { + const source = "

Café <%= x=1 %>

" + const ruby = Herb.extractRuby(source) + + expect(ruby).toBe(" x=1 ; ") + expect(Array.from(ruby)).toHaveLength(Array.from(source).length) + expect(ruby.indexOf("x")).toBe(source.indexOf("x")) + expect(Herb.extractRuby(source, { preserve_positions: false })).toBe(" x=1 ") + }) + test("extractRuby() with semicolons: false", async () => { const source = "<% x = 1 %> <% y = 2 %>" const ruby = Herb.extractRuby(source, { semicolons: false }) diff --git a/javascript/packages/node/test/node.test.ts b/javascript/packages/node/test/node.test.ts index 72546cade..8d5eed687 100644 --- a/javascript/packages/node/test/node.test.ts +++ b/javascript/packages/node/test/node.test.ts @@ -38,6 +38,16 @@ describe("@herb-tools/node", () => { expect(ruby).toBe(' "Hello World" ; ') }) + test("extractRuby() preserves Unicode character positions", () => { + const source = "

Café <%= x=1 %>

" + const ruby = Herb.extractRuby(source) + + expect(ruby).toBe(" x=1 ; ") + expect(Array.from(ruby)).toHaveLength(Array.from(source).length) + expect(ruby.indexOf("x")).toBe(source.indexOf("x")) + expect(Herb.extractRuby(source, { preserve_positions: false })).toBe(" x=1 ") + }) + test("extractRuby() with semicolons: false", async () => { const source = "<% x = 1 %> <% y = 2 %>" const ruby = Herb.extractRuby(source, { semicolons: false }) diff --git a/lib/herb/ruby_program.rb b/lib/herb/ruby_program.rb index 33e625105..ecc923cf9 100644 --- a/lib/herb/ruby_program.rb +++ b/lib/herb/ruby_program.rb @@ -5,8 +5,8 @@ module Herb # Answers what Ruby a span of a template holds, out of the one Prism program the parser built - # for the whole document. `extract_ruby` blanks the HTML in place, so a Prism byte offset and a - # template byte offset are the same number, which is what lets a `Herb::Location` address Ruby. + # for the whole document. The parser uses internal byte-preserving extraction, so a Prism byte + # offset and a template byte offset are the same number, which lets a `Herb::Location` address Ruby. # # program = Herb::RubyProgram.for(document) # program.resolve(node.content.location)&.nodes #=> [#] diff --git a/rust/tests/extract_ruby_options_test.rs b/rust/tests/extract_ruby_options_test.rs index 18c29f3b0..fbc83d750 100644 --- a/rust/tests/extract_ruby_options_test.rs +++ b/rust/tests/extract_ruby_options_test.rs @@ -7,6 +7,24 @@ fn test_extract_ruby_default() { assert_eq!(result, " x = 1 ; y = 2 ;"); } +#[test] +fn test_extract_ruby_preserves_unicode_character_positions() { + let source = "

Café <%= x=1 %>

"; + let result = extract_ruby(source).unwrap(); + let source_column = source[..source.find('x').unwrap()].chars().count(); + let result_column = result[..result.find('x').unwrap()].chars().count(); + + assert_eq!(result, " x=1 ; "); + assert_eq!(result.chars().count(), source.chars().count()); + assert_eq!(result_column, source_column); + + let options = ExtractRubyOptions { + preserve_positions: false, + ..Default::default() + }; + assert_eq!(extract_ruby_with_options(source, &options).unwrap(), " x=1 "); +} + #[test] fn test_extract_ruby_without_semicolons() { let source = "<% x = 1 %> <% y = 2 %>"; diff --git a/src/analyze/analyze.c b/src/analyze/analyze.c index d8a7595c6..21bb13ed5 100644 --- a/src/analyze/analyze.c +++ b/src/analyze/analyze.c @@ -18,6 +18,7 @@ #include "../include/ast/ast_nodes.h" #include "../include/errors.h" #include "../include/extract.h" +#include "../include/extract_internal.h" #include "../include/lexer/token_struct.h" #include "../include/lib/hb_array.h" #include "../include/lib/hb_buffer.h" @@ -1180,7 +1181,7 @@ static tag_helper_scope_T* tag_helper_scope_init(const char* source, hb_allocato .preserve_positions = true, }; - herb_extract_ruby_to_buffer_with_options(source, &scope->buffer, &extract_options, allocator); + herb_extract_ruby_to_buffer_with_options_preserving_bytes(source, &scope->buffer, &extract_options, allocator); if (!scope->buffer.value || scope->buffer.length == 0) { hb_buffer_free(&scope->buffer); diff --git a/src/analyze/parse_errors.c b/src/analyze/parse_errors.c index a5c8ef368..933d60e0c 100644 --- a/src/analyze/parse_errors.c +++ b/src/analyze/parse_errors.c @@ -3,6 +3,7 @@ #include "../include/ast/ast_nodes.h" #include "../include/errors.h" #include "../include/extract.h" +#include "../include/extract_internal.h" #include "../include/lib/hb_allocator.h" #include "../include/lib/hb_string.h" #include "../include/lib/string.h" @@ -83,14 +84,25 @@ void herb_analyze_parse_errors( const parser_options_T* parser_options, hb_allocator_T* allocator ) { - char* extracted_ruby = herb_extract_ruby_with_semicolons_and_openers( + hb_buffer_T extracted_ruby_buffer; + if (!hb_buffer_init(&extracted_ruby_buffer, strlen(source), allocator)) { return; } + + herb_extract_ruby_options_T extract_options = HERB_EXTRACT_RUBY_DEFAULT_OPTIONS; + extract_options.erb_openers = parser_options ? parser_options->erb_openers : NULL; + extract_options.erb_opener_count = parser_options ? parser_options->erb_opener_count : 0; + + herb_extract_ruby_to_buffer_with_options_preserving_bytes( source, - parser_options ? parser_options->erb_openers : NULL, - parser_options ? parser_options->erb_opener_count : 0, + &extracted_ruby_buffer, + &extract_options, allocator ); - if (!extracted_ruby) { return; } + char* extracted_ruby = extracted_ruby_buffer.value; + if (!extracted_ruby) { + hb_buffer_free(&extracted_ruby_buffer); + return; + } bool strict_locals_enabled = parser_options && parser_options->strict_locals; bool has_anonymous_keyword_rest = strict_locals_enabled && document_has_anonymous_keyword_rest(document); @@ -127,5 +139,5 @@ void herb_analyze_parse_errors( pm_node_destroy(&parser, root); pm_parser_free(&parser); pm_options_free(&options); - hb_allocator_dealloc(allocator, extracted_ruby); + hb_buffer_free(&extracted_ruby_buffer); } diff --git a/src/analyze/prism_annotate.c b/src/analyze/prism_annotate.c index 290b37df7..ce1fe4674 100644 --- a/src/analyze/prism_annotate.c +++ b/src/analyze/prism_annotate.c @@ -2,6 +2,7 @@ #include "../include/ast/ast_node.h" #include "../include/ast/ast_nodes.h" #include "../include/extract.h" +#include "../include/extract_internal.h" #include "../include/lib/hb_allocator.h" #include "../include/lib/hb_buffer.h" #include "../include/lib/hb_narray.h" @@ -301,7 +302,7 @@ void herb_annotate_prism_nodes( .erb_opener_count = options->erb_opener_count, }; - herb_extract_ruby_to_buffer_with_options(source, &context->ruby_buf, &extract_options, allocator); + herb_extract_ruby_to_buffer_with_options_preserving_bytes(source, &context->ruby_buf, &extract_options, allocator); if (!context->ruby_buf.value || context->ruby_buf.length == 0) { hb_buffer_free(&context->ruby_buf); diff --git a/src/extract.c b/src/extract.c index 996b9182b..2f8bcaf23 100644 --- a/src/extract.c +++ b/src/extract.c @@ -1,3 +1,4 @@ +#include "include/extract_internal.h" #include "include/herb.h" #include "include/lexer/lexer.h" #include "include/lexer/token.h" @@ -6,6 +7,7 @@ #include "include/lib/hb_buffer.h" #include "include/lib/hb_string.h" #include "include/lib/string.h" +#include "include/util/utf8.h" #include "include/util/util.h" #include @@ -26,22 +28,48 @@ typedef struct { bool is_comment_tag; bool is_erb_comment_tag; bool need_newline; + bool preserve_byte_positions; + const char* source; } extract_ruby_state_T; +static void extract_ruby_mask_data(extract_ruby_state_T* state, const char* data, uint32_t length) { + uint32_t position = 0; + + while (position < length) { + if (is_newline(data[position])) { + hb_buffer_append_char(state->output, data[position]); + state->need_newline = false; + position++; + continue; + } + + uint32_t byte_length = 1; + + if (!state->preserve_byte_positions) { + byte_length = utf8_sequence_length(hb_string_from_data(data + position, length - position)); + } + + hb_buffer_append_char(state->output, ' '); + position += byte_length; + } +} + +static void extract_ruby_mask_range(extract_ruby_state_T* state, range_T range) { + extract_ruby_mask_data(state, state->source + range.from, range_length(range)); +} + static void extract_ruby_data(extract_ruby_state_T* state, const char* data, uint32_t from, uint32_t to) { - uint32_t run_start = from; + if (state->options.preserve_positions) { + extract_ruby_mask_data(state, data + from, to - from); + return; + } for (uint32_t position = from; position < to; position++) { if (!is_newline(data[position])) { continue; } - if (state->options.preserve_positions) { hb_buffer_append_whitespace(state->output, position - run_start); } - hb_buffer_append_char(state->output, data[position]); state->need_newline = false; - run_start = position + 1; } - - if (state->options.preserve_positions) { hb_buffer_append_whitespace(state->output, to - run_start); } } static void extract_ruby_token(extract_ruby_state_T* state, const token_T* token, const token_T* next) { @@ -83,23 +111,19 @@ static void extract_ruby_token(extract_ruby_state_T* state, const token_T* token } else { state->skip_erb_content = true; state->is_comment_tag = true; - if (state->options.preserve_positions) { - hb_buffer_append_whitespace(state->output, range_length(token->range)); - } + if (state->options.preserve_positions) { extract_ruby_mask_range(state, token->range); } } } else if (hb_string_equals(token->value, hb_string("<%%")) || hb_string_equals(token->value, hb_string("<%%=")) || (erb_opening_is_custom(token->value) && !state->options.custom_tags)) { state->skip_erb_content = true; state->is_comment_tag = false; - if (state->options.preserve_positions) { - hb_buffer_append_whitespace(state->output, range_length(token->range)); - } + if (state->options.preserve_positions) { extract_ruby_mask_range(state, token->range); } } else { state->skip_erb_content = false; state->is_comment_tag = false; if (state->options.preserve_positions) { - hb_buffer_append_whitespace(state->output, range_length(token->range)); + extract_ruby_mask_range(state, token->range); } else if (state->need_newline) { hb_buffer_append_char(state->output, '\n'); state->need_newline = false; @@ -124,9 +148,7 @@ static void extract_ruby_token(extract_ruby_state_T* state, const token_T* token } if (is_inline_comment) { - if (state->options.preserve_positions) { - hb_buffer_append_whitespace(state->output, range_length(token->range)); - } + if (state->options.preserve_positions) { extract_ruby_mask_range(state, token->range); } } else if (state->is_erb_comment_tag && !hb_string_is_null(token->value)) { const char* content = token->value.data; size_t content_remaining = token->value.length; @@ -158,21 +180,9 @@ static void extract_ruby_token(extract_ruby_state_T* state, const token_T* token } } else { if (state->is_erb_comment_tag && state->options.preserve_positions && !hb_string_is_null(token->value)) { - const char* content = token->value.data; - size_t content_remaining = token->value.length; - - while (content_remaining > 0) { - if (*content == '\n') { - hb_buffer_append_char(state->output, '\n'); - } else { - hb_buffer_append_char(state->output, ' '); - } - - content++; - content_remaining--; - } + extract_ruby_mask_range(state, token->range); } else if (state->options.preserve_positions) { - hb_buffer_append_whitespace(state->output, range_length(token->range)); + extract_ruby_mask_range(state, token->range); } } @@ -188,9 +198,9 @@ static void extract_ruby_token(extract_ruby_state_T* state, const token_T* token if (state->options.preserve_positions) { if (was_comment) { - hb_buffer_append_whitespace(state->output, range_length(token->range)); + extract_ruby_mask_range(state, token->range); } else if (was_erb_comment && state->options.comments) { - hb_buffer_append_whitespace(state->output, range_length(token->range)); + extract_ruby_mask_range(state, token->range); } else if (state->options.semicolons) { size_t length = range_length(token->range); @@ -198,7 +208,7 @@ static void extract_ruby_token(extract_ruby_state_T* state, const token_T* token if (length >= 1) { hb_buffer_append_char(state->output, ';'); } if (length >= 2) { hb_buffer_append_whitespace(state->output, length - 2); } } else { - hb_buffer_append_whitespace(state->output, range_length(token->range)); + extract_ruby_mask_range(state, token->range); } } @@ -206,15 +216,16 @@ static void extract_ruby_token(extract_ruby_state_T* state, const token_T* token } default: { - if (state->options.preserve_positions) { hb_buffer_append_whitespace(state->output, range_length(token->range)); } + if (state->options.preserve_positions) { extract_ruby_mask_range(state, token->range); } } } } -void herb_extract_ruby_to_buffer_with_options( +static void herb_extract_ruby_to_buffer_internal( const char* source, hb_buffer_T* output, const herb_extract_ruby_options_T* options, + bool preserve_byte_positions, hb_allocator_T* allocator ) { extract_ruby_state_T state = { @@ -224,6 +235,8 @@ void herb_extract_ruby_to_buffer_with_options( .is_comment_tag = false, .is_erb_comment_tag = false, .need_newline = false, + .preserve_byte_positions = preserve_byte_positions, + .source = NULL, }; parser_options_T lex_options = HERB_DEFAULT_PARSER_OPTIONS; @@ -237,6 +250,7 @@ void herb_extract_ruby_to_buffer_with_options( const char* data = lexer.source.data; uint32_t length = (uint32_t) lexer.source.length; token_T* pending = NULL; + state.source = data; while (true) { if (!pending && lexer.state == STATE_DATA) { @@ -265,6 +279,24 @@ void herb_extract_ruby_to_buffer_with_options( } } +void herb_extract_ruby_to_buffer_with_options( + const char* source, + hb_buffer_T* output, + const herb_extract_ruby_options_T* options, + hb_allocator_T* allocator +) { + herb_extract_ruby_to_buffer_internal(source, output, options, false, allocator); +} + +void herb_extract_ruby_to_buffer_with_options_preserving_bytes( + const char* source, + hb_buffer_T* output, + const herb_extract_ruby_options_T* options, + hb_allocator_T* allocator +) { + herb_extract_ruby_to_buffer_internal(source, output, options, true, allocator); +} + void herb_extract_ruby_to_buffer(const char* source, hb_buffer_T* output, hb_allocator_T* allocator) { herb_extract_ruby_to_buffer_with_options(source, output, NULL, allocator); } diff --git a/src/include/extract.h b/src/include/extract.h index 2d13cb49c..929088c5c 100644 --- a/src/include/extract.h +++ b/src/include/extract.h @@ -15,6 +15,7 @@ typedef enum { typedef struct { bool semicolons; bool comments; + // Preserve Unicode character positions, newlines, and Ruby content. bool preserve_positions; bool custom_tags; const hb_string_T* erb_openers; diff --git a/src/include/extract_internal.h b/src/include/extract_internal.h new file mode 100644 index 000000000..31be294af --- /dev/null +++ b/src/include/extract_internal.h @@ -0,0 +1,13 @@ +#ifndef HERB_EXTRACT_INTERNAL_H +#define HERB_EXTRACT_INTERNAL_H + +#include "extract.h" + +void herb_extract_ruby_to_buffer_with_options_preserving_bytes( + const char* source, + hb_buffer_T* output, + const herb_extract_ruby_options_T* options, + hb_allocator_T* allocator +); + +#endif diff --git a/test/c/test_extract.c b/test/c/test_extract.c index a1fd25295..85ffcb671 100644 --- a/test/c/test_extract.c +++ b/test/c/test_extract.c @@ -325,6 +325,35 @@ TEST(extract_ruby_with_options_default) hb_buffer_free(&output); END +TEST(extract_ruby_preserves_unicode_character_positions) + char* source = "

Café <%= x=1 %>

"; + + hb_buffer_T output; + hb_allocator_T malloc_alloc = hb_allocator_with_malloc(); + hb_buffer_init(&output, strlen(source), &malloc_alloc); + + hb_allocator_T allocator; + hb_allocator_init(&allocator, HB_ALLOCATOR_ARENA); + + herb_extract_ruby_to_buffer_with_options(source, &output, &HERB_EXTRACT_RUBY_DEFAULT_OPTIONS, &allocator); + + ck_assert_str_eq(output.value, " x=1 ; "); + ck_assert_uint_eq(strlen(output.value), 22); + ck_assert_uint_eq((size_t) (strchr(output.value, 'x') - output.value), 12); + + hb_buffer_free(&output); + hb_buffer_init(&output, strlen(source), &malloc_alloc); + + herb_extract_ruby_options_T options = HERB_EXTRACT_RUBY_DEFAULT_OPTIONS; + options.preserve_positions = false; + herb_extract_ruby_to_buffer_with_options(source, &output, &options, &allocator); + + ck_assert_str_eq(output.value, " x=1 "); + + hb_allocator_destroy(&allocator); + hb_buffer_free(&output); +END + TCase *extract_tests(void) { TCase *extract = tcase_create("Extract"); @@ -348,6 +377,7 @@ TCase *extract_tests(void) { tcase_add_test(extract, extract_ruby_with_options_preserve_positions_false); tcase_add_test(extract, extract_ruby_with_options_preserve_positions_false_and_comments_true); tcase_add_test(extract, extract_ruby_with_options_default); + tcase_add_test(extract, extract_ruby_preserves_unicode_character_positions); return extract; } diff --git a/test/extractor/extract_ruby_test.rb b/test/extractor/extract_ruby_test.rb index 4aa196c5c..9ed323577 100644 --- a/test/extractor/extract_ruby_test.rb +++ b/test/extractor/extract_ruby_test.rb @@ -26,6 +26,16 @@ class ExtractRubyTest < Minitest::Spec assert_equal " \n RUBY_VERSION ;\n \n", actual end + test "preserves Unicode character positions" do + source = "

Café <%= x=1 %>

" + ruby = Herb.extract_ruby(source) + + assert_equal " x=1 ; ", ruby + assert_equal source.length, ruby.length + assert_equal source.index("x"), ruby.index("x") + assert_equal " x=1 ", Herb.extract_ruby(source, preserve_positions: false) + end + test "nested" do actual = Herb.extract_ruby(<<~HTML) <% array = [1, 2, 3] %> diff --git a/test/ruby_program_test.rb b/test/ruby_program_test.rb index 5f2c4bccc..18e2954f0 100644 --- a/test/ruby_program_test.rb +++ b/test/ruby_program_test.rb @@ -77,6 +77,15 @@ def resolved_sources(template) assert_equal template.byteindex(" draft"), resolved&.offset end + test "keeps internal Prism byte offsets aligned after multibyte template text" do + template = %(

Café

<%= user.name %>) + program = program_for(template) + resolved = program&.resolve(erb_tokens(template).fetch(0).location) + + assert_equal " user.name ", resolved&.source + assert_equal template.byteindex(" user.name"), resolved&.offset + end + test "answers nothing when the document carries no program" do document = Herb.parse(%(

<%= @a %>

)).value From 2cd22f2a19c7226e77b1009b1842da471176cb79 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Tue, 6 Oct 2026 11:45:23 -0600 Subject: [PATCH 12/18] Remove extract option comment Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/include/extract.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/include/extract.h b/src/include/extract.h index 929088c5c..2d13cb49c 100644 --- a/src/include/extract.h +++ b/src/include/extract.h @@ -15,7 +15,6 @@ typedef enum { typedef struct { bool semicolons; bool comments; - // Preserve Unicode character positions, newlines, and Ruby content. bool preserve_positions; bool custom_tags; const hb_string_T* erb_openers; From 92cb2687d4dc86e7c9ceff852ca252d72d4bf02f Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Tue, 6 Oct 2026 11:49:43 -0600 Subject: [PATCH 13/18] Regenerate RubyProgram signature Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- sig/herb/ruby_program.rbs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sig/herb/ruby_program.rbs b/sig/herb/ruby_program.rbs index 54cd5a31c..c628bcbcc 100644 --- a/sig/herb/ruby_program.rbs +++ b/sig/herb/ruby_program.rbs @@ -2,8 +2,8 @@ module Herb # Answers what Ruby a span of a template holds, out of the one Prism program the parser built - # for the whole document. `extract_ruby` blanks the HTML in place, so a Prism byte offset and a - # template byte offset are the same number, which is what lets a `Herb::Location` address Ruby. + # for the whole document. The parser uses internal byte-preserving extraction, so a Prism byte + # offset and a template byte offset are the same number, which lets a `Herb::Location` address Ruby. # # program = Herb::RubyProgram.for(document) # program.resolve(node.content.location)&.nodes #=> [#] From abf9d5a2027271fd67a520c27c77f56d780b5a34 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Tue, 6 Oct 2026 13:16:46 -0600 Subject: [PATCH 14/18] Retry CI Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> From 60646f4397afc70159e195e404fec5ad8b27ccd9 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Wed, 21 Oct 2026 13:21:01 -0600 Subject: [PATCH 15/18] Update Gemfile.lock for rubocop-herb 0.11.1 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 62861432-cba5-4e6b-b0fe-fad60ea33328 --- Gemfile.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 68bfd9f93..c39ba2f81 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -24,8 +24,8 @@ PATH PATH remote: gems/rubocop-herb specs: - rubocop-herb (0.11.0) - herb (= 0.11.0) + rubocop-herb (0.11.1) + herb (= 0.11.1) lint_roller (~> 1.1) rubocop (>= 1.72.1, < 2) From bf9cc7099801bedf3bd3cffc7951aea50e7048a4 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Wed, 21 Oct 2026 14:14:32 -0600 Subject: [PATCH 16/18] Use Herb.extract_ruby directly in RuboCop plugin Herb.extract_ruby now preserves Unicode character positions, so the plugin no longer needs its own position-preserving workaround. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 62861432-cba5-4e6b-b0fe-fad60ea33328 --- gems/rubocop-herb/lib/rubocop/herb/plugin.rb | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/gems/rubocop-herb/lib/rubocop/herb/plugin.rb b/gems/rubocop-herb/lib/rubocop/herb/plugin.rb index cf0cbd281..94d23164c 100644 --- a/gems/rubocop-herb/lib/rubocop/herb/plugin.rb +++ b/gems/rubocop-herb/lib/rubocop/herb/plugin.rb @@ -31,8 +31,7 @@ def supported?(context) def self.extract_ruby(processed_source) return unless processed_source.path&.end_with?(".erb", ".herb") - template = processed_source.raw_source - code = position_preserving_ruby(template) + code = ::Herb.extract_ruby(processed_source.raw_source) return [] if code.strip.empty? source = build_processed_source(code, original: processed_source) @@ -53,20 +52,7 @@ def self.build_processed_source(code, original:) end end - def self.position_preserving_ruby(template) - extracted = ::Herb.extract_ruby(template) - return extracted if extracted.length == template.length - - byte_offset = 0 - template.each_char.map do |character| - extracted_character = extracted.byteslice(byte_offset, character.bytesize) - byte_offset += character.bytesize - - extracted_character == character ? character : extracted_character.each_char.first - end.join - end - - private_class_method :build_processed_source, :position_preserving_ruby + private_class_method :build_processed_source EXTRACT_RUBY = method(:extract_ruby) end From 7a8b82f4e9c145d119d6866417f2fe74bce02158 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Wed, 21 Oct 2026 14:36:21 -0600 Subject: [PATCH 17/18] Address rubocop-herb review findings - Revert extract_ruby Unicode position change; align positions in the plugin - Drop offenses anchored in markup and corrections that would edit markup - Register the extractor once; single source of truth for template extensions - Remove nonexistent Layout/LeadingEmptyLine, exclude cops with ERB false positives - Rewrite tests (default config, markup protection, cop existence) and run them with rake test Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e554b8ed-a44a-4452-9ccf-06e6e96101dc --- Rakefile | 2 + docs/docs/bindings/extracting.md | 4 +- gems/rubocop-herb/README.md | 12 ++- gems/rubocop-herb/config/default.yml | 45 ++++++-- gems/rubocop-herb/lib/rubocop/herb.rb | 4 + .../lib/rubocop/herb/offense_guard.rb | 22 ++++ gems/rubocop-herb/lib/rubocop/herb/plugin.rb | 24 +---- .../lib/rubocop/herb/template_source.rb | 82 ++++++++++++++ gems/rubocop-herb/test/integration_test.rb | 95 ++++++++++++---- .../test/plugin_extraction_test.rb | 51 ++++----- gems/rubocop-herb/test/plugin_test.rb | 33 +++--- gems/rubocop-herb/test/test_helper.rb | 23 ++-- java/org/herb/HerbTest.java | 13 --- .../packages/node-wasm/test/node-wasm.test.ts | 10 -- javascript/packages/node/test/node.test.ts | 10 -- lib/herb/ruby_program.rb | 4 +- rust/tests/extract_ruby_options_test.rs | 18 ---- sig/herb/ruby_program.rbs | 4 +- src/analyze/analyze.c | 3 +- src/analyze/parse_errors.c | 22 +--- src/analyze/prism_annotate.c | 3 +- src/extract.c | 102 ++++++------------ src/include/extract_internal.h | 13 --- test/c/test_extract.c | 30 ------ test/extractor/extract_ruby_test.rb | 10 -- test/ruby_program_test.rb | 9 -- 26 files changed, 335 insertions(+), 313 deletions(-) create mode 100644 gems/rubocop-herb/lib/rubocop/herb/offense_guard.rb create mode 100644 gems/rubocop-herb/lib/rubocop/herb/template_source.rb delete mode 100644 src/include/extract_internal.h diff --git a/Rakefile b/Rakefile index 78f74dcbd..a4f1b2be8 100644 --- a/Rakefile +++ b/Rakefile @@ -34,6 +34,8 @@ Rake::TestTask.new("test:rubocop_herb") do |t| t.test_files = FileList["gems/rubocop-herb/test/**/*_test.rb"] end +task test: "test:rubocop_herb" + task "make" do puts "Running make..." IO.popen("make") do |output| diff --git a/docs/docs/bindings/extracting.md b/docs/docs/bindings/extracting.md index e93e4fdbc..4e9eb097a 100644 --- a/docs/docs/bindings/extracting.md +++ b/docs/docs/bindings/extracting.md @@ -38,7 +38,7 @@ let ruby = extract_ruby("

Hello <%= user.name %>

").unwrap(); ``` ::: -By default every Unicode character keeps the position it had in the template, with non-Ruby content replaced by one space per character. Newlines and Ruby content are retained. A Ruby error on line 12 column 30 of the extracted output is on line 12 column 30 of the template, so character-based positions need no translation. Because UTF-8 characters can contain multiple bytes, the extracted output can have a different byte length than the template. +By default every character keeps the position it had in the template, with the HTML replaced by spaces. A Ruby error on line 12 column 30 of the extracted output is on line 12 column 30 of the template, so positions need no translation. ## Options @@ -46,7 +46,7 @@ By default every Unicode character keeps the position it had in the template, wi |---|---|---|---| | `semicolons` | Boolean | `true` | Add ` ;` at the end of each ERB tag to separate statements | | `comments` | Boolean | `false` | Include ERB comments (`<%# %>`) in the output | -| `preserve_positions` | Boolean | `true` | Preserve Unicode character positions, newlines, and Ruby content | +| `preserve_positions` | Boolean | `true` | Maintain character positions by padding with whitespace | ::: code-group ```ruby [Ruby] diff --git a/gems/rubocop-herb/README.md b/gems/rubocop-herb/README.md index 812350d60..7daedc854 100644 --- a/gems/rubocop-herb/README.md +++ b/gems/rubocop-herb/README.md @@ -23,4 +23,14 @@ bundle exec rubocop -a app/views bundle exec rubocop -A app/views ``` -The plugin honors the project's RuboCop configuration, including custom cops, inherited configuration, target Ruby versions, file exclusions, and autocorrection. Cops that depend on physical Ruby file contents or layout are [excluded for template files by default](config/default.yml). +The plugin honors the project's RuboCop configuration, including custom cops, inherited configuration, target Ruby versions, file exclusions, and autocorrection. Cops that depend on physical Ruby file contents or layout, or that misread template control flow, are [excluded for template files by default](config/default.yml). + +Offenses are only reported for Ruby inside ERB tags, and autocorrections that would touch markup are discarded, so `rubocop -a` and `rubocop -A` never rewrite HTML or ERB delimiters. + +Every `.erb` and `.herb` file is inspected, including non-HTML templates. To skip some of them, exclude them in `.rubocop.yml`: + +```yaml +AllCops: + Exclude: + - "**/*.yml.erb" +``` diff --git a/gems/rubocop-herb/config/default.yml b/gems/rubocop-herb/config/default.yml index d2ee85c74..1a5130c22 100644 --- a/gems/rubocop-herb/config/default.yml +++ b/gems/rubocop-herb/config/default.yml @@ -8,19 +8,22 @@ AllCops: - '**/*.erb' - '**/*.herb' -# These cops depend on physical Ruby file contents or layout that is obscured -# by the position-preserving template representation. +# These cops depend on physical Ruby file contents or layout that is obscured by the +# position-preserving template representation, or misread markup between ERB tags as +# empty or missing Ruby, such as the markup inside a `do` block. Layout/ArgumentAlignment: Exclude: *template_files Layout/ArrayAlignment: Exclude: *template_files Layout/BlockAlignment: Exclude: *template_files +Layout/CaseIndentation: + Exclude: *template_files Layout/ClosingParenthesisIndentation: Exclude: *template_files Layout/CommentIndentation: Exclude: *template_files -Layout/InitialIndentation: +Layout/ElseAlignment: Exclude: *template_files Layout/EmptyLineAfterGuardClause: Exclude: *template_files @@ -46,7 +49,7 @@ Layout/IndentationConsistency: Exclude: *template_files Layout/IndentationWidth: Exclude: *template_files -Layout/LeadingEmptyLine: +Layout/InitialIndentation: Exclude: *template_files Layout/LeadingEmptyLines: Exclude: *template_files @@ -76,25 +79,45 @@ Layout/TrailingEmptyLines: Exclude: *template_files Layout/TrailingWhitespace: Exclude: *template_files -Lint/EmptyFile: +Lint/DuplicateBranch: + Exclude: *template_files +Lint/EmptyBlock: Exclude: *template_files Lint/EmptyConditionalBody: Exclude: *template_files +Lint/EmptyEnsure: + Exclude: *template_files +Lint/EmptyFile: + Exclude: *template_files +Lint/EmptyInPattern: + Exclude: *template_files +Lint/EmptyWhen: + Exclude: *template_files +Lint/SuppressedException: + Exclude: *template_files +Lint/Void: + Exclude: *template_files Metrics/BlockLength: Exclude: *template_files Naming/FileName: Exclude: *template_files Rails/Presence: Exclude: *template_files -Style/FrozenStringLiteralComment: +Style/BlockDelimiters: + Exclude: *template_files +Style/CombinableLoops: Exclude: *template_files Style/EmptyElse: Exclude: *template_files +Style/FrozenStringLiteralComment: + Exclude: *template_files Style/IdenticalConditionalBranches: Exclude: *template_files +Style/IfUnlessModifier: + Exclude: *template_files Style/IfWithSemicolon: Exclude: *template_files -Style/IfUnlessModifier: +Style/MultilineIfModifier: Exclude: *template_files Style/MultilineTernaryOperator: Exclude: *template_files @@ -102,7 +125,15 @@ Style/NestedTernaryOperator: Exclude: *template_files Style/Next: Exclude: *template_files +Style/RedundantCondition: + Exclude: *template_files +Style/SafeNavigation: + Exclude: *template_files Style/Semicolon: Exclude: *template_files +Style/SingleLineDoEndBlock: + Exclude: *template_files +Style/SoleNestedConditional: + Exclude: *template_files Style/WhileUntilModifier: Exclude: *template_files diff --git a/gems/rubocop-herb/lib/rubocop/herb.rb b/gems/rubocop-herb/lib/rubocop/herb.rb index 073986b9e..007a49180 100644 --- a/gems/rubocop-herb/lib/rubocop/herb.rb +++ b/gems/rubocop-herb/lib/rubocop/herb.rb @@ -3,4 +3,8 @@ require "herb" require "rubocop" +require_relative "herb/template_source" +require_relative "herb/offense_guard" require_relative "herb/plugin" + +RuboCop::Cop::Base.prepend(RuboCop::Herb::OffenseGuard) diff --git a/gems/rubocop-herb/lib/rubocop/herb/offense_guard.rb b/gems/rubocop-herb/lib/rubocop/herb/offense_guard.rb new file mode 100644 index 000000000..1494037ab --- /dev/null +++ b/gems/rubocop-herb/lib/rubocop/herb/offense_guard.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +module RuboCop + module Herb + # Drops offenses that point at markup and autocorrections that would edit it. + module OffenseGuard + def add_offense(node_or_range, message: nil, severity: nil, &block) + source = processed_source + return super unless source.is_a?(TemplateSource) && !is_a?(::RuboCop::Cop::Lint::Syntax) + return unless source.anchored_in_ruby?(range_from_node_or_range(node_or_range)) + return super unless block + + super do |corrector| + candidate = ::RuboCop::Cop::Corrector.new(corrector.source_buffer) + yield(candidate) + + corrector.merge!(candidate) if candidate.as_replacements.all? { |range, _| source.within_ruby?(range) } + end + end + end + end +end diff --git a/gems/rubocop-herb/lib/rubocop/herb/plugin.rb b/gems/rubocop-herb/lib/rubocop/herb/plugin.rb index 94d23164c..93758018f 100644 --- a/gems/rubocop-herb/lib/rubocop/herb/plugin.rb +++ b/gems/rubocop-herb/lib/rubocop/herb/plugin.rb @@ -15,7 +15,8 @@ def about end def rules(_context) - RuboCop::Runner.ruby_extractors.unshift(EXTRACT_RUBY) + extractors = RuboCop::Runner.ruby_extractors + extractors.unshift(EXTRACT_RUBY) unless extractors.include?(EXTRACT_RUBY) LintRoller::Rules.new( config_format: :rubocop, @@ -29,31 +30,14 @@ def supported?(context) end def self.extract_ruby(processed_source) - return unless processed_source.path&.end_with?(".erb", ".herb") + return unless TemplateSource.template?(processed_source.path) - code = ::Herb.extract_ruby(processed_source.raw_source) - return [] if code.strip.empty? - - source = build_processed_source(code, original: processed_source) + source = TemplateSource.from(processed_source) return [] if source.valid_syntax? && !source.ast [{ offset: 0, processed_source: source }] end - def self.build_processed_source(code, original:) - ::RuboCop::ProcessedSource.new( - code, - original.ruby_version, - original.path, - parser_engine: original.parser_engine - ).tap do |source| - source.config = original.config - source.registry = original.registry - end - end - - private_class_method :build_processed_source - EXTRACT_RUBY = method(:extract_ruby) end end diff --git a/gems/rubocop-herb/lib/rubocop/herb/template_source.rb b/gems/rubocop-herb/lib/rubocop/herb/template_source.rb new file mode 100644 index 000000000..a045aae69 --- /dev/null +++ b/gems/rubocop-herb/lib/rubocop/herb/template_source.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +module RuboCop + module Herb + # The Ruby of a template, aligned character for character with the template, plus the + # character ranges that hold the Ruby of each ERB tag (without surrounding whitespace). + class TemplateSource < ::RuboCop::ProcessedSource + EXTENSIONS = [".erb", ".herb"].freeze + + def self.template?(path) + path&.end_with?(*EXTENSIONS) + end + + def self.from(original) + template = original.raw_source + code = align(template, ::Herb.extract_ruby(template)) + + new(code, original.ruby_version, original.path, parser_engine: original.parser_engine).tap do |source| + source.config = original.config + source.registry = original.registry + source.ruby_ranges = ruby_ranges(template, code) + end + end + + # `Herb.extract_ruby` preserves byte positions; RuboCop addresses characters. Collapse each + # masked multibyte character to one space and keep every newline so lines stay aligned. + def self.align(template, extracted) + offset = 0 + + template.each_char.map { |char| + slice = extracted.byteslice(offset, char.bytesize) + offset += char.bytesize + + if char == "\n" || slice == char then char + elsif char.bytesize == 1 then slice + else " " + end + }.join + end + + # Ruby characters are the non-whitespace template characters that survive extraction. ERB + # delimiters never survive, so they split the template into one range per tag. + def self.ruby_ranges(template, code) + ranges = [] + start = finish = nil + + template.each_char.zip(code.each_char).each_with_index do |(original, extracted), index| + next if original.match?(/\s/) + + if original == extracted + start ||= index + finish = index + 1 + elsif start + ranges << (start...finish) + start = nil + end + end + + ranges << (start...finish) if start + ranges + end + + attr_accessor :ruby_ranges + + # Both ends of the range touch Ruby, so the offense points at code rather than at markup. + def anchored_in_ruby?(range) + touches_ruby?(range.begin_pos) && touches_ruby?(range.end_pos) + end + + # The range lies within the Ruby of a single tag, so editing it cannot alter markup. + def within_ruby?(range) + ruby_ranges.any? { |ruby| ruby.begin <= range.begin_pos && range.end_pos <= ruby.end } + end + + private + + def touches_ruby?(position) + ruby_ranges.any? { |ruby| position.between?(ruby.begin, ruby.end) } + end + end + end +end diff --git a/gems/rubocop-herb/test/integration_test.rb b/gems/rubocop-herb/test/integration_test.rb index b75c39bbb..aaff682b0 100644 --- a/gems/rubocop-herb/test/integration_test.rb +++ b/gems/rubocop-herb/test/integration_test.rb @@ -5,6 +5,33 @@ class RuboCopHerbIntegrationTest < Minitest::Spec include RuboCopHerbTestHelpers + CLEAN_TEMPLATE = <<~ERB +

<%= @title %>

+

Café <%= user.name %> and <%= count + 1 %>

+ <% if admin? %> + Admin + <% else %> + User + <% end %> +
    + <% items.each do |item| %> +
  • <%= item %>
    • + <% end %> +
+ <%= form_with model: @post do |form| %> + <%= form.text_field :title, class: 'title' %> + <% end %> + <%= link_to root_path do %>Home<% end %> + <% case status %> + <% when :open %> +

Open

+ <% when :closed %> +

Closed

+ <% end %> + <% value = compute %> +

<%= value %>

+ ERB + def setup @directory = Dir.mktmpdir("rubocop-herb") end @@ -39,9 +66,9 @@ def on_send(node) YAML write("example.html.erb", "

Café <%= foo(x=1) %>

\n") - output, error, status = run_rubocop("example.html.erb", chdir: @directory) + output, error, status = run_rubocop("example.html.erb") - refute status.success? + refute_equal 0, status assert_empty error assert_includes output, "Layout/SpaceAroundOperators" assert_includes output, "HerbTest/NoFoo" @@ -55,13 +82,45 @@ def on_send(node) YAML write("example.html.erb", "
Café <%= x=1 %>
\n") - _output, error, status = run_rubocop("-a", "example.html.erb", chdir: @directory) + _output, error, status = run_rubocop("-a", "example.html.erb") - assert status.success? + assert_equal 0, status assert_empty error assert_equal "
Café <%= x = 1 %>
\n", read("example.html.erb") end + test "reports no offenses on a clean template with the default configuration" do + write(".rubocop.yml", <<~YAML) + plugins: + - rubocop-herb + AllCops: + NewCops: enable + SuggestExtensions: false + YAML + write("example.html.erb", CLEAN_TEMPLATE) + + output, error, status = run_rubocop("example.html.erb") + + assert_equal 0, status, output + assert_empty error + + run_rubocop("-A", "example.html.erb") + + assert_equal CLEAN_TEMPLATE, read("example.html.erb") + end + + test "never autocorrects markup between ERB tags" do + write_config(<<~YAML) + Layout/ExtraSpacing: + Enabled: true + YAML + write("example.html.erb", "

Café

<%= a %> <%= b %>\n<%= c + d %>\n") + + run_rubocop("-A", "example.html.erb") + + assert_equal "

Café

<%= a %> <%= b %>\n<%= c + d %>\n", read("example.html.erb") + end + test "does not run cops excluded for ERB fragments" do write_config(<<~YAML) Style/FrozenStringLiteralComment: @@ -69,9 +128,9 @@ def on_send(node) YAML write("example.html.erb", "<%= user.name %>\n") - output, error, status = run_rubocop("example.html.erb", chdir: @directory) + output, error, status = run_rubocop("example.html.erb") - assert status.success?, output + assert_equal 0, status, output assert_empty error refute_includes output, "Style/FrozenStringLiteralComment" end @@ -85,9 +144,9 @@ def on_send(node) YAML write("example.html.erb", "<%= x=1 %>\n") - output, error, status = run_rubocop("example.html.erb", chdir: @directory) + output, error, status = run_rubocop("example.html.erb") - assert status.success?, output + assert_equal 0, status, output assert_empty error refute_includes output, "Layout/SpaceAroundOperators" end @@ -99,9 +158,9 @@ def on_send(node) YAML write("example.html.erb", "<% value = compute %>\n

<%= value %>

\n") - output, error, status = run_rubocop("example.html.erb", chdir: @directory) + output, error, status = run_rubocop("example.html.erb") - assert status.success?, output + assert_equal 0, status, output assert_empty error refute_includes output, "Lint/UselessAssignment" end @@ -113,9 +172,9 @@ def on_send(node) YAML write("example.html.erb", "<%= user. %>\n") - output, error, status = run_rubocop("example.html.erb", chdir: @directory) + output, error, status = run_rubocop("example.html.erb") - refute status.success? + refute_equal 0, status assert_empty error assert_includes output, "Lint/Syntax" assert_includes output, "example.html.erb:1" @@ -129,9 +188,9 @@ def on_send(node) write("example.rss.erb", "<%= rss=1 %>\n") write("example.erb", "<%= generic=1 %>\n") - output, error, status = run_rubocop(".", chdir: @directory) + output, error, status = run_rubocop(".") - refute status.success? + refute_equal 0, status assert_empty error assert_includes output, "example.rss.erb" assert_includes output, "example.erb" @@ -144,9 +203,9 @@ def on_send(node) YAML write("example.herb", "<%= herb=1 %>\n") - output, error, status = run_rubocop("example.herb", chdir: @directory) + output, error, status = run_rubocop("example.herb") - refute status.success? + refute_equal 0, status assert_empty error assert_includes output, "example.herb" assert_includes output, "Layout/SpaceAroundOperators" @@ -159,9 +218,9 @@ def on_send(node) YAML write("example.rb", "x=1\n") - output, error, status = run_rubocop("example.rb", chdir: @directory) + output, error, status = run_rubocop("example.rb") - refute status.success? + refute_equal 0, status assert_empty error assert_includes output, "Layout/SpaceAroundOperators" end diff --git a/gems/rubocop-herb/test/plugin_extraction_test.rb b/gems/rubocop-herb/test/plugin_extraction_test.rb index 483561e55..6a9beff6c 100644 --- a/gems/rubocop-herb/test/plugin_extraction_test.rb +++ b/gems/rubocop-herb/test/plugin_extraction_test.rb @@ -5,7 +5,7 @@ module RuboCop module Herb class PluginExtractionTest < Minitest::Spec - test "builds one position-preserving Ruby source" do + test "builds one character-aligned Ruby source" do template = "

Café <%= x=1 %>

\n" fragment = extract(template).first source = fragment[:processed_source].raw_source @@ -16,6 +16,15 @@ class PluginExtractionTest < Minitest::Spec assert fragment[:processed_source].valid_syntax? end + test "keeps lines aligned after multi-line escaped ERB" do + template = "<%% a\nb %%>\n

Ünï

<%= x %>\n" + source = extract(template).first[:processed_source].raw_source + + assert_equal template.length, source.length + assert_equal template.index("x"), source.index("x") + assert_equal template.count("\n"), source.count("\n") + end + test "preserves Ruby control flow across ERB tags" do fragment = extract(<<~ERB).first <% case status %> @@ -43,18 +52,17 @@ class PluginExtractionTest < Minitest::Spec assert_equal 2, source.ast.each_node(:send).count end - test "uses Herb's Ruby extraction binding" do - template = "<%= first %><%= second %>\n" - source = extract(template).first[:processed_source] + test "preserves multibyte characters in Ruby" do + source = extract("<%= name == \"José\" %>\n").first[:processed_source] - assert_equal ::Herb.extract_ruby(template), source.raw_source + assert_includes source.raw_source, "name == \"José\"" end - test "preserves multibyte characters in Ruby" do - template = "<%= name == \"José\" %>\n" + test "records the Ruby of each tag without surrounding whitespace" do + template = "

Café

<%= a + b %>\n<% if c %>\n<%# note %>\n" source = extract(template).first[:processed_source] - assert_includes source.raw_source, "name == \"José\"" + assert_equal(["a + b", "if c"], source.ruby_ranges.map { |range| template[range] }) end test "skips comments and escaped ERB" do @@ -66,31 +74,14 @@ class PluginExtractionTest < Minitest::Spec assert_empty fragments end - test "returns nil for non-ERB files" do + test "returns nil for non-template files" do assert_nil Plugin.extract_ruby(processed_source("x=1\n", "example.rb")) end - test "extracts Ruby from non-HTML ERB files" do - refute_empty Plugin.extract_ruby(processed_source("<%= x=1 %>\n", "example.rss.erb")) - refute_empty Plugin.extract_ruby(processed_source("<%= x=1 %>\n", "example.erb")) - end - - test "extracts Ruby from Herb files" do - refute_empty Plugin.extract_ruby(processed_source("<%= x=1 %>\n", "example.herb")) - end - - test "preserves processed source metadata" do - original = processed_source("<%= x=1 %>\n", "example.herb") - original.config = Object.new - original.registry = Object.new - - extracted = Plugin.extract_ruby(original).first[:processed_source] - - assert_equal original.path, extracted.path - assert_equal original.ruby_version, extracted.ruby_version - assert_equal original.parser_engine, extracted.parser_engine - assert_same original.config, extracted.config - assert_same original.registry, extracted.registry + test "extracts Ruby from every template extension" do + ["example.rss.erb", "example.erb", "example.herb"].each do |path| + refute_empty Plugin.extract_ruby(processed_source("<%= x=1 %>\n", path)) + end end private diff --git a/gems/rubocop-herb/test/plugin_test.rb b/gems/rubocop-herb/test/plugin_test.rb index 79fd27b7e..896e5c127 100644 --- a/gems/rubocop-herb/test/plugin_test.rb +++ b/gems/rubocop-herb/test/plugin_test.rb @@ -6,26 +6,31 @@ module RuboCop module Herb class PluginTest < Minitest::Spec - test "registers the extractor" do + test "registers the extractor once" do extractors = ::RuboCop::Runner.ruby_extractors - extractor = Plugin::EXTRACT_RUBY - extractors.delete(extractor) - plugin = Plugin.new - context = LintRoller::Context.new(engine: :rubocop, engine_version: "1.84.0") + extractors.delete(Plugin::EXTRACT_RUBY) + context = LintRoller::Context.new(engine: :rubocop, engine_version: ::RuboCop::Version::STRING) - plugin.rules(context) + 2.times { Plugin.new.rules(context) } - assert_equal extractor, extractors.first - ensure - extractors&.delete(extractor) + assert_equal Plugin::EXTRACT_RUBY, extractors.first + assert_equal 1, extractors.count(Plugin::EXTRACT_RUBY) end - test "excludes cops that misinterpret HTML conditional bodies" do - config = YAML.load_file(File.expand_path("../config/default.yml", __dir__), aliases: true) - template_files = ["**/*.erb", "**/*.herb"] + test "includes exactly the template extensions the extractor handles" do + assert_equal(TemplateSource::EXTENSIONS.map { |extension| "**/*#{extension}" }, config.dig("AllCops", "Include")) + end + + test "excludes only cops that exist" do + cops = config.keys.grep(%r{/}).reject { |name| name.start_with?("Rails/") } + + assert_empty(cops.reject { |name| ::RuboCop::Cop::Registry.global.find_by_cop_name(name) }) + end + + private - assert_equal template_files, config.dig("Lint/EmptyConditionalBody", "Exclude") - assert_equal template_files, config.dig("Rails/Presence", "Exclude") + def config + YAML.load_file(File.expand_path("../config/default.yml", __dir__), aliases: true) end end end diff --git a/gems/rubocop-herb/test/test_helper.rb b/gems/rubocop-herb/test/test_helper.rb index ab33de74a..c1ad3d8fa 100644 --- a/gems/rubocop-herb/test/test_helper.rb +++ b/gems/rubocop-herb/test/test_helper.rb @@ -1,13 +1,10 @@ # frozen_string_literal: true -root = File.expand_path("../../..", __dir__) -$LOAD_PATH.unshift File.join(root, "lib") +$LOAD_PATH.unshift File.expand_path("../../../lib", __dir__) $LOAD_PATH.unshift File.expand_path("../lib", __dir__) -require "herb" require "fileutils" require "maxitest/autorun" -require "minitest/spec" require "open3" require "rubocop/herb" require "tmpdir" @@ -15,18 +12,12 @@ Minitest::Spec::DSL.send(:alias_method, :test, :it) module RuboCopHerbTestHelpers - ROOT = File.expand_path("../../..", __dir__) + LOAD_PATHS = [File.expand_path("../../../lib", __dir__), File.expand_path("../lib", __dir__)].freeze - def run_rubocop(*, chdir:) - Open3.capture3( - { "BUNDLE_GEMFILE" => File.join(ROOT, "Gemfile") }, - RbConfig.ruby, - "-S", - "bundle", - "exec", - "rubocop", - *, - chdir: - ) + def run_rubocop(*args) + command = [Gem.ruby, *LOAD_PATHS.map { |path| "-I#{path}" }, Gem.bin_path("rubocop", "rubocop"), "--cache", "false", *args] + output, error, status = Open3.capture3(*command, chdir: @directory) + + [output, error, status.exitstatus] end end diff --git a/java/org/herb/HerbTest.java b/java/org/herb/HerbTest.java index e45a994fd..05d5b92c6 100644 --- a/java/org/herb/HerbTest.java +++ b/java/org/herb/HerbTest.java @@ -79,19 +79,6 @@ void testExtractRuby() { assertTrue(ruby.contains("foo")); } - @Test - void testExtractRubyPreservesUnicodeCharacterPositions() { - String source = "

Café <%= x=1 %>

"; - String ruby = Herb.extractRuby(source); - - assertEquals(" x=1 ; ", ruby); - assertEquals(source.codePointCount(0, source.length()), ruby.codePointCount(0, ruby.length())); - assertEquals(source.indexOf("x"), ruby.indexOf("x")); - - String compact = Herb.extractRuby(source, ExtractRubyOptions.create().preservePositions(false)); - assertEquals(" x=1 ", compact); - } - @Test void testExtractHTML() { String html = Herb.extractHTML("
<%= foo %>
"); diff --git a/javascript/packages/node-wasm/test/node-wasm.test.ts b/javascript/packages/node-wasm/test/node-wasm.test.ts index 2ad29e37f..3e154e35f 100644 --- a/javascript/packages/node-wasm/test/node-wasm.test.ts +++ b/javascript/packages/node-wasm/test/node-wasm.test.ts @@ -40,16 +40,6 @@ describe("@herb-tools/node-wasm", () => { expect(ruby).toBe(' "Hello World" ; ') }) - test("extractRuby() preserves Unicode character positions", () => { - const source = "

Café <%= x=1 %>

" - const ruby = Herb.extractRuby(source) - - expect(ruby).toBe(" x=1 ; ") - expect(Array.from(ruby)).toHaveLength(Array.from(source).length) - expect(ruby.indexOf("x")).toBe(source.indexOf("x")) - expect(Herb.extractRuby(source, { preserve_positions: false })).toBe(" x=1 ") - }) - test("extractRuby() with semicolons: false", async () => { const source = "<% x = 1 %> <% y = 2 %>" const ruby = Herb.extractRuby(source, { semicolons: false }) diff --git a/javascript/packages/node/test/node.test.ts b/javascript/packages/node/test/node.test.ts index 38ba49902..38028b0f7 100644 --- a/javascript/packages/node/test/node.test.ts +++ b/javascript/packages/node/test/node.test.ts @@ -38,16 +38,6 @@ describe("@herb-tools/node", () => { expect(ruby).toBe(' "Hello World" ; ') }) - test("extractRuby() preserves Unicode character positions", () => { - const source = "

Café <%= x=1 %>

" - const ruby = Herb.extractRuby(source) - - expect(ruby).toBe(" x=1 ; ") - expect(Array.from(ruby)).toHaveLength(Array.from(source).length) - expect(ruby.indexOf("x")).toBe(source.indexOf("x")) - expect(Herb.extractRuby(source, { preserve_positions: false })).toBe(" x=1 ") - }) - test("extractRuby() with semicolons: false", async () => { const source = "<% x = 1 %> <% y = 2 %>" const ruby = Herb.extractRuby(source, { semicolons: false }) diff --git a/lib/herb/ruby_program.rb b/lib/herb/ruby_program.rb index ecc923cf9..33e625105 100644 --- a/lib/herb/ruby_program.rb +++ b/lib/herb/ruby_program.rb @@ -5,8 +5,8 @@ module Herb # Answers what Ruby a span of a template holds, out of the one Prism program the parser built - # for the whole document. The parser uses internal byte-preserving extraction, so a Prism byte - # offset and a template byte offset are the same number, which lets a `Herb::Location` address Ruby. + # for the whole document. `extract_ruby` blanks the HTML in place, so a Prism byte offset and a + # template byte offset are the same number, which is what lets a `Herb::Location` address Ruby. # # program = Herb::RubyProgram.for(document) # program.resolve(node.content.location)&.nodes #=> [#] diff --git a/rust/tests/extract_ruby_options_test.rs b/rust/tests/extract_ruby_options_test.rs index fbc83d750..18c29f3b0 100644 --- a/rust/tests/extract_ruby_options_test.rs +++ b/rust/tests/extract_ruby_options_test.rs @@ -7,24 +7,6 @@ fn test_extract_ruby_default() { assert_eq!(result, " x = 1 ; y = 2 ;"); } -#[test] -fn test_extract_ruby_preserves_unicode_character_positions() { - let source = "

Café <%= x=1 %>

"; - let result = extract_ruby(source).unwrap(); - let source_column = source[..source.find('x').unwrap()].chars().count(); - let result_column = result[..result.find('x').unwrap()].chars().count(); - - assert_eq!(result, " x=1 ; "); - assert_eq!(result.chars().count(), source.chars().count()); - assert_eq!(result_column, source_column); - - let options = ExtractRubyOptions { - preserve_positions: false, - ..Default::default() - }; - assert_eq!(extract_ruby_with_options(source, &options).unwrap(), " x=1 "); -} - #[test] fn test_extract_ruby_without_semicolons() { let source = "<% x = 1 %> <% y = 2 %>"; diff --git a/sig/herb/ruby_program.rbs b/sig/herb/ruby_program.rbs index c628bcbcc..54cd5a31c 100644 --- a/sig/herb/ruby_program.rbs +++ b/sig/herb/ruby_program.rbs @@ -2,8 +2,8 @@ module Herb # Answers what Ruby a span of a template holds, out of the one Prism program the parser built - # for the whole document. The parser uses internal byte-preserving extraction, so a Prism byte - # offset and a template byte offset are the same number, which lets a `Herb::Location` address Ruby. + # for the whole document. `extract_ruby` blanks the HTML in place, so a Prism byte offset and a + # template byte offset are the same number, which is what lets a `Herb::Location` address Ruby. # # program = Herb::RubyProgram.for(document) # program.resolve(node.content.location)&.nodes #=> [#] diff --git a/src/analyze/analyze.c b/src/analyze/analyze.c index 21bb13ed5..d8a7595c6 100644 --- a/src/analyze/analyze.c +++ b/src/analyze/analyze.c @@ -18,7 +18,6 @@ #include "../include/ast/ast_nodes.h" #include "../include/errors.h" #include "../include/extract.h" -#include "../include/extract_internal.h" #include "../include/lexer/token_struct.h" #include "../include/lib/hb_array.h" #include "../include/lib/hb_buffer.h" @@ -1181,7 +1180,7 @@ static tag_helper_scope_T* tag_helper_scope_init(const char* source, hb_allocato .preserve_positions = true, }; - herb_extract_ruby_to_buffer_with_options_preserving_bytes(source, &scope->buffer, &extract_options, allocator); + herb_extract_ruby_to_buffer_with_options(source, &scope->buffer, &extract_options, allocator); if (!scope->buffer.value || scope->buffer.length == 0) { hb_buffer_free(&scope->buffer); diff --git a/src/analyze/parse_errors.c b/src/analyze/parse_errors.c index 933d60e0c..a5c8ef368 100644 --- a/src/analyze/parse_errors.c +++ b/src/analyze/parse_errors.c @@ -3,7 +3,6 @@ #include "../include/ast/ast_nodes.h" #include "../include/errors.h" #include "../include/extract.h" -#include "../include/extract_internal.h" #include "../include/lib/hb_allocator.h" #include "../include/lib/hb_string.h" #include "../include/lib/string.h" @@ -84,25 +83,14 @@ void herb_analyze_parse_errors( const parser_options_T* parser_options, hb_allocator_T* allocator ) { - hb_buffer_T extracted_ruby_buffer; - if (!hb_buffer_init(&extracted_ruby_buffer, strlen(source), allocator)) { return; } - - herb_extract_ruby_options_T extract_options = HERB_EXTRACT_RUBY_DEFAULT_OPTIONS; - extract_options.erb_openers = parser_options ? parser_options->erb_openers : NULL; - extract_options.erb_opener_count = parser_options ? parser_options->erb_opener_count : 0; - - herb_extract_ruby_to_buffer_with_options_preserving_bytes( + char* extracted_ruby = herb_extract_ruby_with_semicolons_and_openers( source, - &extracted_ruby_buffer, - &extract_options, + parser_options ? parser_options->erb_openers : NULL, + parser_options ? parser_options->erb_opener_count : 0, allocator ); - char* extracted_ruby = extracted_ruby_buffer.value; - if (!extracted_ruby) { - hb_buffer_free(&extracted_ruby_buffer); - return; - } + if (!extracted_ruby) { return; } bool strict_locals_enabled = parser_options && parser_options->strict_locals; bool has_anonymous_keyword_rest = strict_locals_enabled && document_has_anonymous_keyword_rest(document); @@ -139,5 +127,5 @@ void herb_analyze_parse_errors( pm_node_destroy(&parser, root); pm_parser_free(&parser); pm_options_free(&options); - hb_buffer_free(&extracted_ruby_buffer); + hb_allocator_dealloc(allocator, extracted_ruby); } diff --git a/src/analyze/prism_annotate.c b/src/analyze/prism_annotate.c index 333cb573f..79604ef63 100644 --- a/src/analyze/prism_annotate.c +++ b/src/analyze/prism_annotate.c @@ -2,7 +2,6 @@ #include "../include/ast/ast_node.h" #include "../include/ast/ast_nodes.h" #include "../include/extract.h" -#include "../include/extract_internal.h" #include "../include/lib/hb_allocator.h" #include "../include/lib/hb_buffer.h" #include "../include/lib/hb_narray.h" @@ -325,7 +324,7 @@ void herb_annotate_prism_nodes( .erb_opener_count = options->erb_opener_count, }; - herb_extract_ruby_to_buffer_with_options_preserving_bytes(source, &context->ruby_buf, &extract_options, allocator); + herb_extract_ruby_to_buffer_with_options(source, &context->ruby_buf, &extract_options, allocator); if (!context->ruby_buf.value || context->ruby_buf.length == 0) { hb_buffer_free(&context->ruby_buf); diff --git a/src/extract.c b/src/extract.c index 2f8bcaf23..996b9182b 100644 --- a/src/extract.c +++ b/src/extract.c @@ -1,4 +1,3 @@ -#include "include/extract_internal.h" #include "include/herb.h" #include "include/lexer/lexer.h" #include "include/lexer/token.h" @@ -7,7 +6,6 @@ #include "include/lib/hb_buffer.h" #include "include/lib/hb_string.h" #include "include/lib/string.h" -#include "include/util/utf8.h" #include "include/util/util.h" #include @@ -28,48 +26,22 @@ typedef struct { bool is_comment_tag; bool is_erb_comment_tag; bool need_newline; - bool preserve_byte_positions; - const char* source; } extract_ruby_state_T; -static void extract_ruby_mask_data(extract_ruby_state_T* state, const char* data, uint32_t length) { - uint32_t position = 0; - - while (position < length) { - if (is_newline(data[position])) { - hb_buffer_append_char(state->output, data[position]); - state->need_newline = false; - position++; - continue; - } - - uint32_t byte_length = 1; - - if (!state->preserve_byte_positions) { - byte_length = utf8_sequence_length(hb_string_from_data(data + position, length - position)); - } - - hb_buffer_append_char(state->output, ' '); - position += byte_length; - } -} - -static void extract_ruby_mask_range(extract_ruby_state_T* state, range_T range) { - extract_ruby_mask_data(state, state->source + range.from, range_length(range)); -} - static void extract_ruby_data(extract_ruby_state_T* state, const char* data, uint32_t from, uint32_t to) { - if (state->options.preserve_positions) { - extract_ruby_mask_data(state, data + from, to - from); - return; - } + uint32_t run_start = from; for (uint32_t position = from; position < to; position++) { if (!is_newline(data[position])) { continue; } + if (state->options.preserve_positions) { hb_buffer_append_whitespace(state->output, position - run_start); } + hb_buffer_append_char(state->output, data[position]); state->need_newline = false; + run_start = position + 1; } + + if (state->options.preserve_positions) { hb_buffer_append_whitespace(state->output, to - run_start); } } static void extract_ruby_token(extract_ruby_state_T* state, const token_T* token, const token_T* next) { @@ -111,19 +83,23 @@ static void extract_ruby_token(extract_ruby_state_T* state, const token_T* token } else { state->skip_erb_content = true; state->is_comment_tag = true; - if (state->options.preserve_positions) { extract_ruby_mask_range(state, token->range); } + if (state->options.preserve_positions) { + hb_buffer_append_whitespace(state->output, range_length(token->range)); + } } } else if (hb_string_equals(token->value, hb_string("<%%")) || hb_string_equals(token->value, hb_string("<%%=")) || (erb_opening_is_custom(token->value) && !state->options.custom_tags)) { state->skip_erb_content = true; state->is_comment_tag = false; - if (state->options.preserve_positions) { extract_ruby_mask_range(state, token->range); } + if (state->options.preserve_positions) { + hb_buffer_append_whitespace(state->output, range_length(token->range)); + } } else { state->skip_erb_content = false; state->is_comment_tag = false; if (state->options.preserve_positions) { - extract_ruby_mask_range(state, token->range); + hb_buffer_append_whitespace(state->output, range_length(token->range)); } else if (state->need_newline) { hb_buffer_append_char(state->output, '\n'); state->need_newline = false; @@ -148,7 +124,9 @@ static void extract_ruby_token(extract_ruby_state_T* state, const token_T* token } if (is_inline_comment) { - if (state->options.preserve_positions) { extract_ruby_mask_range(state, token->range); } + if (state->options.preserve_positions) { + hb_buffer_append_whitespace(state->output, range_length(token->range)); + } } else if (state->is_erb_comment_tag && !hb_string_is_null(token->value)) { const char* content = token->value.data; size_t content_remaining = token->value.length; @@ -180,9 +158,21 @@ static void extract_ruby_token(extract_ruby_state_T* state, const token_T* token } } else { if (state->is_erb_comment_tag && state->options.preserve_positions && !hb_string_is_null(token->value)) { - extract_ruby_mask_range(state, token->range); + const char* content = token->value.data; + size_t content_remaining = token->value.length; + + while (content_remaining > 0) { + if (*content == '\n') { + hb_buffer_append_char(state->output, '\n'); + } else { + hb_buffer_append_char(state->output, ' '); + } + + content++; + content_remaining--; + } } else if (state->options.preserve_positions) { - extract_ruby_mask_range(state, token->range); + hb_buffer_append_whitespace(state->output, range_length(token->range)); } } @@ -198,9 +188,9 @@ static void extract_ruby_token(extract_ruby_state_T* state, const token_T* token if (state->options.preserve_positions) { if (was_comment) { - extract_ruby_mask_range(state, token->range); + hb_buffer_append_whitespace(state->output, range_length(token->range)); } else if (was_erb_comment && state->options.comments) { - extract_ruby_mask_range(state, token->range); + hb_buffer_append_whitespace(state->output, range_length(token->range)); } else if (state->options.semicolons) { size_t length = range_length(token->range); @@ -208,7 +198,7 @@ static void extract_ruby_token(extract_ruby_state_T* state, const token_T* token if (length >= 1) { hb_buffer_append_char(state->output, ';'); } if (length >= 2) { hb_buffer_append_whitespace(state->output, length - 2); } } else { - extract_ruby_mask_range(state, token->range); + hb_buffer_append_whitespace(state->output, range_length(token->range)); } } @@ -216,16 +206,15 @@ static void extract_ruby_token(extract_ruby_state_T* state, const token_T* token } default: { - if (state->options.preserve_positions) { extract_ruby_mask_range(state, token->range); } + if (state->options.preserve_positions) { hb_buffer_append_whitespace(state->output, range_length(token->range)); } } } } -static void herb_extract_ruby_to_buffer_internal( +void herb_extract_ruby_to_buffer_with_options( const char* source, hb_buffer_T* output, const herb_extract_ruby_options_T* options, - bool preserve_byte_positions, hb_allocator_T* allocator ) { extract_ruby_state_T state = { @@ -235,8 +224,6 @@ static void herb_extract_ruby_to_buffer_internal( .is_comment_tag = false, .is_erb_comment_tag = false, .need_newline = false, - .preserve_byte_positions = preserve_byte_positions, - .source = NULL, }; parser_options_T lex_options = HERB_DEFAULT_PARSER_OPTIONS; @@ -250,7 +237,6 @@ static void herb_extract_ruby_to_buffer_internal( const char* data = lexer.source.data; uint32_t length = (uint32_t) lexer.source.length; token_T* pending = NULL; - state.source = data; while (true) { if (!pending && lexer.state == STATE_DATA) { @@ -279,24 +265,6 @@ static void herb_extract_ruby_to_buffer_internal( } } -void herb_extract_ruby_to_buffer_with_options( - const char* source, - hb_buffer_T* output, - const herb_extract_ruby_options_T* options, - hb_allocator_T* allocator -) { - herb_extract_ruby_to_buffer_internal(source, output, options, false, allocator); -} - -void herb_extract_ruby_to_buffer_with_options_preserving_bytes( - const char* source, - hb_buffer_T* output, - const herb_extract_ruby_options_T* options, - hb_allocator_T* allocator -) { - herb_extract_ruby_to_buffer_internal(source, output, options, true, allocator); -} - void herb_extract_ruby_to_buffer(const char* source, hb_buffer_T* output, hb_allocator_T* allocator) { herb_extract_ruby_to_buffer_with_options(source, output, NULL, allocator); } diff --git a/src/include/extract_internal.h b/src/include/extract_internal.h deleted file mode 100644 index 31be294af..000000000 --- a/src/include/extract_internal.h +++ /dev/null @@ -1,13 +0,0 @@ -#ifndef HERB_EXTRACT_INTERNAL_H -#define HERB_EXTRACT_INTERNAL_H - -#include "extract.h" - -void herb_extract_ruby_to_buffer_with_options_preserving_bytes( - const char* source, - hb_buffer_T* output, - const herb_extract_ruby_options_T* options, - hb_allocator_T* allocator -); - -#endif diff --git a/test/c/test_extract.c b/test/c/test_extract.c index 85ffcb671..a1fd25295 100644 --- a/test/c/test_extract.c +++ b/test/c/test_extract.c @@ -325,35 +325,6 @@ TEST(extract_ruby_with_options_default) hb_buffer_free(&output); END -TEST(extract_ruby_preserves_unicode_character_positions) - char* source = "

Café <%= x=1 %>

"; - - hb_buffer_T output; - hb_allocator_T malloc_alloc = hb_allocator_with_malloc(); - hb_buffer_init(&output, strlen(source), &malloc_alloc); - - hb_allocator_T allocator; - hb_allocator_init(&allocator, HB_ALLOCATOR_ARENA); - - herb_extract_ruby_to_buffer_with_options(source, &output, &HERB_EXTRACT_RUBY_DEFAULT_OPTIONS, &allocator); - - ck_assert_str_eq(output.value, " x=1 ; "); - ck_assert_uint_eq(strlen(output.value), 22); - ck_assert_uint_eq((size_t) (strchr(output.value, 'x') - output.value), 12); - - hb_buffer_free(&output); - hb_buffer_init(&output, strlen(source), &malloc_alloc); - - herb_extract_ruby_options_T options = HERB_EXTRACT_RUBY_DEFAULT_OPTIONS; - options.preserve_positions = false; - herb_extract_ruby_to_buffer_with_options(source, &output, &options, &allocator); - - ck_assert_str_eq(output.value, " x=1 "); - - hb_allocator_destroy(&allocator); - hb_buffer_free(&output); -END - TCase *extract_tests(void) { TCase *extract = tcase_create("Extract"); @@ -377,7 +348,6 @@ TCase *extract_tests(void) { tcase_add_test(extract, extract_ruby_with_options_preserve_positions_false); tcase_add_test(extract, extract_ruby_with_options_preserve_positions_false_and_comments_true); tcase_add_test(extract, extract_ruby_with_options_default); - tcase_add_test(extract, extract_ruby_preserves_unicode_character_positions); return extract; } diff --git a/test/extractor/extract_ruby_test.rb b/test/extractor/extract_ruby_test.rb index 9ed323577..4aa196c5c 100644 --- a/test/extractor/extract_ruby_test.rb +++ b/test/extractor/extract_ruby_test.rb @@ -26,16 +26,6 @@ class ExtractRubyTest < Minitest::Spec assert_equal " \n RUBY_VERSION ;\n \n", actual end - test "preserves Unicode character positions" do - source = "

Café <%= x=1 %>

" - ruby = Herb.extract_ruby(source) - - assert_equal " x=1 ; ", ruby - assert_equal source.length, ruby.length - assert_equal source.index("x"), ruby.index("x") - assert_equal " x=1 ", Herb.extract_ruby(source, preserve_positions: false) - end - test "nested" do actual = Herb.extract_ruby(<<~HTML) <% array = [1, 2, 3] %> diff --git a/test/ruby_program_test.rb b/test/ruby_program_test.rb index 18e2954f0..5f2c4bccc 100644 --- a/test/ruby_program_test.rb +++ b/test/ruby_program_test.rb @@ -77,15 +77,6 @@ def resolved_sources(template) assert_equal template.byteindex(" draft"), resolved&.offset end - test "keeps internal Prism byte offsets aligned after multibyte template text" do - template = %(

Café

<%= user.name %>) - program = program_for(template) - resolved = program&.resolve(erb_tokens(template).fetch(0).location) - - assert_equal " user.name ", resolved&.source - assert_equal template.byteindex(" user.name"), resolved&.offset - end - test "answers nothing when the document carries no program" do document = Herb.parse(%(

<%= @a %>

)).value From 242a88834608d2106b2b2eb04297cccb4807010b Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Thu, 29 Oct 2026 12:36:02 -0600 Subject: [PATCH 18/18] Fix template autocorrection safety and RuboCop directives --- .github/workflows/build.yml | 5 + Gemfile.lock | 2 +- gems/rubocop-herb/README.md | 10 +- gems/rubocop-herb/config/default.yml | 9 +- gems/rubocop-herb/lib/rubocop/herb.rb | 1 + .../lib/rubocop/herb/directive_source.rb | 63 ++++++++ .../lib/rubocop/herb/template_source.rb | 9 +- gems/rubocop-herb/rubocop-herb.gemspec | 2 +- gems/rubocop-herb/test/integration_test.rb | 150 ++++++++++++++++++ 9 files changed, 246 insertions(+), 5 deletions(-) create mode 100644 gems/rubocop-herb/lib/rubocop/herb/directive_source.rb diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 861fed9d8..a9ddd3929 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -85,6 +85,11 @@ jobs: - name: Run Ruby Tests run: bundle exec rake test:all test:rubocop_herb + - name: Test minimum supported RuboCop + run: | + bundle add rubocop --version 1.92.0 + bundle exec rake test:rubocop_herb + - name: Run C tests run: ./run_herb_tests diff --git a/Gemfile.lock b/Gemfile.lock index 99f3236cd..d0bede135 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -27,7 +27,7 @@ PATH rubocop-herb (0.11.1) herb (= 0.11.1) lint_roller (~> 1.1) - rubocop (>= 1.72.1, < 2) + rubocop (>= 1.92.0, < 2) GEM remote: https://rubygems.org/ diff --git a/gems/rubocop-herb/README.md b/gems/rubocop-herb/README.md index 7daedc854..4bc60f5ea 100644 --- a/gems/rubocop-herb/README.md +++ b/gems/rubocop-herb/README.md @@ -2,7 +2,7 @@ `rubocop-herb` runs a project's configured RuboCop rules against Ruby embedded in `.erb` and `.herb` templates. Herb builds a position-preserving Ruby representation of the entire template so RuboCop can understand control flow and variable usage across ERB tags while reporting offenses at their original template locations. -Add the gem to your bundle: +Requires RuboCop 1.92 or newer. Add the gem to your bundle: ```sh bundle add rubocop-herb @@ -25,6 +25,14 @@ bundle exec rubocop -A app/views The plugin honors the project's RuboCop configuration, including custom cops, inherited configuration, target Ruby versions, file exclusions, and autocorrection. Cops that depend on physical Ruby file contents or layout, or that misread template control flow, are [excluded for template files by default](config/default.yml). +Template-local `rubocop:disable`, `rubocop:enable`, and `rubocop:todo` directives work inside ERB comments: + +```erb +<%# rubocop:disable Layout/SpaceAroundOperators %> +<%= x=1 %> +<%# rubocop:enable Layout/SpaceAroundOperators %> +``` + Offenses are only reported for Ruby inside ERB tags, and autocorrections that would touch markup are discarded, so `rubocop -a` and `rubocop -A` never rewrite HTML or ERB delimiters. Every `.erb` and `.herb` file is inspected, including non-HTML templates. To skip some of them, exclude them in `.rubocop.yml`: diff --git a/gems/rubocop-herb/config/default.yml b/gems/rubocop-herb/config/default.yml index 1a5130c22..33f6421e5 100644 --- a/gems/rubocop-herb/config/default.yml +++ b/gems/rubocop-herb/config/default.yml @@ -10,7 +10,8 @@ AllCops: # These cops depend on physical Ruby file contents or layout that is obscured by the # position-preserving template representation, or misread markup between ERB tags as -# empty or missing Ruby, such as the markup inside a `do` block. +# empty or missing Ruby, such as the markup inside a `do` block. Redundancy cops +# can also mistake implicit template output for unused expressions or unreachable code. Layout/ArgumentAlignment: Exclude: *template_files Layout/ArrayAlignment: @@ -125,8 +126,14 @@ Style/NestedTernaryOperator: Exclude: *template_files Style/Next: Exclude: *template_files +Style/RedundantAssignment: + Exclude: *template_files +Style/RedundantBegin: + Exclude: *template_files Style/RedundantCondition: Exclude: *template_files +Style/RedundantReturn: + Exclude: *template_files Style/SafeNavigation: Exclude: *template_files Style/Semicolon: diff --git a/gems/rubocop-herb/lib/rubocop/herb.rb b/gems/rubocop-herb/lib/rubocop/herb.rb index 007a49180..f0a11f019 100644 --- a/gems/rubocop-herb/lib/rubocop/herb.rb +++ b/gems/rubocop-herb/lib/rubocop/herb.rb @@ -3,6 +3,7 @@ require "herb" require "rubocop" +require_relative "herb/directive_source" require_relative "herb/template_source" require_relative "herb/offense_guard" require_relative "herb/plugin" diff --git a/gems/rubocop-herb/lib/rubocop/herb/directive_source.rb b/gems/rubocop-herb/lib/rubocop/herb/directive_source.rb new file mode 100644 index 000000000..fb8722d93 --- /dev/null +++ b/gems/rubocop-herb/lib/rubocop/herb/directive_source.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +require "delegate" +require "prism" + +module RuboCop + module Herb + # Keep directives separate from parsed Ruby so a comment cannot hide a later tag. + class DirectiveSource < SimpleDelegator + RUBY_OPENERS = ["<%", "<%=", "<%==", "<%-", "<%#"].freeze + + attr_reader :raw_source, :comments + + def initialize(source, original) + super(source) + @raw_source = original.raw_source + @comments = source.comments.reject { |comment| comment.text.include?("rubocop:") } + @comments.concat(directive_comments(original)) + @comments.sort_by! { |comment| comment.loc.expression.begin_pos } + end + + private + + def directive_comments(original) + opener = nil + + ::Herb.lex(raw_source).value.flat_map do |token| + case token.type + when "TOKEN_ERB_START" + opener = token.value + [] + when "TOKEN_ERB_CONTENT" + next [] unless token.value.include?("rubocop:") + + comments_for(token, opener, original.buffer) + else + [] + end + end + end + + def comments_for(token, opener, buffer) + return [] unless RUBY_OPENERS.include?(opener) + + comment_tag = opener == "<%#" + content = comment_tag ? "##{token.value}" : token.value + offset = token.range.from - (comment_tag ? 1 : 0) + + Prism.parse_comments(content).filter_map do |comment| + next unless comment.location.slice.include?("rubocop:") + + comment_from_location(buffer, offset, comment.location) + end + end + + def comment_from_location(buffer, offset, location) + first = raw_source.byteslice(0, offset + location.start_offset).length + last = raw_source.byteslice(0, offset + location.end_offset).length + ::Parser::Source::Comment.new(::Parser::Source::Range.new(buffer, first, last)) + end + end + end +end diff --git a/gems/rubocop-herb/lib/rubocop/herb/template_source.rb b/gems/rubocop-herb/lib/rubocop/herb/template_source.rb index a045aae69..d2b42f74d 100644 --- a/gems/rubocop-herb/lib/rubocop/herb/template_source.rb +++ b/gems/rubocop-herb/lib/rubocop/herb/template_source.rb @@ -19,6 +19,7 @@ def self.from(original) source.config = original.config source.registry = original.registry source.ruby_ranges = ruby_ranges(template, code) + source.original_source = original end end @@ -60,7 +61,13 @@ def self.ruby_ranges(template, code) ranges end - attr_accessor :ruby_ranges + attr_accessor :ruby_ranges, :original_source + + def comment_config + return super unless original_source.raw_source.include?("rubocop:") + + @comment_config ||= ::RuboCop::CommentConfig.new(DirectiveSource.new(self, original_source)) + end # Both ends of the range touch Ruby, so the offense points at code rather than at markup. def anchored_in_ruby?(range) diff --git a/gems/rubocop-herb/rubocop-herb.gemspec b/gems/rubocop-herb/rubocop-herb.gemspec index 94f86a3f5..18d6ad988 100644 --- a/gems/rubocop-herb/rubocop-herb.gemspec +++ b/gems/rubocop-herb/rubocop-herb.gemspec @@ -30,5 +30,5 @@ Gem::Specification.new do |spec| spec.add_dependency "herb", "= #{Herb::VERSION}" spec.add_dependency "lint_roller", "~> 1.1" - spec.add_dependency "rubocop", ">= 1.72.1", "< 2" + spec.add_dependency "rubocop", ">= 1.92.0", "< 2" end diff --git a/gems/rubocop-herb/test/integration_test.rb b/gems/rubocop-herb/test/integration_test.rb index aaff682b0..68baacc04 100644 --- a/gems/rubocop-herb/test/integration_test.rb +++ b/gems/rubocop-herb/test/integration_test.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require_relative "test_helper" +require "erb" class RuboCopHerbIntegrationTest < Minitest::Spec include RuboCopHerbTestHelpers @@ -225,6 +226,155 @@ def on_send(node) assert_includes output, "Layout/SpaceAroundOperators" end + test "does not remove returns before template output" do + write_config(<<~YAML) + Style/RedundantReturn: + Enabled: true + YAML + template = "<% greeting = lambda do %><% return 'Hi' %>ignored<% end %><%= greeting.call %>\n" + write("example.html.erb", template) + + output, error, status = run_rubocop("-a", "example.html.erb") + + assert_equal 0, status, output + assert_empty error + assert_equal template, read("example.html.erb") + assert_equal "Hi\n", ERB.new(read("example.html.erb")).result + end + + test "still corrects redundant returns in ordinary Ruby" do + write_config(<<~YAML) + Style/RedundantReturn: + Enabled: true + YAML + write("example.rb", "def greeting\n return 'Hi'\nend\n") + + output, error, status = run_rubocop("-a", "example.rb") + + assert_equal 0, status, output + assert_empty error + assert_equal "def greeting\n 'Hi'\nend\n", read("example.rb") + end + + test "does not remove assignments consumed by output tags" do + write_config(<<~YAML) + Style/RedundantAssignment: + Enabled: true + YAML + template = "<% def greeting %><% x = 'Hi' %><%= x %><% end %><%= greeting %>\n" + write("example.html.erb", template) + + output, error, status = run_rubocop("-a", "example.html.erb") + + assert_equal 0, status, output + assert_empty error + assert_equal template, read("example.html.erb") + assert_equal "HiHi\n", ERB.new(read("example.html.erb"), eoutvar: "@output").result(Object.new.instance_eval { binding }) + end + + test "does not remove begin and end across template tags" do + write_config(<<~YAML) + Style/RedundantBegin: + Enabled: true + YAML + template = "<% begin %>Hi<%= 'there' %><% end %>\n" + write("example.html.erb", template) + + output, error, status = run_rubocop("-a", "example.html.erb") + + assert_equal 0, status, output + assert_empty error + assert_equal template, read("example.html.erb") + assert_equal "Hithere\n", ERB.new(read("example.html.erb")).result + end + + test "honors disable and enable directives in ERB comments" do + write_config(<<~YAML) + Layout/SpaceAroundOperators: + Enabled: true + YAML + template = <<~ERB +

Caf\u00e9

<%# rubocop:disable Layout/SpaceAroundOperators %> + <%= x=1 %> + <%# rubocop:enable Layout/SpaceAroundOperators %> + <%= y=2 %> + ERB + write("example.html.erb", template) + + output, error, status = run_rubocop("-a", "example.html.erb") + + assert_equal 0, status, output + assert_empty error + assert_equal template.sub("y=2", "y = 2"), read("example.html.erb") + end + + test "does not let a same-line directive hide later Ruby tags" do + write_config(<<~YAML) + Layout/SpaceAroundOperators: + Enabled: true + Style/StringLiterals: + Enabled: true + YAML + template = "<%# rubocop:todo Layout/SpaceAroundOperators %><%= x=1 %><%= \"hello\" %>\n<%= y=2 %>\n" + write("example.html.erb", template) + + output, error, status = run_rubocop("-a", "example.html.erb") + + assert_equal 0, status, output + assert_empty error + assert_equal template.sub('"hello"', "'hello'").sub("y=2", "y = 2"), read("example.html.erb") + end + + test "honors directives in Ruby comment tags" do + write_config(<<~YAML) + Layout/SpaceAroundOperators: + Enabled: true + YAML + template = "<% # rubocop:disable Layout/SpaceAroundOperators %>\n<%= x=1 %>\n" + write("example.html.erb", template) + + output, error, status = run_rubocop("-a", "example.html.erb") + + assert_equal 0, status, output + assert_empty error + assert_equal template, read("example.html.erb") + end + + test "honors disable-next directives without losing the following statement" do + write_config(<<~YAML) + Layout/SpaceAroundOperators: + Enabled: true + YAML + template = "<%# rubocop:disable-next Layout/SpaceAroundOperators %>\n<%= x=1 %>\n<%= y=2 %>\n" + write("example.html.erb", template) + + output, error, status = run_rubocop("-a", "example.html.erb") + + assert_equal 0, status, output + assert_empty error + assert_equal template.sub("y=2", "y = 2"), read("example.html.erb") + end + + test "ignores directives in strings markup and escaped tags" do + write_config(<<~YAML) + Layout/SpaceAroundOperators: + Enabled: true + YAML + template = <<~ERB +

# rubocop:disable Layout/SpaceAroundOperators

+ <%%# rubocop:disable Layout/SpaceAroundOperators %> + <%= '# rubocop:disable Layout/SpaceAroundOperators' %> + <%= x=1 %> + ERB + write("example.html.erb", template) + + output, error, status = run_rubocop("-a", "example.html.erb") + + assert_equal 0, status, output + assert_empty error + assert_equal template.sub("x=1", "x = 1"), read("example.html.erb") + end + private def write_config(cops)