Skip to content

Adopt stricter oxlint config#194

Merged
0x80 merged 6 commits into
mainfrom
0521-stricter-linting
May 21, 2026
Merged

Adopt stricter oxlint config#194
0x80 merged 6 commits into
mainfrom
0521-stricter-linting

Conversation

@0x80

@0x80 0x80 commented May 21, 2026

Copy link
Copy Markdown
Owner

Expand the oxlint configuration with type-aware safety rules
(no-unsafe-*, no-floating-promises, strict-void-return,
no-confusing-void-expression, no-deprecated,
prefer-nullish-coalescing), consistency rules
(consistent-type-imports/exports/definitions, array-type,
promise-function-async), and unicorn rules
(prefer-node-protocol, no-useless-undefined,
no-await-expression-member, catch-error-name). The pedantic
category is enabled at the warn level so its findings are visible
without being blocking.

Test files and src/lib/lockfile/helpers/** get scoped overrides
that relax the no-unsafe-* rules — those helpers interact with
untyped third-party APIs (@npmcli/arborist, @pnpm/lockfile-file)
where the warnings would be noise.

Existing source is conformed to the new rules: interfaces converted
to type aliases, node: import protocol, || to ??, non-null
assertions replaced, err catch parameters renamed to error, and
other minor cleanups.

Two small unrelated fixes are bundled in:

  • process.exit(1) is now reached from both branches of the CLI's
    top-level .catch, so non-Error rejections no longer exit zero.
  • pack() passes cwd to exec instead of mutating process.cwd,
    removing a leak where a failed pack left the global cwd pointing
    at the package source dir.

Decisions:

  • eslint/require-await and typescript/require-await are off.
    They conflict with promise-function-async + return-await for
    pure delegation functions, and promise-function-async already
    guards the property they protect.
  • For the pack() cwd fix, used exec's cwd option instead of
    the reviewer-suggested try/finally — eliminates the bug class
    rather than patching it, and makes the function concurrency-safe.

Expand the oxlint config with type-aware safety rules (no-unsafe-*,
no-floating-promises, strict-void-return, no-confusing-void-expression,
no-deprecated, prefer-nullish-coalescing), consistency rules
(consistent-type-imports/exports/definitions, array-type,
promise-function-async), and unicorn rules (prefer-node-protocol,
no-useless-undefined, no-await-expression-member, catch-error-name).
Add overrides relaxing the unsafe-* rules in tests and in
lockfile/helpers (which interact with untyped third-party APIs).

Conform existing code: switch interfaces to types, add node: prefixes,
replace || with ??, eliminate non-null assertions, rename catch
variables from err to error, and resolve other minor rule violations.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit e943e62. Configure here.

Comment thread package.json
@coderabbitai

coderabbitai Bot commented May 21, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

Failed to post review comments

Walkthrough

This PR expands oxlint configuration and updates lint scripts to enable import-aware, type-aware checks; converts getBuildOutputDir from async to sync and updates its call site; standardizes catch variables to error with safe message extraction; modernizes types and traversal patterns (nullish coalescing, explicit queue pops); and refactors utilities/tests (pack cwd, Promise flows, cleanup awaits, minor test formatting).

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 56.52% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Adopt stricter oxlint config' accurately summarizes the primary change: expanding and strengthening the oxlint linting configuration with new rules and plugins.
Description check ✅ Passed The description comprehensively explains the oxlint configuration changes, the rule categories being added, scoped overrides, source code conformance updates, and additional fixes included in the changeset.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 0521-stricter-linting

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 1cfd071d-52fa-4b00-aa91-c7d34106a23c

📥 Commits

Reviewing files that changed from the base of the PR and between 6613230 and 9f72f21.

📒 Files selected for processing (26)
  • .oxlintrc.json
  • package.json
  • src/isolate-bin.ts
  • src/isolate.ts
  • src/lib/config.ts
  • src/lib/lockfile/helpers/bun-lockfile.ts
  • src/lib/lockfile/helpers/generate-bun-lockfile.ts
  • src/lib/lockfile/helpers/generate-npm-lockfile.ts
  • src/lib/lockfile/helpers/generate-pnpm-lockfile.ts
  • src/lib/lockfile/helpers/generate-yarn-lockfile.ts
  • src/lib/manifest/helpers/adopt-pnpm-fields-from-root.ts
  • src/lib/manifest/helpers/resolve-catalog-dependencies.ts
  • src/lib/manifest/validate-manifest.ts
  • src/lib/output/get-build-output-dir.ts
  • src/lib/package-manager/helpers/infer-from-files.ts
  • src/lib/patches/collect-installed-names-bun.ts
  • src/lib/patches/collect-installed-names-pnpm.ts
  • src/lib/patches/copy-patches.test.ts
  • src/lib/registry/create-packages-registry.ts
  • src/lib/types.ts
  • src/lib/utils/get-dirname.ts
  • src/lib/utils/json.ts
  • src/lib/utils/pack.ts
  • src/lib/utils/reset-isolate-dir.ts
  • src/lib/utils/wait-for-complete-file.ts
  • src/lib/utils/yaml.ts
💤 Files with no reviewable changes (1)
  • src/lib/patches/copy-patches.test.ts

Comment thread src/isolate-bin.ts
Comment thread src/lib/utils/pack.ts
0x80 added 4 commits May 21, 2026 14:23
Always exit non-zero from the CLI on rejection, including when the
rejected value is not an Error. Previously the CLI would log the
failure and exit 0.

Pass cwd to exec instead of mutating process.cwd in pack(). The
previous code restored cwd only on the success path, so any rejection
or assertion failure left the process in srcDir.
@0x80 0x80 merged commit 2c9a520 into main May 21, 2026
5 checks passed
@0x80 0x80 deleted the 0521-stricter-linting branch May 21, 2026 14:36
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