feat(linter-rules/no-unused-vars): declared var must be used (#2955)

Author: sidvishnoi

src/core/defaults.js

@@ -10,11 +10,13 @@ import linter from "./linter.js";
 import { rule as localRefsExist } from "./linter-rules/local-refs-exist.js";
 import { rule as noHeadinglessSectionsRule } from "./linter-rules/no-headingless-sections.js";
 import { rule as noHttpPropsRule } from "./linter-rules/no-http-props.js";
+import { rule as noUnusedVars } from "./linter-rules/no-unused-vars.js";
 import { rule as privsecSection } from "./linter-rules/privsec-section.js";
 
 linter.register(
   noHttpPropsRule,
   noHeadinglessSectionsRule,
+  noUnusedVars,
   checkPunctuation,
   localRefsExist,
   checkInternalSlots,
@@ -26,6 +28,7 @@ export const coreDefaults = {
   lint: {
     "no-headingless-sections": true,
     "no-http-props": true,
+    "no-unused-vars": false,
     "check-punctuation": false,
     "local-refs-exist": true,
     "check-internal-slots": false,

src/core/linter-rules/no-unused-vars.js

@@ -0,0 +1,86 @@
+// @ts-check
+/**
+ * Linter rule "no-unused-vars".
+ *
+ * Checks that an variable is used if declared (the first use is treated as
+ * declaration).
+ */
+import LinterRule from "../LinterRule.js";
+import { lang as defaultLang } from "../l10n.js";
+import { norm } from "../utils.js";
+
+const name = "no-unused-vars";
+
+const meta = {
+  en: {
+    description: "Variable was defined, but never used.",
+    howToFix: "Add a `data-ignore-unused` attribute to the `<var>`.",
+    help: "See developer console.",
+  },
+};
+// Fall back to english, if language is missing
+const lang = defaultLang in meta ? defaultLang : "en";
+
+/**
+ * @param {*} _
+ * @param {Document} doc
+ * @return {import("../LinterRule").LinterResult}
+ */
+function linterFunction(_, doc) {
+  const offendingElements = [];
+
+  /**
+   * Check if a <section> contains a `".algorithm"`
+   *
+   * The selector matches:
+   * ``` html
+   * <section><ul class="algorithm"></ul></section>
+   * <section><div><ul class="algorithm"></ul></div></section>
+   * ```
+   * The selector does not match:
+   * ``` html
+   * <section><section><ul class="algorithm"></ul></section></section>
+   * ```
+   * @param {HTMLElement} section
+   */
+  const sectionContainsAlgorithm = section =>
+    !!section.querySelector(
+      ":scope > :not(section) ~ .algorithm, :scope > :not(section) .algorithm"
+    );
+
+  for (const section of doc.querySelectorAll("section")) {
+    if (!sectionContainsAlgorithm(section)) continue;
+
+    /**
+     * `<var>` in this section, but excluding those in child sections.
+     * @type {NodeListOf<HTMLElement>}
+     */
+    const varElems = section.querySelectorAll(":scope > :not(section) var");
+    if (!varElems.length) continue;
+
+    /** @type {Map<string, HTMLElement[]>} */
+    const varUsage = new Map();
+    for (const varElem of varElems) {
+      const key = norm(varElem.textContent);
+      const elems = varUsage.get(key) || varUsage.set(key, []).get(key);
+      elems.push(varElem);
+    }
+
+    for (const vars of varUsage.values()) {
+      if (vars.length === 1 && !vars[0].hasAttribute("data-ignore-unused")) {
+        offendingElements.push(vars[0]);
+      }
+    }
+  }
+
+  if (!offendingElements.length) {
+    return;
+  }
+  return {
+    name,
+    offendingElements,
+    occurrences: offendingElements.length,
+    ...meta[lang],
+  };
+}
+export const rule = new LinterRule(name, linterFunction);

tests/spec/core/linter-rules/no-unused-vars-spec.js

@@ -0,0 +1,82 @@
+"use strict";
+
+import { rule } from "../../../../src/core/linter-rules/no-unused-vars.js";
+
+describe("Core Linter Rule - 'no-unused-vars'", () => {
+  const config = {
+    lint: { "no-unused-vars": true },
+  };
+  const doc = document.implementation.createHTMLDocument("test doc");
+  beforeEach(() => {
+    // Make sure every unordered test get an empty document
+    // See: https://github.com/w3c/respec/pull/1495
+    while (doc.body.firstChild) {
+      doc.body.removeChild(doc.body.firstChild);
+    }
+  });
+
+  it("skips sections without .algorithm", async () => {
+    doc.body.innerHTML = `
+      <section>
+        <p><var>varA</var></p>
+        <section>
+          <p><var>varC</var></p>
+        </section>
+      </section>
+    `;
+
+    const result = await rule.lint(config, doc);
+    expect(result).toBeUndefined();
+  });
+
+  it("complains on unused vars", async () => {
+    doc.body.innerHTML = `
+      <section>
+        <p><var>A</var>  is unused</p>
+        <p><var>B</var></p>
+        <p><var>Z</var> is unused</p>
+        <ul class="algorithm">
+          <li><var>B</var></li>
+          <li><var>C</var> is unused</li>
+          <li><var>D</var></li>
+          <li><var>D</var></li>
+        </ul>
+        <section>
+          <p><var>E</var> is unused</p>
+          <ul class="algorithm">
+            <li><var>F</var> is unused</li>
+            <li><var>G</var></li>
+            <li><var>G</var></li>
+            <li><var>Z</var> is unused even though it's used in grandparent</li>
+          </ul>
+        </section>
+      </section>
+    `;
+
+    const result = await rule.lint(config, doc);
+    expect(result.name).toBe("no-unused-vars");
+    const unusedVars = result.offendingElements.map(v => v.textContent);
+    expect(unusedVars).toEqual(["A", "Z", "C", "E", "F", "Z"]);
+    expect(result.occurrences).toBe(6);
+  });
+
+  it("ignores unused vars with data-ignore-unused", async () => {
+    doc.body.innerHTML = `
+      <section>
+        <p><var data-ignore-unused>varA</var>  is unused</p>
+        <ul class="algorithm">
+          <li><var>varB</var> is unused</li>
+          <li><var data-ignore-unused>varC</var> is unused</li>
+          <li><var>varD</var></li>
+          <li><var>varD</var></li>
+        </ul>
+      </section>
+    `;
+
+    const result = await rule.lint(config, doc);
+    expect(result.name).toBe("no-unused-vars");
+    expect(result.occurrences).toBe(1);
+    const unusedVars = result.offendingElements.map(v => v.textContent);
+    expect(unusedVars).toEqual(["varB"]);
+  });
+});

tests/spec/geonovum/defaults-spec.js

@@ -16,6 +16,7 @@ describe("Geonovum — Defaults", () => {
       "no-headingless-sections": true,
       "privsec-section": true,
       "no-http-props": true,
+      "no-unused-vars": false,
       "local-refs-exist": true,
       "check-punctuation": false,
       "check-internal-slots": false,
@@ -53,6 +54,7 @@ describe("Geonovum — Defaults", () => {
       "no-headingless-sections": true,
       "privsec-section": false,
       "no-http-props": false,
+      "no-unused-vars": false,
       "local-refs-exist": true,
       "check-punctuation": false,
       "fake-linter-rule": "foo",

tests/spec/w3c/defaults-spec.js

@@ -15,6 +15,7 @@ describe("W3C — Defaults", () => {
       "no-headingless-sections": true,
       "privsec-section": true,
       "no-http-props": true,
+      "no-unused-vars": false,
       "local-refs-exist": true,
       "check-punctuation": false,
       "check-internal-slots": false,
@@ -53,6 +54,7 @@ describe("W3C — Defaults", () => {
       "no-headingless-sections": true,
       "privsec-section": false,
       "no-http-props": false,
+      "no-unused-vars": false,
       "local-refs-exist": true,
       "check-punctuation": false,
       "fake-linter-rule": "foo",