Add rule to warn against @escaping retain cycle
Nobody has claimed this yet.
- Dominant language
- Swift
- Stars
- 19.7k
- Forks
- 2.3k
- Avg merge
- 1d 1h
- Merged PRs (30d)
- 11
Description
I'm not quite sure if this is doable, or falls within the realm of what SwiftLint wants to warn against, but I recently ran into this issue and it seemed like in might be a common Swift "gotcha"
Consider the following (contrived) example:
class Sync {
var callback: () -> Void = {}
func perform(andThen completionHandler: @escaping () -> Void) {
callback = completionHandler
}
}
class Other {
let sync = Sync()
deinit {
print("Deinit called")
}
func start() {
sync.perform(andThen: cleanUp)
}
func cleanUp() {
// Do cleanup
}
}
If start() is called on an instance of Other, that instance will never deallocate because passing cleanUp to Sync results in a retain cycle. This can be remedied by changing the implementation like so:
sync.perform { [weak self] in
self?.cleanUp()
}
This seems to happen often enough that it has led to the creation of a µFramework called Weakify, but that doesn't help you find occurrences of this issue. And at least in my case it involved a lot of head scratching to figure out what was going on.
Swift 3 made @nonescaping the default for closure parameters, which makes me think that if we can detect @escaping parameters we will have solved half the puzzle of "should SwiftLint emit a warning".
The other half of the puzzle is detecting that you're passing something to that parameter which will result in a retain cycle. My earlier example could also be expressed as a partially applied currying method:
sync.perform(andThen: Other.cleanUp(self))
So I imagine that there would be a way to detect the above example and then emit a warning.
In addition (but perhaps more complex to detect) would be the following case, in which a capture list is not provided, and self is strongly retained:
sync.perform {
self.cleanUp()
}
I would appreciate any thoughts on this rule. Are there cases where the retain cycle would be desirable? Does it seem susceptible to false-positives? Does it seem doable?
Thank you very much for your time,
Noah
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
No source file, test, or entry point is named. Start by evaluating whether SwiftLint can detect @escaping parameters, method references, and closures that strongly capture self; done would mean a defined rule scope with acceptable false-positive behavior for the examples described.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- swift
- Domain
- tooling
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100