Expand conformance suite to per-rule fixture coverage, gated (herb-embedded-ag7)

CHARTER.md

@@ -157,11 +157,18 @@ already sitting in the `herb` gem's native C extension the whole time.
    `herb-lint-rb` are the current names. If Marco's gem claims `Herb::Linter` or a
    conflicting convention before herb-embedded sunsets, a rename may be needed in the
    meantime — low stakes given the planned sunset, but worth a glance when his gem surfaces.
-3. **Conformance punch list isn't closed.** The design spec's differential run against
-   `npx @herb-tools/linter` wasn't yet clean end-to-end — missing `LintContext` (filename),
-   missing `.herb.yml` config application, and per-rule `parserOptions` forwarding. Each has
-   a named cause and is tracked as implementation work in `bd` (see `herb-embedded-6dp` and
-   related beads), not a design gap.
+3. **Conformance punch list, fixture-coverage dimension: closed.** The design spec's
+   differential run against `npx @herb-tools/linter` originally wasn't clean end-to-end —
+   missing `LintContext` (filename), missing `.herb.yml` config application, and per-rule
+   `parserOptions` forwarding. `LintContext` and `parserOptions` forwarding (including
+   `prism_program`/`prism_nodes`, see the Shape decisions entry above) are resolved; each had
+   a named cause tracked as implementation work in `bd` (`herb-embedded-6dp` and related
+   beads), not a design gap. `spec/conformance/conformance_spec.rb` now runs the differential
+   offense-for-offense diff against a hand-written fixture for every one of the 100 rules the
+   vendored bundle registers (`herb-embedded-ag7`), gated so an upstream rule with no fixture
+   fails the build — not just the 5 general-purpose fixtures from before. `.herb.yml` config
+   application is unaffected by that work and still worth a look if it's ever suspected of
+   drifting from upstream.
 4. **Config schema stability.** `.herb.yml` config currently passes through to the engine
    whole. Per-rule options can change across Herb releases, so Ruby may eventually need
    version-aware translation instead of passthrough. Unresolved until it's actually observed

js/ruby_backend.js

