chore(Lint): extend FE linter rules

.github/workflows/frontend-pull-request.yml

@@ -38,6 +38,10 @@ jobs:
   lint:
     name: Lint changed files
     runs-on: ubuntu-latest
+    permissions:
+      contents: read
+      # reviewdog reports Stylelint results as a check run.
+      checks: write
 
     defaults:
       run:
@@ -64,3 +68,15 @@ jobs:
         env:
           ESLINT_PLUGIN_DIFF_COMMIT: ${{ github.event.pull_request.base.sha }}
         run: npm run lint
+
+      # Fails only on problems in lines this PR added, so existing styles
+      # don't fail it.
+      - name: Lint changed SCSS lines
+        uses: reviewdog/action-stylelint@v1.33.0
+        with:
+          workdir: frontend
+          # Quoted so Stylelint expands the glob, not the action's shell.
+          stylelint_input: "'**/*.scss'"
+          reporter: github-pr-check
+          filter_mode: added
+          fail_level: error

.husky/pre-commit

@@ -18,4 +18,5 @@ else
   cd ./frontend
 fi
 
-npx lint-staged --allow-empty
+# LINT_STAGED makes eslint check only the staged lines (see .eslintrc.js).
+LINT_STAGED=true npx lint-staged --allow-empty

frontend/.eslintignore

@@ -6,6 +6,5 @@ public
 tests_output
 web/static
 web/project/libs.js
-e2e
 webpack
 api

frontend/.eslintrc.js

