Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

unused_capture_list false positive with explicit capturing of strong self in Swift 5.3 #3267

Closed
2 tasks done
holgerde opened this issue Jul 31, 2020 · 5 comments · Fixed by #3352
Closed
2 tasks done
Labels
enhancement Ideas for improvements of existing features and rules.

Comments

@holgerde
Copy link

New Issue Checklist

Describe the bug

A new feature in Swift 5.3 allows to explicitly capture strong self and then drop the self. prefix in the closure (see SE-0269). However, SwiftLint's unused_capture_list then treats the capture as unused, because self is not explicitly written out. It is still used implicitly though, so it should not produce an error.

See below for an example.

Complete output when running SwiftLint, including the stack trace and command used
$ swiftlint lint Example.swift --no-cache                   
Linting Swift files at paths Example.swift
Linting 'Example.swift' (1/1)
Example.swift:14:50: warning: Unused Capture List Violation: Unused reference self in a capture list should be removed. (unused_capture_list)
Done linting! Found 1 violation, 0 serious in 1 file.

Environment

  • SwiftLint version (run swiftlint version to be sure)?

    • 0.39.2
  • Installation method used (Homebrew, CocoaPods, building from source, etc)?

    • homebrew
  • Paste your configuration file:

    • none, unused_capture_list is enabled by default
  • Are you using nested configurations?
    If so, paste their relative paths and respective contents.

    • no
  • Which Xcode version are you using (check xcodebuild -version)?

Xcode 12.0
Build version 12A8169g
  • Do you have a sample that shows the issue?
URLSession.shared.dataTask(with: url) { [self] (data, _, _) in
    handle(data)
}
@marcelofabri marcelofabri added the enhancement Ideas for improvements of existing features and rules. label Aug 4, 2020
@marcelofabri
Copy link
Collaborator

Hmm, looks like the Swift compiler actually warns on unused captures, so I wonder if we should just remove the rule instead:

$ cat file.swift 
final class A {
    func b() {
        let x = { [weak self] in print("") }
        x()
    }
}

$ swift file.swift 
file.swift:3:25: warning: variable 'self' was written to, but never read
        let x = { [weak self] in print("") }

jpsim pushed a commit that referenced this issue Sep 18, 2020
…3352)

## Summary

This pr fixes: #3267 .

Since Swift 5.3 is released. It support implicit self in @escaping closures([SE-0269](https://github.com/apple/swift-evolution/blob/master/proposals/0269-implicit-self-explicit-capture.md))

I think we should exclude self keyword from capture list rule.

## Test Plan

I have already add some test in example.

Thanks you so much for your code review!
@jberkel
Copy link

jberkel commented Oct 5, 2020

Looks like this isn't handling all cases, if self is qualified with unowned it still triggers:

class A {
  func foo() {}

  func bar() {
    _ = { [unowned self] in // Unused reference self in a capture list should be removed. (SwiftLint: unused_capture_list)
       foo()
    }
  }
}

@guykogus
Copy link

Looks like this isn't handling all cases, if self is qualified with unowned it still triggers:

class A {
  func foo() {}

  func bar() {
    _ = { [unowned self] in // Unused reference self in a capture list should be removed. (SwiftLint: unused_capture_list)
       foo()
    }
  }
}

I'm having the same issue. I think that this issue should be reopened.

@lordzsolt
Copy link
Contributor

lordzsolt commented Oct 20, 2020

@jberkel @guykogus Opened a new issue with unowned

@danielaRiesgo
Copy link

For anyone looking around like me, the unowned issue was opened here. It was fixed and merged but not yet released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Ideas for improvements of existing features and rules.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants