chore(lint): let the linter see tests/ and scripts/ (#783)

`npm run lint` was `eslint src`, so two whole trees were never linted. Across
the fleet that hid roughly 3,900 errors, none of which any CI leg had shown.

Most of it was the config, not the code, and the same two defects were in
every app.

**scripts/ had no config block at all.** These are CommonJS Node CLI checkers,
and flat config defaults every `.js` to ESM with browser-ish globals, so eslint
read the CommonJS wrapper itself as undefined identifiers: `require`,
`process`, `__dirname`, `__filename`, `module`. A `scripts/**` block now
declares the environment. Declaring beats suppressing here: `no-undef` is the
rule that catches a genuinely misspelled identifier, and dozens of fake
findings would bury a real one. A second block covers `scripts/**/*.mjs`, which
is ESM and needs Node's globals without the CommonJS wrapper.

**The tests block applied a non-TypeScript-aware rule to TypeScript.** It named
`tests/**/*.ts` while setting the CORE `no-unused-vars`, which v9 deliberately
turns off for `.ts` in favour of the `@typescript-eslint` version. The core
rule reads the parameter names inside a function TYPE as bindings, so

    t?: (app: string, key: string) => string

reports `app` and `key` as unused variables, and every unused `catch (e)` in a
`.ts` spec reports twice. The block is split now: `.js`/`.mjs` on the core
rule, `.ts`/`.tsx` on the TypeScript one, same patterns on both.

Also: stale `eslint-disable` comments naming plugins eslint 10 no longer
registers, which are themselves errors ("Definition for rule ... was not
found"), and a rule that must not parse shell scripts.

The genuinely real findings were the useful part: dead locals, unused imports,
dead helper functions, unused `catch` bindings, extensionless relative imports,
and a handful of `== null` comparisons spelled out so they still match null AND
undefined.

Verified per app: `npm run lint` 0 errors over src + tests + scripts,
`prettier --check` clean, and the unit suite still green.

Author: rubenvdlinde

eslint.config.mjs

@@ -144,11 +144,32 @@ export default [
 			'no-console': 'off',
 			'n/no-process-exit': 'off',
 			'n/hashbang': 'off',
-			// `_` / `__` as a deliberate throwaway binding — `catch (_)`, a
-			// discarded destructuring slot. Narrow on purpose: the pattern matches
-			// UNDERSCORES ONLY, so a real name that happens to start with `_` is
-			// still reported. v9 drives plain `.js` through the CORE rule (the
-			// `@typescript-eslint` swap is per-file-type), so it is set here.
+			// Tests import devDependencies by definition; this rule is about what
+			// ships in the published package, which tests/ never does.
+			'n/no-unpublished-import': 'off',
+		},
+	},
+
+	{
+		// `_` / `__` as a deliberate throwaway binding — `catch (_)`, a discarded
+		// destructuring slot. Narrow on purpose: the pattern matches UNDERSCORES
+		// ONLY, so a real name that happens to start with `_` is still reported.
+		//
+		// 🔴 `.js` / `.mjs` ONLY, NOT `.ts`. The CORE rule is not TypeScript-aware:
+		// applied to a `.ts` file it reads the parameter NAMES inside a function
+		// TYPE as bindings and reports them unused. Measured on humaniq —
+		//
+		//   t?: (app: string, key: string) => string
+		//
+		// produced four `no-unused-vars` errors for `app` and `key`, which are
+		// documentation, not variables. The same mis-scoping made every unused
+		// `catch (e)` in a `.ts` spec report TWICE, once per rule.
+		//
+		// v9 already turns the core rule off for `.ts` and drives
+		// `@typescript-eslint/no-unused-vars` instead; naming `.ts` here switched
+		// it back on. TypeScript files are handled by the block below.
+		files: ['tests/**/*.js', 'tests/**/*.mjs'],
+		rules: {
 			'no-unused-vars': [
 				'error',
 				{
@@ -165,12 +186,117 @@ export default [
 					ignoreRestSiblings: true,
 				},
 			],
-			// Tests import devDependencies by definition; this rule is about what
-			// ships in the published package, which tests/ never does.
-			'n/no-unpublished-import': 'off',
 		},
 	},
 
+	{
+		// The TypeScript half of the block above. Same intent, same patterns, on
+		// the rule that actually understands the language: it knows a name inside
+		// a function type is not a binding, so type annotations stay quiet while a
+		// genuinely dead local is still reported.
+		files: ['tests/**/*.ts', 'tests/**/*.tsx'],
+		rules: {
+			'@typescript-eslint/no-unused-vars': [
+				'error',
+				{
+					varsIgnorePattern: '^_+$',
+					caughtErrors: 'all',
+					caughtErrorsIgnorePattern: '^_+$',
+					argsIgnorePattern: '^_',
+					ignoreRestSiblings: true,
+				},
+			],
+		},
+	},
+
+	{
+		// 🔴 Node-side CLI tooling under `scripts/`, which is COMMONJS. Flat
+		// config defaults every `.js` to ESM with browser-ish globals, so without
+		// this block eslint reports the CommonJS wrapper itself as undefined
+		// identifiers. Measured on this app: 52 of the 233 errors under
+		// `tests/` + `scripts/` were `no-undef`, ALL of them in `scripts/`, and
+		// all five names were the environment rather than a typo — `process` 23,
+		// `require` 20, `__dirname` 6, `__filename` 2, `module` 1.
+		//
+		// This is describing the environment, not relaxing a rule, and it is the
+		// same argument the test-globals block below makes: declaring them keeps
+		// `no-undef` able to do its real job, which is catching a genuinely
+		// misspelled identifier. Suppressing the rule instead would bury that.
+		//
+		// `no-console` is off because printing its report is what a CLI checker
+		// is FOR.
+		//
+		// 🔴 NO `n/*` ENTRIES HERE, DELIBERATELY. `eslint-plugin-n` is NOT
+		// registered for these files under eslint 10 + @nextcloud/eslint-config
+		// 9, so `'n/no-process-exit': 'off'` would be dead config that reads as
+		// if it were doing something. Measured both ways on this app: 0 `n/`
+		// findings with the entries and 0 without.
+		//
+		// What DID report was the opposite — four `scripts/*.js` carried
+		// `/* eslint-disable n/no-process-exit */` and `/* eslint-disable
+		// n/shebang */` left over from the eslintrc era, and an inline disable
+		// naming an unregistered plugin is itself an error ("Definition for rule
+		// 'n/shebang' was not found"). Those 8 comments are removed; do not add
+		// `n/*` rules back to replace them.
+		//
+		// ⚠️ `.js` and `.cjs` ONLY. A `scripts/*.mjs` is genuinely ESM and must
+		// keep the default `sourceType`, or `import` stops parsing there.
+		files: ['scripts/**/*.js', 'scripts/**/*.cjs'],
+		languageOptions: {
+			sourceType: 'commonjs',
+			globals: {
+				require: 'readonly',
+				module: 'writable',
+				exports: 'writable',
+				process: 'readonly',
+				__dirname: 'readonly',
+				__filename: 'readonly',
+				console: 'readonly',
+				Buffer: 'readonly',
+				global: 'readonly',
+				URL: 'readonly',
+				TextEncoder: 'readonly',
+				TextDecoder: 'readonly',
+			},
+		},
+		rules: {
+			'no-console': 'off',
+		},
+	},
+
+	{
+		// The ESM half of the block above. A `scripts/*.mjs` is genuinely a module
+		// and must keep the default `sourceType`, so it gets Node's globals but
+		// none of the CommonJS wrapper. Measured: `process` reported undefined 2x
+		// in hermiq's generate-opengemeenten-icons.mjs and 4x in openregister's
+		// l10n/runtime-check.mjs, which the `.js`/`.cjs` block deliberately does
+		// not match.
+		files: ['scripts/**/*.mjs', 'tests/**/*.mjs'],
+		languageOptions: {
+			globals: {
+				process: 'readonly',
+				console: 'readonly',
+				Buffer: 'readonly',
+				global: 'readonly',
+				URL: 'readonly',
+				TextEncoder: 'readonly',
+				TextDecoder: 'readonly',
+			},
+		},
+		rules: {
+			'no-console': 'off',
+		},
+	},
+
+	{
+		// eslint must not try to PARSE a shell script. `tests/e2e/seed.test.sh`
+		// matches the `**/*.test.*` glob some presets use, and eslint then reads
+		// it as JavaScript and reports "Parsing error: Unexpected character" —
+		// a finding about a file it should never have opened.
+		ignores: ['**/*.sh', '**/*.bash'],
+	},
+
+
 	{
 		// Test globals. Several apps keep their spec files INSIDE `src/`, which the
 		// lint script scans, and neither `@nextcloud/eslint-config` nor the runner

package.json

@@ -10,7 +10,7 @@
     "build": "NODE_ENV=production webpack --config webpack.config.js --progress",
     "dev": "NODE_ENV=development webpack --config webpack.config.js --progress",
     "watch": "NODE_ENV=development webpack --config webpack.config.js --progress --watch",
-    "lint": "eslint src",
+    "lint": "eslint src tests scripts",
     "lint-fix": "npm run lint -- --fix",
     "stylelint": "stylelint \"src/**/*.{vue,scss,css}\"",
     "format": "prettier --check \"**/*.{js,ts,vue,css,scss}\"",

scripts/build-l10n-js.js

@@ -101,6 +101,9 @@ function renderJs(id, translations, pluralForm) {
 	].join('\n')
 }
 
+/**
+ *
+ */
 function main() {
 	const check = process.argv.includes('--check')
 	const id = appId()

scripts/check-schema-l10n.js

@@ -113,6 +113,9 @@ function collect(node, where, sink) {
 	for (const value of Object.values(node)) collect(value, where, sink)
 }
 
+/**
+ *
+ */
 function main() {
 	const update = process.argv.includes('--update')
 	const list = process.argv.includes('--list')

tests/e2e/credential-verify.spec.ts

@@ -1,4 +1,4 @@
-import { test, expect } from './fixtures'
+import { expect, test } from './fixtures.ts'
 
 /**
  * Credential verification page tests.

tests/e2e/docs-screenshots.spec.ts

@@ -37,9 +37,11 @@
  * Pattern reference: ADR-030 (hydra/openspec/architecture/).
  */
 
-import { test, expect, type Page } from '@playwright/test'
-import * as path from 'path'
+import type { Page } from '@playwright/test'
+
+import { expect, test } from '@playwright/test'
 import * as fs from 'fs'
+import * as path from 'path'
 
 const SHOT_ROOT = path.resolve(
 	__dirname,

tests/e2e/fixtures.ts

@@ -1,4 +1,6 @@
-import { test as base, Page } from '@playwright/test'
+import type { Page } from '@playwright/test'
+
+import { test as base } from '@playwright/test'
 
 /**
  * Navigate to the app and verify the session is active.

tests/e2e/global-setup.ts

@@ -2,7 +2,7 @@ import { chromium } from '@playwright/test'
 import { execFileSync } from 'child_process'
 import * as fs from 'fs'
 import * as path from 'path'
-import { baseUrl } from './base-url'
+import { baseUrl } from './base-url.ts'
 
 /** Repo root — `tests/e2e/` is two levels down. */
 const APP_ROOT = path.resolve(__dirname, '..', '..')
@@ -152,7 +152,7 @@ async function globalSetup(): Promise<void> {
 						'cn-walkthrough-seen:learniq',
 						'999.0.0',
 					)
-				} catch (e) {
+				} catch {
 					// localStorage unavailable — specs fall back to dismissing by hand.
 				}
 			})

tests/e2e/l10n-browser-catalogue.spec.ts

@@ -39,11 +39,10 @@
  * nothing about apps whose translations differ.
  */
 
+import { expect, test } from '@playwright/test'
 import { readFileSync } from 'node:fs'
 import path from 'node:path'
 
-import { expect, test } from '@playwright/test'
-
 /**
  * The app id this repo declares. Resolved from the repo root by walking up
  * from this file, so it does not depend on the working directory playwright

tests/e2e/pages.spec.ts

@@ -1,4 +1,4 @@
-import { test, expect } from './fixtures'
+import { expect, test } from './fixtures.ts'
 
 /**
  * Page-level smoke tests — navigate to every manifest route and assert that: