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

redundant_self_in_closure false positive #5010

Closed
la-pieuvre opened this issue May 15, 2023 · 4 comments · Fixed by #5027
Closed

redundant_self_in_closure false positive #5010

la-pieuvre opened this issue May 15, 2023 · 4 comments · Fixed by #5027
Labels
bug Unexpected and reproducible misbehavior.

Comments

@la-pieuvre
Copy link

New Issue Checklist

Describe the bug

in the typical case where a guard let is used :

guard let lastTimerCallTime else {return}
self.lastTimerCallTime = Date()

affecting a new value to the object property "self.lastTimerCallTime" trigger a false positive and remove the self creating an error since local lastTimerCallTime is immutable

Environment

  • SwiftLint version 0.52.2
  • Installation method used Homebrew
  • Paste your configuration file:
opt_in_rules:
 - redundant_self_in_closure
  • Which Xcode version are you using ? 14.3
guard let lastTimerCallTime else {return}
self.lastTimerCallTime = Date()
@SimplyDanny
Copy link
Collaborator

This is a known shortcoming of this rule that must generally be accepted, unfortunately. The reason is that normal SwiftLint rules only operate on the Swift AST without knowing anything about types.

The line self.lastTimerCallTime = lastTimerCallTime would be the only special case that is allowed. The rule doesn't trigger on the self. Besides your example there are a bunch of other conceivable cases that cannot easily and especially fast be detected, however.

@SimplyDanny SimplyDanny added bug Unexpected and reproducible misbehavior. acceptable-false-positive False positives caused by rules that are unavoidable due to missing type information. labels May 15, 2023
@la-pieuvre
Copy link
Author

Thank you for your fast answer.
What about detecting the value assignment combined with the guard let to prevent the self to be remove?

@SimplyDanny
Copy link
Collaborator

SimplyDanny commented May 16, 2023

Well, it's unimportant where the symbol is defined. You can just have

let x = 1
self.x = 2

to make the rule fail. The point is that there can be a symbol named x anywhere in the outer scopes. Consider for example:

class X {
    var x = 1
    func f(x: Int = 2) {
        var x = 3
        g {
            var x = 4
            self.x = x
        }
    }
    func g(_ work: () -> Void) { work() }
}

This is valid Swift code with 3 reasons why self cannot be left out. Yes, it's possible to track any defined symbols in the same class/struct and trigger the rule only based on this knowledge. This is quite some effort, though.

@SimplyDanny
Copy link
Collaborator

I came up with a way to track all identifiers declared up to a certain statement in the code. With that knowledge, the redundant_self_in_closure rule is now able to know better which self. qualification is needed and which isn't.

That is supposed to fix your specific issue and a lot more similar cases.

@SimplyDanny SimplyDanny removed the acceptable-false-positive False positives caused by rules that are unavoidable due to missing type information. label Jun 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Unexpected and reproducible misbehavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants