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 rule can behave unexpectedly with withAnimation(_:_:) #4988

Closed
2 tasks done
ski-u opened this issue May 11, 2023 · 3 comments · Fixed by #4996
Closed
2 tasks done

redundant_self_in_closure rule can behave unexpectedly with withAnimation(_:_:) #4988

ski-u opened this issue May 11, 2023 · 3 comments · Fixed by #4996

Comments

@ski-u
Copy link

ski-u commented May 11, 2023

New Issue Checklist

Describe the bug

  1. Run SwiftLint v0.52.0 to the following code, for example
import SwiftUI

struct SampleView: View {
    @State private var number = 0

    var body: some View {
        Text("Sample")
    }

    private func add(_ number: Int) {
        withAnimation {
            self.number = number
        }
    }
}
  1. SwiftLint removed self and we have an error like ↓
スクリーンショット 2023-05-11 13 57 25
Complete output when running SwiftLint, including the stack trace and command used
$ swiftlint lint --enable-all-rules --fix
Correcting Swift files in current working directory
Correcting 'Contents.swift' (1/1)
/Users/sakai.yunosuke/test/Contents.swift:12:13 Corrected Redundant Self in Closure
Done correcting 1 file!

Environment

  • SwiftLint version (run swiftlint version to be sure)?: 0.52.0
  • Installation method used (Homebrew, CocoaPods, building from source, etc)?: Homebrew
  • Paste your configuration file: No configuration file
  • Are you using nested configurations?: No
    If so, paste their relative paths and respective contents.
  • Which Xcode version are you using (check xcodebuild -version)?: Xcode 14.3
  • Do you have a sample that shows the issue? Run echo "[string here]" | swiftlint lint --no-cache --use-stdin --enable-all-rules
    to quickly test if your example is really demonstrating the issue. If your example is more
    complex, you can use swiftlint lint --path [file here] --no-cache --enable-all-rules.
    • I have an example in "Describe the bug" section above.
@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.

However, as this case of updating state is quite common it seems reasonable to exclude simple assignments of the form self.x = x from being reported. There are still a lot of conceivable other cases that can not be recognized. So we have to expect and accept a certain amount of false positives in this rule.

@SimplyDanny SimplyDanny added enhancement acceptable-false-positive A false positive caused by a rule that is unavoidable due to missing type information labels May 11, 2023
@ski-u
Copy link
Author

ski-u commented May 12, 2023

Thank you for your explanation and working to fix this!

@PauliusPetru
Copy link

maybe there is a rule to which asks for selfs? Like it was before 14.3 basically

@SimplyDanny SimplyDanny removed the acceptable-false-positive A false positive caused by a rule that is 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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants