Skip to content

Commit 9561066

Browse files
xedinallevato
authored andcommitted
[Lint] Add a rule to replace .forEach with a for-in loop
If there is no chaining using for-in loop more understandable and requires less chaining/nesting.
1 parent 5df0dd2 commit 9561066

File tree

5 files changed

+90
-0
lines changed

5 files changed

+90
-0
lines changed

Sources/SwiftFormat/Core/Pipelines+Generated.swift

+1
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ class LintPipeline: SyntaxVisitor {
143143
override func visit(_ node: FunctionCallExprSyntax) -> SyntaxVisitorContinueKind {
144144
visitIfEnabled(NoEmptyTrailingClosureParentheses.visit, for: node)
145145
visitIfEnabled(OnlyOneTrailingClosureArgument.visit, for: node)
146+
visitIfEnabled(ReplaceForEachWithForLoop.visit, for: node)
146147
return .visitChildren
147148
}
148149

Sources/SwiftFormat/Core/RuleNameCache+Generated.swift

+1
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ public let ruleNameCache: [ObjectIdentifier: String] = [
4242
ObjectIdentifier(OneVariableDeclarationPerLine.self): "OneVariableDeclarationPerLine",
4343
ObjectIdentifier(OnlyOneTrailingClosureArgument.self): "OnlyOneTrailingClosureArgument",
4444
ObjectIdentifier(OrderedImports.self): "OrderedImports",
45+
ObjectIdentifier(ReplaceForEachWithForLoop.self): "ReplaceForEachWithForLoop",
4546
ObjectIdentifier(ReturnVoidInsteadOfEmptyTuple.self): "ReturnVoidInsteadOfEmptyTuple",
4647
ObjectIdentifier(TypeNamesShouldBeCapitalized.self): "TypeNamesShouldBeCapitalized",
4748
ObjectIdentifier(UseEarlyExits.self): "UseEarlyExits",
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// This source file is part of the Swift.org open source project
4+
//
5+
// Copyright (c) 2014 - 2023 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See https://swift.org/LICENSE.txt for license information
9+
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
//===----------------------------------------------------------------------===//
12+
13+
import SwiftSyntax
14+
15+
/// Replace `forEach` with `for-in` loop unless its argument is a function reference.
16+
///
17+
/// Lint: invalid use of `forEach` yield will yield a lint error.
18+
@_spi(Rules)
19+
public final class ReplaceForEachWithForLoop : SyntaxLintRule {
20+
public override func visit(_ node: FunctionCallExprSyntax) -> SyntaxVisitorContinueKind {
21+
// We are only interested in calls with a single trailing closure
22+
// argument.
23+
if !node.arguments.isEmpty || node.trailingClosure == nil ||
24+
!node.additionalTrailingClosures.isEmpty {
25+
return .visitChildren
26+
}
27+
28+
guard let member = node.calledExpression.as(MemberAccessExprSyntax.self) else {
29+
return .visitChildren
30+
}
31+
32+
guard let memberName = member.declName.baseName.as(TokenSyntax.self),
33+
memberName.text == "forEach" else {
34+
return .visitChildren
35+
}
36+
37+
// If there is another chained member after `.forEach`,
38+
// let's skip the diagnostic because resulting code might
39+
// be less understandable.
40+
if node.parent?.is(MemberAccessExprSyntax.self) == true {
41+
return .visitChildren
42+
}
43+
44+
diagnose(.replaceForEachWithLoop(), on: memberName)
45+
return .visitChildren
46+
}
47+
}
48+
49+
extension Finding.Message {
50+
public static func replaceForEachWithLoop() -> Finding.Message {
51+
"replace use of '.forEach { ... }' with for-in loop"
52+
}
53+
}

Sources/SwiftFormatConfiguration/RuleRegistry+Generated.swift

+1
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ enum RuleRegistry {
4141
"OneVariableDeclarationPerLine": true,
4242
"OnlyOneTrailingClosureArgument": true,
4343
"OrderedImports": true,
44+
"ReplaceForEachWithForLoop": true,
4445
"ReturnVoidInsteadOfEmptyTuple": true,
4546
"TypeNamesShouldBeCapitalized": true,
4647
"UseEarlyExits": false,
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import _SwiftFormatTestSupport
2+
3+
@_spi(Rules) import SwiftFormat
4+
5+
6+
final class ReplaceForEachWithForLoopTests: LintOrFormatRuleTestCase {
7+
func test() {
8+
assertLint(
9+
ReplaceForEachWithForLoop.self,
10+
"""
11+
values.1️⃣forEach { $0 * 2 }
12+
values.map { $0 }.2️⃣forEach { print($0) }
13+
values.forEach(callback)
14+
values.forEach { $0 }.chained()
15+
values.forEach({ $0 }).chained()
16+
values.3️⃣forEach {
17+
let arg = $0
18+
return arg + 1
19+
}
20+
values.forEach {
21+
let arg = $0
22+
return arg + 1
23+
} other: {
24+
42
25+
}
26+
""",
27+
findings: [
28+
FindingSpec("1️⃣", message: "replace use of '.forEach { ... }' with for-in loop"),
29+
FindingSpec("2️⃣", message: "replace use of '.forEach { ... }' with for-in loop"),
30+
FindingSpec("3️⃣", message: "replace use of '.forEach { ... }' with for-in loop")
31+
]
32+
)
33+
}
34+
}

0 commit comments

Comments
 (0)