From b22b857221c34f9204cf2d9cc15f11aa2272cc3c Mon Sep 17 00:00:00 2001 From: jleo3 Date: Tue, 18 Aug 2026 11:36:49 -0400 Subject: [PATCH] Wire prism_nodes injection for the remaining 10 prism-dependent rules (herb-embedded-ada) ResultEnvelope.parse now handles prism_nodes/prism_nodes_deep the same way gu7 handled prism_program: never forward the flag to Herb.parse itself (which returns raw ASCII-8BIT bytes and breaks .to_json), but compute the Prism bytes separately as a JSON-safe Integer array and inject them. Unlike prism_program's single whole-document parse, prism_nodes needs one Prism parse per AST_ERB_* node, scoped to just that node's own embedded-Ruby content token but still offset-correct against the whole file: every byte outside the node's content range gets blanked (preserving newlines) before re-dumping, so the resulting parse's only real statement is that node's own expression, at its true position in the file. Ruby's Prism.dump has no API to serialize an arbitrary sub-node, so the injected bytes always deserialize to a ProgramNode - but every prism_nodes-dependent rule expects prismNode to be the single embedded expression node directly (e.g. isAssignmentNode checks prismNode.constructor.name). js/ruby_backend.js patches every ERB*Node's prismNode getter, once, to unwrap a single-statement ProgramNode down to its inner node, rather than touching the vendored bundle. Manually verified all 10 target rules (a11y-no-autofocus-attribute, actionview-no-silent-helper, actionview-no-unnecessary-tag-attributes, erb-no-output-in-attribute-position, erb-no-silent-statement, erb-no-unsafe-raw, erb-no-unsafe-script-interpolation, erb-no-unused-expressions, erb-no-unused-literals, erb-prefer-direct-output) produce offense-for-offense matching diagnostics against the real reference linter, including offset-derived locations. --- CHARTER.md | 50 ++++++++++-------- js/ruby_backend.js | 33 ++++++++++++ lib/herb/embedded/result_envelope.rb | 77 +++++++++++++++++++++++++--- spec/bridge_lint_spec.rb | 16 ++++++ spec/result_envelope_spec.rb | 25 ++++++++- 5 files changed, 174 insertions(+), 27 deletions(-) diff --git a/CHARTER.md b/CHARTER.md index d93efad..52b3a11 100644 --- a/CHARTER.md +++ b/CHARTER.md @@ -74,26 +74,36 @@ already sitting in the `herb` gem's native C extension the whole time. every future adapter. Prism-backed rules need real walkable `PrismNode` objects, not JSON, so `Prism.dump`'s self-describing versioned binary format crosses as a `Uint8Array` via `MiniRacer::Binary` — verified byte-accurate including non-ASCII source. - - **`prism_program` rules are wired; `prism_nodes`/`prism_nodes_deep` rules are not (yet).** - 12 rules in the vendored bundle declare which Prism mode they need via `parserOptions`: - `erb-no-debug-output` and `erb-no-instance-variables-in-partials` - ask for `prism_program` (one whole-document Prism parse, attached to the root - `DocumentNode`); the other 10 (`a11y-no-autofocus-attribute`, - `actionview-no-silent-helper`, `actionview-no-unnecessary-tag-attributes`, - `erb-no-output-in-attribute-position`, `erb-no-silent-statement`, `erb-no-unsafe-raw`, - `erb-no-unsafe-script-interpolation`, `erb-no-unused-expressions`, - `erb-no-unused-literals`, `erb-prefer-direct-output`) ask for `prism_nodes` (a separate - Prism parse per embedded-Ruby node). `ResultEnvelope.parse` implements the `prism_program` - case: it never asks `Herb.parse` itself to embed `prism_node` (that comes back as a raw - ASCII-8BIT String and blows up `.to_json`, the bug `FORWARDABLE_OPTIONS` still guards - against); instead it separately computes `Prism.dump(Herb.extract_ruby(source)).bytes` — a - plain JSON-safe `Integer` array — and injects it onto the root value's `prism_node` key. - `Herb.extract_ruby` blanks non-Ruby content but preserves byte length/position, so the - resulting Prism byte offsets still line up with `DocumentNode#prismNode`'s use of the - original (whole-file) `source`. The `prism_nodes`/`prism_nodes_deep` case — injection onto - individual `ERBContentNode`s rather than the document root — needs its own design pass - (which nodes carry embedded Ruby, how offsets there compose) and remains unimplemented; see - herb-embedded-gu7. + - **Both `prism_program` and `prism_nodes`/`prism_nodes_deep` are wired.** 12 rules in the + vendored bundle declare which Prism mode they need via `parserOptions`: + `erb-no-debug-output` and `erb-no-instance-variables-in-partials` ask for `prism_program` + (one whole-document Prism parse, attached to the root `DocumentNode`); the other 10 + (`a11y-no-autofocus-attribute`, `actionview-no-silent-helper`, + `actionview-no-unnecessary-tag-attributes`, `erb-no-output-in-attribute-position`, + `erb-no-silent-statement`, `erb-no-unsafe-raw`, `erb-no-unsafe-script-interpolation`, + `erb-no-unused-expressions`, `erb-no-unused-literals`, `erb-prefer-direct-output`) ask for + `prism_nodes` (a separate Prism parse per embedded-Ruby node — no rule currently requests + `prism_nodes_deep`, but `ResultEnvelope` treats it identically). Neither mode ever asks + `Herb.parse` itself to embed `prism_node` (that comes back as a raw ASCII-8BIT String and + blows up `.to_json`, the bug `FORWARDABLE_OPTIONS` still guards against); both compute a + plain JSON-safe `Integer` array of `Prism.dump` bytes separately and inject it into the + parsed value hash before serialization. + - **`prism_program`** computes `Prism.dump(Herb.extract_ruby(source)).bytes` once and + injects it onto the root value's `prism_node` key. `Herb.extract_ruby` blanks non-Ruby + content but preserves byte length/position, so the resulting offsets line up with + `DocumentNode#prismNode`'s use of the original (whole-file) `source`. + - **`prism_nodes`** does the equivalent per node: every `AST_ERB_*` node carries its own + embedded-Ruby snippet in a `content` token with a byte range into the whole file: for + each one, `ResultEnvelope` blanks every byte *outside* that range (not just non-Ruby + content) and re-dumps, so the resulting parse contains just that one node's own + statement at its correct absolute offset. Ruby's `Prism.dump` has no API to serialize an + arbitrary sub-node directly, so this always yields a `ProgramNode` — but every + `prism_nodes`-dependent rule expects `prismNode` to be the single embedded expression + node itself (e.g. `isAssignmentNode` checks `prismNode.constructor.name`). `js/ruby_backend.js` + patches every `ERB*Node`'s `prismNode` getter, once, to unwrap a single-statement + `ProgramNode` down to that inner node — a deliberate, narrowly-scoped patch in *our* + integration file, not the vendored bundle, since there's no other way to reach this + shape through Ruby's public Prism API. - **Exceptions cross the boundary raw, not as structured errors.** An earlier design assumed exceptions needed to be caught and translated before crossing. Sixteen probes showed raw crossing is catchable, non-poisoning, and preserves the original Ruby class in the outward diff --git a/js/ruby_backend.js b/js/ruby_backend.js index f257a5b..722a630 100644 --- a/js/ruby_backend.js +++ b/js/ruby_backend.js @@ -29,6 +29,39 @@ var libHerbBackend = { }, }; +// rbParse (via ResultEnvelope#inject_prism_nodes) injects prism_node bytes +// produced by Ruby's Prism.dump, which always serializes a whole +// ProgramNode — there is no public API to dump an arbitrary sub-node +// directly. But every prism_nodes-dependent rule (see CHARTER.md) expects +// an ERB*Node's prismNode to BE the single embedded-Ruby expression node +// itself (e.g. isAssignmentNode checks prismNode.constructor.name), not a +// Program wrapping it. Unwrap here, once, for every ERB node class that +// defines the prismNode getter, rather than patching the vendored bundle. +// Only unwraps when the parse yielded exactly one top-level statement — +// the case single-tag Ruby content always produces — leaving anything +// else (multiple ';'-separated statements in one tag) as the ProgramNode, +// same as an unhandled edge case would fall back to. +Object.keys(HerbLinter).forEach(function (name) { + if (!/^ERB.*Node$/.test(name)) return; + + var proto = HerbLinter[name] && HerbLinter[name].prototype; + var descriptor = proto && Object.getOwnPropertyDescriptor(proto, "prismNode"); + if (!descriptor || typeof descriptor.get !== "function") return; + + var originalGet = descriptor.get; + + Object.defineProperty(proto, "prismNode", { + configurable: true, + enumerable: descriptor.enumerable, + get: function () { + var raw = originalGet.call(this); + var body = raw && raw.constructor && raw.constructor.name === "ProgramNode" && raw.statements && raw.statements.body; + + return body && body.length === 1 ? body[0] : raw; + }, + }); +}); + class RubyBackend extends HerbLinter.HerbBackend { backendVersion() { return "mini_racer"; diff --git a/lib/herb/embedded/result_envelope.rb b/lib/herb/embedded/result_envelope.rb index c9796d2..60f1575 100644 --- a/lib/herb/embedded/result_envelope.rb +++ b/lib/herb/embedded/result_envelope.rb @@ -14,11 +14,10 @@ module ResultEnvelope # caller-supplied options. Deliberately excludes prism_nodes, # prism_nodes_deep, and prism_program: asking Herb.parse itself to # embed prism_node populates it with a raw ASCII-8BIT String, and - # forwarding that through raises JSON::GeneratorError. prism_program - # is instead handled below by computing a JSON-safe byte array - # ourselves; prism_nodes/prism_nodes_deep (per-ERBContentNode - # injection) remain unimplemented — see CHARTER.md. Also excludes - # timeout and max_errors (timing/error-cap options, not shape). + # forwarding that through raises JSON::GeneratorError. Both are + # instead handled below by computing JSON-safe byte arrays + # ourselves. Also excludes timeout and max_errors (timing/error-cap + # options, not shape). FORWARDABLE_OPTIONS = %i[ strict track_whitespace @@ -33,6 +32,14 @@ module ResultEnvelope def parse(source, options_hash = {}) options_hash = (options_hash || {}).transform_keys(&:to_sym) + envelope = build_envelope(source, options_hash) + + return envelope.to_json unless options_hash[:prism_nodes] || options_hash[:prism_nodes_deep] + + with_injected_prism_nodes(envelope, source) + end + + def build_envelope(source, options_hash) result = Herb.parse(source, **forwardable(options_hash)) value_hash = result.value.to_hash @@ -44,8 +51,9 @@ def parse(source, options_hash = {}) warnings: result.warnings, errors: result.errors, options: result.options.to_h, - }.to_json + } end + private_class_method :build_envelope def lex(source) result = Herb.lex(source) @@ -58,6 +66,17 @@ def lex(source) }.to_json end + # value_hash's children are still live Herb::AST::Node objects + # (#to_hash is shallow), so per-node injection needs them as plain + # Hashes first. A JSON round-trip is the simplest way to get that + # without hand-walking Node#child_nodes ourselves. + def with_injected_prism_nodes(envelope, source) + parsed_envelope = JSON.parse(envelope.to_json) + inject_prism_nodes(parsed_envelope["value"], source) + parsed_envelope.to_json + end + private_class_method :with_injected_prism_nodes + def forwardable(options_hash) options_hash.each_with_object({}) do |(key, value), forwarded| symbol_key = key.to_sym @@ -77,6 +96,52 @@ def prism_program_bytes(source) Prism.dump(Herb.extract_ruby(source)).bytes end private_class_method :prism_program_bytes + + # Every AST_ERB_* node (ERBContentNode, ERBBlockNode, ERBIfNode, + # ...) carries its own embedded-Ruby snippet in a `content` token + # with a byte `range` into the whole file. Unlike prism_program's + # single whole-document parse, each of these needs its own Prism + # parse scoped to just that snippet — but still offset-correct + # against the whole-file `source`, since that's what every + # ERB*Node#prismNode getter deserializes against (ruby_backend.js + # unwraps the resulting single-statement ProgramNode down to the + # inner expression node the vendored rules actually expect). + def inject_prism_nodes(node, source) + case node + when Hash + inject_prism_node_for(node, source) + node.each_value { |value| inject_prism_nodes(value, source) } + when Array + node.each { |value| inject_prism_nodes(value, source) } + end + end + private_class_method :inject_prism_nodes + + def inject_prism_node_for(node, source) + return unless node["type"].is_a?(String) && node["type"].start_with?("AST_ERB_") + + range = node.dig("content", "range") + return unless range.is_a?(Array) && range.length == 2 + + node["prism_node"] = prism_nodes_bytes(source, range[0], range[1]) + end + private_class_method :inject_prism_node_for + + # Blanks (space, newlines preserved) every byte outside [from, to) + # so the one node's own Ruby content parses alone — at the correct + # absolute offset — rather than pulling in unrelated HTML or other + # ERB tags' Ruby. + def prism_nodes_bytes(source, from, to) + bytes = source.b.bytes + bytes.each_index do |i| + next if i >= from && i < to + + bytes[i] = 0x20 unless bytes[i] == 0x0A + end + + Prism.dump(bytes.pack("C*").force_encoding(source.encoding)).bytes + end + private_class_method :prism_nodes_bytes end end end diff --git a/spec/bridge_lint_spec.rb b/spec/bridge_lint_spec.rb index b756014..9e852e9 100644 --- a/spec/bridge_lint_spec.rb +++ b/spec/bridge_lint_spec.rb @@ -108,6 +108,22 @@ class FakeCrashRule { expect(non_partial_diagnostics).to be_empty end + it "produces a real offense for a prism_nodes-backed rule with offset-sensitive location, not just isEnabled() firing" do + source = %(<%= raw(@untrusted) %>\n) + + diagnostics = bridge.lint(source, file: "x.html.erb", rules: ["erb-no-unsafe-raw"]) + + expect(diagnostics.size).to eq(1) + expect(diagnostics.first.column).to eq(4) + + expect(bridge.lint(%(<%= @untrusted %>\n), file: "x.html.erb", rules: ["erb-no-unsafe-raw"])).to be_empty + end + + it "skips assignments for a prism_nodes-backed rule that must inspect the embedded Ruby's node type" do + expect(bridge.lint("<% @foo = 1 %>\n", file: "x.html.erb", rules: ["erb-no-silent-statement"])).to be_empty + expect(bridge.lint("<% do_something_silently %>\n", file: "x.html.erb", rules: ["erb-no-silent-statement"])).not_to be_empty + end + it "excludes a rule not enabled by default when rules: is nil, but includes it when explicitly selected" do bridge diff --git a/spec/result_envelope_spec.rb b/spec/result_envelope_spec.rb index 3ecd1ef..d7b2261 100644 --- a/spec/result_envelope_spec.rb +++ b/spec/result_envelope_spec.rb @@ -34,12 +34,35 @@ .not_to raise_error end - it "never forwards prism_nodes, even when present in options_hash" do + it "never forwards prism_nodes to Herb.parse itself, even when present in options_hash" do json = described_class.parse("<%= 1 + 1 %>", prism_nodes: true) expect { JSON.parse(json) }.not_to raise_error end + it "injects a JSON-safe prism_node byte array on ERB nodes when prism_nodes is requested" do + json = described_class.parse("<%= 1 + 1 %>", prism_nodes: true) + erb_node = JSON.parse(json)["value"]["children"].find { |c| c["type"] == "AST_ERB_CONTENT_NODE" } + + expect(erb_node["prism_node"]).to be_an(Array) + expect(erb_node["prism_node"]).not_to be_empty + expect(erb_node["prism_node"]).to all(be_an(Integer)) + end + + it "also injects prism_node byte arrays when prism_nodes_deep is requested" do + json = described_class.parse("<%= 1 + 1 %>", prism_nodes_deep: true) + erb_node = JSON.parse(json)["value"]["children"].find { |c| c["type"] == "AST_ERB_CONTENT_NODE" } + + expect(erb_node["prism_node"]).to be_an(Array) + end + + it "leaves prism_node nil on ERB nodes when neither prism_nodes flag is requested" do + json = described_class.parse("<%= 1 + 1 %>", {}) + erb_node = JSON.parse(json)["value"]["children"].find { |c| c["type"] == "AST_ERB_CONTENT_NODE" } + + expect(erb_node["prism_node"]).to be_nil + end + it "injects a JSON-safe prism_node byte array on the root value when prism_program is requested" do json = described_class.parse("
<%= @foo %>
\n", prism_program: true) envelope = JSON.parse(json)