realm / SwiftLint

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

Autocorrect for `redundant_void_return` can cause code breakage #5609

Open mikaryyn opened 1 month ago

mikaryyn commented 1 month ago

New Issue Checklist

Describe the bug

After upgrading to SwiftLint to 0.55.1 code like this (below) breaks when applying --fix. Reverting to version 0.53.0 makes it work. I found some easy workarounds but I don't think it's good to break existing code.

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

main.swift

func takesClosure(_ closure: () -> Void) { }

@discardableResult func returnsInt() -> Int { 42 }

let testClosure = { () -> Void in returnsInt() }

takesClosure(testClosure)

This code works but after swiftlint --fix, swift main.swift causes error:

main.swift:7:14: error: cannot convert value of type '() -> Int' to expected argument type '() -> Void'

Environment

vadimvitvickiy commented 3 weeks ago

Having the same issue.

SimplyDanny commented 2 weeks ago

As SwiftLint works on the syntax level only, it cannot behave entirely correct here. Due to implicit returns, the statement is ambiguous here. Without full type information for the function, it's impossible to resolve the ambivalence.

There are three ways to "fix" this that come into my mind. They all come with their own drawbacks:

  1. Ignore closures with just a single statement. This would suddenly silence a lot of valid findings from then on as well.
  2. The rule has the option include_closures which can be set to false to exclude closure signatures entirely if there are too many false positives.
  3. An explicit // swiftlint:disable command can be used.

From all these, I would actually prefer and recommend the last one as we cannot achieve full correctness in any way.