refactor: move the project's conventions into the linters

.github/workflows/ci.yml

@@ -195,6 +195,10 @@ jobs:
         with:
           playwright: true
           exitOnceUploaded: true
+          # A build on the baseline branch does not accept its own snapshots.
+          # Without this they stay unreviewed, and a later pull request finds no
+          # accepted ancestor to compare against, so it re-reports every one.
+          autoAcceptChanges: main
 
   # A job rather than its own workflow, because `needs` is what makes the publish
   # wait on the gate. The write permissions sit here so the job running untrusted

.stylelintrc.json

@@ -7,6 +7,118 @@
       {
         "message": "Class selectors are camelCase, so a component reads them back as styles.thisName rather than through a bracketed string"
       }
+    ],
+    "property-disallowed-list": [
+      [
+        "left",
+        "right",
+        "margin-left",
+        "margin-right",
+        "padding-left",
+        "padding-right",
+        "border-left",
+        "border-right",
+        "border-left-color",
+        "border-left-style",
+        "border-left-width",
+        "border-right-color",
+        "border-right-style",
+        "border-right-width"
+      ],
+      {
+        "message": "Inline-axis geometry is logical, so one stylesheet serves both reading directions: use inset-inline-start, margin-inline-end, padding-inline or border-inline-end"
+      }
+    ],
+    "declaration-property-value-disallowed-list": [
+      {
+        "text-align": ["/^(?:left|right)$/"],
+        "float": ["/^(?:left|right)$/"],
+        "clear": ["/^(?:left|right)$/"],
+        "outline": [
+          "/(?<![\\w.-])(?:none|0(?:px|rem|em)?|transparent)(?![\\w.%-])/"
+        ],
+        "outline-style": ["/^(?:none|0(?:px|rem|em)?|transparent)$/"],
+        "outline-width": ["/^(?:none|0(?:px|rem|em)?|transparent)$/"],
+        "outline-color": ["/^(?:none|0(?:px|rem|em)?|transparent)$/"]
+      },
+      {
+        "message": "Two kinds of value are disallowed on a property here. An inline-axis value follows the document rather than the page: use text-align: start, float: inline-start or clear: inline-end. And a value that cancels an outline removes the only keyboard affordance on that control: the focus ring is drawn once, globally, from the ring token, and no component cancels it"
+      }
+    ],
+    "selector-disallowed-list": [
+      ["/:dir\\(/"],
+      {
+        "message": "The :dir() pseudo-class landed in Chrome 120 against this application's floor of 111, so it ships inert in the very browsers the floor exists to name: select the direction on the attribute, as [dir=\"rtl\"] .navButton svg does"
+      }
     ]
-  }
+  },
+  "overrides": [
+    {
+      "files": ["src/**/*.{css,scss}"],
+      "rules": {
+        "color-no-hex": [
+          true,
+          {
+            "message": "Colour in a component stylesheet arrives through var(--color-*) and nowhere else, so a theme switch moves every value at once"
+          }
+        ],
+        "color-named": [
+          "never",
+          {
+            "message": "Colour in a component stylesheet arrives through var(--color-*) and nowhere else; a keyword is as fixed as a hex and flips with no theme"
+          }
+        ],
+        "declaration-property-unit-allowed-list": [
+          {
+            "/^(margin|padding)(-(block|inline))?(-(start|end))?$/": ["rem"],
+            "/^(margin|padding)-(top|bottom|left|right)$/": ["rem"],
+            "/^(gap|row-gap|column-gap|font-size)$/": ["rem"]
+          },
+          {
+            "message": "Spacing and type are authored in rem through a token, so the layout follows the reader's browser font-size setting: reach for var(--space-*) or var(--font-size-*)"
+          }
+        ],
+        "function-disallowed-list": [
+          [
+            "rgb",
+            "rgba",
+            "hsl",
+            "hsla",
+            "hwb",
+            "lab",
+            "lch",
+            "oklab",
+            "oklch",
+            "color-mix",
+            "color"
+          ],
+          {
+            "message": "Colour in a component stylesheet arrives through var(--color-*) and nowhere else; a functional notation is as fixed as a hex and flips with no theme"
+          }
+        ]
+      }
+    },
+    {
+      "files": ["src/index.css"],
+      "rules": {
+        "color-no-hex": null,
+        "function-disallowed-list": null
+      }
+    },
+    {
+      "files": ["src/styles/_visually-hidden.scss"],
+      "rules": {
+        "declaration-property-unit-allowed-list": [
+          {
+            "/^padding(-(block|inline))?(-(start|end))?$/": ["rem"],
+            "/^padding-(top|bottom|left|right)$/": ["rem"],
+            "/^(gap|row-gap|column-gap|font-size)$/": ["rem"]
+          },
+          {
+            "message": "Spacing and type are authored in rem through a token. Only margin is exempt here, for the off-screen clip's margin: -1px, which is a magnitude a unit allowed-list cannot express"
+          }
+        ]
+      }
+    }
+  ]
 }

