Wire prism_nodes injection for the remaining 10 prism-dependent rules (herb-embedded-ada)

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

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";

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

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
 

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("<div><%= @foo %></div>\n", prism_program: true)
       envelope = JSON.parse(json)