Conversation
Summary of ChangesHello @fansenze, 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 resolves a critical bug in the linter's auto-fixer that caused unintended code duplication when multiple configuration files applied the same diagnostic fix to a single source file. The core change involves refining the fix conflict detection mechanism to accurately identify and prevent redundant insertion operations, ensuring that auto-fixes are applied cleanly and correctly without introducing artifacts. 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. 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
This pull request effectively addresses a bug that caused duplicate auto-fixes when a file is processed by multiple tsconfig projects. The core change introduces more sophisticated conflict detection logic that correctly handles overlapping and adjacent fixes, particularly for insertion operations. The solution is robust and is supported by an excellent and comprehensive set of new tests, including a specific regression test for the issue. The code is well-commented and the logic appears sound. I have one minor suggestion to improve the readability of the new conflict detection logic.
… match the same file When a file is matched by multiple tsconfig projects, the same diagnostic is reported multiple times, leading to duplicate fix insertions (e.g., "async async async function"). This change improves the fix conflict detection logic to properly handle insertion operations (where pos == end). The key changes: 1. Track whether the previous fix was an insertion via `lastWasInsertion` 2. Skip fixes when `lastFixEnd == pos` and either current or previous fix is an insertion (prevents duplicate insertions and position conflicts) 3. Allow adjacent replacement fixes (end == start) since they don't overlap Also adds comprehensive test cases for the fix application logic.
64e5f40 to
dce724e
Compare
Summary
Problem
When a file is matched by multiple tsconfig projects (e.g.,
tsconfig.jsonandtsconfig.app.json), each project reports the same diagnostic independently. This causes the auto-fixer to apply the same fix multiple times, resulting in incorrect output likeasync async async function render().Root Cause
The original conflict detection logic only checked
lastFixEnd > pos, which works for overlapping replacement fixes but fails to detect duplicate insertions. For insertion operations wherepos == end, after applying the first insertion,lastFixEndequalspos(not greater than), so subsequent duplicate insertions were not detected as conflicts.Solution
Improved the fix conflict detection in
ApplyRuleFixesto properly handle insertion operations:lastWasInsertionflag to track whether the previous fix was an insertionlastFixEnd == posAND either current or previous fix is an insertionend == start) are still allowed since they operate on non-overlapping rangesFix Conflict Strategy
Replace [0,3]→Replace [3,6][0,3)and[3,6)Insert at 0→Insert at 0Insert at 0→Replace [0,3]Replace [0,3]→Insert at 3This strategy is more precise than ESLint's approach (which uses
lastPos >= startand skips all adjacent fixes). Our implementation correctly allows adjacent replacement fixes while preventing duplicate insertions and position conflicts.Related Links
Checklist