diff --git a/CHARTER.md b/CHARTER.md index 52b3a11..e83a7fd 100644 --- a/CHARTER.md +++ b/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 diff --git a/js/ruby_backend.js b/js/ruby_backend.js index 722a630..fa4a4dc 100644 --- a/js/ruby_backend.js +++ b/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,6 +171,15 @@ 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 }; @@ -178,7 +187,7 @@ function __herbLint(source, file, ruleNames) { 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) { diff --git a/spec/conformance/conformance_spec.rb b/spec/conformance/conformance_spec.rb index 3d419af..fcff14f 100644 --- a/spec/conformance/conformance_spec.rb +++ b/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 diff --git a/spec/conformance/fixtures/rules/_actionview-strict-locals-first-line.html.erb b/spec/conformance/fixtures/rules/_actionview-strict-locals-first-line.html.erb new file mode 100644 index 0000000..3653be2 --- /dev/null +++ b/spec/conformance/fixtures/rules/_actionview-strict-locals-first-line.html.erb @@ -0,0 +1,2 @@ +
Header
+<%# locals: (title:) %> diff --git a/spec/conformance/fixtures/rules/_erb-no-instance-variables-in-partials.html.erb b/spec/conformance/fixtures/rules/_erb-no-instance-variables-in-partials.html.erb new file mode 100644 index 0000000..02613be --- /dev/null +++ b/spec/conformance/fixtures/rules/_erb-no-instance-variables-in-partials.html.erb @@ -0,0 +1 @@ +<%= @widget_name %> diff --git a/spec/conformance/fixtures/rules/_erb-strict-locals-required.html.erb b/spec/conformance/fixtures/rules/_erb-strict-locals-required.html.erb new file mode 100644 index 0000000..98d12dd --- /dev/null +++ b/spec/conformance/fixtures/rules/_erb-strict-locals-required.html.erb @@ -0,0 +1 @@ +Hi
diff --git a/spec/conformance/fixtures/rules/a11y-avoid-generic-link-text.html.erb b/spec/conformance/fixtures/rules/a11y-avoid-generic-link-text.html.erb new file mode 100644 index 0000000..aa414e6 --- /dev/null +++ b/spec/conformance/fixtures/rules/a11y-avoid-generic-link-text.html.erb @@ -0,0 +1 @@ +Click here diff --git a/spec/conformance/fixtures/rules/a11y-disabled-attribute.html.erb b/spec/conformance/fixtures/rules/a11y-disabled-attribute.html.erb new file mode 100644 index 0000000..2aa0e78 --- /dev/null +++ b/spec/conformance/fixtures/rules/a11y-disabled-attribute.html.erb @@ -0,0 +1 @@ +
diff --git a/spec/conformance/fixtures/rules/a11y-svg-has-accessible-text.html.erb b/spec/conformance/fixtures/rules/a11y-svg-has-accessible-text.html.erb
new file mode 100644
index 0000000..4a65311
--- /dev/null
+++ b/spec/conformance/fixtures/rules/a11y-svg-has-accessible-text.html.erb
@@ -0,0 +1 @@
+
diff --git a/spec/conformance/fixtures/rules/actionview-no-silent-helper.html.erb b/spec/conformance/fixtures/rules/actionview-no-silent-helper.html.erb
new file mode 100644
index 0000000..af2de83
--- /dev/null
+++ b/spec/conformance/fixtures/rules/actionview-no-silent-helper.html.erb
@@ -0,0 +1 @@
+<% link_to "Home", "/" %>
diff --git a/spec/conformance/fixtures/rules/actionview-no-silent-render.html.erb b/spec/conformance/fixtures/rules/actionview-no-silent-render.html.erb
new file mode 100644
index 0000000..d420d57
--- /dev/null
+++ b/spec/conformance/fixtures/rules/actionview-no-silent-render.html.erb
@@ -0,0 +1 @@
+<% render "header" %>
diff --git a/spec/conformance/fixtures/rules/actionview-no-unnecessary-tag-attributes.html.erb b/spec/conformance/fixtures/rules/actionview-no-unnecessary-tag-attributes.html.erb
new file mode 100644
index 0000000..21dfc2e
--- /dev/null
+++ b/spec/conformance/fixtures/rules/actionview-no-unnecessary-tag-attributes.html.erb
@@ -0,0 +1 @@
+Content
diff --git a/spec/conformance/fixtures/rules/erb-comment-syntax.html.erb b/spec/conformance/fixtures/rules/erb-comment-syntax.html.erb new file mode 100644 index 0000000..0200c1f --- /dev/null +++ b/spec/conformance/fixtures/rules/erb-comment-syntax.html.erb @@ -0,0 +1 @@ +<% # oops this should be an ERB comment %> diff --git a/spec/conformance/fixtures/rules/erb-no-case-node-children.html.erb b/spec/conformance/fixtures/rules/erb-no-case-node-children.html.erb new file mode 100644 index 0000000..e5b537e --- /dev/null +++ b/spec/conformance/fixtures/rules/erb-no-case-node-children.html.erb @@ -0,0 +1,5 @@ +<% case status %> + stray text +<% when :ok %> + fine +<% end %> diff --git a/spec/conformance/fixtures/rules/erb-no-commented-out-output-tags.html.erb b/spec/conformance/fixtures/rules/erb-no-commented-out-output-tags.html.erb new file mode 100644 index 0000000..e5fc01d --- /dev/null +++ b/spec/conformance/fixtures/rules/erb-no-commented-out-output-tags.html.erb @@ -0,0 +1 @@ +<%#= old_helper %> diff --git a/spec/conformance/fixtures/rules/erb-no-conditional-html-element.html.erb b/spec/conformance/fixtures/rules/erb-no-conditional-html-element.html.erb new file mode 100644 index 0000000..07cfdce --- /dev/null +++ b/spec/conformance/fixtures/rules/erb-no-conditional-html-element.html.erb @@ -0,0 +1,7 @@ +<% if show %> +first
+ + + +second
diff --git a/spec/conformance/fixtures/rules/erb-no-extra-whitespace-inside-tags.html.erb b/spec/conformance/fixtures/rules/erb-no-extra-whitespace-inside-tags.html.erb new file mode 100644 index 0000000..4e4f1c6 --- /dev/null +++ b/spec/conformance/fixtures/rules/erb-no-extra-whitespace-inside-tags.html.erb @@ -0,0 +1 @@ +<%= name %> diff --git a/spec/conformance/fixtures/rules/erb-no-inline-case-conditions.html.erb b/spec/conformance/fixtures/rules/erb-no-inline-case-conditions.html.erb new file mode 100644 index 0000000..6808eb5 --- /dev/null +++ b/spec/conformance/fixtures/rules/erb-no-inline-case-conditions.html.erb @@ -0,0 +1,3 @@ +<% case status; when :ok %> + fine +<% end %> diff --git a/spec/conformance/fixtures/rules/erb-no-interpolated-class-names.html.erb b/spec/conformance/fixtures/rules/erb-no-interpolated-class-names.html.erb new file mode 100644 index 0000000..e6b8caf --- /dev/null +++ b/spec/conformance/fixtures/rules/erb-no-interpolated-class-names.html.erb @@ -0,0 +1 @@ + diff --git a/spec/conformance/fixtures/rules/erb-no-javascript-tag-helper.html.erb b/spec/conformance/fixtures/rules/erb-no-javascript-tag-helper.html.erb new file mode 100644 index 0000000..8d3f499 --- /dev/null +++ b/spec/conformance/fixtures/rules/erb-no-javascript-tag-helper.html.erb @@ -0,0 +1,3 @@ +<%= javascript_tag do %> + console.log("hi"); +<% end %> diff --git a/spec/conformance/fixtures/rules/erb-no-output-control-flow.html.erb b/spec/conformance/fixtures/rules/erb-no-output-control-flow.html.erb new file mode 100644 index 0000000..bb76757 --- /dev/null +++ b/spec/conformance/fixtures/rules/erb-no-output-control-flow.html.erb @@ -0,0 +1,3 @@ +<%= if cond %> + yes +<% end %> diff --git a/spec/conformance/fixtures/rules/erb-no-output-in-attribute-name.html.erb b/spec/conformance/fixtures/rules/erb-no-output-in-attribute-name.html.erb new file mode 100644 index 0000000..f28b365 --- /dev/null +++ b/spec/conformance/fixtures/rules/erb-no-output-in-attribute-name.html.erb @@ -0,0 +1 @@ +Hello +
diff --git a/spec/conformance/fixtures/rules/erb-no-unsafe-js-attribute.html.erb b/spec/conformance/fixtures/rules/erb-no-unsafe-js-attribute.html.erb new file mode 100644 index 0000000..4553f1e --- /dev/null +++ b/spec/conformance/fixtures/rules/erb-no-unsafe-js-attribute.html.erb @@ -0,0 +1 @@ + diff --git a/spec/conformance/fixtures/rules/erb-no-unsafe-raw.html.erb b/spec/conformance/fixtures/rules/erb-no-unsafe-raw.html.erb new file mode 100644 index 0000000..1726b8a --- /dev/null +++ b/spec/conformance/fixtures/rules/erb-no-unsafe-raw.html.erb @@ -0,0 +1 @@ +Hi
+ diff --git a/spec/conformance/fixtures/rules/erb-require-whitespace-inside-tags.html.erb b/spec/conformance/fixtures/rules/erb-require-whitespace-inside-tags.html.erb new file mode 100644 index 0000000..9a472a2 --- /dev/null +++ b/spec/conformance/fixtures/rules/erb-require-whitespace-inside-tags.html.erb @@ -0,0 +1 @@ +<%=x%> diff --git a/spec/conformance/fixtures/rules/erb-right-trim.html.erb b/spec/conformance/fixtures/rules/erb-right-trim.html.erb new file mode 100644 index 0000000..6ca0b26 --- /dev/null +++ b/spec/conformance/fixtures/rules/erb-right-trim.html.erb @@ -0,0 +1 @@ +<% x = 1 =%> diff --git a/spec/conformance/fixtures/rules/erb-strict-locals-comment-syntax.html.erb b/spec/conformance/fixtures/rules/erb-strict-locals-comment-syntax.html.erb new file mode 100644 index 0000000..7c0bea1 --- /dev/null +++ b/spec/conformance/fixtures/rules/erb-strict-locals-comment-syntax.html.erb @@ -0,0 +1 @@ +<% # locals: (name:) %> diff --git a/spec/conformance/fixtures/rules/herb-disable-comment-malformed.html.erb b/spec/conformance/fixtures/rules/herb-disable-comment-malformed.html.erb new file mode 100644 index 0000000..ec4af8a --- /dev/null +++ b/spec/conformance/fixtures/rules/herb-disable-comment-malformed.html.erb @@ -0,0 +1 @@ +Content
+ + + diff --git a/spec/conformance/fixtures/rules/html-boolean-attributes-no-value.html.erb b/spec/conformance/fixtures/rules/html-boolean-attributes-no-value.html.erb new file mode 100644 index 0000000..9190a02 --- /dev/null +++ b/spec/conformance/fixtures/rules/html-boolean-attributes-no-value.html.erb @@ -0,0 +1 @@ + diff --git a/spec/conformance/fixtures/rules/html-details-has-summary.html.erb b/spec/conformance/fixtures/rules/html-details-has-summary.html.erb new file mode 100644 index 0000000..4e90180 --- /dev/null +++ b/spec/conformance/fixtures/rules/html-details-has-summary.html.erb @@ -0,0 +1,3 @@ +Content
+
diff --git a/spec/conformance/fixtures/rules/html-input-require-autocomplete.html.erb b/spec/conformance/fixtures/rules/html-input-require-autocomplete.html.erb
new file mode 100644
index 0000000..6c92fdc
--- /dev/null
+++ b/spec/conformance/fixtures/rules/html-input-require-autocomplete.html.erb
@@ -0,0 +1 @@
+
diff --git a/spec/conformance/fixtures/rules/html-navigation-has-label.html.erb b/spec/conformance/fixtures/rules/html-navigation-has-label.html.erb
new file mode 100644
index 0000000..47f176d
--- /dev/null
+++ b/spec/conformance/fixtures/rules/html-navigation-has-label.html.erb
@@ -0,0 +1 @@
+
diff --git a/spec/conformance/fixtures/rules/html-no-abstract-roles.html.erb b/spec/conformance/fixtures/rules/html-no-abstract-roles.html.erb
new file mode 100644
index 0000000..29b5eb0
--- /dev/null
+++ b/spec/conformance/fixtures/rules/html-no-abstract-roles.html.erb
@@ -0,0 +1 @@
+Content
+ diff --git a/spec/conformance/fixtures/rules/html-no-aria-hidden-on-focusable.html.erb b/spec/conformance/fixtures/rules/html-no-aria-hidden-on-focusable.html.erb new file mode 100644 index 0000000..fddfa8f --- /dev/null +++ b/spec/conformance/fixtures/rules/html-no-aria-hidden-on-focusable.html.erb @@ -0,0 +1 @@ + diff --git a/spec/conformance/fixtures/rules/html-no-block-inside-inline.html.erb b/spec/conformance/fixtures/rules/html-no-block-inside-inline.html.erb new file mode 100644 index 0000000..2665f1d --- /dev/null +++ b/spec/conformance/fixtures/rules/html-no-block-inside-inline.html.erb @@ -0,0 +1 @@ +1 < 2
diff --git a/spec/conformance/fixtures/rules/html-no-unknown-tag.html.erb b/spec/conformance/fixtures/rules/html-no-unknown-tag.html.erb new file mode 100644 index 0000000..64f094f --- /dev/null +++ b/spec/conformance/fixtures/rules/html-no-unknown-tag.html.erb @@ -0,0 +1 @@ +