chore(lint): cleanup unused eslint-disable directives#23212
chore(lint): cleanup unused eslint-disable directives#23212mattKorwel wants to merge 3 commits intomainfrom
Conversation
mattKorwel
added
the
🔒 maintainer only
mattKorwel
added
the
🔒 maintainer only
Summary of ChangesHello, 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 significantly improves the performance and reliability of the ESLint process by correctly configuring ignore patterns. By preventing the linter from processing irrelevant directories and files, it resolves issues of excessive lint errors, high memory consumption, and terminal lockups. The changes also contribute to cleaner code by removing numerous redundant ESLint disable comments. Highlights
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. Footnotes
|
mattKorwel
force-pushed
the
fix/lint-cleanup
branch
from
f2b812e to
ea49fd4
Compare
mattKorwel
changed the title
|
Size Change: -413 B (0%) Total Size: 26.3 MB
ℹ️ View Unchanged
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly updates the ESLint configuration to ignore the .gemini/ directory and nested node_modules directories, which should resolve the described performance and linting issues. The associated cleanup of eslint-disable directives in test utility files is also appropriate, as those files are correctly excluded from the stricter type-aware linting rules.
However, I've raised a concern regarding the removal of no-restricted-syntax directives in several product source files. The corresponding lint rule appears to still be active, which could cause the build to fail. Please see the specific comment for more details.
Note: Security Review is unavailable for this PR.
| typeof part.data['callId'] !== 'string' || |
There was a problem hiding this comment.
This PR removes several eslint-disable-next-line no-restricted-syntax directives across multiple files (e.g., this file, packages/core/src/tools/mcp-tool.ts, etc.). While the cleanup is appreciated, the lint rule that flags the typeof obj['prop'] pattern appears to still be active for product source code in eslint.config.js.
The rule is defined with the selector UnaryExpression[operator="typeof"] > MemberExpression[computed=true][property.type="Literal"], which matches code patterns like typeof part.data['callId'] !== 'string'.
Removing these directives might cause the linter to fail. Could you please double-check that the linter passes with these directives removed? It's possible I'm missing a configuration detail, but based on the provided files, this seems like it would cause an error.
gemini-cli
bot
added
the
area/platform
| } else if (responseObj && typeof responseObj === 'object') { | ||
| if ( | ||
| 'output' in responseObj && | ||
| // eslint-disable-next-line no-restricted-syntax |
There was a problem hiding this comment.
@alisa-alisa any idea what's going on here? I thought we re-enabled this rule.
There was a problem hiding this comment.
@mattKorwel for context I introduced a linter to ban this type of syntax because it leads to confusing and verbose code with no explicit typings. In some cases we've had bugs creep in where the ad hoc type assertions got out of sync: #21485
# Conflicts: # packages/core/src/test-utils/mock-message-bus.ts # packages/core/src/tools/mcp-tool.ts
2 participants
Summary
This PR cleans up over 30 unused
eslint-disabledirectives across the codebase. These directives became redundant following the global exclusion of the.gemini/directory and recursivenode_modules(handled in a separate PR).Details
eslint-disablecomments forno-restricted-syntax,@typescript-eslint/no-unsafe-type-assertion, and other rules.Related Issues
Related to #23210
How to Validate
npm run lint. (Note: This may require the ESLint config changes from Bug: ESLint processes nested .gemini/tmp worktrees, causing terminal lockup #23210 or equivalent to be applied to complete without noise).Pre-Merge Checklist