[Tech debt] Enable eslint-plugin-unicorn on backend with staged fixes #161

Open
opened 2026-11-03 22:15:01 +03:00 by codex-bot · 0 comments
Owner

Context

Split from WealthLedger/main#18 (ESLint 10 and quality checks). The backend part of that Issue was measured and paused: enabling the unicorn preset on backend produces about 1250 findings. Tuning brought it to about 286 findings in src plus test noise, and manual fixes in sync-related code were not safe to bundle with a lint change.

Related: backend#119 (sync reservation/claim protocol). Sync code must not be changed by lint-only work before backend#119 is done.

Agreed decisions (already recorded in main#18)

  • Enable unicorn.configs.recommended on backend.
  • Keep these rules off, with reasons in eslint.config.mjs:
    • unicorn/no-null: Prisma columns and API DTOs use null by contract.
    • unicorn/name-replacements: renaming DI parameters or exported DTO classes changes Nest provider names and OpenAPI schema names.
    • unicorn/consistent-boolean-name: would rename persisted and API field names.
    • unicorn/prefer-set-methods: tsconfig target/lib is ES2023, Set.prototype.union is not in the typings.
    • unicorn/explicit-length-check: conflicts with the no-magic-numbers policy (test/eslint-config.cjs, "0 requires a name").
  • Do not disable rules broadly to get a green lint.

Known pitfalls found during measurement

  • unicorn/prefer-math-min-max on Date values (src/modules/investments/income/income-period.ts): Math.min/max coerces to number and breaks types. Keep < comparison with a narrow eslint-disable.
  • unicorn/prefer-array-index-of on a narrowed union (portfolio-structure.ts): indexOf needs the narrowed type. Keep findIndex with a narrow disable.
  • Autofix (eslint --fix) rewrites about 90 files and must be checked against tsc after each batch. Do not apply it repo-wide in one commit.
  • Autofix of consistent-conditional-object-spread changes ...(x ? {a} : {}) to ...(x && {a}). Runtime equivalence is not verified per site.

Scope

  • In: remaining unicorn findings in backend src, test, scripts, prisma.config.ts; config changes only as narrow, justified disables.
  • Out: changing behaviour of sync, reservation or claim code (backend#119), DTO or DI renames, API or OpenAPI contract changes, lint configuration relaxation beyond the list above.

Acceptance Criteria

  • npm run lint (eslint . --max-warnings=0) passes on backend develop.
  • npm run typecheck:app and npm run typecheck:test pass.
  • npm test (unit) and npm run test:e2e (SQLite) pass with no changed assertions.
  • npm run openapi:check passes (no contract drift).
  • Every eslint-disable added has a concrete reason and is listed in the PR description.
  • No production behaviour change: each PR states that it is lint-only and lists any file where a rewrite was non-trivial.

Suggested split (PRs, each green on its own)

  1. Config and rules that have no code changes, plus the already-known narrow disables.
  2. Test/spec noise (no-await-expression-member, consistent-function-scoping, max-nested-calls, prefer-promise-with-resolvers) in batches of about 10 files.
  3. src batches by module, after backend#119 is merged: prefer-await, no-array-callback-reference, no-array-sort, consistent-class-member-order, prefer-simple-condition-first.

Each batch: typecheck + unit tests before the next batch.

Verification

Not started. Baseline on develop: npm run lint passes; remaining work is to enable the rules.

## Context Split from WealthLedger/main#18 (ESLint 10 and quality checks). The backend part of that Issue was measured and paused: enabling the unicorn preset on backend produces about 1250 findings. Tuning brought it to about 286 findings in `src` plus test noise, and manual fixes in sync-related code were not safe to bundle with a lint change. Related: backend#119 (sync reservation/claim protocol). Sync code must not be changed by lint-only work before backend#119 is done. ## Agreed decisions (already recorded in main#18) - Enable `unicorn.configs.recommended` on backend. - Keep these rules off, with reasons in `eslint.config.mjs`: - `unicorn/no-null`: Prisma columns and API DTOs use `null` by contract. - `unicorn/name-replacements`: renaming DI parameters or exported DTO classes changes Nest provider names and OpenAPI schema names. - `unicorn/consistent-boolean-name`: would rename persisted and API field names. - `unicorn/prefer-set-methods`: `tsconfig` target/lib is ES2023, `Set.prototype.union` is not in the typings. - `unicorn/explicit-length-check`: conflicts with the `no-magic-numbers` policy (`test/eslint-config.cjs`, "0 requires a name"). - Do not disable rules broadly to get a green lint. ## Known pitfalls found during measurement - `unicorn/prefer-math-min-max` on `Date` values (`src/modules/investments/income/income-period.ts`): `Math.min/max` coerces to number and breaks types. Keep `<` comparison with a narrow `eslint-disable`. - `unicorn/prefer-array-index-of` on a narrowed union (`portfolio-structure.ts`): `indexOf` needs the narrowed type. Keep `findIndex` with a narrow disable. - Autofix (`eslint --fix`) rewrites about 90 files and must be checked against `tsc` after each batch. Do not apply it repo-wide in one commit. - Autofix of `consistent-conditional-object-spread` changes `...(x ? {a} : {})` to `...(x && {a})`. Runtime equivalence is not verified per site. ## Scope - In: remaining unicorn findings in backend `src`, `test`, `scripts`, `prisma.config.ts`; config changes only as narrow, justified disables. - Out: changing behaviour of sync, reservation or claim code (backend#119), DTO or DI renames, API or OpenAPI contract changes, lint configuration relaxation beyond the list above. ## Acceptance Criteria - [ ] `npm run lint` (`eslint . --max-warnings=0`) passes on backend develop. - [ ] `npm run typecheck:app` and `npm run typecheck:test` pass. - [ ] `npm test` (unit) and `npm run test:e2e` (SQLite) pass with no changed assertions. - [ ] `npm run openapi:check` passes (no contract drift). - [ ] Every `eslint-disable` added has a concrete reason and is listed in the PR description. - [ ] No production behaviour change: each PR states that it is lint-only and lists any file where a rewrite was non-trivial. ## Suggested split (PRs, each green on its own) 1. Config and rules that have no code changes, plus the already-known narrow disables. 2. Test/spec noise (`no-await-expression-member`, `consistent-function-scoping`, `max-nested-calls`, `prefer-promise-with-resolvers`) in batches of about 10 files. 3. `src` batches by module, after backend#119 is merged: `prefer-await`, `no-array-callback-reference`, `no-array-sort`, `consistent-class-member-order`, `prefer-simple-condition-first`. Each batch: `typecheck` + unit tests before the next batch. ## Verification Not started. Baseline on develop: `npm run lint` passes; remaining work is to enable the rules.
codex-bot added the Kind/ProcessKind/Refactor labels 2026-11-03 22:15:01 +03:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: WealthLedger/backend#161