Add rule to convert `forEach { ... }` calls to for loops

[calda]

This PR adds a new disabled-by-default forLoop rule which converts .forEach { ... } calls to use for loops instead:

  let strings = ["foo", "bar", "baaz"]
- strings.forEach { placeholder in
+ for placeholder in strings {
      print(placeholder)
  }

// Supports anonymous closures!
- strings.forEach {
+ for string in strings {
-     print($0)
+     print(string)
  }

- foo.item().bar[2].baazValues(option: true).forEach {
+ for baazValue in foo.item().bar[2].baazValues(option: true) {
-     print($0)
+     print(baazValue)
  }

// Doesn't affect long multiline functional chains ✅
placeholderStrings
    .filter { $0.style == .fooBar }
    .map { $0.uppercased() }
    .forEach { print($0) }

The secret sauce here for converting from anonymous closures to non-anonymous closures is the GrammaticalNumber package, which I added as a dependency.

[calda]

Seems I need to tweak how the dependency is set up. Probably need to configure it in the Package.swift and not just the Xcode project.

The dependency is just a single file so it may be easier to just copy into the repo instead of adding it as an SPM dependency. Any preference @nicklockwood?

[nicklockwood]

@calda yeah, I'd definitely prefer to avoid the package dependency as it complicates the build process.

[nicklockwood]

(This is a great idea for a rule btw - and something I've wanted to add for a while 👍)

[codecov[bot]]

Codecov Report

Merging #1490 (fc6b5d0) into develop (5b0a2c2) will decrease coverage by 0.01%. The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           develop   #1490      +/-   ##
==========================================
- Coverage     0.00%   0.00%   -0.01%     
==========================================
  Files           44      45       +1     
  Lines        55802   56420     +618     
==========================================
  Hits             2       2              
- Misses       55800   56418     +618     

Files Changed	Coverage Δ
Sources/Examples.swift	0.00% <0.00%> (ø)
Sources/FormattingHelpers.swift	0.00% <0.00%> (ø)
Sources/GrammaticalNumber.swift	0.00% <0.00%> (ø)
Sources/Rules.swift	0.00% <0.00%> (ø)
Tests/RulesTests+Syntax.swift	0.00% <0.00%> (ø)
Tests/SwiftFormatTests.swift	0.00% <0.00%> (ø)

[calda]

Seems like the code coverage job is broken.

I see the windows CI job is uploading code coverage data, but not actually running any tests (it runs some test target but that target includes 0 actual test cases). Maybe that's what's breaking the code coverage reporting?

[calda]

CI is passing now @nicklockwood

[calda]

Ran this rule on our codebase and found some issues. Will post fixes in follow-up. Some examples include:

silly case where the code is using return void but which breaks when converted to continue:

We may want to special-case the reversed function because planets.reversed().forEach { ... } currently gets converted to for reversedItem in planets { ... } which is funny looking. Probably not a general way to handle this.

We probably need to check for cases where the forEach closure contains $1, $2 etc and not attempt to modify those closures, since we don't have a good name to use instead.

[calda]

I'm also thinking we want to add linebreaks when converting from a single-line forEach, so convert:

placeholderStrings.forEach { print($0) }

to:

for placeholderString in placeholderStrings {
  print(placeholderString)
}

[nicklockwood]

I'm thinking single-line loops should maybe stay on one line. And/or there should be an option to not convert one-liners 🤔

[calda]

An option makes sense to me

[nicklockwood]

@calda this rule landed in 0.53.0. FYI I made a few changes:

• Single line forEach is no longer converted by default. you can enabled that with an option
• Single line foreach is not wrapped when it's converted - there's a new wrapLoopBodies rule for that instead
• I rewrote the GrammaticalNumber logic to be simpler and handle more cases. You may find it behaves slightly differently with respect to your codebase

[calda]

Thanks for letting me know! I updated our tools to reflect the new options (https://github.com/airbnb/swift/pull/255)