oclif / oclif/core

eslint --fix corrupts source and desyncs the suppressions baseline because lint rules conflict with the repo's actual TS setup

Open
#1,644 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

BUG P2
Dominant language
TypeScript
Stars
316
Forks
98
Avg merge
12h 32m
Merged PRs (30d)
2

Description

Summary

The repo's ESLint configuration enables autofixable rules that fight each other and disagree with how the source is actually written and loaded. Running eslint --fix, which the lint-staged pre-commit hook does on every commit, can silently rewrite source into a state that fails tsc and breaks tests, while report-mode yarn lint stays green. The result is that a routine commit touching otherwise-fine files can land broken code.

Impact

  • A commit that stages a handful of source files triggers eslint --fix, which can rewrite those files' imports and types, break the build (tsc) and the test suite, yet still pass the pre-commit hook (the hook runs ESLint, not tsc or tests).
  • yarn lint (report mode) and the hook's eslint --fix (fix mode) disagree, so "lint is green" does not mean "a commit will be safe."
  • Fixing lint findings one file at a time surfaces a new, unrelated failure each time, because the underlying conflicts are config-level, not file-level.

Root cause 1: autofixable rules conflict with the actual source layout, and the baseline hides it

Several rules are set to error with autofixers, but the committed source is written the opposite way and the violations are mass-suppressed in eslint-suppressions.json rather than fixed. Two concrete instances:

  • n/file-extension-in-import: ["error", "always"] requires .js extensions on relative imports, but the source is extension-less TypeScript loaded via ts-node/@oclif/test at test time. The baseline suppresses this rule across ~134 files (~466 occurrences). When eslint --fix touches a file, it "fixes" the import by adding .js, which the test-time loader cannot resolve, so module loading fails and commands/hooks silently produce no output.
  • @typescript-eslint/no-unsafe-return / no-null (and related type-aware rules) get autofixed on individual files (for example rewriting | null return types to | undefined) without touching the corresponding value expressions, producing tsc errors like Type 'null' is not assignable to type 'string | undefined'.

Because these are suppressed globally, they are invisible until an unrelated --fix run converts a suppression into an autofix on one file and desyncs it from the rest of the codebase.

Root cause 2: two import-ordering systems run together and produce circular fixes

Both perfectionist/sort-imports and the eslint-plugin-import (import-x) ordering rules (import-x/first, import-x/newline-after-import, import-x/order) are enabled with autofixers. On files that mix a top-level require with ESM imports (for example src/module-loader.ts, where get-package-type ships no types and is required rather than imported), their fixers disagree on placement and oscillate, emitting ESLintCircularFixesWarning: Circular fixes detected ... likely that you have conflicting rules. The perfectionist/sort-imports docs explicitly recommend disabling the eslint-plugin-import ordering rules to avoid exactly this conflict.

Why the pre-commit flow makes it worse

.lintstagedrc.json runs eslint --fix on staged files. Because the suppressions baseline records pre-fix counts, any --fix that resolves a suppressed violation leaves the baseline stale (There are suppressions left that do not occur anymore), which fails the hook on its own. Working around that (for example adding --prune-suppressions) then lets the other autofixes through, which is how broken .js extensions and type rewrites reach a commit.

Suggested direction (not prescriptive)

  • Reconcile each autofixable rule with how the code is actually written and loaded: either conform the source to the rule, or turn the rule off, rather than mass-suppressing it. In particular decide whether relative imports should carry .js extensions given the ts-node test loader, and set n/file-extension-in-import accordingly.
  • Pick a single import-ordering authority (perfectionist/sort-imports or import-x, not both) per the perfectionist docs.
  • Consider adding tsc --noEmit and/or a test run to the pre-commit or CI gate so autofix-induced breakage cannot pass on a green ESLint alone.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by inspecting the ESLint configuration, eslint-suppressions.json, .lintstagedrc.json, and src/module-loader.ts; compare yarn lint with the staged eslint --fix flow. Trace the suppressed extension and type-aware rules, the competing import-order rules, and the loader behavior. Done means the chosen configuration matches the TypeScript setup, no circular fixes or stale suppressions remain, and autofix cannot pass while tsc or tests fail.

Written by the indexing model from the issue text.

Assessment

Tech stack
node.js, typescript
Domain
cli, testing-qa, tooling
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.