eslint.config.js

@@ -24,13 +24,88 @@ const TEST_SCAFFOLDING_IMPORT = {
     "src/test/ holds test scaffolding and is excluded from coverage. Importing it from application code moves executing code outside the coverage gate.",
 };
 
+// The four attributes whose string value a reader perceives. `aria-sort` and
+// `aria-live` carry strings in that layer too and are deliberately outside the
+// set: both take a value the standard defines and assistive technology matches
+// on, so translating either would break the feature rather than localize it.
+//
+// no-restricted-syntax is configured per rule and not per selector, exactly as
+// no-restricted-imports is above, so each selector set is declared once here and
+// composed per block below. A block that named the rule and forgot a set would
+// silently unrestrict it for every file that block matches.
+const READER_FACING_ATTRIBUTE = {
+  selector:
+    'JSXAttribute[name.name=/^(aria-label|title|placeholder|alt)$/]:matches([value.type="Literal"], [value.expression.type="Literal"], [value.expression.type="TemplateLiteral"][value.expression.expressions.length=0])',
+  message:
+    "src/components/ renders any collection for any reader, so a string here is a claim about which reader. Add the entry to the catalogs and read it off DataTableLabels, PaginationLabels or SearchInputLabels.",
+};
+
+// Every way a file asks the platform for a locale: a namespace construction, and
+// a call to one of the six value-level helpers that read a locale from the
+// machine when called with no argument. A fifth surface resolving a locale of its
+// own reintroduces the defect the locale layer closed, and does it invisibly on a
+// machine whose own preference is the base tag.
+//
+// Matched on the tree rather than on the text, which matters twice here: a
+// namespace named inside a comment is not a call site, and a type annotation
+// naming the same constructor is not one either.
+const LOCALE_CALL_SITE = [
+  {
+    selector:
+      ':matches(NewExpression, CallExpression)[callee.object.name="Intl"]',
+    message:
+      "src/i18n/format.ts is the one module that builds a platform locale object, so its caches hold one instance per resolved tag. Reach for collatorFor, numberFormatFor or pluralRulesFor instead.",
+  },
+  {
+    selector:
+      "CallExpression[callee.property.name=/^(localeCompare|toLocaleString|toLocaleDateString|toLocaleTimeString|toLocaleLowerCase|toLocaleUpperCase)$/]",
+    message:
+      "This reads a locale from the machine rather than the one the application resolved, and builds a formatter per call. Take a collator or a formatter from src/i18n/format.ts instead.",
+  },
+];
+
+// The way the mirror rule is undone in JavaScript rather than in CSS: a branch
+// choosing between two glyph components on the reading direction. It is
+// invisible to the stylelint rules that hold the stylesheets, because it is not
+// CSS, and invisible to the literal rule above, because a glyph component is
+// neither a text child nor a string.
+const DIRECTION_BRANCH = [
+  {
+    selector:
+      "ConditionalExpression[consequent.type=/^JSX(Element|Fragment)$/][alternate.type=/^JSX(Element|Fragment)$/]",
+    message:
+      "Two glyph components chosen on a condition are a prop, a branch and a coverage line for what one transform: scaleX(-1) under an attribute selector already does. Mirror in the stylesheet.",
+  },
+  {
+    // Both sides, by shape: esquery cannot scope :has() to a named property,
+    // so an else-if chain and a return that is not first in its block are out
+    // of reach. Recorded in GUARD-DISPOSITION.md rather than answered with a
+    // looser selector, because the guarded render this used to flag is the
+    // false positive that gets a rule deleted.
+    selector:
+      "IfStatement" +
+      ":matches([consequent.argument.type=/^JSX(Element|Fragment)$/], [consequent.body.0.argument.type=/^JSX(Element|Fragment)$/])" +
+      ":matches([alternate.argument.type=/^JSX(Element|Fragment)$/], [alternate.body.0.argument.type=/^JSX(Element|Fragment)$/])",
+    message:
+      "Two glyph components returned from two branches are a prop, a branch and a coverage line for what one transform: scaleX(-1) under an attribute selector already does. Mirror in the stylesheet.",
+  },
+];
+
 export default defineConfig([
   // Build output and test-runner output, not authored source. `eslint .` walks
   // the working tree, so without this the gate reports parse errors for an
   // emitted bundle, a coverage report, and the page resources the visual spec
   // archives under test-results/.
   {
-    ignores: ["dist/", "coverage/", "test-results/", "playwright-report/"],
+    ignores: [
+      "dist/",
+      "coverage/",
+      "test-results/",
+      "playwright-report/",
+      "junit/",
+      "src/__screenshots__/",
+      ".vitest-attachments/",
+    ],
   },
   {
     files: ["**/*.{js,mjs,cjs,jsx,mjsx,ts,tsx,mtsx}"],
@@ -63,6 +138,9 @@ export default defineConfig([
       },
     },
     rules: {
+      // Spread again: the key above replaces the recommended rules wholesale
+      // rather than merging with them, which silently disabled all 22.
+      ...react.configs.flat.recommended.rules,
       "react/react-in-jsx-scope": "off",
     },
   },
@@ -116,6 +194,59 @@ export default defineConfig([
       ],
     },
   },
+  // The formatter module is exempt because it is the module the rule names. The
+  // test globs are exempt because a test asserting a formatted string has to
+  // compute its expectation through the platform rather than type it: the French
+  // group separator is a narrow no-break space, and a typed literal fails on a
+  // difference no terminal renders. src/test/ is not exempt: it holds executing
+  // helpers rather than assertions, and the sweep it drives is shipped code's
+  // reach.
+  {
+    files: ["src/**/*.{ts,tsx}"],
+    ignores: [
+      "src/i18n/format.ts",
+      "src/**/*.test.{ts,tsx}",
+      "src/**/*.test-d.ts",
+    ],
+    rules: {
+      "no-restricted-syntax": ["error", ...LOCALE_CALL_SITE],
+    },
+  },
+  // The same layer rule one level down from the imports above: a hardcoded
+  // sentence needs no import, so the two import rules cannot see it. ignoreProps
+  // is deliberate, because the attribute half is narrower than every prop and is
+  // the selector beside it.
+  {
+    files: ["src/components/**/*.{ts,tsx}"],
+    ignores: ["src/**/*.test.{ts,tsx}", "src/**/*.test-d.ts"],
+    rules: {
+      "react/jsx-no-literals": [
+        "error",
+        { noStrings: true, ignoreProps: true, allowedStrings: [] },
+      ],
+      "no-restricted-syntax": [
+        "error",
+        READER_FACING_ATTRIBUTE,
+        ...LOCALE_CALL_SITE,
+      ],
+    },
+  },
+  // The component holding the four glyphs that mean a direction. All three
+  // selector sets, because no-restricted-syntax is configured per rule: this
+  // block replaces the two above outright for this file, and naming only the
+  // direction set would unrestrict the other two for exactly the component most
+  // likely to regain a literal.
+  {
+    files: ["src/components/DataTable/Pagination.tsx"],
+    rules: {
+      "no-restricted-syntax": [
+        "error",
+        READER_FACING_ATTRIBUTE,
+        ...LOCALE_CALL_SITE,
+        ...DIRECTION_BRANCH,
+      ],
+    },
+  },
   // The type-aware rules need declarations to reason about, and the plain
   // JavaScript in this tree has none: a build script reading an untyped CSV
   // parser and a flat config importing a plugin that ships no types. Every

playwright.config.ts

@@ -6,8 +6,10 @@ import globalSetup from "./e2e/globalSetup";
 // all. The hundred percent threshold stays measured over the deterministic
 // jsdom project alone, which is the project the only command asking for
 // coverage is scoped to. The other half of that agreement lives in the coverage
-// block of vite.config.ts, and the guard that keeps the end-to-end script from
-// quietly growing a coverage flag lives in src/toolchain.test.ts.
+// block of vite.config.ts. A coverage flag added to this script would emit a
+// second report into the directory the static analysis import reads, and
+// nothing fails on that flag appearing, so the carve rests on the scripts being
+// read rather than on a gate.
 //
 // Two alternatives were rejected. Merging this runner's numbers into the one
 // report the static analysis import reads would cost a hand-rolled protocol

src/App.tsx

@@ -9,15 +9,8 @@ import { RootLayout } from "./features/RootLayout";
 import { CityTable, parseSearchTerm } from "./features/CityTable";
 
 const App = () => {
-  // Seeded from the address so the first render already asks for the right
-  // rows. Without this the effect below issues a request for the empty term and
-  // then immediately another for the restored one, so every shared link costs
-  // two requests on a cold start; the stale-result guard makes that survivable
-  // rather than correct.
-  //
-  // This is a read of the address and stays one. The single write lives with
-  // the table's view state, one layer down, and adding a second writer here is
-  // what would make the address a thing two components argue over.
+  // Seeded from the address, or every shared link costs two requests on a cold
+  // start. A read and it stays one: the single write is one layer down.
   const [searchTerm, setSearchTerm] = useState(() =>
     parseSearchTerm(window.location.search),
   );
@@ -26,11 +19,8 @@ const App = () => {
   const { catalog, tag } = useLocale();
 
   useEffect(() => {
-    // The asynchronous work sits directly in the effect rather than behind a
-    // memoized callback, because that is what lets this one variable guard
-    // every state write below, the settle handler included. A result that
-    // arrives after the cleanup has run belongs to a search the user has
-    // already moved past, so it is dropped rather than rendered.
+    // The fetch sits in the effect so this one flag guards every write below.
+    // A result arriving after cleanup belongs to a search already moved past.
     let ignore = false;
 
     // Clear the last failure as the new attempt starts, so a retry does not
@@ -47,10 +37,8 @@ const App = () => {
         if (err instanceof Error) {
           dispatch({ type: "failed", error: err });
         } else {
-          // A rejection carrying no error at all, which the loader never
-          // produces and a stubbed seam can. It enters state as a dataset
-          // error so what the reducer holds is always something the translator
-          // below has a sentence for.
+          // A rejection carrying no error, which only a stubbed seam produces.
+          // It enters state as a dataset error so a sentence always exists.
           dispatch({
             type: "failed",
             error: new DatasetError(
@@ -71,29 +59,20 @@ const App = () => {
     };
   }, [searchTerm, retryAttempt]);
 
-  // Receives the term the search box has settled on rather than every
-  // keystroke: the debounce lives with the box now, so what arrives here is
-  // already the term worth issuing a request for. Memoized because the child
-  // holds on to it, and an empty dependency array is what makes that hold safe.
+  // Receives the settled term rather than every keystroke. Memoized with an
+  // empty dependency array, because the child holds on to it.
   const handleSearchChange = useCallback((term: string) => {
     setSearchTerm(term);
   }, []);
 
-  // The next keystroke would re-run the search too, but nothing on screen says
-  // so, which leaves a reader of the error with no way forward. Retrying on a
-  // timer with backoff was rejected instead: it hides a misconfigured
-  // deployment behind a spinner and re-downloads several megabytes of city data
-  // with nobody watching.
+  // Deliberately manual. A backoff timer would hide a misconfigured deployment
+  // behind a spinner and re-download the dataset with nobody watching.
   const handleRetry = useCallback(() => {
     dispatch({ type: "retry" });
   }, []);
 
-  // Derived here, during render, rather than at the catch above, and that is
-  // the whole reason this line is not two lines further up. The catch is inside
-  // the fetch effect, so reading the catalog there would make the locale a
-  // dependency of the effect and a reader changing language would re-issue the
-  // search. Here the catalog is already in scope and the effect's dependencies
-  // are untouched.
+  // Derived during render rather than at the catch, which is inside the fetch
+  // effect: reading the catalog there would make the locale a dependency of it.
   const errorMessage =
     error === null ? null : datasetErrorText(error, catalog, tag);