@@ -161,7 +161,7 @@ function __herbRegisterCustomRule(rewrittenSource, path) {
 function __herbAutofix(source, file, ruleNames, includeUnsafe) {
   var ruleClasses = __herbSelectRules(ruleNames);
   var context = { fileName: file, filename: file };
-  var linter = new HerbLinter.Linter(__herbEmbeddedBridge.instance, ruleClasses);
+  var linter = new HerbLinter.Linter(__herbEmbeddedBridge.instance, ruleClasses, undefined, HerbLinter.rules);
   var result = linter.autofix(source, context, undefined, { includeUnsafe: !!includeUnsafe });
 
   return JSON.stringify({ source: result.source, fixed: result.fixed });
@@ -171,14 +171,23 @@ function __herbAutofix(source, file, ruleNames, includeUnsafe) {
 // throws must not abort the run (spike 2 finding) — the real Linter#lint
 // has no such isolation internally, since it runs every selected rule's
 // check() in one uncaught loop.
+//
+// The 4th constructor arg (allAvailableRules) must be the full registry,
+// not just the one rule being run: herb-disable-comment-unnecessary reads
+// context.validRuleNames (built from Linter#getAvailableRules, which
+// falls back to allAvailableRules) to decide whether a `herb:disable
+// some-other-rule` comment references a real rule — omitting this arg
+// left validRuleNames scoped to whatever single rule __herbLint happened
+// to be running, so the rule silently never matched anything outside
+// itself. Caught by conformance fixture coverage (herb-embedded-ag7).
 function __herbLint(source, file, ruleNames) {
   var ruleClasses = __herbSelectRules(ruleNames);
   var context = { fileName: file, filename: file };
   var offenses = [];
 
   ruleClasses.forEach(function (ruleClass) {
     try {
-      var linter = new HerbLinter.Linter(__herbEmbeddedBridge.instance, [ruleClass]);
+      var linter = new HerbLinter.Linter(__herbEmbeddedBridge.instance, [ruleClass], undefined, HerbLinter.rules);
       var result = linter.lint(source, context);
       offenses = offenses.concat(result.offenses);
     } catch (e) {

spec/conformance/conformance_spec.rb

@@ -10,6 +10,18 @@
 CONFORMANCE_ROOT = File.expand_path("../..", __dir__)
 CONFORMANCE_REFERENCE_LINTER = File.join(CONFORMANCE_ROOT, "node_modules", ".bin", "herb-lint")
 CONFORMANCE_FIXTURES = Dir.glob(File.join(__dir__, "fixtures", "*.html.erb"))
+CONFORMANCE_RULE_FIXTURES = Dir.glob(File.join(__dir__, "fixtures", "rules", "*.html.erb"))
+
+# A handful of rules (erb-no-instance-variables-in-partials and friends)
+# only enable themselves for partial files (basename starting with `_`),
+# so their fixture is named with that leading underscore to trigger
+# correctly — this strips it back off to recover the rule code the
+# coverage-gate test and --only/rules: need. Top-level (not an instance
+# method) since it's needed both at spec-definition time, to build each
+# `it` description, and at example-run time.
+def conformance_rule_name_for(fixture_path)
+  File.basename(fixture_path, ".html.erb").sub(/\A_/, "")
+end
 
 # Node at test time is acceptable — the guarantee this repo makes is
 # about *user* time (see spec/no_node_spec.rb). This is the differential
@@ -23,16 +35,24 @@
 
   after { adapter.dispose }
 
-  def reference_offenses(fixture_path)
+  def reference_offenses(fixture_path, only: nil)
     # --no-github: herb-lint auto-enables GitHub Actions annotations whenever
     # GITHUB_ACTIONS=true is set (i.e. inside our own CI job), which then
     # conflicts fatally with --json ("--github cannot be used with --json
     # format"). Explicitly disabling it makes --json behavior independent of
     # the invoking environment.
-    stdout, stderr, status = Open3.capture3(
+    #
+    # --only ignores .herb.yml config entirely (including a rule's own
+    # defaultConfig.enabled), matching rules: [...] on the bridge side —
+    # both let a per-rule fixture exercise a not-enabled-by-default rule
+    # directly, the same way gu7/ada's Bridge#lint specs already do.
+    args = [
       CONFORMANCE_REFERENCE_LINTER, fixture_path, "--json", "--no-custom-rules", "--no-color", "--no-github",
-      "--jobs", "1", chdir: CONFORMANCE_ROOT
-    )
+      "--jobs", "1"
+    ]
+    args += ["--only", only] if only
+
+    stdout, stderr, status = Open3.capture3(*args, chdir: CONFORMANCE_ROOT)
     unless status.exitstatus.zero? || status.exitstatus == 1
       raise "reference herb-lint failed (exit #{status.exitstatus}): #{stderr}"
     end
@@ -44,9 +64,9 @@ def reference_offenses(fixture_path)
     JSON.parse(stdout)["offenses"].map { |o| [o["code"], o["location"]["start"]["line"], o["location"]["start"]["column"]] }
   end
 
-  def bridge_offenses(fixture_path)
+  def bridge_offenses(fixture_path, rules: nil)
     source = File.read(fixture_path)
-    bridge.lint(source, file: File.basename(fixture_path)).map { |d| [d.rule, d.line, d.column] }
+    bridge.lint(source, file: File.basename(fixture_path), rules: rules).map { |d| [d.rule, d.line, d.column] }
   end
 
   CONFORMANCE_FIXTURES.each do |fixture_path|
@@ -71,4 +91,36 @@ def bridge_offenses(fixture_path)
 
     expect(present).to include(*required)
   end
+
+  CONFORMANCE_RULE_FIXTURES.each do |fixture_path|
+    rule = conformance_rule_name_for(fixture_path)
+
+    it "matches npx @herb-tools/linter offense-for-offense on the #{rule} fixture, exercising the rule" do
+      reference = reference_offenses(fixture_path, only: rule).sort
+      actual = bridge_offenses(fixture_path, rules: [rule]).sort
+
+      expect(reference).not_to be_empty,
+                               "Fixture #{File.basename(fixture_path)} produced no reference offenses for " \
+                               "#{rule} — it doesn't actually trigger the rule"
+
+      missing = reference - actual
+      extra = actual - reference
+
+      expect([missing, extra]).to eq([[], []]), <<~MSG
+        Conformance mismatch for #{rule} (#{File.basename(fixture_path)}) — [rule, line, column] tuples:
+          Missing (npx found it, bridge did not): #{missing.inspect}
+          Extra   (bridge found it, npx did not): #{extra.inspect}
+      MSG
+    end
+  end
+
+  it "has a conformance fixture for every rule the vendored bundle registers" do
+    registered = bridge.rule_names
+    covered = CONFORMANCE_RULE_FIXTURES.map { |f| conformance_rule_name_for(f) }
+
+    missing = registered - covered
+    expect(missing).to be_empty,
+                       "Rules registered by the vendored bundle with no conformance fixture under " \
+                       "spec/conformance/fixtures/rules/: #{missing.sort.inspect}"
+  end
 end

spec/conformance/fixtures/rules/_actionview-strict-locals-first-line.html.erb

@@ -0,0 +1,2 @@
+<p>Header</p>
+<%# locals: (title:) %>

spec/conformance/fixtures/rules/_erb-no-instance-variables-in-partials.html.erb

@@ -0,0 +1 @@
+<%= @widget_name %>

spec/conformance/fixtures/rules/_erb-strict-locals-required.html.erb

@@ -0,0 +1 @@
+<p>Hi</p>

spec/conformance/fixtures/rules/a11y-avoid-generic-link-text.html.erb

@@ -0,0 +1 @@
+<a href="/details">Click here</a>

spec/conformance/fixtures/rules/a11y-disabled-attribute.html.erb

@@ -0,0 +1 @@
+<div disabled>Unavailable</div>

spec/conformance/fixtures/rules/a11y-nested-interactive-elements.html.erb

@@ -0,0 +1 @@
+<a href="/go"><button type="button">Go</button></a>

spec/conformance/fixtures/rules/a11y-no-accesskey-attribute.html.erb

@@ -0,0 +1 @@
+<div accesskey="s">Save</div>

spec/conformance/fixtures/rules/a11y-no-aria-label-misuse.html.erb

@@ -0,0 +1 @@
+<h1 aria-label="Page Title">Welcome</h1>

spec/conformance/fixtures/rules/a11y-no-aria-unsupported-elements.html.erb

@@ -0,0 +1 @@
+<meta aria-hidden="true">

spec/conformance/fixtures/rules/a11y-no-autofocus-attribute.html.erb

@@ -0,0 +1 @@
+<input type="text" autofocus>

spec/conformance/fixtures/rules/a11y-no-redundant-image-alt.html.erb

@@ -0,0 +1 @@
+<img src="cat.png" alt="a cat image">

spec/conformance/fixtures/rules/a11y-svg-has-accessible-text.html.erb

@@ -0,0 +1 @@
+<svg><rect width="10" height="10"></rect></svg>

spec/conformance/fixtures/rules/actionview-no-silent-helper.html.erb

@@ -0,0 +1 @@
+<% link_to "Home", "/" %>

spec/conformance/fixtures/rules/actionview-no-silent-render.html.erb

@@ -0,0 +1 @@
+<% render "header" %>

spec/conformance/fixtures/rules/actionview-no-unnecessary-tag-attributes.html.erb

@@ -0,0 +1 @@
+<div <%= tag.attributes(class: "box", id: "main") %>></div>

spec/conformance/fixtures/rules/actionview-no-void-element-content.html.erb

@@ -0,0 +1 @@
+<%= tag.hr "divider text" %>

spec/conformance/fixtures/rules/actionview-strict-locals-partial-only.html.erb

@@ -0,0 +1,3 @@
+<%# locals: (title:) %>
+
+<p>Content</p>

spec/conformance/fixtures/rules/erb-comment-syntax.html.erb

@@ -0,0 +1 @@
+<%  # oops this should be an ERB comment %>

spec/conformance/fixtures/rules/erb-no-case-node-children.html.erb

@@ -0,0 +1,5 @@
+<% case status %>
+  stray text
+<% when :ok %>
+  fine
+<% end %>

spec/conformance/fixtures/rules/erb-no-commented-out-output-tags.html.erb

@@ -0,0 +1 @@
+<%#= old_helper %>

spec/conformance/fixtures/rules/erb-no-conditional-html-element.html.erb

@@ -0,0 +1,7 @@
+<% if show %>
+<div>
+<% end %>
+content
+<% if show %>
+</div>
+<% end %>

spec/conformance/fixtures/rules/erb-no-conditional-open-tag.html.erb

@@ -0,0 +1,7 @@
+<% if active %>
+<div class="a">
+<% else %>
+<div class="b">
+<% end %>
+content
+</div>

spec/conformance/fixtures/rules/erb-no-debug-output.html.erb

@@ -0,0 +1 @@
+<% byebug %>

spec/conformance/fixtures/rules/erb-no-duplicate-branch-elements.html.erb

@@ -0,0 +1,5 @@
+<% if cond %>
+  <div>A</div>
+<% else %>
+  <div>B</div>
+<% end %>

spec/conformance/fixtures/rules/erb-no-empty-control-flow.html.erb

@@ -0,0 +1,2 @@
+<% if cond %>
+<% end %>

spec/conformance/fixtures/rules/erb-no-empty-tags.html.erb

@@ -0,0 +1 @@
+<%  %>

spec/conformance/fixtures/rules/erb-no-extra-newline.html.erb

@@ -0,0 +1,5 @@
+<p>first</p>
+
+
+
+<p>second</p>

spec/conformance/fixtures/rules/erb-no-extra-whitespace-inside-tags.html.erb

@@ -0,0 +1 @@
+<%=  name %>

spec/conformance/fixtures/rules/erb-no-inline-case-conditions.html.erb

@@ -0,0 +1,3 @@
+<% case status; when :ok %>
+  fine
+<% end %>

spec/conformance/fixtures/rules/erb-no-interpolated-class-names.html.erb

@@ -0,0 +1 @@
+<div class="btn-<%= size %>"></div>

spec/conformance/fixtures/rules/erb-no-javascript-tag-helper.html.erb

@@ -0,0 +1,3 @@
+<%= javascript_tag do %>
+  console.log("hi");
+<% end %>

spec/conformance/fixtures/rules/erb-no-output-control-flow.html.erb

@@ -0,0 +1,3 @@
+<%= if cond %>
+  yes
+<% end %>

spec/conformance/fixtures/rules/erb-no-output-in-attribute-name.html.erb

@@ -0,0 +1 @@
+<div <%= dynamic_attr %>="value"></div>

spec/conformance/fixtures/rules/erb-no-output-in-attribute-position.html.erb

@@ -0,0 +1 @@
+<div <%= extra_attrs %>></div>

spec/conformance/fixtures/rules/erb-no-raw-output-in-attribute-value.html.erb

@@ -0,0 +1 @@
+<div title="<%== @name %>"></div>

spec/conformance/fixtures/rules/erb-no-silent-statement.html.erb

@@ -0,0 +1 @@
+<% track_view %>

spec/conformance/fixtures/rules/erb-no-silent-tag-in-attribute-name.html.erb

@@ -0,0 +1 @@
+<div aria-<% role %>-label="x"></div>

spec/conformance/fixtures/rules/erb-no-statement-in-script.html.erb

@@ -0,0 +1,3 @@
+<script>
+<% do_something %>
+</script>

spec/conformance/fixtures/rules/erb-no-then-in-control-flow.html.erb

@@ -0,0 +1,3 @@
+<% if true then %>
+  yes
+<% end %>

spec/conformance/fixtures/rules/erb-no-trailing-whitespace.html.erb

@@ -0,0 +1,2 @@
+<p>Hello   
+</p>

spec/conformance/fixtures/rules/erb-no-unsafe-js-attribute.html.erb

@@ -0,0 +1 @@
+<button onclick="<%= handler %>">Click</button>

spec/conformance/fixtures/rules/erb-no-unsafe-raw.html.erb

@@ -0,0 +1 @@
+<div><%= raw(comment_body) %></div>

spec/conformance/fixtures/rules/erb-no-unsafe-script-interpolation.html.erb

@@ -0,0 +1,3 @@
+<script>
+var config = <%= settings %>;
+</script>

spec/conformance/fixtures/rules/erb-no-unused-expressions.html.erb

@@ -0,0 +1 @@
+<% @user.name %>

spec/conformance/fixtures/rules/erb-no-unused-literals.html.erb

@@ -0,0 +1 @@
+<% "unused" %>

spec/conformance/fixtures/rules/erb-prefer-direct-output.html.erb

@@ -0,0 +1 @@
+<%= "hello" %>

spec/conformance/fixtures/rules/erb-prefer-image-tag-helper.html.erb

@@ -0,0 +1 @@
+<img src="<%= @photo %>" alt="pic">

spec/conformance/fixtures/rules/erb-require-trailing-newline.html.erb

@@ -0,0 +1,2 @@
+<p>Hi</p>
+

spec/conformance/fixtures/rules/erb-require-whitespace-inside-tags.html.erb

@@ -0,0 +1 @@
+<%=x%>

spec/conformance/fixtures/rules/erb-right-trim.html.erb

@@ -0,0 +1 @@
+<% x = 1 =%>

spec/conformance/fixtures/rules/erb-strict-locals-comment-syntax.html.erb

@@ -0,0 +1 @@
+<% # locals: (name:) %>

spec/conformance/fixtures/rules/herb-disable-comment-malformed.html.erb

@@ -0,0 +1 @@
+<div><%# herb:disable foo, %></div>

spec/conformance/fixtures/rules/herb-disable-comment-missing-rules.html.erb

@@ -0,0 +1 @@
+<div><%# herb:disable %></div>

spec/conformance/fixtures/rules/herb-disable-comment-no-duplicate-rules.html.erb

@@ -0,0 +1 @@
+<div><%# herb:disable parser-no-errors, parser-no-errors %></div>

spec/conformance/fixtures/rules/herb-disable-comment-no-redundant-all.html.erb

@@ -0,0 +1 @@
+<div><%# herb:disable all, parser-no-errors %></div>

spec/conformance/fixtures/rules/herb-disable-comment-unnecessary.html.erb

@@ -0,0 +1 @@
+<div><%# herb:disable svg-tag-name-capitalization %></div>

spec/conformance/fixtures/rules/herb-disable-comment-valid-rule-name.html.erb

@@ -0,0 +1 @@
+<div><%# herb:disable totally-not-a-real-rule %></div>

spec/conformance/fixtures/rules/html-allowed-script-type.html.erb

@@ -0,0 +1 @@
+<script type="text/babel">console.log(1)</script>

spec/conformance/fixtures/rules/html-anchor-require-href.html.erb

@@ -0,0 +1 @@
+<a>Learn more</a>

spec/conformance/fixtures/rules/html-aria-attribute-must-be-valid.html.erb

@@ -0,0 +1 @@
+<div aria-frobnicate="true">Content</div>

spec/conformance/fixtures/rules/html-aria-label-is-well-formatted.html.erb

@@ -0,0 +1 @@
+<div aria-label="myLabel">Content</div>

spec/conformance/fixtures/rules/html-aria-level-must-be-valid.html.erb

@@ -0,0 +1 @@
+<div role="heading" aria-level="9">Content</div>

spec/conformance/fixtures/rules/html-aria-role-heading-requires-level.html.erb

@@ -0,0 +1 @@
+<div role="heading">Content</div>

spec/conformance/fixtures/rules/html-aria-role-must-be-valid.html.erb

@@ -0,0 +1 @@
+<div role="fizzbuzz">Content</div>

spec/conformance/fixtures/rules/html-attribute-double-quotes.html.erb

@@ -0,0 +1 @@
+<div class='wrapper'>Content</div>

spec/conformance/fixtures/rules/html-attribute-equals-spacing.html.erb

@@ -0,0 +1 @@
+<div class ="wrapper">Content</div>

spec/conformance/fixtures/rules/html-attribute-values-require-quotes.html.erb

@@ -0,0 +1 @@
+<div class=wrapper>Content</div>

spec/conformance/fixtures/rules/html-avoid-both-disabled-and-aria-disabled.html.erb

@@ -0,0 +1 @@
+<button disabled aria-disabled="true">Save</button>

spec/conformance/fixtures/rules/html-body-only-elements.html.erb

@@ -0,0 +1,6 @@
+<html>
+<head>
+<p>Content</p>
+</head>
+<body></body>
+</html>

spec/conformance/fixtures/rules/html-boolean-attributes-no-value.html.erb

@@ -0,0 +1 @@
+<input disabled="disabled">

spec/conformance/fixtures/rules/html-details-has-summary.html.erb

@@ -0,0 +1,3 @@
+<details>
+<p>Content</p>
+</details>

spec/conformance/fixtures/rules/html-head-only-elements.html.erb

@@ -0,0 +1,6 @@
+<html>
+<head></head>
+<body>
+<base href="/">
+</body>
+</html>

spec/conformance/fixtures/rules/html-iframe-has-title.html.erb

@@ -0,0 +1 @@
+<iframe src="https://example.com"></iframe>

spec/conformance/fixtures/rules/html-img-require-alt.html.erb

@@ -0,0 +1 @@
+<img src="logo.png">

spec/conformance/fixtures/rules/html-input-require-autocomplete.html.erb

@@ -0,0 +1 @@
+<input type="email">

spec/conformance/fixtures/rules/html-navigation-has-label.html.erb

@@ -0,0 +1 @@
+<nav>Content</nav>

spec/conformance/fixtures/rules/html-no-abstract-roles.html.erb

@@ -0,0 +1 @@
+<div role="widget">Content</div>

spec/conformance/fixtures/rules/html-no-aria-hidden-on-body.html.erb

@@ -0,0 +1,3 @@
+<body aria-hidden="true">
+  <p>Content</p>
+</body>

spec/conformance/fixtures/rules/html-no-aria-hidden-on-focusable.html.erb

@@ -0,0 +1 @@
+<a href="/somewhere" aria-hidden="true">Go</a>

spec/conformance/fixtures/rules/html-no-block-inside-inline.html.erb

@@ -0,0 +1 @@
+<span><div>Nested block</div></span>

spec/conformance/fixtures/rules/html-no-duplicate-attributes.html.erb

@@ -0,0 +1 @@
+<div class="a" class="b">Content</div>

spec/conformance/fixtures/rules/html-no-duplicate-ids.html.erb

@@ -0,0 +1,2 @@
+<div id="widget">A</div>
+<div id="widget">B</div>

spec/conformance/fixtures/rules/html-no-duplicate-meta-names.html.erb

@@ -0,0 +1,4 @@
+<head>
+  <meta name="description" content="first">
+  <meta name="description" content="second">
+</head>

spec/conformance/fixtures/rules/html-no-empty-attributes.html.erb

@@ -0,0 +1 @@
+<div id="">Content</div>

spec/conformance/fixtures/rules/html-no-empty-headings.html.erb

@@ -0,0 +1 @@
+<h1></h1>

spec/conformance/fixtures/rules/html-no-nested-links.html.erb

@@ -0,0 +1 @@
+<a href="/outer"><a href="/inner">Inner</a></a>

spec/conformance/fixtures/rules/html-no-positive-tab-index.html.erb

@@ -0,0 +1 @@
+<div tabindex="5">Content</div>

spec/conformance/fixtures/rules/html-no-self-closing.html.erb

@@ -0,0 +1 @@
+<div />

spec/conformance/fixtures/rules/html-no-space-in-tag.html.erb

@@ -0,0 +1 @@
+<div  id="x">Content</div>

spec/conformance/fixtures/rules/html-no-title-attribute.html.erb

@@ -0,0 +1 @@
+<div title="Tooltip">Content</div>

spec/conformance/fixtures/rules/html-no-underscores-in-attribute-names.html.erb

@@ -0,0 +1 @@
+<div data_foo="bar">Content</div>

spec/conformance/fixtures/rules/html-no-unescaped-entities.html.erb

@@ -0,0 +1 @@
+<p>1 < 2</p>