@@ -1,3 +1,96 @@
+// Patterns we keep catching in review. CI and pre-commit lint changed lines
+// only, so existing uses don't fail a PR. For a real exception, disable the
+// rule on that line and say why.
+const colourLiterals = [
+  {
+    message: 'Use a colour token, not a hex, rgb or hsl literal.',
+    selector:
+      'Literal[value=/^\\s*(#([0-9a-fA-F]{3,4}|[0-9a-fA-F]{6}|[0-9a-fA-F]{8})$|(rgba?|hsla?)\\()/]',
+  },
+  {
+    message: 'Use a colour token, not a hex, rgb or hsl literal.',
+    selector:
+      'TemplateElement[value.raw=/#[0-9a-fA-F]{6}\\b|(rgba?|hsla?)\\(/]',
+  },
+]
+
+// The project's own SVGs, through Icon, are the only icon set.
+const onlyIcon = 'Add the SVG to the project and render it through Icon.'
+const iconLibraries = {
+  paths: [
+    { importNames: ['IonIcon'], message: onlyIcon, name: '@ionic/react' },
+  ],
+  patterns: [{ group: ['ionicons', 'ionicons/*'], message: onlyIcon }],
+}
+
+const jsxPatterns = [
+  {
+    message: onlyIcon,
+    selector: "JSXOpeningElement[name.name='IonIcon']",
+  },
+  {
+    message:
+      'Use a SCSS class with tokens. For a runtime value, set a CSS custom property.',
+    selector: "JSXAttribute[name.name='style']",
+  },
+  {
+    message: 'Give the element a role or accessible name instead of data-test.',
+    selector: "JSXAttribute[name.name='data-test']",
+  },
+  {
+    message: 'Use IconButton.',
+    selector:
+      "JSXAttribute[name.name='className'] Literal[value=/(^|\\s)btn-with-icon(\\s|$)/]",
+  },
+  {
+    message: 'Use IconButton.',
+    selector:
+      "JSXOpeningElement[name.name='Button'] > JSXAttribute[name.name='theme'][value.value='icon']",
+  },
+  {
+    message: 'Pass icons as children.',
+    selector: 'JSXAttribute[name.name=/^icon(Left|Right)$/]',
+  },
+  {
+    message: "Use BareButton instead of role='button'.",
+    selector: "JSXAttribute[name.name='role'][value.value='button']",
+  },
+  {
+    message:
+      'Use the Bootstrap 5 name (fw-*, ms-*, me-*, ps-*, pe-*, float-start, text-end).',
+    selector:
+      "JSXAttribute[name.name='className'] Literal[value=/(^|\\s)(font-weight-[a-z]+|float-(left|right)|text-(left|right)|[mp][lr]-([0-9]|auto))(\\s|$)/]",
+  },
+]
+
+const e2eSelectors = [
+  {
+    message: 'Use getByRole or getByLabel.',
+    selector: "CallExpression[callee.name='byId']",
+  },
+  {
+    message: 'Use getByRole or getByLabel.',
+    selector:
+      "CallExpression[callee.property.name='locator'] :matches(Literal[value=/data-test/], TemplateElement[value.raw=/data-test/])",
+  },
+]
+
+// A warning, not an error: some data is only in the stores until the RTK
+// migration, and touching a line that already reads one shouldn't fail CI.
+const fluxStores = {
+  group: ['common/stores/*', '**/stores/*-store'],
+  message: 'Read data through RTK Query services, not Flux stores.',
+}
+const fetchCalls = {
+  message: 'Add an RTK Query endpoint in common/services/ instead of fetch().',
+  selector:
+    "CallExpression[callee.object.name=/^(window|globalThis|self)$/][callee.property.name='fetch']",
+}
+const fetchGlobal = {
+  message: 'Add an RTK Query endpoint in common/services/ instead of fetch().',
+  name: 'fetch',
+}
+
 module.exports = {
   'env': {
     'browser': true,
@@ -11,9 +104,9 @@ module.exports = {
     'plugin:prettier/recommended',
     'plugin:@dword-design/import-alias/recommended',
     'plugin:storybook/recommended',
-    // Lints only changed lines in CI (no-op locally), so the pre-existing
-    // eslint backlog doesn't fail PRs that merely touch a file.
-    'plugin:diff/ci',
+    // Lints only changed lines in CI and in the pre-commit hook (which sets
+    // LINT_STAGED), so the pre-existing backlog doesn't fail a commit or PR.
+    process.env.LINT_STAGED ? 'plugin:diff/staged' : 'plugin:diff/ci',
   ],
   'globals': {
     '$': true,
@@ -99,6 +192,45 @@ module.exports = {
     'zE': true,
   },
   'ignorePatterns': ['server/index.js', 'next.config.js', 'babel.config.js'],
+  'overrides': [
+    {
+      // Where raw colour values are defined or are brand colours.
+      'files': [
+        '.storybook/**',
+        'common/constants.ts',
+        'common/theme/**',
+        'documentation/**',
+        'web/components/icons/**',
+      ],
+      'rules': {
+        'no-restricted-syntax': ['error', ...jsxPatterns, fetchCalls],
+      },
+    },
+    {
+      'files': ['common/stores/**', 'common/providers/**'],
+      'rules': {
+        '@typescript-eslint/no-restricted-imports': 'off',
+      },
+    },
+    {
+      // The legacy request layer RTK Query is built on.
+      'files': ['common/data/base/**'],
+      'rules': {
+        'no-restricted-globals': 'off',
+      },
+    },
+    {
+      // E2E was outside lint until these rules, so it keeps its own formatting
+      // and relative imports.
+      'files': ['e2e/**'],
+      'rules': {
+        '@dword-design/import-alias/prefer-alias': 'off',
+        'no-restricted-globals': 'off',
+        'no-restricted-syntax': ['error', ...colourLiterals, ...e2eSelectors],
+        'prettier/prettier': 'off',
+      },
+    },
+  ],
   'parser': '@typescript-eslint/parser',
   'parserOptions': {
     'ecmaFeatures': {
@@ -129,9 +261,18 @@ module.exports = {
     ],
     '@typescript-eslint/ban-ts-comment': 'off',
     '@typescript-eslint/ban-types': 'off',
+    '@typescript-eslint/consistent-type-assertions': [
+      'error',
+      { 'assertionStyle': 'never' },
+    ],
     '@typescript-eslint/no-empty-function': 'off',
     '@typescript-eslint/no-explicit-any': 'off',
     '@typescript-eslint/no-namespace': 'off',
+    '@typescript-eslint/no-non-null-assertion': 'error',
+    '@typescript-eslint/no-restricted-imports': [
+      'warn',
+      { 'patterns': [fluxStores] },
+    ],
     '@typescript-eslint/no-unused-vars': 'off',
     '@typescript-eslint/no-var-requires': 'off',
     'default-case': 'error',
@@ -144,6 +285,14 @@ module.exports = {
     'no-empty': 'off',
     'no-empty-pattern': 'off',
     'no-nested-ternary': 'error',
+    'no-restricted-globals': ['error', fetchGlobal],
+    'no-restricted-imports': ['error', iconLibraries],
+    'no-restricted-syntax': [
+      'error',
+      ...colourLiterals,
+      ...jsxPatterns,
+      fetchCalls,
+    ],
     'no-unused-vars': 'off',
     'no-var': 'error',
     'object-curly-spacing': ['error', 'always'],
@@ -158,6 +307,7 @@ module.exports = {
       },
     ],
     'react/jsx-uses-react': 'off',
+    'react/no-danger': 'error',
     'react/no-direct-mutation-state': 'off',
     'react/no-find-dom-node': 'off',
     'react/no-string-refs': 'off',

frontend/.stylelintrc.cjs

@@ -0,0 +1,27 @@
+// Only added lines are checked, by reviewdog in CI, so existing styles don't
+// fail a PR.
+module.exports = {
+  customSyntax: 'postcss-scss',
+  overrides: [
+    {
+      // Where raw colour values and the dark theme are defined.
+      files: [
+        '.storybook/**',
+        'documentation/**',
+        'web/styles/_primitives.scss',
+        'web/styles/_tokens.scss',
+      ],
+      rules: {
+        'color-no-hex': null,
+        'selector-disallowed-list': null,
+      },
+    },
+  ],
+  rules: {
+    'color-no-hex': [true, { message: 'Use a colour token, not a hex literal.' }],
+    'selector-disallowed-list': [
+      [/\.dark(?![\w-])/],
+      { message: 'Use a token whose value already changes in dark mode.' },
+    ],
+  },
+}