realm / SwiftLint

A tool to enforce Swift style and conventions.
https://realm.github.io/SwiftLint
MIT License
18.46k stars 2.2k forks source link

`trailing_closure` rule disagrees with Swift 5.8 forward-scan matching #5590

Open defagos opened 1 month ago

defagos commented 1 month ago

New Issue Checklist

Describe the bug

With changes made between SwiftLint 0.54.0 and 0.55.1 the trailing_closure rule now enforces a practice that the Swift compiler considers as deprecated.

Consider a function (or method) whose signature involves several trailing closures with default arguments, like:

func trailingClosures(closure1: () -> Void = {}, closure2: () -> Void = {}) {}

Swift 5.8 introduced changes which lead to a warning being reported when calling the above function as follows:

func someCode() {
    // Xcode warning: "Backward matching of the unlabeled trailing closure is deprecated; label the argument
    // with 'closure2' to suppress this warning"
    trailingClosures {}
}

When fixing the code to silence the Xcode warning:

func someCode() {
    // swiftlint 0.55.1 warning: "Trailing closure syntax should be used whenever possible"
    trailingClosures(closure2: {})
}

SwiftLint 0.55.1 reports a warning. This was not the case with SwiftLint 0.54.0.

Complete output when running SwiftLint, including the stack trace and command used
$ swiftlint lint

Environment

opt_in_rules:
  - trailing_closure
func trailingClosures(closure1: () -> Void = {}, closure2: () -> Void = {}) {}

func someCode() {
    trailingClosures(closure2: {})
}
SimplyDanny commented 1 month ago

Unfortunately, this is impossible to detect on a syntax-level only. To analyse that correctly, full type information is necessary on the call site about the object that's called. So this is a false positive we have to accept.

defagos commented 1 month ago

Thanks for the feedback. Feel free to close this issue.

SimplyDanny commented 1 month ago

Thanks for the feedback. Feel free to close this issue.

It's still a bug. Maybe one day there's a new way to support this in the tool. Then we may want to revisit all so far "acceptable false positives".