Skip to content

diagnostic: Fix false positive for kwargs used in other kwarg defaults#593

Merged
aviatesk merged 1 commit intomasterfrom
avi/592
Mar 14, 2026
Merged

diagnostic: Fix false positive for kwargs used in other kwarg defaults#593
aviatesk merged 1 commit intomasterfrom
avi/592

Conversation

@aviatesk
Copy link
Owner

When keyword argument defaults depend on other keyword arguments, JuliaLowering's scope_nest creates :local bindings in let blocks instead of :argument bindings. This caused the unused argument analysis to miss the connection between the :argument binding in the body method and the :local bindings where the keyword is actually used in default expressions.

Three changes fix this:

  1. Include :argument bindings in same_location_bindings unconditionally (not just for @generated functions), so they get unified with :local bindings at the same source location.
  2. Extend skip_arguments in compute_binding_occurrences! to also skip :local bindings in self/kwsorter calls, preventing internal call machinery from being counted as genuine usage.
  3. Add has_matching_argument_binding check in analyze_unused_bindings! to suppress duplicate :local diagnostics when a corresponding :argument binding exists at the same location.

Closes #592

When keyword argument defaults depend on other keyword arguments,
JuliaLowering's `scope_nest` creates `:local` bindings in `let`
blocks instead of `:argument` bindings. This caused the unused
argument analysis to miss the connection between the `:argument`
binding in the body method and the `:local` bindings where the
keyword is actually used in default expressions.

Three changes fix this:

1. Include `:argument` bindings in `same_location_bindings`
   unconditionally (not just for `@generated` functions), so they
   get unified with `:local` bindings at the same source location.
2. Extend `skip_arguments` in `compute_binding_occurrences!` to
   also skip `:local` bindings in self/kwsorter calls, preventing
   internal call machinery from being counted as genuine usage.
3. Add `has_matching_argument_binding` check in
   `analyze_unused_bindings!` to suppress duplicate `:local`
   diagnostics when a corresponding `:argument` binding exists at
   the same location.

Closes #592

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@codecov
Copy link

codecov bot commented Mar 13, 2026

Codecov Report

❌ Patch coverage is 78.94737% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.87%. Comparing base (00f46f2) to head (4ed285b).
⚠️ Report is 3 commits behind head on master.

Files with missing lines Patch % Lines
src/diagnostic.jl 63.63% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #593      +/-   ##
==========================================
- Coverage   67.88%   67.87%   -0.01%     
==========================================
  Files          51       51              
  Lines        8295     8303       +8     
==========================================
+ Hits         5631     5636       +5     
- Misses       2664     2667       +3     
Flag Coverage Δ
JETLS.jl 67.87% <78.94%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 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.

@aviatesk aviatesk merged commit 9b9e391 into master Mar 14, 2026
16 of 17 checks passed
@aviatesk aviatesk deleted the avi/592 branch March 14, 2026 01:58
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.

Unused argument reported when arg is used in other arg defaults

1 participant