Skip to content

Fix redundantClosure false positive for async let - #2659

Merged
calda merged 5 commits into
developfrom
copilot/fix-redundantclosure-false-positive
Aug 21, 2026
Merged

Fix redundantClosure false positive for async let#2659
calda merged 5 commits into
developfrom
copilot/fix-redundantclosure-false-positive

Conversation

Copilot AI commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

redundantClosure incorrectly flags closures in async let declarations as redundant. The closure is not redundant here — it's required to defer execution until the value is awaited.

// This closure should not be removed
async let doAsync = {
    if 1 == 1 {
        print()
    }
}()
await doAsync

Changes:

  • Added a guard in RedundantClosure.swift that detects async let/var declarations by walking backward from the = sign and skips the rule
  • Added test case testKeepsClosureInAsyncLet

Closes #2658

Co-authored-by: calda <1811727+calda@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix redundantClosure false positive for async let Fix redundantClosure false positive for async let Aug 20, 2026
Copilot AI requested a review from calda August 20, 2026 14:36
@codecov

codecov Bot commented Aug 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.45%. Comparing base (df2c5bd) to head (443d8ed).
⚠️ Report is 4 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2659      +/-   ##
===========================================
- Coverage    95.45%   95.45%   -0.01%     
===========================================
  Files          178      178              
  Lines        27137    27209      +72     
===========================================
+ Hits         25904    25971      +67     
- Misses        1233     1238       +5     

☔ 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.

@calda

calda commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

@copilot, actually the issue is that this closure is not redundant because an if with no else is not a valid if expression:

// Not valid
async let doAsync =
    if 1 == 1 { // error: 'if' must have an unconditional 'else' to be used as expression
        print()
    }

// Valid
async let doAsync =
    if 1 == 1 {
        print(1)
    } else {
        print(2)
    }

Ensure redundantClosure doesn't try to remove closures that only contain a single if branch

…nc let

Don't remove closures containing non-exhaustive if expressions (if without
else), since those are not valid Swift expressions. This correctly handles
the async let case from #2658 and any other context where a non-exhaustive
if statement is wrapped in an immediately-invoked closure.

Co-authored-by: calda <1811727+calda@users.noreply.github.com>

Copilot AI commented Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

@copilot, actually the issue is that this closure is not redundant because an if with no else is not a valid if expression:

...

Updated the fix to check for non-exhaustive if expressions instead of specifically checking for async let. The rule now verifies that the conditional statement inside the closure is exhaustive (has an unconditional else for if statements) before removing the closure. Added tests for if without else, if/else if without else, and confirmed exhaustive if/else closures are still removed correctly. See commit 0f50f47.

Comment thread Sources/Rules/RedundantClosure.swift Outdated
extension Formatter {
/// Finds the `if` or `switch` keyword index inside a closure body,
/// skipping over `return`, `try`, `try?`, `try!`, and `await` keywords.
func indexOfConditionalKeywordInClosure(startOfBody: Int) -> Int? {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@copilot this new helper / the complex code in it shouldn't be necessary, check how other if expression rules like redundantReturn or conditionalAssignment handle this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Replaced the custom indexOfConditionalKeywordInClosure helper with a direct call to conditionalBranches(at:), which already handles skipping try/await keywords. See commit 7e1f5a3.

Co-authored-by: calda <1811727+calda@users.noreply.github.com>
Copilot AI requested a review from calda August 20, 2026 15:32
@calda

calda commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

@copilot please lint

@calda
calda marked this pull request as ready for review August 20, 2026 16:15
Co-authored-by: calda <1811727+calda@users.noreply.github.com>

Copilot AI commented Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

@copilot please lint

Done in commit d89d44d.

Copilot AI requested a review from calda August 20, 2026 16:23
Copilot AI linked an issue Aug 20, 2026 that may be closed by this pull request
@calda
calda merged commit 78eb87a into develop Aug 21, 2026
15 of 16 checks passed
@calda
calda deleted the copilot/fix-redundantclosure-false-positive branch August 21, 2026 01:38
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.

redundantClosure false positive for async let

2 participants