Skip to content

Report unfixable diagnostics when using scan --update-all#2684

Merged
HerringtonDarkholme merged 1 commit into
ast-grep:mainfrom
morgan-coded:fix/2681-update-all-unfixable-diagnostics
Jun 6, 2026
Merged

Report unfixable diagnostics when using scan --update-all#2684
HerringtonDarkholme merged 1 commit into
ast-grep:mainfrom
morgan-coded:fix/2681-update-all-unfixable-diagnostics

Conversation

@morgan-coded

@morgan-coded morgan-coded commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Problem I saw:
ast-grep scan --update-all applies fixable rewrites, but it was also hiding diagnostics from rules that do not have a fix.
In a mixed scan, the fixable rule was applied, but the remaining unfixable warning was not printed.
What I changed:
In accept-all mode, diff payloads still auto-confirm rewrites without printing the interactive diff preview.
Highlight diagnostics now continue through the normal printer, so unfixable findings are still shown after --update-all applies the available fixes.
I also added CLI regression coverage with one fixable rule and one unfixable rule. The test checks that the fix is applied, the fixed rule is no longer reported, the unfixable rule is still printed, and stderr reports the applied change.
How I checked it:
I ran:
cargo fmt --all
cargo fmt --all -- --check
focused regression test, 1 test
full scan integration test, 31 tests
git diff --check
Closes #2681

Summary by CodeRabbit

  • Bug Fixes
    • Fixed highlight processing in interactive mode—highlights are now properly forwarded and output instead of being suppressed when using the accept-all option.

Copilot AI review requested due to automatic review settings June 6, 2026 12:34
@coderabbitai

coderabbitai Bot commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4090143b-4051-4095-a284-ee4e89f0c082

📥 Commits

Reviewing files that changed from the base of the PR and between 3f66e2b and cedd881.

📒 Files selected for processing (2)
  • crates/cli/src/print/interactive_print.rs
  • crates/cli/tests/scan_test.rs

📝 Walkthrough

Walkthrough

Interactive printer's accept-all mode now forwards highlights to the inner printer instead of suppressing output. This fix allows unfixable rules to be reported when --update-all is used. A new integration test validates that fixable rules apply changes while unfixable rules are still reported in the output.

Changes

Interactive Printer Highlight Processing Fix

Layer / File(s) Summary
Process highlights in accept-all mode
crates/cli/src/print/interactive_print.rs
InteractivePrinter::process_highlights now processes highlights via self.inner.process(inner) when accept_all=true, instead of returning Ok(()) without output.
Test fixable vs unfixable rules with --update-all
crates/cli/tests/scan_test.rs
Added fixture constants for fixable and unfixable JS rules, and integration test test_sg_scan_update_all_reports_unfixable_rules that verifies unfixable rules are reported while fixable rules apply transformations with --update-all.

Estimated Code Review Effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 When highlights were suppressed without a trace,
Rules without fixes vanished without a face,
Now with accept-all, the printer does share,
All findings reported with newfound care!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 60.00% 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
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: ensuring unfixable diagnostics are reported when using scan --update-all, which aligns with the PR's core objective.
Linked Issues check ✅ Passed The PR directly addresses issue #2681 by forwarding highlights in accept-all mode and adding test coverage for reporting unfixable rules, fulfilling the objective.
Out of Scope Changes check ✅ Passed All changes are directly scoped to the linked issue: fixing highlight reporting in accept-all mode and adding regression test coverage.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR adjusts scan --update-all output behavior so that unfixable findings are still reported while fixable findings are auto-applied, and adds a regression test to validate that behavior.

Changes:

  • Added CLI integration test covering scan --update-all reporting only unfixable rules and applying fixes.
  • Updated interactive printing logic so accept_all mode forwards results to the underlying printer rather than suppressing output.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
crates/cli/tests/scan_test.rs Adds a test ensuring --update-all applies fixable changes while still reporting unfixable rules.
crates/cli/src/print/interactive_print.rs Changes accept_all behavior to print via the inner printer instead of returning early with no output.

Comment thread crates/cli/tests/scan_test.rs
Comment thread crates/cli/src/print/interactive_print.rs
@codecov

codecov Bot commented Jun 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.44%. Comparing base (3f66e2b) to head (cedd881).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2684      +/-   ##
==========================================
+ Coverage   85.48%   86.44%   +0.95%     
==========================================
  Files         116      116              
  Lines       20174    20174              
==========================================
+ Hits        17246    17439     +193     
+ Misses       2928     2735     -193     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@HerringtonDarkholme

Copy link
Copy Markdown
Member

Thanks

@HerringtonDarkholme HerringtonDarkholme added this pull request to the merge queue Jun 6, 2026
Merged via the queue into ast-grep:main with commit 796ccba Jun 6, 2026
6 checks passed
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.

Unfixable rule silently ignored with --update-all

3 participants