Skip to content

chore(lint): enforce single-parameter function style with custom check#2429

Closed
andrew-bierman wants to merge 1 commit into
developmentfrom
codex/update-lint-rules-for-max-params
Closed

chore(lint): enforce single-parameter function style with custom check#2429
andrew-bierman wants to merge 1 commit into
developmentfrom
codex/update-lint-rules-for-max-params

Conversation

@andrew-bierman
Copy link
Copy Markdown
Collaborator

Motivation

  • The repository lint baseline (biome) is set to allow up to 2 parameters, but the team wants to tighten style to prefer a single typed object parameter for first-party code.
  • Introduce a stricter, repo-specific check to enforce a one-parameter rule while avoiding noise from callbacks and third-party code.
  • Provide an explicit opt-out when multiple parameters are genuinely required to make incremental migration feasible.

Description

  • Added a new custom check script packages/checks/src/check-max-params.ts which scans apps/ and packages/ TypeScript files and reports any function-like node with more than one parameter.
  • The checker ignores common test/build/generated paths, excludes node_modules/dist/build, and treats callbacks passed directly into call expressions as non-violations to reduce false positives.
  • Introduced an escape hatch annotation @allow-multi-param that can be placed on a leading comment to exempt specific functions.
  • Wired the new script into npm scripts: added check:max-params to packages/checks/package.json and a root-level check:max-params and appended it to the lint:custom pipeline in the root package.json while leaving Biome's useMaxParams at max: 2 as the baseline.

Testing

  • Ran bunx biome format --write packages/checks/src/check-max-params.ts which formatted the new file successfully.
  • Ran biome check --no-errors-on-unmatched against the changed files as part of the pre-commit hook which succeeded after formatting.
  • Ran bun run --cwd packages/checks check:max-params which intentionally failed because it reported existing multi-parameter functions in first-party code (this is expected and indicates the check is detecting current violations).

Codex Task

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 16, 2026

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: bdbc4176-a8e3-457c-a4b2-1d73b8b7a82d

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/update-lint-rules-for-max-params

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@github-actions github-actions Bot added the dependencies Pull requests that update a dependency file label May 16, 2026
@github-actions
Copy link
Copy Markdown
Contributor

Coverage Report for API Unit Tests Coverage (./packages/api)

Status Category Percentage Covered / Total
🔵 Lines 69.62% 502 / 721
🔵 Statements 69.62% (🎯 65%) 502 / 721
🔵 Functions 92.68% 38 / 41
🔵 Branches 88.32% 227 / 257
File CoverageNo changed files found.
Generated in workflow #1269 for commit 927e26f by the Vitest Coverage Report Action

@github-actions
Copy link
Copy Markdown
Contributor

Coverage Report for Expo Unit Tests Coverage (./apps/expo)

Status Category Percentage Covered / Total
🔵 Lines 82.61% 480 / 581
🔵 Statements 82.61% (🎯 75%) 480 / 581
🔵 Functions 92.59% 50 / 54
🔵 Branches 90.9% 170 / 187
File CoverageNo changed files found.
Generated in workflow #1269 for commit 927e26f by the Vitest Coverage Report Action

@andrew-bierman
Copy link
Copy Markdown
Collaborator Author

Closing as superseded by #2422.

After comparing the two checks, #2422's scripts/lint/no-owned-max-params.ts is the better-designed implementation:

  • Distinguishes owned function definitions from inline framework callbacks (e.g. fetch, queue, resolveRequest on Cloudflare Workers; React event handlers like onChange/keyExtractor); this check's heuristic is broader and would flag legitimate framework signatures.
  • Has explicit per-file exclusions for cases that intentionally mirror external APIs (e.g. packages/api/src/services/r2-bucket.ts mirrors R2's positional API).
  • Documents the why inline so the next contributor understands the rationale.

#2422 also ships the fix alongside the check (206 violations refactored across 260 files), which is what unblocks the check from being red on day one. #2429 only added the check without addressing existing violations.

If we later want the @allow-multi-param escape-hatch annotation from this branch, it's a small follow-up commit on top of #2422's check rather than a separate parallel implementation. Filing as docs/plans follow-up if useful.

Branch codex/update-lint-rules-for-max-params is preserved for reference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

codex dependencies Pull requests that update a dependency file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant