Conversation
Summary of ChangesHello @ScriptedAlchemy, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request integrates the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
The pull request successfully ports the @typescript-eslint/max-params rule to the linter. The implementation is robust, covering a wide range of function-like nodes and correctly handling the special this: void parameter in TypeScript through the countVoidThis option. The unit tests are comprehensive, and the documentation is clear. I have provided one suggestion to simplify the options parsing logic by removing redundant type checks.
There was a problem hiding this comment.
Pull request overview
Ports the @typescript-eslint/max-params rule into the TypeScript plugin, wiring it into the global registry and enabling its test runner coverage.
Changes:
- Added a new TypeScript rule implementation for
@typescript-eslint/max-paramswith option parsing and diagnostics. - Added Go rule tests plus rstest snapshots, and enabled the TypeScript rule test file in
rstest.config.mts. - Registered the rule in the global config registry and updated
rslint.jsondefaults.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| rslint.json | Adds the new rule to the repo config (currently disabled). |
| packages/rslint-test-tools/rstest.config.mts | Enables the TypeScript rule test file for max-params. |
| packages/rslint-test-tools/tests/typescript-eslint/rules/snapshots/max-params.test.ts.snap | Adds snapshots for the new rule’s diagnostics. |
| internal/plugins/typescript/rules/max_params/max_params.go | Implements the rule logic, option parsing, and reporting. |
| internal/plugins/typescript/rules/max_params/max_params_test.go | Adds Go-side rule tests for valid/invalid cases and options. |
| internal/plugins/typescript/rules/max_params/max_params.md | Adds rule documentation page. |
| internal/config/config.go | Registers @typescript-eslint/max-params in the global rule registry. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| { | ||
| Code: ` | ||
| interface Foo { | ||
| method(a: number, b: number, c: number, d: number): void; | ||
| } | ||
| `, | ||
| }, | ||
| { | ||
| Code: ` | ||
| type CallSig = { | ||
| (a: number, b: number, c: number, d: number): void; | ||
| }; | ||
| `, | ||
| }, | ||
| { | ||
| Code: ` | ||
| type Ctor = new (a: number, b: number, c: number, d: number) => Foo; | ||
| `, | ||
| }, |
There was a problem hiding this comment.
These “valid” cases include interface/type signatures with 4 parameters while the default max is 3. Given the current listener list, they won’t be checked at all, so this test may be accidentally encoding a coverage gap. Either add coverage + listeners for the relevant TS signature node kinds (and move these to invalid), or remove them and clarify that the rule only targets runtime functions.
| { | |
| Code: ` | |
| interface Foo { | |
| method(a: number, b: number, c: number, d: number): void; | |
| } | |
| `, | |
| }, | |
| { | |
| Code: ` | |
| type CallSig = { | |
| (a: number, b: number, c: number, d: number): void; | |
| }; | |
| `, | |
| }, | |
| { | |
| Code: ` | |
| type Ctor = new (a: number, b: number, c: number, d: number) => Foo; | |
| `, | |
| }, |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 94ffd147f3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Deploying rslint with
|
| Latest commit: |
a63a0ec
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://9351beda.rslint.pages.dev |
| Branch Preview URL: | https://codex-ts-eslint-max-params-a.rslint.pages.dev |
…arams-ac37 # Conflicts: # rslint.json
| func buildExceedMessage(count int, maxCount int) rule.RuleMessage { | ||
| return rule.RuleMessage{ | ||
| Id: "exceed", | ||
| Description: fmt.Sprintf("Function has too many parameters (%d). Maximum allowed is %d.", count, maxCount), |
There was a problem hiding this comment.
The original ESLint core max-params rule uses a dynamic name in the message — e.g. "Function 'foo'...", "Arrow function...", "Method 'bar'..." depending on the function type. Currently this is hardcoded as "Function" for all cases.
It might be worth generating the appropriate name based on the node kind to stay consistent with the upstream behavior. Something like:
KindFunctionDeclaration→Function 'name'KindArrowFunction→Arrow functionKindMethodDeclaration→Method 'name'KindConstructor→ConstructorKindGetAccessor→Getter 'name'KindSetAccessor→Setter 'name'KindFunctionType→Function type
Not a blocker, but would be nice to align!
…arams-ac37 # Conflicts: # rslint.json
…arams-ac37 # Conflicts: # rslint.json
… into codex/ts-eslint-max-params-ac37
…arams-ac37 # Conflicts: # rslint.json
…arams-ac37 # Conflicts: # rslint.json
Summary: Port @typescript-eslint/max-params from ScriptedAlchemy#13.
Related Links:
Checklist: