nicklockwood / SwiftFormat

A command-line tool and Xcode Extension for formatting Swift code
MIT License
7.62k stars 623 forks source link

named tuple in closure argument destructuring breaks after formatting #1709

Closed alistra closed 3 weeks ago

alistra commented 1 month ago

When I have a file with named tuple destructuring in closure argument like this:

❯ cat bug1.swift
    let dict: [String: String] = ["a":"b"]

    dict.forEach { (header: (key: String, value: String)) in
        print(header.key)
        print(header.value)
    }
❯ swiftc bug1.swift && ./bug1
a
b

and I try to format it using

❯ swiftformat --version
0.53.10

without any configuration file

❯ swiftformat bug1.swift
Running SwiftFormat...
warning: No Swift version was specified, so some formatting features were disabled. Specify the version of Swift you are using with the --swiftversion option, or by adding a .swift-version file to your project.
SwiftFormat completed in 0.02s.
1/1 files formatted.

I get this error when compiling

❯ swiftc bug1.swift && ./bug1
bug1.swift:4:18: error: value of type 'String' has no member 'key'
    print(header.key)
          ~~~~~~ ^~~
bug1.swift:5:18: error: value of type 'String' has no member 'value'
    print(header.value)
          ~~~~~~ ^~~~~
error: fatalError

because the code formats to this

❯ cat bug1.swift
  let dict: [String: String] = ["a": "b"]

  for (header, value) in dict {
      print(header.key)
      print(header.value)
  }

and realistically should either format to this:

  let dict: [String: String] = ["a": "b"]

  for (key, value) in dict {
      print(key)
      print(value)
  }

or not be changed at all.

nicklockwood commented 1 month ago

@alistra thanks for the report. I've never actually seen single-element tuples in action (though I was aware they'd been added recently), and I didn't know that they could be used this way!

It's not really clear what the right solution is here since there doesn't appear to be an equivalent syntax for for loop pattern matching. Probably it's safest just to detect this scenario and disable the preferForLoop rule in these cases.

cc: @calda

calda commented 1 month ago

Agreed, we can disable the preferForLoop rule if any of the arguments have a type that is a tuple with labeled arguments. I can post a fix for this.

edit: Oh I see, we aren't renaming the tuple arguments or anything, the output for (header, value) in dict is just totally wrong. Should be an easy fix to make it just use for header in dict.

alistra commented 1 month ago

@calda seems to work like you say.

  let dict: [String: String] = ["a": "b"]

  for header in dict {
      print(header.key)
      print(header.value)
  }

is a good result. Question is should the formatter always emit .key and .value or stick to labels in the original code

alistra commented 1 month ago

I saw the implementation - it uses the custom labels, but compiler emits a warning - seems great.

nicklockwood commented 3 weeks ago

@alistra fix landed in 0.54.0