Skip to content

refactor(linters): add rule to enforce inline single use fixtures - #45230

Open
secustor wants to merge 11 commits into
mainfrom
feat/lint-rule-inline-single-use-fixtures
Open

refactor(linters): add rule to enforce inline single use fixtures#45230
secustor wants to merge 11 commits into
mainfrom
feat/lint-rule-inline-single-use-fixtures

Conversation

@secustor

@secustor secustor commented Aug 11, 2026

Copy link
Copy Markdown
Member

Changes

Add a rule which forces Fixtures to be declared inside of a test scope if they are only used in that block.
It does NOT enforce right now that the actual string is inline, rather it allows to have also simply a Fixtures.get() in test scope.
The idea has been that if we have truly big fixtures we can still move them out, though we could also adapt this and use oxlint ignore.
This could be a follow up to this.

Context

Please select one of the following:

  • This closes an existing Issue, Closes: #
  • This doesn't close an Issue, but I accept the risk that this PR may be closed if maintainers disagree with its opening or implementation

AI assistance disclosure

Did you use AI tools to create any part of this pull request?

Please select one option and, if yes, briefly describe how AI was used (e.g., code, tests, docs) and which tool(s) you used.

  • No — I did not use AI for this contribution.
  • Yes — minimal assistance (e.g., IDE autocomplete, small code completions, grammar fixes).
  • Yes — substantive assistance (AI-generated non‑trivial portions of code, tests, or documentation).
  • Yes — other (please describe):

Documentation (please check one with an [x])

  • I have updated the documentation, or
  • No documentation update is required

How I've tested my work (please select one)

I have verified these changes via:

  • Code inspection only, or
  • Newly added/modified unit tests, or
  • No unit tests, but ran on a real repository, or
  • Both unit tests + ran on a real repository

The public repository:


Supersedes #44687 (branch moved into this repo so the stack can use the parent branch as base).

secustor and others added 10 commits August 11, 2026 20:40
…ect rules

Flag ad-hoc type checks that have @sindresorhus/is equivalents:
- .filter(Boolean) -> .filter(isTruthy)
- typeof x === 'string' -> isString(x)
- x === null || x === undefined -> isNullOrUndefined(x) (and negation)

The typeof x === 'object' shape lives in a separate suggestion-only rule
(renovate/prefer-is-object, kept at warn) because isObject()/isPlainObject()
are not drop-in equivalents: null, arrays and functions behave differently.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replace ad-hoc type checks with @sindresorhus/is helpers across lib/:
- 13x .filter(Boolean) -> .filter(isTruthy)
- 36x typeof x ==/!== 'string' -> isString(x) / !isString(x)
- 6x x === null || x === undefined -> isNullOrUndefined(x) (and negation)

Promote renovate/prefer-is-helpers to error now that the codebase is clean.
renovate/prefer-is-object stays at warn: its 15 remaining sites guard
parsed YAML/JSON or unknown values where isObject()/isPlainObject() are
not provably equivalent (null/array/function semantics).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Resolve the 15 remaining warnings and raise the rule from warn to error:

- Migrate 10 sites to isObject() where the guarded value provably cannot
  be a function (parsed YAML/JSON, typed exec command arguments, or
  test-controlled fixtures): helm-values, woodpecker, bun, pre-commit
  managers, exec utils, file package cache, and the github http spec.
  In the bun manager this also fixes a latent crash where a null
  `workspaces` value would throw on the `in` operator.
- Keep 5 sites as reasoned oxlint-disable comments where excluding
  functions is load-bearing: the byte-exact fingerprint canonicalizer
  (2), the circularity walker, the zod result guard, and the generic
  hasKey helper.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
In spec files, a module-scope const fixture that is referenced from
exactly one it()/test() block should be inlined into that test so the
test is self-contained. The rule is a warn-level nudge (naming for
clarity is a valid exception) with no autofix (moving code is risky).

Uses oxlint jsPlugins scope analysis (getDeclaredVariables) for exact,
shadowing-safe reference counting and bails out conservatively: only
top-level non-exported single-declarator consts with multiline,
non-function initializers whose every reference sits inside the callback
function of one and the same test call are flagged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fix 26 renovate/inline-single-use-fixtures warnings in specs where the
move is mechanical and value-preserving: codeBlock fixtures were
re-indented uniformly (codeBlock strips the common indentation, so the
produced string is unchanged) and Fixtures.get() calls resolve relative
to the spec file, so relocating them inside the test body has no effect.

The remaining 59 warnings are left for per-site judgment, as the rule is
a warn-level nudge by design.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Move the 24 remaining single-use module-scope fixtures whose initializer
is a codeBlock tagged template or a Fixtures accessor call into the one
it() block that uses them. Both shapes are value-preserving to relocate:
codeBlock strips the common leading indentation, so the uniformly
re-indented templates produce identical strings, and Fixtures.get()
resolves relative to the spec file, not the call site's position in it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Narrow the rule to the initializer shapes where moving the declaration
into the test body is provably value-preserving and unambiguously an
improvement: codeBlock/stripIndent tagged templates (indentation-
normalizing, so re-indenting is safe) and Fixtures accessor calls
(resolve relative to the spec file). Plain object/array literals,
untagged template literals (whose leading-newline formatting would need
ugly re-indentation) and arbitrary call expressions remain per-site
judgment calls and are no longer flagged.

With the rule reduced to true defects and all remaining violations
inlined, bump it from warn to error in the spec override block.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The previous commits moved single-use module-scope Fixtures.get() consts
into their one it() block, which only relocated the file indirection.
Replace those relocated Fixtures.get() calls with the fixture file's
actual content — codeBlock tagged templates for text fixtures — so the
tests are self-contained, and delete the fixture files that no longer
have any references. Relocated fixtures too large to inline comfortably
(terraform-provider releases, sbt private-variable file, nuget v2 pkg
list, bundled package-lock) keep their Fixtures.get() calls.

Value preservation was verified by unchanged vitest results for every
touched spec; all consumers parse the content, so codeBlock's trailing
newline normalization is behavior-neutral.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Inline the four Fixtures.get() sites that were previously exempted for
size, so every single-use fixture is now inlined regardless of size:

- nuget v2_withoutProjectUrl.xml -> codeBlock (served via http mock)
- terraform-provider releaseBackendIndexGoogleBeta.json -> plain object
  literal passed to the nock reply
- npm locked-dependency bundled.package-lock.json -> codeBlock JSON
  string (consumed as raw lockFileContent)
- sbt private-variable-dependency-file.scala -> codeBlock

All consumers parse the content, so codeBlock's trailing-newline
normalization is behavior-neutral; vitest results are unchanged for
every touched spec, including the two pre-existing nuget failures.

The nuget, terraform-provider and sbt fixture files have no remaining
references and are deleted. bundled.package-lock.json is kept because
package-lock/get-locked.spec.ts still reads it via Fixtures.getJson().

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The bundled package-lock.json inlined into the "fails remediation if
bundled" test is ~150 KB (~4,400 lines); inlining it as a codeBlock
hurts readability more than it helps. Read it via Fixtures.get inside
the test block instead — the inline-single-use-fixtures rule permits a
Fixtures.get() call in test scope, and the fixture file is still present.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Base automatically changed from feat/lint-rule-prefer-is-helpers to main August 13, 2026 14:30
…ne-single-use-fixtures

# Conflicts:
#	.oxlintrc.json
#	lib/modules/datasource/go/goproxy-parser.ts
#	lib/modules/manager/terraform/lockfile/hash.spec.ts
#	lib/modules/versioning/bazel-module/bzlmod-version.ts
#	lib/modules/versioning/bazel-module/index.ts
#	tools/lint/rules/prefer-is-helpers.js
#	tools/lint/rules/prefer-is-object.js
@secustor
secustor marked this pull request as ready for review August 14, 2026 19:26
@github-actions
github-actions Bot requested a review from viceice August 14, 2026 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant