Skip to content

[eslint] strict-codegen: add ultra-strict rules to the universal lint overlay, with repo-wide bulk suppressions - #5923

Open
Sean Larkin (TheLarkInn) wants to merge 16 commits into
mainfrom
thelarkinn-strict-lint-rules-rollout
Open

[eslint] strict-codegen: add ultra-strict rules to the universal lint overlay, with repo-wide bulk suppressions#5923
Sean Larkin (TheLarkInn) wants to merge 16 commits into
mainfrom
thelarkinn-strict-lint-rules-rollout

Conversation

@TheLarkInn

Copy link
Copy Markdown
Member

Summary

Newly generated packages (e.g. the upcoming rushd wire-layer packages) need an ultra-strict lint rule set, and we want it enforced repo-wide rather than as an opt-in mixin. This PR adds the "strict-codegen" rules to the repo's universal ESLint overlay (localCommonConfig in rigs/decoupled-local-node-rig/.../flat/profile/_common.js), which every project composes after the published @rushstack/eslint-config profile, and records all pre-existing violations with @rushstack/eslint-bulk so the ratchet can flip to 'error' without breaking builds.

Also adds a root AGENTS.md lint policy and a "Lint Policy" section in .github/copilot-instructions.md: no new inline eslint-disable comments, no ignores-entry additions to hide violations, and .eslint-bulk-suppressions.json (managed only via the eslint-bulk CLI) as the sole sanctioned suppression mechanism; the file may only shrink in a PR except in dedicated onboarding PRs.

Details

Rule set (all rules already ship with the repo's ESLint toolchain; no new plugins): complexity budget (complexity: 3, max-depth: 3, max-lines-per-function: 30, max-lines: 100, max-params: 4), @typescript-eslint/no-magic-numbers (enums/readonly class props exempt), @typescript-eslint/prefer-nullish-coalescing (all exemptions disabled), sort-imports member sorting, import/order with alphabetization + full grouping (merged with the existing @{rushstack,microsoft}/** pathGroups convention), import/enforce-node-protocol-usage, @typescript-eslint/consistent-type-imports (keeps the inline-type-imports fixStyle house style), import/no-relative-parent-imports (test files exempt per repo convention), and @typescript-eslint/no-implied-eval replacing the core rule to avoid double-reporting.

Rollout strategy (why 'warn' first): the universal overlay is all-or-nothing, and bulk suppressions can only be captured for rules that are enabled. So the rules land at 'warn' with // strict-codegen: ratchet to 'error' markers, every package's existing violations are captured into per-package .eslint-bulk-suppressions.json files (6,510 suppressions across 114 packages), and a follow-up PR flips the severities to 'error' (a ~15-line diff; suppressions are severity-independent, so builds stay green). A later phase strips the remaining inline eslint-disable comments repo-wide and then enables noInlineConfig + reportUnusedDisableDirectives: 'error' in the overlay.

Commits are sharded for review: hand-written changes (overlay rules, docs, config wiring, 7 package.json devDep fixes, 2 stale inline-disable removals) are separate from the 10 per-folder machine-generated suppression commits; review the file lists, not the JSON.

Notable execution findings:

  • Suppression capture must run on a built repo: eslint-plugin-import rules silently skip imports that don't resolve yet (e.g. apps/lockfile-explorer's generated build/lfx-shared).
  • @rushstack/eslint-bulk requires eslint to be a declared dependency of the target project; 7 projects that linted via their rig without declaring it were fixed (plus lockfile).
  • 10 legacy .eslintrc.js-only test projects (eslint 7/8 matrix fixtures) are unaffected - the universal overlay is flat-config only. build-tests-subspace/typescript-v4-test consumes the published profile directly and is intentionally excluded.
  • build-tests/eslint-bulk-suppressions-test-flat self-regenerates its versioned fixture; the strict rules legitimately added one entry to it.
  • Two inline import/order disables (in apps/rush, heft-webpack4-plugin) became unused once bulk suppressions covered them and were removed.

Backwards compatibility: no published package's shipped code changes; all version-bump change files are type: "none". The overlay change itself is in a private rig package.

How it was tested

  • Full-repo rush build (192 projects) on the final commit: exit 0, zero lint warnings or errors.
  • Smoke-tested the composed config (published profile + overlay) against sample files with eslint@9.37 / typescript-eslint (@typescript-eslint)@8.56.1: strict rules fire at 'warn', no-eval/no-implied-eval override behaves, no plugin-redefinition conflicts, and a compliant file passes clean.
  • Per-project verification pass: eslint src --max-warnings 0 across all 141 wired config folders comes back clean (excluding the self-testing eslint-bulk fixture folders, whose own build passes).
  • eslint-bulk suppress + rebuild piloted end-to-end on libraries/tree-pattern before mass rollout.

Impacted documentation

N/A for the website. Repo-local docs added: AGENTS.md (new lint policy + rollout phases) and .github/copilot-instructions.md (new section 7 referencing it).

Sean Larkin (TheLarkInn) and others added 15 commits August 14, 2026 11:59
Add an ultra-strict rule set (small functions/files, named constants, nullish
coalescing, strict import hygiene, no eval) to the rig overlay
(localCommonConfig) that every repo project's lint config composes after the
published @rushstack/eslint-config profile. Rules land at 'warn' with
strict-codegen ratchet markers; AGENTS.md and copilot-instructions document
the lint policy (no inline eslint-disable comments, no ignore-entry additions,
bulk suppressions as the only sanctioned suppression mechanism) and the
phased rollout (warn -> per-package bulk suppressions -> error ->
noInlineConfig).

Rule reconciliation vs. the existing profiles: import/order gains
alphabetize + full grouping while keeping the @{rushstack,microsoft}
pathGroups convention; core no-implied-eval is replaced by the
@typescript-eslint extension rule (kept at 'error');
prefer-nullish-coalescing disables the ignoreConditionalTests exemption that
would otherwise defeat it; consistent-type-imports keeps the repo's
inline-type-imports fixStyle.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Adds the flat-config bulk-suppressions patch require to all 138 project
eslint.config.js/eslint.config.cjs files (path chosen per rig surface already
consumed by each project), enabling per-package .eslint-bulk-suppressions.json
for the strict-codegen rollout. Three tool-fixture projects already had it;
build-tests-subspace/typescript-v4-test (published-profile-only TS4 fixture)
is intentionally excluded.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@rushstack/eslint-bulk requires eslint to be resolvable as a dependency of the
project being suppressed. These 7 projects lint via their rig but did not
declare eslint directly. Lockfile + repo-state regenerated via rush update.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The bulk suppressions recorded for these files make the inline
eslint-disable-next-line comments redundant; the repo's
reportUnusedDisableDirectives linting was flagging them as unused warnings.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Machine-generated by @rushstack/eslint-bulk (eslint-bulk suppress) after
enabling the strict-codegen rules repo-wide at 'warn'. Each entry records a
{file, scopeId, rule} triple for a pre-existing violation so the ratchet can
flip to 'error' without breaking builds. Review the file list, not the JSON.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Machine-generated by @rushstack/eslint-bulk (eslint-bulk suppress) after
enabling the strict-codegen rules repo-wide at 'warn'. Each entry records a
{file, scopeId, rule} triple for a pre-existing violation so the ratchet can
flip to 'error' without breaking builds. Review the file list, not the JSON.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Machine-generated by @rushstack/eslint-bulk (eslint-bulk suppress) after
enabling the strict-codegen rules repo-wide at 'warn'. Each entry records a
{file, scopeId, rule} triple for a pre-existing violation so the ratchet can
flip to 'error' without breaking builds. Review the file list, not the JSON.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Machine-generated by @rushstack/eslint-bulk (eslint-bulk suppress) after
enabling the strict-codegen rules repo-wide at 'warn'. Each entry records a
{file, scopeId, rule} triple for a pre-existing violation so the ratchet can
flip to 'error' without breaking builds. Review the file list, not the JSON.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Machine-generated by @rushstack/eslint-bulk (eslint-bulk suppress) after
enabling the strict-codegen rules repo-wide at 'warn'. Each entry records a
{file, scopeId, rule} triple for a pre-existing violation so the ratchet can
flip to 'error' without breaking builds. Review the file list, not the JSON.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Machine-generated by @rushstack/eslint-bulk (eslint-bulk suppress) after
enabling the strict-codegen rules repo-wide at 'warn'. Each entry records a
{file, scopeId, rule} triple for a pre-existing violation so the ratchet can
flip to 'error' without breaking builds. Review the file list, not the JSON.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Machine-generated by @rushstack/eslint-bulk (eslint-bulk suppress) after
enabling the strict-codegen rules repo-wide at 'warn'. Each entry records a
{file, scopeId, rule} triple for a pre-existing violation so the ratchet can
flip to 'error' without breaking builds. Review the file list, not the JSON.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Machine-generated by @rushstack/eslint-bulk (eslint-bulk suppress) after
enabling the strict-codegen rules repo-wide at 'warn'. Each entry records a
{file, scopeId, rule} triple for a pre-existing violation so the ratchet can
flip to 'error' without breaking builds. Review the file list, not the JSON.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Machine-generated by @rushstack/eslint-bulk (eslint-bulk suppress) after
enabling the strict-codegen rules repo-wide at 'warn'. Each entry records a
{file, scopeId, rule} triple for a pre-existing violation so the ratchet can
flip to 'error' without breaking builds. Review the file list, not the JSON.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Machine-generated by @rushstack/eslint-bulk (eslint-bulk suppress) after
enabling the strict-codegen rules repo-wide at 'warn'. Each entry records a
{file, scopeId, rule} triple for a pre-existing violation so the ratchet can
flip to 'error' without breaking builds. Review the file list, not the JSON.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Satisfies rush change --verify for the 82 published projects that gained an
eslint.config.js patch require and/or .eslint-bulk-suppressions.json. No
shipping code changed.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
rush change --verify requires changes for lockstep members to be filed
against the policy's main project (@microsoft/rush), which already has one.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Needs triage

Development

Successfully merging this pull request may close these issues.

1 participant