From 2b0462a96461b1dd5caba2fe39e6661190f6d8e1 Mon Sep 17 00:00:00 2001 From: Alfons Hoogervorst Date: Sun, 18 Jun 2023 23:34:41 +0200 Subject: [PATCH 01/28] Implement ReduceIntoInsteadOfLoop rule --- .../Idiomatic/ReduceIntoInsteadOfLoop.swift | 463 ++++++++++++++++++ .../ReduceIntoInsteadOfLoopExamples.swift | 80 +++ 2 files changed, 543 insertions(+) create mode 100644 Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift create mode 100644 Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopExamples.swift diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift new file mode 100644 index 0000000000..7b4bc3ec1f --- /dev/null +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift @@ -0,0 +1,463 @@ +import SwiftSyntax + +struct ReduceIntoInsteadOfLoop: ConfigurationProviderRule, SwiftSyntaxRule, OptInRule { + var configuration = SeverityConfiguration(.warning) + + static let description = RuleDescription( + identifier: "reduce_into_instead_of_loop", + name: "Reduce Into Instead Of Loop", + description: "Prefer using reduce(into:) instead of a loop", + kind: .idiomatic, + nonTriggeringExamples: nonTriggeringExamples, + triggeringExamples: triggeringExamples + ) + + init() {} + + func makeVisitor(file: SwiftLintFile) -> ViolationsSyntaxVisitor { + Visitor(viewMode: .sourceAccurate) + } +} + +fileprivate extension ReduceIntoInsteadOfLoop { + final class Visitor: ViolationsSyntaxVisitor { + /// Collects all VariableDecl of collection types, finds a ForInStmt, and checks if the ForInStmts + /// body references one of the earlier encounterd VariableDecls. + override func visitPost(_ node: CodeBlockItemListSyntax) { + // reduce into forInStmts and variableDecls map + guard let all = node.allVariableDeclsForInStatmts() else { + return + } + // reduce variableDecls into the ones we're interested in + let selected = all.reduce(into: [ForInStmtSyntax: [VariableDeclSyntax]]()) { partialResult, element in + // we're interested in a couple of variable decls: + // * fully type declared + // * implicitly declared by initializer + let interestingVariableDecls = element.value.filter { variableDecl in + return variableDecl.isTypeAnnotatedAndInitializer + || variableDecl.isCollectionTypeInitializer + } + guard !interestingVariableDecls.isEmpty else { + return + } + partialResult[element.key] = interestingVariableDecls + } + guard !selected.isEmpty else { + return + } + let referencedVars = selected.reduce(into: Set()) { partialResult, keyValue in + let (forInStmt, variableDecls) = keyValue + if let allReferencedVars = forInStmt.referencedVariables(for: variableDecls) { + partialResult.formUnion(allReferencedVars) + } + } + guard !referencedVars.isEmpty else { + return + } + self.violations.append(contentsOf: referencedVars.map { $0.position }) + } + } + + static let collectionTypes: [CollectionType] = [ + CollectionType(name: "Set", genericArguments: 1), + CollectionType(name: "Array", genericArguments: 1), + CollectionType(name: "Dictionary", genericArguments: 2) + ] + + static let collectionNames: [String: CollectionType] = + ReduceIntoInsteadOfLoop.collectionTypes.reduce(into: [String: CollectionType]()) { partialResult, type in + return partialResult[type.name] = type + } +} + +private extension CodeBlockItemListSyntax { + /// Returns a dictionary with all VariableDecls preceding a ForInStmt, at the same scope level + func allVariableDeclsForInStatmts() -> [ForInStmtSyntax: [VariableDeclSyntax]]? { + typealias IndexRange = Range + typealias IndexRangeForStmts = (IndexRange, ForInStmtSyntax) + typealias IndexRangeStmts = (IndexRange, CodeBlockItemSyntax) + // collect all ForInStmts and track their index ranges + let indexed: [IndexRangeForStmts] = self.reduce(into: [IndexRangeStmts]()) { partialResult, codeBlockItem in + guard codeBlockItem.is(ForInStmtSyntax.self) else { + return + } + // Not sure whether ForInStmtSyntax.index == CodeBlockItem.index + guard let last = partialResult.last else { + partialResult.append((self.startIndex.. = []`, `: Array<> = []`, or `: Dictionary<> = [:]` + var isTypeAnnotatedAndInitializer: Bool { + guard self.isVar && self.identifier != nil, + let idIndex = self.firstIndexOf(IdentifierPatternSyntax.self) else { + return false + } + // Is type-annotated, and initialized? + guard let typeAnnotationIndex = self.next(after: idIndex), + let typeAnnotation = typeAnnotationIndex.as(TypeAnnotationSyntax.self), + let type = typeAnnotation.collectionDeclarationType(), + let initializerClause = self.next(after: typeAnnotationIndex)?.as(InitializerClauseSyntax.self) else { + return false + } + return initializerClause.isTypeInitializer(for: type) + } + + /// Is initialized with empty collection + /// E.g. + /// `= Set(), = Array(), = Dictionary[:]` + /// but a couple of more, see `InitializerClauseExprSyntax.isCollectionInitializer` + var isCollectionTypeInitializer: Bool { + guard self.isVar && self.identifier != nil, + let idIndex = self.firstIndexOf(IdentifierPatternSyntax.self) else { + return false + } + let initializerClause = self.next(after: idIndex, of: InitializerClauseSyntax.self) + guard initializerClause?.isTypeInitializer() ?? false else { + return false + } + return true + } + + func firstOf(_ type: T.Type) -> T? { + return self.bindings.first { patternBinding in + return patternBinding.as(type) != nil + } as? T + } + + func firstIndexOf(_ type: T.Type) -> PatternBindingListSyntax.Index? { + return self.bindings.firstIndex(where: { patternBinding in + return patternBinding.as(type) != nil + }) + } +} + +private extension TypeAnnotationSyntax { + /// Returns collection's type name, or `nil` if unknown + func collectionDeclarationType() -> CollectionType? { + if let genericTypeName = self.genericCollectionDeclarationType() { + return genericTypeName + } else if let array = self.arrayDeclarationType() { + return array + } else if let dictionary = self.dictionaryDeclarationType() { + return dictionary + } else { + return nil + } + } + + /// var x: Set<> + /// var x: Array<> + /// var x: Dictionary<> + func genericCollectionDeclarationType() -> CollectionType? { + guard let simpleTypeIdentifier = self.type.as(SimpleTypeIdentifierSyntax.self), + let genericArgumentClause = simpleTypeIdentifier.genericArgumentClause, + genericArgumentClause.leftAngleBracket.tokenKind == .leftAngle, + genericArgumentClause.rightAngleBracket.tokenKind == .rightAngle, + case .identifier(let name) = simpleTypeIdentifier.name.tokenKind, + let collectionType = CollectionType.names[name], + genericArgumentClause.arguments.count == collectionType.genericArguments else { + return nil + } + return collectionType + } + + /// var x: [Y] + func arrayDeclarationType() -> CollectionType? { + guard let arrayType = self.type.as(ArrayTypeSyntax.self), + case .leftSquareBracket = arrayType.leftSquareBracket.tokenKind, + case .rightSquareBracket = arrayType.rightSquareBracket.tokenKind, + arrayType.elementType.kind == .simpleTypeIdentifier else { + return nil + } + return .array + } + + /// var x: [K: V] + func dictionaryDeclarationType() -> CollectionType? { + guard let dictionaryType = self.type.as(DictionaryTypeSyntax.self), + case .leftSquareBracket = dictionaryType.leftSquareBracket.tokenKind, + case .rightSquareBracket = dictionaryType.rightSquareBracket.tokenKind, + case .colon = dictionaryType.colon.tokenKind else { + return nil + } + return .dictionary + } +} + +private extension InitializerClauseSyntax { + /// --- + /// If `nil` we don't know the type and investigate the following, which is a + /// `FunctionCallExpr`: + /// + /// var x = Set(...) + /// var y = Array(...) + /// var z = Dictionary(...) + /// + /// Otherwise we investigate, which checks for `FunctionCallExpr`, + /// `MemberAccessExpr`, `DictionaryExpr` and `ArrayExpr`: + /// + /// 1. `= Set(...)` | `Set(...)` | `.init(...)` | `[]` + /// 2. `= Array(...)` | `Array(...)` | `.init(...)` | `[]` + /// 3. `= Dictionary()` | `Dictionary()` | `.init(..)` | `[:]` + func isTypeInitializer(for collectionType: CollectionType? = nil) -> Bool { + func isSupportedType(with name: String) -> Bool { + if let collectionType { + return collectionType.name == name + } else { + return CollectionType.names[name] != nil + } + } + guard self.equal.tokenKind == .equal else { + return false + } + if let functionCallExpr = self.value.as(FunctionCallExprSyntax.self) { + // either construction using explicit specialisation, or general construction + if let specializeExpr = functionCallExpr.calledExpression.as(SpecializeExprSyntax.self), + let identifierExpr = specializeExpr.expression.as(IdentifierExprSyntax.self), + case .identifier(let typename) = identifierExpr.identifier.tokenKind { + return isSupportedType(with: typename) + } else if let identifierExpr = functionCallExpr.calledExpression.as(IdentifierExprSyntax.self), + case .identifier(let typename) = identifierExpr.identifier.tokenKind { + return isSupportedType(with: typename) + } else if let memberAccessExpr = functionCallExpr.calledExpression.as(MemberAccessExprSyntax.self), + memberAccessExpr.name.tokenKind == .keyword(.`init`) { + return true + } + } else if collectionType == .dictionary, + self.value.as(DictionaryExprSyntax.self) != nil { + return true + } else if collectionType == .array || collectionType == .set, + self.value.as(ArrayExprSyntax.self) != nil { + return true + } + return false + } +} + +private extension ForInStmtSyntax { + /// Checks whether a ForInStmtSyntax references the vars in `variables`. + /// Defers to `CodeBlockItemSyntax.referencedVariable(for:)` + func referencedVariables(for variables: [VariableDeclSyntax]?) -> Set? { + guard let variables, !variables.isEmpty, + let codeBlock = self.body.as(CodeBlockSyntax.self), + let codeBlockItemList = codeBlock.statements.as(CodeBlockItemListSyntax.self) else { + return nil + } + // check whether each of the items references a var + let references: Set = codeBlockItemList.reduce(into: .init(), { partialResult, codeBlock in + // see if each codeblock item references variable + // one variable reference should be plenty, so it covers semicolon-ized statements: + // someMutation(); someOtherMutation() + variables.forEach { variableDecl in + guard let referenced = codeBlock.referencedVariable(for: variableDecl) else { + return + } + partialResult.insert(referenced) + } + }) + return references.isEmpty ? nil : references + } +} + +private extension CodeBlockItemSyntax { + func referencedVariable(for variableDecl: VariableDeclSyntax?) -> ReferencedVariable? { + guard let identifier = variableDecl?.identifier else { + return nil + } + return self.referencedVariable(for: identifier) + } + + func referencedVariable(for varName: String) -> ReferencedVariable? { + if let functionCallExpr = self.item.as(FunctionCallExprSyntax.self) { + return functionCallExpr.referencedVariable(for: varName) + } + if let sequenceExpr = self.item.as(SequenceExprSyntax.self) { + return sequenceExpr.referencedVariable(for: varName) + } + return nil + } +} + +private extension FunctionCallExprSyntax { + /// varName.method(x, y, n) + func referencedVariable(for varName: String) -> ReferencedVariable? { + guard self.leftParen?.tokenKind == .leftParen, + self.rightParen?.tokenKind == .rightParen, + let memberAccessExpr = self.calledExpression.as(MemberAccessExprSyntax.self), + memberAccessExpr.dot.tokenKind == .period, + let arguments = self.argumentList.as(TupleExprElementListSyntax.self)?.count, + let identifierExpr = memberAccessExpr.base?.as(IdentifierExprSyntax.self), + identifierExpr.identifier.tokenKind == .identifier(varName), + case .identifier(let name) = memberAccessExpr.name.tokenKind else { + return nil + } + return .init( + name: varName, + position: memberAccessExpr.positionAfterSkippingLeadingTrivia, + kind: .method(name: name, arguments: arguments) + ) + } +} + +private extension SequenceExprSyntax { + /// varName[xxx] = ... + func referencedVariable(for varName: String) -> ReferencedVariable? { + guard let exprList = self.as(ExprListSyntax.self), exprList.count >= 2 else { + return nil + } + let first = exprList.startIndex + let second = exprList.index(after: first) + guard let subscrExpr = exprList[first].as(SubscriptExprSyntax.self), + let assignmentExpr = exprList[second].as(AssignmentExprSyntax.self), + assignmentExpr.assignToken.text == "=", + subscrExpr.leftBracket.tokenKind == .leftSquareBracket, + subscrExpr.rightBracket.tokenKind == .rightSquareBracket, + subscrExpr.argumentList.is(TupleExprElementListSyntax.self), + let identifierExpr = subscrExpr.calledExpression.as(IdentifierExprSyntax.self), + identifierExpr.identifier.tokenKind == .identifier(varName) else { + return nil + } + return .init( + name: varName, + position: exprList.positionAfterSkippingLeadingTrivia, + kind: .assignment(subscript: true) + ) + } +} + +private extension PatternBindingListSyntax { + func optionalIndex(after: PatternBindingListSyntax.Index?) -> PatternBindingListSyntax.Index? { + guard let after else { + return nil + } + return self.index(after: after) + } +} + +private extension VariableDeclSyntax { + func next(after index: PatternBindingListSyntax.Index?) -> PatternBindingSyntax? { + guard let index = self.bindings.optionalIndex(after: index), + index >= self.bindings.startIndex && index < self.bindings.endIndex else { + return nil + } + return self.bindings[index] + } + + func next(after index: PatternBindingListSyntax.Index?, of type: T.Type) -> T? { + guard let index = self.bindings.optionalIndex(after: index), + index >= self.bindings.startIndex && index < self.bindings.endIndex else { + return nil + } + return self.bindings[index].as(type) + } + + func next(after: PatternBindingSyntax?) -> PatternBindingSyntax? { + guard let after, let index = self.bindings.firstIndex(where: { patterBindingSyntax in + return patterBindingSyntax == after + }) else { + return nil + } + let newIndex = self.bindings.index(after: index) + guard newIndex >= self.bindings.startIndex && newIndex < self.bindings.endIndex else { + return nil + } + return self.bindings[newIndex] + } + + var isVar: Bool { + return self.bindingKeyword.tokenKind == .keyword(.var) + } + + var identifier: String? { + guard let identifierPattern = self.firstOf(IdentifierPatternSyntax.self), + case .identifier(let name) = identifierPattern.identifier.tokenKind else { + return nil + } + return name + } +} + +private struct ReferencedVariable: Hashable { + let name: String + let position: AbsolutePosition + let kind: Kind + + func hash(into hasher: inout Hasher) { + hasher.combine(self.name) + hasher.combine(self.position.utf8Offset) + hasher.combine(self.kind) + } + + enum Kind: Hashable { + case method(name: String, arguments: Int) + case assignment(subscript: Bool) + + func hash(into hasher: inout Hasher) { + switch self { + case let .method(name, arguments): + hasher.combine("method") + hasher.combine(name) + hasher.combine(arguments) + case let .assignment(`subscript`): + hasher.combine("assignment") + hasher.combine(`subscript`) + } + } + } +} + +private struct CollectionType: Hashable { + let name: String + let genericArguments: Int + + static let types: [CollectionType] = [ + .set, + .array, + .dictionary + ] + + static let array = CollectionType(name: "Array", genericArguments: 1) + static let set = CollectionType(name: "Set", genericArguments: 1) + static let dictionary = CollectionType(name: "Dictionary", genericArguments: 2) + + static let names: [String: CollectionType] = { + return CollectionType.types.reduce(into: [String: CollectionType]()) { partialResult, collectionType in + partialResult[collectionType.name] = collectionType + } + }() +} diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopExamples.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopExamples.swift new file mode 100644 index 0000000000..ff1b4a82ce --- /dev/null +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopExamples.swift @@ -0,0 +1,80 @@ +// var encountered1: Set = [] +// var encountered1a = Set() +// var encountered1b: Set = Set() +// var encountered1c: Set = .init() +// var encountered2: [String] = [] +// var encountered2a = [String]() +// var encountered2b: [String] = [1, 2, 3, 4] +// var encountered2c: [String] = Array(contentsOf: other) +// var encountered3: Array = [] +// var encountered4: Dictionary = [] +// var encountered4b: [String: Int] = [:] +// var encountered4c: [String: Int] = ["2": 2, "3": 3] +// for eachN in someArray { +// encountered.insert(eachN) +// encountered1[2] = 45 +// let newSet = encountered.popFirst() +// } + +internal extension ReduceIntoInsteadOfLoop { + static let nonTriggeringExamples: [Example] = [ +// Example(""" +// class Foo { +// static let constant: Int = 1 +// var variable: Int = 2 +// } +// """), +// Example(""" +// struct Foo { +// static let constant: Int = 1 +// } +// """), +// Example(""" +// enum InstFooance { +// static let constant = 1 +// } +// """), +// Example(""" +// struct Foo { +// let property1 +// let property2 +// init(property1: Int, property2: String) { +// self.property1 = property1 +// self.property2 = property2 +// } +// } +// """) + Example(""" + let encountered: Set = someArray.reduce(into: Set(), { result, eachN in + result.insert(eachN) + }) + """) + ] + + static let triggeringExamples: [Example] = [ +// Example(""" +// class Foo { +// static let one = 32 +// ↓let constant: Int = 1 +// } +// """), +// Example(""" +// struct Foo { +// ↓let constant: Int = 1 +// } +// """), +// Example(""" +// enum Foo { +// ↓let constant: Int = 1 +// } +// """), + Example(""" + var encountered: Set = [] + for eachN in someArray { + ↓encountered.insert(eachN) + } + """) + ] +} + + From abf8ea91ddcee8ddeb93c21c80593893a1eb6280 Mon Sep 17 00:00:00 2001 From: Alfons Hoogervorst Date: Sun, 18 Jun 2023 23:53:27 +0200 Subject: [PATCH 02/28] Cleanups --- .../Idiomatic/ReduceIntoInsteadOfLoop.swift | 47 +++++-------------- 1 file changed, 11 insertions(+), 36 deletions(-) diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift index 7b4bc3ec1f..e9ada2fe81 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift @@ -21,8 +21,6 @@ struct ReduceIntoInsteadOfLoop: ConfigurationProviderRule, SwiftSyntaxRule, OptI fileprivate extension ReduceIntoInsteadOfLoop { final class Visitor: ViolationsSyntaxVisitor { - /// Collects all VariableDecl of collection types, finds a ForInStmt, and checks if the ForInStmts - /// body references one of the earlier encounterd VariableDecls. override func visitPost(_ node: CodeBlockItemListSyntax) { // reduce into forInStmts and variableDecls map guard let all = node.allVariableDeclsForInStatmts() else { @@ -30,9 +28,7 @@ fileprivate extension ReduceIntoInsteadOfLoop { } // reduce variableDecls into the ones we're interested in let selected = all.reduce(into: [ForInStmtSyntax: [VariableDeclSyntax]]()) { partialResult, element in - // we're interested in a couple of variable decls: - // * fully type declared - // * implicitly declared by initializer + // we're interested fully type declared and implicitly declared by initializer let interestingVariableDecls = element.value.filter { variableDecl in return variableDecl.isTypeAnnotatedAndInitializer || variableDecl.isCollectionTypeInitializer @@ -98,8 +94,7 @@ private extension CodeBlockItemListSyntax { guard !indexed.isEmpty else { return nil } - // Attach VariableDecls to ForInStmt. Note that we only attach VariableDecls that - // are at the same level of scope of the ForInStmt. + // only VariableDecls on same level of scope of the ForInStmt. let result = self.reduce(into: [ForInStmtSyntax: [VariableDeclSyntax]]()) { partialResult, codeBlockItem in guard let variableDecl = codeBlockItem.as(VariableDeclSyntax.self) else { return @@ -119,9 +114,7 @@ private extension CodeBlockItemListSyntax { } private extension VariableDeclSyntax { - /// Is type declared with initializer - /// E.g: - /// `: Set<> = []`, `: Array<> = []`, or `: Dictionary<> = [:]` + /// Is type declared with initializer: `: Set<> = []`, `: Array<> = []`, or `: Dictionary<> = [:]` var isTypeAnnotatedAndInitializer: Bool { guard self.isVar && self.identifier != nil, let idIndex = self.firstIndexOf(IdentifierPatternSyntax.self) else { @@ -137,9 +130,7 @@ private extension VariableDeclSyntax { return initializerClause.isTypeInitializer(for: type) } - /// Is initialized with empty collection - /// E.g. - /// `= Set(), = Array(), = Dictionary[:]` + /// Is initialized with empty collection: `= Set(), = Array(), = Dictionary[:]` /// but a couple of more, see `InitializerClauseExprSyntax.isCollectionInitializer` var isCollectionTypeInitializer: Bool { guard self.isVar && self.identifier != nil, @@ -167,7 +158,6 @@ private extension VariableDeclSyntax { } private extension TypeAnnotationSyntax { - /// Returns collection's type name, or `nil` if unknown func collectionDeclarationType() -> CollectionType? { if let genericTypeName = self.genericCollectionDeclarationType() { return genericTypeName @@ -180,9 +170,7 @@ private extension TypeAnnotationSyntax { } } - /// var x: Set<> - /// var x: Array<> - /// var x: Dictionary<> + /// var x: Set<>, var x: Array<>, var x: Dictionary<> func genericCollectionDeclarationType() -> CollectionType? { guard let simpleTypeIdentifier = self.type.as(SimpleTypeIdentifierSyntax.self), let genericArgumentClause = simpleTypeIdentifier.genericArgumentClause, @@ -221,16 +209,11 @@ private extension TypeAnnotationSyntax { private extension InitializerClauseSyntax { /// --- - /// If `nil` we don't know the type and investigate the following, which is a - /// `FunctionCallExpr`: - /// + /// If `nil` we don't know the type and investigate the following, which is a`FunctionCallExpr`: /// var x = Set(...) /// var y = Array(...) /// var z = Dictionary(...) - /// - /// Otherwise we investigate, which checks for `FunctionCallExpr`, - /// `MemberAccessExpr`, `DictionaryExpr` and `ArrayExpr`: - /// + /// Otherwise checks for `FunctionCallExpr`,`MemberAccessExpr`,`DictionaryExpr` and `ArrayExpr`: /// 1. `= Set(...)` | `Set(...)` | `.init(...)` | `[]` /// 2. `= Array(...)` | `Array(...)` | `.init(...)` | `[]` /// 3. `= Dictionary()` | `Dictionary()` | `.init(..)` | `[:]` @@ -242,9 +225,7 @@ private extension InitializerClauseSyntax { return CollectionType.names[name] != nil } } - guard self.equal.tokenKind == .equal else { - return false - } + guard self.equal.tokenKind == .equal else { return false } if let functionCallExpr = self.value.as(FunctionCallExprSyntax.self) { // either construction using explicit specialisation, or general construction if let specializeExpr = functionCallExpr.calledExpression.as(SpecializeExprSyntax.self), @@ -270,24 +251,18 @@ private extension InitializerClauseSyntax { } private extension ForInStmtSyntax { - /// Checks whether a ForInStmtSyntax references the vars in `variables`. - /// Defers to `CodeBlockItemSyntax.referencedVariable(for:)` func referencedVariables(for variables: [VariableDeclSyntax]?) -> Set? { guard let variables, !variables.isEmpty, let codeBlock = self.body.as(CodeBlockSyntax.self), let codeBlockItemList = codeBlock.statements.as(CodeBlockItemListSyntax.self) else { return nil } - // check whether each of the items references a var let references: Set = codeBlockItemList.reduce(into: .init(), { partialResult, codeBlock in - // see if each codeblock item references variable - // one variable reference should be plenty, so it covers semicolon-ized statements: - // someMutation(); someOtherMutation() + // no need to cover one liner: someMutation(); someOtherMutation() variables.forEach { variableDecl in - guard let referenced = codeBlock.referencedVariable(for: variableDecl) else { - return + if let referenced = codeBlock.referencedVariable(for: variableDecl) { + partialResult.insert(referenced) } - partialResult.insert(referenced) } }) return references.isEmpty ? nil : references From 9d90eba49a368936ead450700f83d6c71abf2d59 Mon Sep 17 00:00:00 2001 From: Alfons Hoogervorst Date: Mon, 19 Jun 2023 04:50:06 +0200 Subject: [PATCH 03/28] Fix build --- .../Models/BuiltInRules.swift | 1 + .../Idiomatic/ReduceIntoInsteadOfLoop.swift | 4 +- .../ReduceIntoInsteadOfLoopExamples.swift | 38 +++++++++---------- 3 files changed, 21 insertions(+), 22 deletions(-) diff --git a/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift b/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift index b3f6a1a45e..55335d7a25 100644 --- a/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift +++ b/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift @@ -161,6 +161,7 @@ public let builtInRules: [Rule.Type] = [ RawValueForCamelCasedCodableEnumRule.self, ReduceBooleanRule.self, ReduceIntoRule.self, + ReduceIntoInsteadOfLoop.self, RedundantDiscardableLetRule.self, RedundantNilCoalescingRule.self, RedundantObjcAttributeRule.self, diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift index e9ada2fe81..ea6bda8001 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift @@ -8,8 +8,8 @@ struct ReduceIntoInsteadOfLoop: ConfigurationProviderRule, SwiftSyntaxRule, OptI name: "Reduce Into Instead Of Loop", description: "Prefer using reduce(into:) instead of a loop", kind: .idiomatic, - nonTriggeringExamples: nonTriggeringExamples, - triggeringExamples: triggeringExamples + nonTriggeringExamples: ReduceIntoInsteadOfLoopExamples.nonTriggeringExamples, + triggeringExamples: ReduceIntoInsteadOfLoopExamples.triggeringExamples ) init() {} diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopExamples.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopExamples.swift index ff1b4a82ce..4498998c22 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopExamples.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopExamples.swift @@ -1,22 +1,4 @@ -// var encountered1: Set = [] -// var encountered1a = Set() -// var encountered1b: Set = Set() -// var encountered1c: Set = .init() -// var encountered2: [String] = [] -// var encountered2a = [String]() -// var encountered2b: [String] = [1, 2, 3, 4] -// var encountered2c: [String] = Array(contentsOf: other) -// var encountered3: Array = [] -// var encountered4: Dictionary = [] -// var encountered4b: [String: Int] = [:] -// var encountered4c: [String: Int] = ["2": 2, "3": 3] -// for eachN in someArray { -// encountered.insert(eachN) -// encountered1[2] = 45 -// let newSet = encountered.popFirst() -// } - -internal extension ReduceIntoInsteadOfLoop { +internal struct ReduceIntoInsteadOfLoopExamples { static let nonTriggeringExamples: [Example] = [ // Example(""" // class Foo { @@ -77,4 +59,20 @@ internal extension ReduceIntoInsteadOfLoop { ] } - +// var encountered1: Set = [] +// var encountered1a = Set() +// var encountered1b: Set = Set() +// var encountered1c: Set = .init() +// var encountered2: [String] = [] +// var encountered2a = [String]() +// var encountered2b: [String] = [1, 2, 3, 4] +// var encountered2c: [String] = Array(contentsOf: other) +// var encountered3: Array = [] +// var encountered4: Dictionary = [] +// var encountered4b: [String: Int] = [:] +// var encountered4c: [String: Int] = ["2": 2, "3": 3] +// for eachN in someArray { +// encountered.insert(eachN) +// encountered1[2] = 45 +// let newSet = encountered.popFirst() +// } From 437da087481024ab8f14d9b55dab9f2a05f29c72 Mon Sep 17 00:00:00 2001 From: Alfons Hoogervorst Date: Mon, 19 Jun 2023 17:48:28 +0200 Subject: [PATCH 04/28] Move models to separate file --- .../Idiomatic/ReduceIntoInsteadOfLoop.swift | 54 ++-------------- .../ReduceIntoInsteadOfLoopModels.swift | 61 +++++++++++++++++++ 2 files changed, 65 insertions(+), 50 deletions(-) create mode 100644 Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopModels.swift diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift index ea6bda8001..a8e63c74ee 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift @@ -1,5 +1,8 @@ import SwiftSyntax +typealias ReferencedVariable = ReduceIntoInsteadOfLoop.ReferencedVariable +typealias CollectionType = ReduceIntoInsteadOfLoop.CollectionType + struct ReduceIntoInsteadOfLoop: ConfigurationProviderRule, SwiftSyntaxRule, OptInRule { var configuration = SeverityConfiguration(.warning) @@ -19,7 +22,7 @@ struct ReduceIntoInsteadOfLoop: ConfigurationProviderRule, SwiftSyntaxRule, OptI } } -fileprivate extension ReduceIntoInsteadOfLoop { +internal extension ReduceIntoInsteadOfLoop { final class Visitor: ViolationsSyntaxVisitor { override func visitPost(_ node: CodeBlockItemListSyntax) { // reduce into forInStmts and variableDecls map @@ -387,52 +390,3 @@ private extension VariableDeclSyntax { } } -private struct ReferencedVariable: Hashable { - let name: String - let position: AbsolutePosition - let kind: Kind - - func hash(into hasher: inout Hasher) { - hasher.combine(self.name) - hasher.combine(self.position.utf8Offset) - hasher.combine(self.kind) - } - - enum Kind: Hashable { - case method(name: String, arguments: Int) - case assignment(subscript: Bool) - - func hash(into hasher: inout Hasher) { - switch self { - case let .method(name, arguments): - hasher.combine("method") - hasher.combine(name) - hasher.combine(arguments) - case let .assignment(`subscript`): - hasher.combine("assignment") - hasher.combine(`subscript`) - } - } - } -} - -private struct CollectionType: Hashable { - let name: String - let genericArguments: Int - - static let types: [CollectionType] = [ - .set, - .array, - .dictionary - ] - - static let array = CollectionType(name: "Array", genericArguments: 1) - static let set = CollectionType(name: "Set", genericArguments: 1) - static let dictionary = CollectionType(name: "Dictionary", genericArguments: 2) - - static let names: [String: CollectionType] = { - return CollectionType.types.reduce(into: [String: CollectionType]()) { partialResult, collectionType in - partialResult[collectionType.name] = collectionType - } - }() -} diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopModels.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopModels.swift new file mode 100644 index 0000000000..f1ed30592e --- /dev/null +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopModels.swift @@ -0,0 +1,61 @@ +// +// ReduceIntoInsteadOfLoop+Models.swift +// SwiftLintBuiltInRules +// +// Created by Alfons Hoogervorst on 19/06/2023. +// +import SwiftSyntax + +internal extension ReduceIntoInsteadOfLoop { + + struct ReferencedVariable: Hashable { + let name: String + let position: AbsolutePosition + let kind: Kind + + func hash(into hasher: inout Hasher) { + hasher.combine(self.name) + hasher.combine(self.position.utf8Offset) + hasher.combine(self.kind) + } + + enum Kind: Hashable { + case method(name: String, arguments: Int) + case assignment(subscript: Bool) + + func hash(into hasher: inout Hasher) { + switch self { + case let .method(name, arguments): + hasher.combine("method") + hasher.combine(name) + hasher.combine(arguments) + case let .assignment(`subscript`): + hasher.combine("assignment") + hasher.combine(`subscript`) + } + } + } + } + + struct CollectionType: Hashable { + let name: String + let genericArguments: Int + + static let types: [CollectionType] = [ + .set, + .array, + .dictionary + ] + + static let array = CollectionType(name: "Array", genericArguments: 1) + static let set = CollectionType(name: "Set", genericArguments: 1) + static let dictionary = CollectionType(name: "Dictionary", genericArguments: 2) + + static let names: [String: CollectionType] = { + return CollectionType.types.reduce(into: [String: CollectionType]()) { partialResult, collectionType in + partialResult[collectionType.name] = collectionType + } + }() + } + +} From 31363d45c81c9176558ec94b5903aa298f725a2c Mon Sep 17 00:00:00 2001 From: Alfons Hoogervorst Date: Mon, 19 Jun 2023 18:00:36 +0200 Subject: [PATCH 05/28] Move couple of extension helper functions --- .../Idiomatic/ReduceIntoInsteadOfLoop.swift | 65 ------------------ .../ReduceIntoInsteadOfLoopHelpers.swift | 67 +++++++++++++++++++ .../ReduceIntoInsteadOfLoopModels.swift | 38 +++++------ 3 files changed, 83 insertions(+), 87 deletions(-) create mode 100644 Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopHelpers.swift diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift index a8e63c74ee..363418e1e5 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift @@ -146,18 +146,6 @@ private extension VariableDeclSyntax { } return true } - - func firstOf(_ type: T.Type) -> T? { - return self.bindings.first { patternBinding in - return patternBinding.as(type) != nil - } as? T - } - - func firstIndexOf(_ type: T.Type) -> PatternBindingListSyntax.Index? { - return self.bindings.firstIndex(where: { patternBinding in - return patternBinding.as(type) != nil - }) - } } private extension TypeAnnotationSyntax { @@ -337,56 +325,3 @@ private extension SequenceExprSyntax { ) } } - -private extension PatternBindingListSyntax { - func optionalIndex(after: PatternBindingListSyntax.Index?) -> PatternBindingListSyntax.Index? { - guard let after else { - return nil - } - return self.index(after: after) - } -} - -private extension VariableDeclSyntax { - func next(after index: PatternBindingListSyntax.Index?) -> PatternBindingSyntax? { - guard let index = self.bindings.optionalIndex(after: index), - index >= self.bindings.startIndex && index < self.bindings.endIndex else { - return nil - } - return self.bindings[index] - } - - func next(after index: PatternBindingListSyntax.Index?, of type: T.Type) -> T? { - guard let index = self.bindings.optionalIndex(after: index), - index >= self.bindings.startIndex && index < self.bindings.endIndex else { - return nil - } - return self.bindings[index].as(type) - } - - func next(after: PatternBindingSyntax?) -> PatternBindingSyntax? { - guard let after, let index = self.bindings.firstIndex(where: { patterBindingSyntax in - return patterBindingSyntax == after - }) else { - return nil - } - let newIndex = self.bindings.index(after: index) - guard newIndex >= self.bindings.startIndex && newIndex < self.bindings.endIndex else { - return nil - } - return self.bindings[newIndex] - } - - var isVar: Bool { - return self.bindingKeyword.tokenKind == .keyword(.var) - } - - var identifier: String? { - guard let identifierPattern = self.firstOf(IdentifierPatternSyntax.self), - case .identifier(let name) = identifierPattern.identifier.tokenKind else { - return nil - } - return name - } -} - diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopHelpers.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopHelpers.swift new file mode 100644 index 0000000000..39686dbfaa --- /dev/null +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopHelpers.swift @@ -0,0 +1,67 @@ +import SwiftSyntax + +struct ReduceIntoInsteadOfLoopHelpers { } + +extension PatternBindingListSyntax { + func optionalIndex(after: PatternBindingListSyntax.Index?) -> PatternBindingListSyntax.Index? { + guard let after else { + return nil + } + return self.index(after: after) + } +} + +extension VariableDeclSyntax { + var isVar: Bool { + return self.bindingKeyword.tokenKind == .keyword(.var) + } + + var identifier: String? { + guard let identifierPattern = self.firstOf(IdentifierPatternSyntax.self), + case .identifier(let name) = identifierPattern.identifier.tokenKind else { + return nil + } + return name + } + + func next(after index: PatternBindingListSyntax.Index?) -> PatternBindingSyntax? { + guard let index = self.bindings.optionalIndex(after: index), + index >= self.bindings.startIndex && index < self.bindings.endIndex else { + return nil + } + return self.bindings[index] + } + + func next(after index: PatternBindingListSyntax.Index?, of type: T.Type) -> T? { + guard let index = self.bindings.optionalIndex(after: index), + index >= self.bindings.startIndex && index < self.bindings.endIndex else { + return nil + } + return self.bindings[index].as(type) + } + + func next(after: PatternBindingSyntax?) -> PatternBindingSyntax? { + guard let after, let index = self.bindings.firstIndex(where: { patterBindingSyntax in + return patterBindingSyntax == after + }) else { + return nil + } + let newIndex = self.bindings.index(after: index) + guard newIndex >= self.bindings.startIndex && newIndex < self.bindings.endIndex else { + return nil + } + return self.bindings[newIndex] + } + + func firstOf(_ type: T.Type) -> T? { + return self.bindings.first { patternBinding in + return patternBinding.as(type) != nil + } as? T + } + + func firstIndexOf(_ type: T.Type) -> PatternBindingListSyntax.Index? { + return self.bindings.firstIndex(where: { patternBinding in + return patternBinding.as(type) != nil + }) + } +} diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopModels.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopModels.swift index f1ed30592e..9581d076f7 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopModels.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopModels.swift @@ -1,13 +1,8 @@ -// -// ReduceIntoInsteadOfLoop+Models.swift -// SwiftLintBuiltInRules -// -// Created by Alfons Hoogervorst on 19/06/2023. -// import SwiftSyntax +typealias ReduceIntoInsteadOfLoopModels = ReduceIntoInsteadOfLoop + internal extension ReduceIntoInsteadOfLoop { - struct ReferencedVariable: Hashable { let name: String let position: AbsolutePosition @@ -18,21 +13,21 @@ internal extension ReduceIntoInsteadOfLoop { hasher.combine(self.position.utf8Offset) hasher.combine(self.kind) } + } + + enum Kind: Hashable { + case method(name: String, arguments: Int) + case assignment(subscript: Bool) - enum Kind: Hashable { - case method(name: String, arguments: Int) - case assignment(subscript: Bool) - - func hash(into hasher: inout Hasher) { - switch self { - case let .method(name, arguments): - hasher.combine("method") - hasher.combine(name) - hasher.combine(arguments) - case let .assignment(`subscript`): - hasher.combine("assignment") - hasher.combine(`subscript`) - } + func hash(into hasher: inout Hasher) { + switch self { + case let .method(name, arguments): + hasher.combine("method") + hasher.combine(name) + hasher.combine(arguments) + case let .assignment(`subscript`): + hasher.combine("assignment") + hasher.combine(`subscript`) } } } @@ -57,5 +52,4 @@ internal extension ReduceIntoInsteadOfLoop { } }() } - } From 3746b589f63c5c05209940638582c13134dc69ef Mon Sep 17 00:00:00 2001 From: Alfons Hoogervorst Date: Sun, 18 Jun 2023 23:34:41 +0200 Subject: [PATCH 06/28] Implement ReduceIntoInsteadOfLoop rule --- .../Idiomatic/ReduceIntoInsteadOfLoop.swift | 463 ++++++++++++++++++ .../ReduceIntoInsteadOfLoopExamples.swift | 80 +++ 2 files changed, 543 insertions(+) create mode 100644 Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift create mode 100644 Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopExamples.swift diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift new file mode 100644 index 0000000000..7b4bc3ec1f --- /dev/null +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift @@ -0,0 +1,463 @@ +import SwiftSyntax + +struct ReduceIntoInsteadOfLoop: ConfigurationProviderRule, SwiftSyntaxRule, OptInRule { + var configuration = SeverityConfiguration(.warning) + + static let description = RuleDescription( + identifier: "reduce_into_instead_of_loop", + name: "Reduce Into Instead Of Loop", + description: "Prefer using reduce(into:) instead of a loop", + kind: .idiomatic, + nonTriggeringExamples: nonTriggeringExamples, + triggeringExamples: triggeringExamples + ) + + init() {} + + func makeVisitor(file: SwiftLintFile) -> ViolationsSyntaxVisitor { + Visitor(viewMode: .sourceAccurate) + } +} + +fileprivate extension ReduceIntoInsteadOfLoop { + final class Visitor: ViolationsSyntaxVisitor { + /// Collects all VariableDecl of collection types, finds a ForInStmt, and checks if the ForInStmts + /// body references one of the earlier encounterd VariableDecls. + override func visitPost(_ node: CodeBlockItemListSyntax) { + // reduce into forInStmts and variableDecls map + guard let all = node.allVariableDeclsForInStatmts() else { + return + } + // reduce variableDecls into the ones we're interested in + let selected = all.reduce(into: [ForInStmtSyntax: [VariableDeclSyntax]]()) { partialResult, element in + // we're interested in a couple of variable decls: + // * fully type declared + // * implicitly declared by initializer + let interestingVariableDecls = element.value.filter { variableDecl in + return variableDecl.isTypeAnnotatedAndInitializer + || variableDecl.isCollectionTypeInitializer + } + guard !interestingVariableDecls.isEmpty else { + return + } + partialResult[element.key] = interestingVariableDecls + } + guard !selected.isEmpty else { + return + } + let referencedVars = selected.reduce(into: Set()) { partialResult, keyValue in + let (forInStmt, variableDecls) = keyValue + if let allReferencedVars = forInStmt.referencedVariables(for: variableDecls) { + partialResult.formUnion(allReferencedVars) + } + } + guard !referencedVars.isEmpty else { + return + } + self.violations.append(contentsOf: referencedVars.map { $0.position }) + } + } + + static let collectionTypes: [CollectionType] = [ + CollectionType(name: "Set", genericArguments: 1), + CollectionType(name: "Array", genericArguments: 1), + CollectionType(name: "Dictionary", genericArguments: 2) + ] + + static let collectionNames: [String: CollectionType] = + ReduceIntoInsteadOfLoop.collectionTypes.reduce(into: [String: CollectionType]()) { partialResult, type in + return partialResult[type.name] = type + } +} + +private extension CodeBlockItemListSyntax { + /// Returns a dictionary with all VariableDecls preceding a ForInStmt, at the same scope level + func allVariableDeclsForInStatmts() -> [ForInStmtSyntax: [VariableDeclSyntax]]? { + typealias IndexRange = Range + typealias IndexRangeForStmts = (IndexRange, ForInStmtSyntax) + typealias IndexRangeStmts = (IndexRange, CodeBlockItemSyntax) + // collect all ForInStmts and track their index ranges + let indexed: [IndexRangeForStmts] = self.reduce(into: [IndexRangeStmts]()) { partialResult, codeBlockItem in + guard codeBlockItem.is(ForInStmtSyntax.self) else { + return + } + // Not sure whether ForInStmtSyntax.index == CodeBlockItem.index + guard let last = partialResult.last else { + partialResult.append((self.startIndex.. = []`, `: Array<> = []`, or `: Dictionary<> = [:]` + var isTypeAnnotatedAndInitializer: Bool { + guard self.isVar && self.identifier != nil, + let idIndex = self.firstIndexOf(IdentifierPatternSyntax.self) else { + return false + } + // Is type-annotated, and initialized? + guard let typeAnnotationIndex = self.next(after: idIndex), + let typeAnnotation = typeAnnotationIndex.as(TypeAnnotationSyntax.self), + let type = typeAnnotation.collectionDeclarationType(), + let initializerClause = self.next(after: typeAnnotationIndex)?.as(InitializerClauseSyntax.self) else { + return false + } + return initializerClause.isTypeInitializer(for: type) + } + + /// Is initialized with empty collection + /// E.g. + /// `= Set(), = Array(), = Dictionary[:]` + /// but a couple of more, see `InitializerClauseExprSyntax.isCollectionInitializer` + var isCollectionTypeInitializer: Bool { + guard self.isVar && self.identifier != nil, + let idIndex = self.firstIndexOf(IdentifierPatternSyntax.self) else { + return false + } + let initializerClause = self.next(after: idIndex, of: InitializerClauseSyntax.self) + guard initializerClause?.isTypeInitializer() ?? false else { + return false + } + return true + } + + func firstOf(_ type: T.Type) -> T? { + return self.bindings.first { patternBinding in + return patternBinding.as(type) != nil + } as? T + } + + func firstIndexOf(_ type: T.Type) -> PatternBindingListSyntax.Index? { + return self.bindings.firstIndex(where: { patternBinding in + return patternBinding.as(type) != nil + }) + } +} + +private extension TypeAnnotationSyntax { + /// Returns collection's type name, or `nil` if unknown + func collectionDeclarationType() -> CollectionType? { + if let genericTypeName = self.genericCollectionDeclarationType() { + return genericTypeName + } else if let array = self.arrayDeclarationType() { + return array + } else if let dictionary = self.dictionaryDeclarationType() { + return dictionary + } else { + return nil + } + } + + /// var x: Set<> + /// var x: Array<> + /// var x: Dictionary<> + func genericCollectionDeclarationType() -> CollectionType? { + guard let simpleTypeIdentifier = self.type.as(SimpleTypeIdentifierSyntax.self), + let genericArgumentClause = simpleTypeIdentifier.genericArgumentClause, + genericArgumentClause.leftAngleBracket.tokenKind == .leftAngle, + genericArgumentClause.rightAngleBracket.tokenKind == .rightAngle, + case .identifier(let name) = simpleTypeIdentifier.name.tokenKind, + let collectionType = CollectionType.names[name], + genericArgumentClause.arguments.count == collectionType.genericArguments else { + return nil + } + return collectionType + } + + /// var x: [Y] + func arrayDeclarationType() -> CollectionType? { + guard let arrayType = self.type.as(ArrayTypeSyntax.self), + case .leftSquareBracket = arrayType.leftSquareBracket.tokenKind, + case .rightSquareBracket = arrayType.rightSquareBracket.tokenKind, + arrayType.elementType.kind == .simpleTypeIdentifier else { + return nil + } + return .array + } + + /// var x: [K: V] + func dictionaryDeclarationType() -> CollectionType? { + guard let dictionaryType = self.type.as(DictionaryTypeSyntax.self), + case .leftSquareBracket = dictionaryType.leftSquareBracket.tokenKind, + case .rightSquareBracket = dictionaryType.rightSquareBracket.tokenKind, + case .colon = dictionaryType.colon.tokenKind else { + return nil + } + return .dictionary + } +} + +private extension InitializerClauseSyntax { + /// --- + /// If `nil` we don't know the type and investigate the following, which is a + /// `FunctionCallExpr`: + /// + /// var x = Set(...) + /// var y = Array(...) + /// var z = Dictionary(...) + /// + /// Otherwise we investigate, which checks for `FunctionCallExpr`, + /// `MemberAccessExpr`, `DictionaryExpr` and `ArrayExpr`: + /// + /// 1. `= Set(...)` | `Set(...)` | `.init(...)` | `[]` + /// 2. `= Array(...)` | `Array(...)` | `.init(...)` | `[]` + /// 3. `= Dictionary()` | `Dictionary()` | `.init(..)` | `[:]` + func isTypeInitializer(for collectionType: CollectionType? = nil) -> Bool { + func isSupportedType(with name: String) -> Bool { + if let collectionType { + return collectionType.name == name + } else { + return CollectionType.names[name] != nil + } + } + guard self.equal.tokenKind == .equal else { + return false + } + if let functionCallExpr = self.value.as(FunctionCallExprSyntax.self) { + // either construction using explicit specialisation, or general construction + if let specializeExpr = functionCallExpr.calledExpression.as(SpecializeExprSyntax.self), + let identifierExpr = specializeExpr.expression.as(IdentifierExprSyntax.self), + case .identifier(let typename) = identifierExpr.identifier.tokenKind { + return isSupportedType(with: typename) + } else if let identifierExpr = functionCallExpr.calledExpression.as(IdentifierExprSyntax.self), + case .identifier(let typename) = identifierExpr.identifier.tokenKind { + return isSupportedType(with: typename) + } else if let memberAccessExpr = functionCallExpr.calledExpression.as(MemberAccessExprSyntax.self), + memberAccessExpr.name.tokenKind == .keyword(.`init`) { + return true + } + } else if collectionType == .dictionary, + self.value.as(DictionaryExprSyntax.self) != nil { + return true + } else if collectionType == .array || collectionType == .set, + self.value.as(ArrayExprSyntax.self) != nil { + return true + } + return false + } +} + +private extension ForInStmtSyntax { + /// Checks whether a ForInStmtSyntax references the vars in `variables`. + /// Defers to `CodeBlockItemSyntax.referencedVariable(for:)` + func referencedVariables(for variables: [VariableDeclSyntax]?) -> Set? { + guard let variables, !variables.isEmpty, + let codeBlock = self.body.as(CodeBlockSyntax.self), + let codeBlockItemList = codeBlock.statements.as(CodeBlockItemListSyntax.self) else { + return nil + } + // check whether each of the items references a var + let references: Set = codeBlockItemList.reduce(into: .init(), { partialResult, codeBlock in + // see if each codeblock item references variable + // one variable reference should be plenty, so it covers semicolon-ized statements: + // someMutation(); someOtherMutation() + variables.forEach { variableDecl in + guard let referenced = codeBlock.referencedVariable(for: variableDecl) else { + return + } + partialResult.insert(referenced) + } + }) + return references.isEmpty ? nil : references + } +} + +private extension CodeBlockItemSyntax { + func referencedVariable(for variableDecl: VariableDeclSyntax?) -> ReferencedVariable? { + guard let identifier = variableDecl?.identifier else { + return nil + } + return self.referencedVariable(for: identifier) + } + + func referencedVariable(for varName: String) -> ReferencedVariable? { + if let functionCallExpr = self.item.as(FunctionCallExprSyntax.self) { + return functionCallExpr.referencedVariable(for: varName) + } + if let sequenceExpr = self.item.as(SequenceExprSyntax.self) { + return sequenceExpr.referencedVariable(for: varName) + } + return nil + } +} + +private extension FunctionCallExprSyntax { + /// varName.method(x, y, n) + func referencedVariable(for varName: String) -> ReferencedVariable? { + guard self.leftParen?.tokenKind == .leftParen, + self.rightParen?.tokenKind == .rightParen, + let memberAccessExpr = self.calledExpression.as(MemberAccessExprSyntax.self), + memberAccessExpr.dot.tokenKind == .period, + let arguments = self.argumentList.as(TupleExprElementListSyntax.self)?.count, + let identifierExpr = memberAccessExpr.base?.as(IdentifierExprSyntax.self), + identifierExpr.identifier.tokenKind == .identifier(varName), + case .identifier(let name) = memberAccessExpr.name.tokenKind else { + return nil + } + return .init( + name: varName, + position: memberAccessExpr.positionAfterSkippingLeadingTrivia, + kind: .method(name: name, arguments: arguments) + ) + } +} + +private extension SequenceExprSyntax { + /// varName[xxx] = ... + func referencedVariable(for varName: String) -> ReferencedVariable? { + guard let exprList = self.as(ExprListSyntax.self), exprList.count >= 2 else { + return nil + } + let first = exprList.startIndex + let second = exprList.index(after: first) + guard let subscrExpr = exprList[first].as(SubscriptExprSyntax.self), + let assignmentExpr = exprList[second].as(AssignmentExprSyntax.self), + assignmentExpr.assignToken.text == "=", + subscrExpr.leftBracket.tokenKind == .leftSquareBracket, + subscrExpr.rightBracket.tokenKind == .rightSquareBracket, + subscrExpr.argumentList.is(TupleExprElementListSyntax.self), + let identifierExpr = subscrExpr.calledExpression.as(IdentifierExprSyntax.self), + identifierExpr.identifier.tokenKind == .identifier(varName) else { + return nil + } + return .init( + name: varName, + position: exprList.positionAfterSkippingLeadingTrivia, + kind: .assignment(subscript: true) + ) + } +} + +private extension PatternBindingListSyntax { + func optionalIndex(after: PatternBindingListSyntax.Index?) -> PatternBindingListSyntax.Index? { + guard let after else { + return nil + } + return self.index(after: after) + } +} + +private extension VariableDeclSyntax { + func next(after index: PatternBindingListSyntax.Index?) -> PatternBindingSyntax? { + guard let index = self.bindings.optionalIndex(after: index), + index >= self.bindings.startIndex && index < self.bindings.endIndex else { + return nil + } + return self.bindings[index] + } + + func next(after index: PatternBindingListSyntax.Index?, of type: T.Type) -> T? { + guard let index = self.bindings.optionalIndex(after: index), + index >= self.bindings.startIndex && index < self.bindings.endIndex else { + return nil + } + return self.bindings[index].as(type) + } + + func next(after: PatternBindingSyntax?) -> PatternBindingSyntax? { + guard let after, let index = self.bindings.firstIndex(where: { patterBindingSyntax in + return patterBindingSyntax == after + }) else { + return nil + } + let newIndex = self.bindings.index(after: index) + guard newIndex >= self.bindings.startIndex && newIndex < self.bindings.endIndex else { + return nil + } + return self.bindings[newIndex] + } + + var isVar: Bool { + return self.bindingKeyword.tokenKind == .keyword(.var) + } + + var identifier: String? { + guard let identifierPattern = self.firstOf(IdentifierPatternSyntax.self), + case .identifier(let name) = identifierPattern.identifier.tokenKind else { + return nil + } + return name + } +} + +private struct ReferencedVariable: Hashable { + let name: String + let position: AbsolutePosition + let kind: Kind + + func hash(into hasher: inout Hasher) { + hasher.combine(self.name) + hasher.combine(self.position.utf8Offset) + hasher.combine(self.kind) + } + + enum Kind: Hashable { + case method(name: String, arguments: Int) + case assignment(subscript: Bool) + + func hash(into hasher: inout Hasher) { + switch self { + case let .method(name, arguments): + hasher.combine("method") + hasher.combine(name) + hasher.combine(arguments) + case let .assignment(`subscript`): + hasher.combine("assignment") + hasher.combine(`subscript`) + } + } + } +} + +private struct CollectionType: Hashable { + let name: String + let genericArguments: Int + + static let types: [CollectionType] = [ + .set, + .array, + .dictionary + ] + + static let array = CollectionType(name: "Array", genericArguments: 1) + static let set = CollectionType(name: "Set", genericArguments: 1) + static let dictionary = CollectionType(name: "Dictionary", genericArguments: 2) + + static let names: [String: CollectionType] = { + return CollectionType.types.reduce(into: [String: CollectionType]()) { partialResult, collectionType in + partialResult[collectionType.name] = collectionType + } + }() +} diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopExamples.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopExamples.swift new file mode 100644 index 0000000000..ff1b4a82ce --- /dev/null +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopExamples.swift @@ -0,0 +1,80 @@ +// var encountered1: Set = [] +// var encountered1a = Set() +// var encountered1b: Set = Set() +// var encountered1c: Set = .init() +// var encountered2: [String] = [] +// var encountered2a = [String]() +// var encountered2b: [String] = [1, 2, 3, 4] +// var encountered2c: [String] = Array(contentsOf: other) +// var encountered3: Array = [] +// var encountered4: Dictionary = [] +// var encountered4b: [String: Int] = [:] +// var encountered4c: [String: Int] = ["2": 2, "3": 3] +// for eachN in someArray { +// encountered.insert(eachN) +// encountered1[2] = 45 +// let newSet = encountered.popFirst() +// } + +internal extension ReduceIntoInsteadOfLoop { + static let nonTriggeringExamples: [Example] = [ +// Example(""" +// class Foo { +// static let constant: Int = 1 +// var variable: Int = 2 +// } +// """), +// Example(""" +// struct Foo { +// static let constant: Int = 1 +// } +// """), +// Example(""" +// enum InstFooance { +// static let constant = 1 +// } +// """), +// Example(""" +// struct Foo { +// let property1 +// let property2 +// init(property1: Int, property2: String) { +// self.property1 = property1 +// self.property2 = property2 +// } +// } +// """) + Example(""" + let encountered: Set = someArray.reduce(into: Set(), { result, eachN in + result.insert(eachN) + }) + """) + ] + + static let triggeringExamples: [Example] = [ +// Example(""" +// class Foo { +// static let one = 32 +// ↓let constant: Int = 1 +// } +// """), +// Example(""" +// struct Foo { +// ↓let constant: Int = 1 +// } +// """), +// Example(""" +// enum Foo { +// ↓let constant: Int = 1 +// } +// """), + Example(""" + var encountered: Set = [] + for eachN in someArray { + ↓encountered.insert(eachN) + } + """) + ] +} + + From 63bed4047b9a0beb2a8a4489f0ff09d6f6759e9e Mon Sep 17 00:00:00 2001 From: Alfons Hoogervorst Date: Sun, 18 Jun 2023 23:53:27 +0200 Subject: [PATCH 07/28] Cleanups --- .../Idiomatic/ReduceIntoInsteadOfLoop.swift | 47 +++++-------------- 1 file changed, 11 insertions(+), 36 deletions(-) diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift index 7b4bc3ec1f..e9ada2fe81 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift @@ -21,8 +21,6 @@ struct ReduceIntoInsteadOfLoop: ConfigurationProviderRule, SwiftSyntaxRule, OptI fileprivate extension ReduceIntoInsteadOfLoop { final class Visitor: ViolationsSyntaxVisitor { - /// Collects all VariableDecl of collection types, finds a ForInStmt, and checks if the ForInStmts - /// body references one of the earlier encounterd VariableDecls. override func visitPost(_ node: CodeBlockItemListSyntax) { // reduce into forInStmts and variableDecls map guard let all = node.allVariableDeclsForInStatmts() else { @@ -30,9 +28,7 @@ fileprivate extension ReduceIntoInsteadOfLoop { } // reduce variableDecls into the ones we're interested in let selected = all.reduce(into: [ForInStmtSyntax: [VariableDeclSyntax]]()) { partialResult, element in - // we're interested in a couple of variable decls: - // * fully type declared - // * implicitly declared by initializer + // we're interested fully type declared and implicitly declared by initializer let interestingVariableDecls = element.value.filter { variableDecl in return variableDecl.isTypeAnnotatedAndInitializer || variableDecl.isCollectionTypeInitializer @@ -98,8 +94,7 @@ private extension CodeBlockItemListSyntax { guard !indexed.isEmpty else { return nil } - // Attach VariableDecls to ForInStmt. Note that we only attach VariableDecls that - // are at the same level of scope of the ForInStmt. + // only VariableDecls on same level of scope of the ForInStmt. let result = self.reduce(into: [ForInStmtSyntax: [VariableDeclSyntax]]()) { partialResult, codeBlockItem in guard let variableDecl = codeBlockItem.as(VariableDeclSyntax.self) else { return @@ -119,9 +114,7 @@ private extension CodeBlockItemListSyntax { } private extension VariableDeclSyntax { - /// Is type declared with initializer - /// E.g: - /// `: Set<> = []`, `: Array<> = []`, or `: Dictionary<> = [:]` + /// Is type declared with initializer: `: Set<> = []`, `: Array<> = []`, or `: Dictionary<> = [:]` var isTypeAnnotatedAndInitializer: Bool { guard self.isVar && self.identifier != nil, let idIndex = self.firstIndexOf(IdentifierPatternSyntax.self) else { @@ -137,9 +130,7 @@ private extension VariableDeclSyntax { return initializerClause.isTypeInitializer(for: type) } - /// Is initialized with empty collection - /// E.g. - /// `= Set(), = Array(), = Dictionary[:]` + /// Is initialized with empty collection: `= Set(), = Array(), = Dictionary[:]` /// but a couple of more, see `InitializerClauseExprSyntax.isCollectionInitializer` var isCollectionTypeInitializer: Bool { guard self.isVar && self.identifier != nil, @@ -167,7 +158,6 @@ private extension VariableDeclSyntax { } private extension TypeAnnotationSyntax { - /// Returns collection's type name, or `nil` if unknown func collectionDeclarationType() -> CollectionType? { if let genericTypeName = self.genericCollectionDeclarationType() { return genericTypeName @@ -180,9 +170,7 @@ private extension TypeAnnotationSyntax { } } - /// var x: Set<> - /// var x: Array<> - /// var x: Dictionary<> + /// var x: Set<>, var x: Array<>, var x: Dictionary<> func genericCollectionDeclarationType() -> CollectionType? { guard let simpleTypeIdentifier = self.type.as(SimpleTypeIdentifierSyntax.self), let genericArgumentClause = simpleTypeIdentifier.genericArgumentClause, @@ -221,16 +209,11 @@ private extension TypeAnnotationSyntax { private extension InitializerClauseSyntax { /// --- - /// If `nil` we don't know the type and investigate the following, which is a - /// `FunctionCallExpr`: - /// + /// If `nil` we don't know the type and investigate the following, which is a`FunctionCallExpr`: /// var x = Set(...) /// var y = Array(...) /// var z = Dictionary(...) - /// - /// Otherwise we investigate, which checks for `FunctionCallExpr`, - /// `MemberAccessExpr`, `DictionaryExpr` and `ArrayExpr`: - /// + /// Otherwise checks for `FunctionCallExpr`,`MemberAccessExpr`,`DictionaryExpr` and `ArrayExpr`: /// 1. `= Set(...)` | `Set(...)` | `.init(...)` | `[]` /// 2. `= Array(...)` | `Array(...)` | `.init(...)` | `[]` /// 3. `= Dictionary()` | `Dictionary()` | `.init(..)` | `[:]` @@ -242,9 +225,7 @@ private extension InitializerClauseSyntax { return CollectionType.names[name] != nil } } - guard self.equal.tokenKind == .equal else { - return false - } + guard self.equal.tokenKind == .equal else { return false } if let functionCallExpr = self.value.as(FunctionCallExprSyntax.self) { // either construction using explicit specialisation, or general construction if let specializeExpr = functionCallExpr.calledExpression.as(SpecializeExprSyntax.self), @@ -270,24 +251,18 @@ private extension InitializerClauseSyntax { } private extension ForInStmtSyntax { - /// Checks whether a ForInStmtSyntax references the vars in `variables`. - /// Defers to `CodeBlockItemSyntax.referencedVariable(for:)` func referencedVariables(for variables: [VariableDeclSyntax]?) -> Set? { guard let variables, !variables.isEmpty, let codeBlock = self.body.as(CodeBlockSyntax.self), let codeBlockItemList = codeBlock.statements.as(CodeBlockItemListSyntax.self) else { return nil } - // check whether each of the items references a var let references: Set = codeBlockItemList.reduce(into: .init(), { partialResult, codeBlock in - // see if each codeblock item references variable - // one variable reference should be plenty, so it covers semicolon-ized statements: - // someMutation(); someOtherMutation() + // no need to cover one liner: someMutation(); someOtherMutation() variables.forEach { variableDecl in - guard let referenced = codeBlock.referencedVariable(for: variableDecl) else { - return + if let referenced = codeBlock.referencedVariable(for: variableDecl) { + partialResult.insert(referenced) } - partialResult.insert(referenced) } }) return references.isEmpty ? nil : references From 810a508478a21129c18f6980b15283de79dbb231 Mon Sep 17 00:00:00 2001 From: Alfons Hoogervorst Date: Mon, 19 Jun 2023 04:50:06 +0200 Subject: [PATCH 08/28] Fix build --- .../Models/BuiltInRules.swift | 1 + .../Idiomatic/ReduceIntoInsteadOfLoop.swift | 4 +- .../ReduceIntoInsteadOfLoopExamples.swift | 38 +++++++++---------- 3 files changed, 21 insertions(+), 22 deletions(-) diff --git a/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift b/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift index 0138cc743e..c5b719ac94 100644 --- a/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift +++ b/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift @@ -170,6 +170,7 @@ public let builtInRules: [any Rule.Type] = [ RawValueForCamelCasedCodableEnumRule.self, ReduceBooleanRule.self, ReduceIntoRule.self, + ReduceIntoInsteadOfLoop.self, RedundantDiscardableLetRule.self, RedundantNilCoalescingRule.self, RedundantObjcAttributeRule.self, diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift index e9ada2fe81..ea6bda8001 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift @@ -8,8 +8,8 @@ struct ReduceIntoInsteadOfLoop: ConfigurationProviderRule, SwiftSyntaxRule, OptI name: "Reduce Into Instead Of Loop", description: "Prefer using reduce(into:) instead of a loop", kind: .idiomatic, - nonTriggeringExamples: nonTriggeringExamples, - triggeringExamples: triggeringExamples + nonTriggeringExamples: ReduceIntoInsteadOfLoopExamples.nonTriggeringExamples, + triggeringExamples: ReduceIntoInsteadOfLoopExamples.triggeringExamples ) init() {} diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopExamples.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopExamples.swift index ff1b4a82ce..4498998c22 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopExamples.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopExamples.swift @@ -1,22 +1,4 @@ -// var encountered1: Set = [] -// var encountered1a = Set() -// var encountered1b: Set = Set() -// var encountered1c: Set = .init() -// var encountered2: [String] = [] -// var encountered2a = [String]() -// var encountered2b: [String] = [1, 2, 3, 4] -// var encountered2c: [String] = Array(contentsOf: other) -// var encountered3: Array = [] -// var encountered4: Dictionary = [] -// var encountered4b: [String: Int] = [:] -// var encountered4c: [String: Int] = ["2": 2, "3": 3] -// for eachN in someArray { -// encountered.insert(eachN) -// encountered1[2] = 45 -// let newSet = encountered.popFirst() -// } - -internal extension ReduceIntoInsteadOfLoop { +internal struct ReduceIntoInsteadOfLoopExamples { static let nonTriggeringExamples: [Example] = [ // Example(""" // class Foo { @@ -77,4 +59,20 @@ internal extension ReduceIntoInsteadOfLoop { ] } - +// var encountered1: Set = [] +// var encountered1a = Set() +// var encountered1b: Set = Set() +// var encountered1c: Set = .init() +// var encountered2: [String] = [] +// var encountered2a = [String]() +// var encountered2b: [String] = [1, 2, 3, 4] +// var encountered2c: [String] = Array(contentsOf: other) +// var encountered3: Array = [] +// var encountered4: Dictionary = [] +// var encountered4b: [String: Int] = [:] +// var encountered4c: [String: Int] = ["2": 2, "3": 3] +// for eachN in someArray { +// encountered.insert(eachN) +// encountered1[2] = 45 +// let newSet = encountered.popFirst() +// } From a4f3b782768688ff4788cd381da69d2a2875662d Mon Sep 17 00:00:00 2001 From: Alfons Hoogervorst Date: Mon, 19 Jun 2023 17:48:28 +0200 Subject: [PATCH 09/28] Move models to separate file --- .../Idiomatic/ReduceIntoInsteadOfLoop.swift | 54 ++-------------- .../ReduceIntoInsteadOfLoopModels.swift | 61 +++++++++++++++++++ 2 files changed, 65 insertions(+), 50 deletions(-) create mode 100644 Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopModels.swift diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift index ea6bda8001..a8e63c74ee 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift @@ -1,5 +1,8 @@ import SwiftSyntax +typealias ReferencedVariable = ReduceIntoInsteadOfLoop.ReferencedVariable +typealias CollectionType = ReduceIntoInsteadOfLoop.CollectionType + struct ReduceIntoInsteadOfLoop: ConfigurationProviderRule, SwiftSyntaxRule, OptInRule { var configuration = SeverityConfiguration(.warning) @@ -19,7 +22,7 @@ struct ReduceIntoInsteadOfLoop: ConfigurationProviderRule, SwiftSyntaxRule, OptI } } -fileprivate extension ReduceIntoInsteadOfLoop { +internal extension ReduceIntoInsteadOfLoop { final class Visitor: ViolationsSyntaxVisitor { override func visitPost(_ node: CodeBlockItemListSyntax) { // reduce into forInStmts and variableDecls map @@ -387,52 +390,3 @@ private extension VariableDeclSyntax { } } -private struct ReferencedVariable: Hashable { - let name: String - let position: AbsolutePosition - let kind: Kind - - func hash(into hasher: inout Hasher) { - hasher.combine(self.name) - hasher.combine(self.position.utf8Offset) - hasher.combine(self.kind) - } - - enum Kind: Hashable { - case method(name: String, arguments: Int) - case assignment(subscript: Bool) - - func hash(into hasher: inout Hasher) { - switch self { - case let .method(name, arguments): - hasher.combine("method") - hasher.combine(name) - hasher.combine(arguments) - case let .assignment(`subscript`): - hasher.combine("assignment") - hasher.combine(`subscript`) - } - } - } -} - -private struct CollectionType: Hashable { - let name: String - let genericArguments: Int - - static let types: [CollectionType] = [ - .set, - .array, - .dictionary - ] - - static let array = CollectionType(name: "Array", genericArguments: 1) - static let set = CollectionType(name: "Set", genericArguments: 1) - static let dictionary = CollectionType(name: "Dictionary", genericArguments: 2) - - static let names: [String: CollectionType] = { - return CollectionType.types.reduce(into: [String: CollectionType]()) { partialResult, collectionType in - partialResult[collectionType.name] = collectionType - } - }() -} diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopModels.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopModels.swift new file mode 100644 index 0000000000..f1ed30592e --- /dev/null +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopModels.swift @@ -0,0 +1,61 @@ +// +// ReduceIntoInsteadOfLoop+Models.swift +// SwiftLintBuiltInRules +// +// Created by Alfons Hoogervorst on 19/06/2023. +// +import SwiftSyntax + +internal extension ReduceIntoInsteadOfLoop { + + struct ReferencedVariable: Hashable { + let name: String + let position: AbsolutePosition + let kind: Kind + + func hash(into hasher: inout Hasher) { + hasher.combine(self.name) + hasher.combine(self.position.utf8Offset) + hasher.combine(self.kind) + } + + enum Kind: Hashable { + case method(name: String, arguments: Int) + case assignment(subscript: Bool) + + func hash(into hasher: inout Hasher) { + switch self { + case let .method(name, arguments): + hasher.combine("method") + hasher.combine(name) + hasher.combine(arguments) + case let .assignment(`subscript`): + hasher.combine("assignment") + hasher.combine(`subscript`) + } + } + } + } + + struct CollectionType: Hashable { + let name: String + let genericArguments: Int + + static let types: [CollectionType] = [ + .set, + .array, + .dictionary + ] + + static let array = CollectionType(name: "Array", genericArguments: 1) + static let set = CollectionType(name: "Set", genericArguments: 1) + static let dictionary = CollectionType(name: "Dictionary", genericArguments: 2) + + static let names: [String: CollectionType] = { + return CollectionType.types.reduce(into: [String: CollectionType]()) { partialResult, collectionType in + partialResult[collectionType.name] = collectionType + } + }() + } + +} From 4df2cb9effb16256e66901732f7bdcac14bc2758 Mon Sep 17 00:00:00 2001 From: Alfons Hoogervorst Date: Mon, 19 Jun 2023 18:00:36 +0200 Subject: [PATCH 10/28] Move couple of extension helper functions --- .../Idiomatic/ReduceIntoInsteadOfLoop.swift | 65 ------------------ .../ReduceIntoInsteadOfLoopHelpers.swift | 67 +++++++++++++++++++ .../ReduceIntoInsteadOfLoopModels.swift | 38 +++++------ 3 files changed, 83 insertions(+), 87 deletions(-) create mode 100644 Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopHelpers.swift diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift index a8e63c74ee..363418e1e5 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift @@ -146,18 +146,6 @@ private extension VariableDeclSyntax { } return true } - - func firstOf(_ type: T.Type) -> T? { - return self.bindings.first { patternBinding in - return patternBinding.as(type) != nil - } as? T - } - - func firstIndexOf(_ type: T.Type) -> PatternBindingListSyntax.Index? { - return self.bindings.firstIndex(where: { patternBinding in - return patternBinding.as(type) != nil - }) - } } private extension TypeAnnotationSyntax { @@ -337,56 +325,3 @@ private extension SequenceExprSyntax { ) } } - -private extension PatternBindingListSyntax { - func optionalIndex(after: PatternBindingListSyntax.Index?) -> PatternBindingListSyntax.Index? { - guard let after else { - return nil - } - return self.index(after: after) - } -} - -private extension VariableDeclSyntax { - func next(after index: PatternBindingListSyntax.Index?) -> PatternBindingSyntax? { - guard let index = self.bindings.optionalIndex(after: index), - index >= self.bindings.startIndex && index < self.bindings.endIndex else { - return nil - } - return self.bindings[index] - } - - func next(after index: PatternBindingListSyntax.Index?, of type: T.Type) -> T? { - guard let index = self.bindings.optionalIndex(after: index), - index >= self.bindings.startIndex && index < self.bindings.endIndex else { - return nil - } - return self.bindings[index].as(type) - } - - func next(after: PatternBindingSyntax?) -> PatternBindingSyntax? { - guard let after, let index = self.bindings.firstIndex(where: { patterBindingSyntax in - return patterBindingSyntax == after - }) else { - return nil - } - let newIndex = self.bindings.index(after: index) - guard newIndex >= self.bindings.startIndex && newIndex < self.bindings.endIndex else { - return nil - } - return self.bindings[newIndex] - } - - var isVar: Bool { - return self.bindingKeyword.tokenKind == .keyword(.var) - } - - var identifier: String? { - guard let identifierPattern = self.firstOf(IdentifierPatternSyntax.self), - case .identifier(let name) = identifierPattern.identifier.tokenKind else { - return nil - } - return name - } -} - diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopHelpers.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopHelpers.swift new file mode 100644 index 0000000000..39686dbfaa --- /dev/null +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopHelpers.swift @@ -0,0 +1,67 @@ +import SwiftSyntax + +struct ReduceIntoInsteadOfLoopHelpers { } + +extension PatternBindingListSyntax { + func optionalIndex(after: PatternBindingListSyntax.Index?) -> PatternBindingListSyntax.Index? { + guard let after else { + return nil + } + return self.index(after: after) + } +} + +extension VariableDeclSyntax { + var isVar: Bool { + return self.bindingKeyword.tokenKind == .keyword(.var) + } + + var identifier: String? { + guard let identifierPattern = self.firstOf(IdentifierPatternSyntax.self), + case .identifier(let name) = identifierPattern.identifier.tokenKind else { + return nil + } + return name + } + + func next(after index: PatternBindingListSyntax.Index?) -> PatternBindingSyntax? { + guard let index = self.bindings.optionalIndex(after: index), + index >= self.bindings.startIndex && index < self.bindings.endIndex else { + return nil + } + return self.bindings[index] + } + + func next(after index: PatternBindingListSyntax.Index?, of type: T.Type) -> T? { + guard let index = self.bindings.optionalIndex(after: index), + index >= self.bindings.startIndex && index < self.bindings.endIndex else { + return nil + } + return self.bindings[index].as(type) + } + + func next(after: PatternBindingSyntax?) -> PatternBindingSyntax? { + guard let after, let index = self.bindings.firstIndex(where: { patterBindingSyntax in + return patterBindingSyntax == after + }) else { + return nil + } + let newIndex = self.bindings.index(after: index) + guard newIndex >= self.bindings.startIndex && newIndex < self.bindings.endIndex else { + return nil + } + return self.bindings[newIndex] + } + + func firstOf(_ type: T.Type) -> T? { + return self.bindings.first { patternBinding in + return patternBinding.as(type) != nil + } as? T + } + + func firstIndexOf(_ type: T.Type) -> PatternBindingListSyntax.Index? { + return self.bindings.firstIndex(where: { patternBinding in + return patternBinding.as(type) != nil + }) + } +} diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopModels.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopModels.swift index f1ed30592e..9581d076f7 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopModels.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopModels.swift @@ -1,13 +1,8 @@ -// -// ReduceIntoInsteadOfLoop+Models.swift -// SwiftLintBuiltInRules -// -// Created by Alfons Hoogervorst on 19/06/2023. -// import SwiftSyntax +typealias ReduceIntoInsteadOfLoopModels = ReduceIntoInsteadOfLoop + internal extension ReduceIntoInsteadOfLoop { - struct ReferencedVariable: Hashable { let name: String let position: AbsolutePosition @@ -18,21 +13,21 @@ internal extension ReduceIntoInsteadOfLoop { hasher.combine(self.position.utf8Offset) hasher.combine(self.kind) } + } + + enum Kind: Hashable { + case method(name: String, arguments: Int) + case assignment(subscript: Bool) - enum Kind: Hashable { - case method(name: String, arguments: Int) - case assignment(subscript: Bool) - - func hash(into hasher: inout Hasher) { - switch self { - case let .method(name, arguments): - hasher.combine("method") - hasher.combine(name) - hasher.combine(arguments) - case let .assignment(`subscript`): - hasher.combine("assignment") - hasher.combine(`subscript`) - } + func hash(into hasher: inout Hasher) { + switch self { + case let .method(name, arguments): + hasher.combine("method") + hasher.combine(name) + hasher.combine(arguments) + case let .assignment(`subscript`): + hasher.combine("assignment") + hasher.combine(`subscript`) } } } @@ -57,5 +52,4 @@ internal extension ReduceIntoInsteadOfLoop { } }() } - } From 06cf6165ec5076d393e23f4ad9d3929f4dad8df0 Mon Sep 17 00:00:00 2001 From: Alfons Hoogervorst Date: Sun, 11 May 2025 21:36:47 +0200 Subject: [PATCH 11/28] Make it work with new SwiftSyntax --- .../Idiomatic/ReduceIntoInsteadOfLoop.swift | 165 +++++++++--------- .../ReduceIntoInsteadOfLoopExamples.swift | 4 +- .../ReduceIntoInsteadOfLoopHelpers.swift | 25 +-- .../ReduceIntoInsteadOfLoopModels.swift | 14 +- 4 files changed, 103 insertions(+), 105 deletions(-) diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift index 363418e1e5..e207f38270 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift @@ -3,7 +3,8 @@ import SwiftSyntax typealias ReferencedVariable = ReduceIntoInsteadOfLoop.ReferencedVariable typealias CollectionType = ReduceIntoInsteadOfLoop.CollectionType -struct ReduceIntoInsteadOfLoop: ConfigurationProviderRule, SwiftSyntaxRule, OptInRule { +@SwiftSyntaxRule +struct ReduceIntoInsteadOfLoop: Rule { var configuration = SeverityConfiguration(.warning) static let description = RuleDescription( @@ -14,26 +15,20 @@ struct ReduceIntoInsteadOfLoop: ConfigurationProviderRule, SwiftSyntaxRule, OptI nonTriggeringExamples: ReduceIntoInsteadOfLoopExamples.nonTriggeringExamples, triggeringExamples: ReduceIntoInsteadOfLoopExamples.triggeringExamples ) - - init() {} - - func makeVisitor(file: SwiftLintFile) -> ViolationsSyntaxVisitor { - Visitor(viewMode: .sourceAccurate) - } } internal extension ReduceIntoInsteadOfLoop { - final class Visitor: ViolationsSyntaxVisitor { + final class Visitor: ViolationsSyntaxVisitor { override func visitPost(_ node: CodeBlockItemListSyntax) { // reduce into forInStmts and variableDecls map guard let all = node.allVariableDeclsForInStatmts() else { return } // reduce variableDecls into the ones we're interested in - let selected = all.reduce(into: [ForInStmtSyntax: [VariableDeclSyntax]]()) { partialResult, element in + let selected = all.reduce(into: [ForStmtSyntax: [VariableDeclSyntax]]()) { partialResult, element in // we're interested fully type declared and implicitly declared by initializer let interestingVariableDecls = element.value.filter { variableDecl in - return variableDecl.isTypeAnnotatedAndInitializer + variableDecl.isTypeAnnotatedAndInitializer || variableDecl.isCollectionTypeInitializer } guard !interestingVariableDecls.isEmpty else { @@ -60,56 +55,59 @@ internal extension ReduceIntoInsteadOfLoop { static let collectionTypes: [CollectionType] = [ CollectionType(name: "Set", genericArguments: 1), CollectionType(name: "Array", genericArguments: 1), - CollectionType(name: "Dictionary", genericArguments: 2) + CollectionType(name: "Dictionary", genericArguments: 2), ] static let collectionNames: [String: CollectionType] = ReduceIntoInsteadOfLoop.collectionTypes.reduce(into: [String: CollectionType]()) { partialResult, type in - return partialResult[type.name] = type + partialResult[type.name] = type } } private extension CodeBlockItemListSyntax { /// Returns a dictionary with all VariableDecls preceding a ForInStmt, at the same scope level - func allVariableDeclsForInStatmts() -> [ForInStmtSyntax: [VariableDeclSyntax]]? { + func allVariableDeclsForInStatmts() -> [ForStmtSyntax: [VariableDeclSyntax]]? { typealias IndexRange = Range - typealias IndexRangeForStmts = (IndexRange, ForInStmtSyntax) - typealias IndexRangeStmts = (IndexRange, CodeBlockItemSyntax) - // collect all ForInStmts and track their index ranges - let indexed: [IndexRangeForStmts] = self.reduce(into: [IndexRangeStmts]()) { partialResult, codeBlockItem in - guard codeBlockItem.is(ForInStmtSyntax.self) else { + typealias IndexRangeForStmts = (range: IndexRange, forStmt: ForStmtSyntax) + // Collect all ForInStmts and track their index ranges + let indexRangeForStatements: [IndexRangeForStmts] = self.reduce(into: [IndexRangeForStmts]()) { partialResult, codeBlockItem in + guard let codeBlockItemIndex = self.index(of: codeBlockItem) else { + assertionFailure("Unreachable") return } - // Not sure whether ForInStmtSyntax.index == CodeBlockItem.index - guard let last = partialResult.last else { - partialResult.append((self.startIndex..(), = Array(), = Dictionary[:]` /// but a couple of more, see `InitializerClauseExprSyntax.isCollectionInitializer` var isCollectionTypeInitializer: Bool { - guard self.isVar && self.identifier != nil, - let idIndex = self.firstIndexOf(IdentifierPatternSyntax.self) else { + guard self.isVar && self.identifier != nil else { + return false + } + guard let initializerClausePatternBinding = self.bindings.first(where: { patternBindingSyntax in + patternBindingSyntax.initializer != nil + }) else { + return false + } + guard let initializerClause = initializerClausePatternBinding.initializer else { return false } - let initializerClause = self.next(after: idIndex, of: InitializerClauseSyntax.self) - guard initializerClause?.isTypeInitializer() ?? false else { + guard initializerClause.isTypeInitializer() else { return false } return true @@ -149,24 +153,26 @@ private extension VariableDeclSyntax { } private extension TypeAnnotationSyntax { + /// Returns one of the collection types we define func collectionDeclarationType() -> CollectionType? { if let genericTypeName = self.genericCollectionDeclarationType() { return genericTypeName - } else if let array = self.arrayDeclarationType() { + } + if let array = self.arrayDeclarationType() { return array - } else if let dictionary = self.dictionaryDeclarationType() { + } + if let dictionary = self.dictionaryDeclarationType() { return dictionary - } else { - return nil } + return nil } /// var x: Set<>, var x: Array<>, var x: Dictionary<> func genericCollectionDeclarationType() -> CollectionType? { - guard let simpleTypeIdentifier = self.type.as(SimpleTypeIdentifierSyntax.self), + guard let simpleTypeIdentifier = self.type.as(IdentifierTypeSyntax.self), let genericArgumentClause = simpleTypeIdentifier.genericArgumentClause, - genericArgumentClause.leftAngleBracket.tokenKind == .leftAngle, - genericArgumentClause.rightAngleBracket.tokenKind == .rightAngle, + genericArgumentClause.leftAngle.tokenKind == .leftAngle, + genericArgumentClause.rightAngle.tokenKind == .rightAngle, case .identifier(let name) = simpleTypeIdentifier.name.tokenKind, let collectionType = CollectionType.names[name], genericArgumentClause.arguments.count == collectionType.genericArguments else { @@ -178,9 +184,9 @@ private extension TypeAnnotationSyntax { /// var x: [Y] func arrayDeclarationType() -> CollectionType? { guard let arrayType = self.type.as(ArrayTypeSyntax.self), - case .leftSquareBracket = arrayType.leftSquareBracket.tokenKind, - case .rightSquareBracket = arrayType.rightSquareBracket.tokenKind, - arrayType.elementType.kind == .simpleTypeIdentifier else { + case .leftSquare = arrayType.leftSquare.tokenKind, + case .rightSquare = arrayType.rightSquare.tokenKind, + arrayType.element.kind == .identifierType else { return nil } return .array @@ -189,8 +195,8 @@ private extension TypeAnnotationSyntax { /// var x: [K: V] func dictionaryDeclarationType() -> CollectionType? { guard let dictionaryType = self.type.as(DictionaryTypeSyntax.self), - case .leftSquareBracket = dictionaryType.leftSquareBracket.tokenKind, - case .rightSquareBracket = dictionaryType.rightSquareBracket.tokenKind, + case .leftSquare = dictionaryType.leftSquare.tokenKind, + case .rightSquare = dictionaryType.rightSquare.tokenKind, case .colon = dictionaryType.colon.tokenKind else { return nil } @@ -212,22 +218,23 @@ private extension InitializerClauseSyntax { func isSupportedType(with name: String) -> Bool { if let collectionType { return collectionType.name == name - } else { - return CollectionType.names[name] != nil } + return CollectionType.names[name] != nil } guard self.equal.tokenKind == .equal else { return false } if let functionCallExpr = self.value.as(FunctionCallExprSyntax.self) { // either construction using explicit specialisation, or general construction - if let specializeExpr = functionCallExpr.calledExpression.as(SpecializeExprSyntax.self), - let identifierExpr = specializeExpr.expression.as(IdentifierExprSyntax.self), - case .identifier(let typename) = identifierExpr.identifier.tokenKind { + if let specializeExpr = functionCallExpr.calledExpression.as(GenericSpecializationExprSyntax.self), + let identifierExpr = specializeExpr.expression.as(DeclReferenceExprSyntax.self), + case .identifier(let typename) = identifierExpr.baseName.tokenKind { return isSupportedType(with: typename) - } else if let identifierExpr = functionCallExpr.calledExpression.as(IdentifierExprSyntax.self), - case .identifier(let typename) = identifierExpr.identifier.tokenKind { + } + if let identifierExpr = functionCallExpr.calledExpression.as(DeclReferenceExprSyntax.self), + case .identifier(let typename) = identifierExpr.baseName.tokenKind { return isSupportedType(with: typename) - } else if let memberAccessExpr = functionCallExpr.calledExpression.as(MemberAccessExprSyntax.self), - memberAccessExpr.name.tokenKind == .keyword(.`init`) { + } + if let memberAccessExpr = functionCallExpr.calledExpression.as(MemberAccessExprSyntax.self), + memberAccessExpr.declName.baseName.tokenKind == .keyword(.`init`) { return true } } else if collectionType == .dictionary, @@ -241,13 +248,12 @@ private extension InitializerClauseSyntax { } } -private extension ForInStmtSyntax { +private extension ForStmtSyntax { func referencedVariables(for variables: [VariableDeclSyntax]?) -> Set? { - guard let variables, !variables.isEmpty, - let codeBlock = self.body.as(CodeBlockSyntax.self), - let codeBlockItemList = codeBlock.statements.as(CodeBlockItemListSyntax.self) else { + guard let variables, !variables.isEmpty else { return nil } + let codeBlockItemList = self.body.statements let references: Set = codeBlockItemList.reduce(into: .init(), { partialResult, codeBlock in // no need to cover one liner: someMutation(); someOtherMutation() variables.forEach { variableDecl in @@ -285,17 +291,16 @@ private extension FunctionCallExprSyntax { guard self.leftParen?.tokenKind == .leftParen, self.rightParen?.tokenKind == .rightParen, let memberAccessExpr = self.calledExpression.as(MemberAccessExprSyntax.self), - memberAccessExpr.dot.tokenKind == .period, - let arguments = self.argumentList.as(TupleExprElementListSyntax.self)?.count, - let identifierExpr = memberAccessExpr.base?.as(IdentifierExprSyntax.self), - identifierExpr.identifier.tokenKind == .identifier(varName), - case .identifier(let name) = memberAccessExpr.name.tokenKind else { + memberAccessExpr.period.tokenKind == .period, + let identifierExpr = memberAccessExpr.base?.as(DeclReferenceExprSyntax.self), + identifierExpr.baseName.tokenKind == .identifier(varName), + case .identifier(let name) = memberAccessExpr.declName.baseName.tokenKind else { return nil } return .init( name: varName, position: memberAccessExpr.positionAfterSkippingLeadingTrivia, - kind: .method(name: name, arguments: arguments) + kind: .method(name: name, arguments: self.arguments.count) ) } } @@ -303,19 +308,19 @@ private extension FunctionCallExprSyntax { private extension SequenceExprSyntax { /// varName[xxx] = ... func referencedVariable(for varName: String) -> ReferencedVariable? { - guard let exprList = self.as(ExprListSyntax.self), exprList.count >= 2 else { + let exprList = self.elements + guard exprList.count >= 2 else { return nil } let first = exprList.startIndex let second = exprList.index(after: first) - guard let subscrExpr = exprList[first].as(SubscriptExprSyntax.self), + guard let subscrExpr = exprList[first].as(SubscriptCallExprSyntax.self), let assignmentExpr = exprList[second].as(AssignmentExprSyntax.self), - assignmentExpr.assignToken.text == "=", - subscrExpr.leftBracket.tokenKind == .leftSquareBracket, - subscrExpr.rightBracket.tokenKind == .rightSquareBracket, - subscrExpr.argumentList.is(TupleExprElementListSyntax.self), - let identifierExpr = subscrExpr.calledExpression.as(IdentifierExprSyntax.self), - identifierExpr.identifier.tokenKind == .identifier(varName) else { + assignmentExpr.equal.text == "=", + subscrExpr.leftSquare.tokenKind == .leftSquare, + subscrExpr.rightSquare.tokenKind == .rightSquare, + let identifierExpr = subscrExpr.calledExpression.as(DeclReferenceExprSyntax.self), + identifierExpr.baseName.tokenKind == .identifier(varName) else { return nil } return .init( diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopExamples.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopExamples.swift index 4498998c22..267dd98997 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopExamples.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopExamples.swift @@ -30,7 +30,7 @@ internal struct ReduceIntoInsteadOfLoopExamples { let encountered: Set = someArray.reduce(into: Set(), { result, eachN in result.insert(eachN) }) - """) + """), ] static let triggeringExamples: [Example] = [ @@ -55,7 +55,7 @@ internal struct ReduceIntoInsteadOfLoopExamples { for eachN in someArray { ↓encountered.insert(eachN) } - """) + """), ] } diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopHelpers.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopHelpers.swift index 39686dbfaa..2b3321d958 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopHelpers.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopHelpers.swift @@ -12,8 +12,9 @@ extension PatternBindingListSyntax { } extension VariableDeclSyntax { + /// Binding is a var: "`var` someVar = <...>" var isVar: Bool { - return self.bindingKeyword.tokenKind == .keyword(.var) + self.bindingSpecifier == .keyword(.var) } var identifier: String? { @@ -32,17 +33,9 @@ extension VariableDeclSyntax { return self.bindings[index] } - func next(after index: PatternBindingListSyntax.Index?, of type: T.Type) -> T? { - guard let index = self.bindings.optionalIndex(after: index), - index >= self.bindings.startIndex && index < self.bindings.endIndex else { - return nil - } - return self.bindings[index].as(type) - } - func next(after: PatternBindingSyntax?) -> PatternBindingSyntax? { guard let after, let index = self.bindings.firstIndex(where: { patterBindingSyntax in - return patterBindingSyntax == after + patterBindingSyntax == after }) else { return nil } @@ -53,15 +46,15 @@ extension VariableDeclSyntax { return self.bindings[newIndex] } - func firstOf(_ type: T.Type) -> T? { - return self.bindings.first { patternBinding in - return patternBinding.as(type) != nil + func firstOf(_ type: T.Type) -> T? { + self.bindings.first { patternBinding in + patternBinding.pattern.as(type) != nil } as? T } - func firstIndexOf(_ type: T.Type) -> PatternBindingListSyntax.Index? { - return self.bindings.firstIndex(where: { patternBinding in - return patternBinding.as(type) != nil + func firstIndexOf(_ type: T.Type) -> PatternBindingListSyntax.Index? { + self.bindings.firstIndex(where: { patternBinding in + patternBinding.pattern.as(type) != nil }) } } diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopModels.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopModels.swift index 9581d076f7..3fecea8ded 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopModels.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopModels.swift @@ -36,18 +36,18 @@ internal extension ReduceIntoInsteadOfLoop { let name: String let genericArguments: Int - static let types: [CollectionType] = [ + static let types: [Self] = [ .set, .array, - .dictionary + .dictionary, ] - static let array = CollectionType(name: "Array", genericArguments: 1) - static let set = CollectionType(name: "Set", genericArguments: 1) - static let dictionary = CollectionType(name: "Dictionary", genericArguments: 2) + static let array = Self(name: "Array", genericArguments: 1) + static let set = Self(name: "Set", genericArguments: 1) + static let dictionary = Self(name: "Dictionary", genericArguments: 2) - static let names: [String: CollectionType] = { - return CollectionType.types.reduce(into: [String: CollectionType]()) { partialResult, collectionType in + static let names: [String: Self] = { + Self.types.reduce(into: [String: Self]()) { partialResult, collectionType in partialResult[collectionType.name] = collectionType } }() From 74a4372e22d0e1f8ee1ceaa40915e8623683222f Mon Sep 17 00:00:00 2001 From: Alfons Hoogervorst Date: Sun, 11 May 2025 21:42:27 +0200 Subject: [PATCH 12/28] Rename --- ...oop.swift => ReduceIntoInsteadOfLoopRule.swift} | 14 +++++++------- ...t => ReduceIntoInsteadOfLoopRuleExamples.swift} | 2 +- ...ft => ReduceIntoInsteadOfLoopRuleHelpers.swift} | 2 +- ...ift => ReduceIntoInsteadOfLoopRuleModels.swift} | 4 ++-- 4 files changed, 11 insertions(+), 11 deletions(-) rename Source/SwiftLintBuiltInRules/Rules/Idiomatic/{ReduceIntoInsteadOfLoop.swift => ReduceIntoInsteadOfLoopRule.swift} (96%) rename Source/SwiftLintBuiltInRules/Rules/Idiomatic/{ReduceIntoInsteadOfLoopExamples.swift => ReduceIntoInsteadOfLoopRuleExamples.swift} (97%) rename Source/SwiftLintBuiltInRules/Rules/Idiomatic/{ReduceIntoInsteadOfLoopHelpers.swift => ReduceIntoInsteadOfLoopRuleHelpers.swift} (97%) rename Source/SwiftLintBuiltInRules/Rules/Idiomatic/{ReduceIntoInsteadOfLoopModels.swift => ReduceIntoInsteadOfLoopRuleModels.swift} (96%) diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRule.swift similarity index 96% rename from Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift rename to Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRule.swift index e207f38270..49b4cd6188 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRule.swift @@ -1,10 +1,10 @@ import SwiftSyntax -typealias ReferencedVariable = ReduceIntoInsteadOfLoop.ReferencedVariable -typealias CollectionType = ReduceIntoInsteadOfLoop.CollectionType +typealias ReferencedVariable = ReduceIntoInsteadOfLoopRule.ReferencedVariable +typealias CollectionType = ReduceIntoInsteadOfLoopRule.CollectionType @SwiftSyntaxRule -struct ReduceIntoInsteadOfLoop: Rule { +struct ReduceIntoInsteadOfLoopRule: Rule { var configuration = SeverityConfiguration(.warning) static let description = RuleDescription( @@ -12,12 +12,12 @@ struct ReduceIntoInsteadOfLoop: Rule { name: "Reduce Into Instead Of Loop", description: "Prefer using reduce(into:) instead of a loop", kind: .idiomatic, - nonTriggeringExamples: ReduceIntoInsteadOfLoopExamples.nonTriggeringExamples, - triggeringExamples: ReduceIntoInsteadOfLoopExamples.triggeringExamples + nonTriggeringExamples: ReduceIntoInsteadOfLoopRuleExamples.nonTriggeringExamples, + triggeringExamples: ReduceIntoInsteadOfLoopRuleExamples.triggeringExamples ) } -internal extension ReduceIntoInsteadOfLoop { +internal extension ReduceIntoInsteadOfLoopRule { final class Visitor: ViolationsSyntaxVisitor { override func visitPost(_ node: CodeBlockItemListSyntax) { // reduce into forInStmts and variableDecls map @@ -59,7 +59,7 @@ internal extension ReduceIntoInsteadOfLoop { ] static let collectionNames: [String: CollectionType] = - ReduceIntoInsteadOfLoop.collectionTypes.reduce(into: [String: CollectionType]()) { partialResult, type in + ReduceIntoInsteadOfLoopRule.collectionTypes.reduce(into: [String: CollectionType]()) { partialResult, type in partialResult[type.name] = type } } diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopExamples.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRuleExamples.swift similarity index 97% rename from Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopExamples.swift rename to Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRuleExamples.swift index 267dd98997..575ce41a64 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopExamples.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRuleExamples.swift @@ -1,4 +1,4 @@ -internal struct ReduceIntoInsteadOfLoopExamples { +internal struct ReduceIntoInsteadOfLoopRuleExamples { static let nonTriggeringExamples: [Example] = [ // Example(""" // class Foo { diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopHelpers.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRuleHelpers.swift similarity index 97% rename from Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopHelpers.swift rename to Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRuleHelpers.swift index 2b3321d958..4d351d566a 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopHelpers.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRuleHelpers.swift @@ -1,6 +1,6 @@ import SwiftSyntax -struct ReduceIntoInsteadOfLoopHelpers { } +struct ReduceIntoInsteadOfLoopRuleHelpers { } extension PatternBindingListSyntax { func optionalIndex(after: PatternBindingListSyntax.Index?) -> PatternBindingListSyntax.Index? { diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopModels.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRuleModels.swift similarity index 96% rename from Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopModels.swift rename to Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRuleModels.swift index 3fecea8ded..99f2c70ab7 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopModels.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRuleModels.swift @@ -1,8 +1,8 @@ import SwiftSyntax -typealias ReduceIntoInsteadOfLoopModels = ReduceIntoInsteadOfLoop +typealias ReduceIntoInsteadOfLoopModels = ReduceIntoInsteadOfLoopRule -internal extension ReduceIntoInsteadOfLoop { +internal extension ReduceIntoInsteadOfLoopRule { struct ReferencedVariable: Hashable { let name: String let position: AbsolutePosition From 5016d85b7208aff46987467d3d5388ce2d58e1b7 Mon Sep 17 00:00:00 2001 From: Alfons Hoogervorst Date: Sun, 11 May 2025 22:14:25 +0200 Subject: [PATCH 13/28] Fix new SwiftSyntax forStmt --- .../Rules/Idiomatic/ReduceIntoInsteadOfLoopRule.swift | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRule.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRule.swift index 49b4cd6188..8f2bd3da59 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRule.swift @@ -75,7 +75,9 @@ private extension CodeBlockItemListSyntax { assertionFailure("Unreachable") return } - guard codeBlockItem.kind == .forStmt, let forStmt = codeBlockItem.item.as(ForStmtSyntax.self), forStmt.inKeyword == .keyword(.in) else { + guard codeBlockItem.item.kind == .forStmt, + let forStmt = codeBlockItem.item.as(ForStmtSyntax.self), + forStmt.inKeyword.tokenKind == .keyword(.in) else { return } guard let lastEncountered = partialResult.last else { From 86560c50645f752c5314d3fd0b56cf078a822aec Mon Sep 17 00:00:00 2001 From: Alfons Hoogervorst Date: Mon, 12 May 2025 04:59:52 +0200 Subject: [PATCH 14/28] Fix new SwiftSyntax-isms --- .../Idiomatic/ReduceIntoInsteadOfLoopRule.swift | 9 +++++---- .../ReduceIntoInsteadOfLoopRuleHelpers.swift | 12 ++++++++---- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRule.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRule.swift index 8f2bd3da59..cf35a7b7c3 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRule.swift @@ -119,15 +119,16 @@ private extension CodeBlockItemListSyntax { private extension VariableDeclSyntax { /// Is type declared with initializer: `: Set<> = []`, `: Array<> = []`, or `: Dictionary<> = [:]` var isTypeAnnotatedAndInitializer: Bool { - guard self.isVar && self.identifier != nil, - let idIndex = self.firstIndexOf(IdentifierPatternSyntax.self) else { + guard self.isVar, + self.identifier != nil, + let identifiePatternSyntax = self.firstPatternOf(IdentifierPatternSyntax.self) else { return false } // Is type-annotated, and initialized? - guard let patternBindingSyntax = self.next(after: idIndex), + guard let patternBindingSyntax = identifiePatternSyntax.parent?.as(PatternBindingSyntax.self), let typeAnnotation = patternBindingSyntax.typeAnnotation, let type = typeAnnotation.collectionDeclarationType(), - let initializerClause = self.next(after: patternBindingSyntax)?.initializer else { + let initializerClause = patternBindingSyntax.initializer else { return false } return initializerClause.isTypeInitializer(for: type) diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRuleHelpers.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRuleHelpers.swift index 4d351d566a..dc1b931f75 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRuleHelpers.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRuleHelpers.swift @@ -14,11 +14,11 @@ extension PatternBindingListSyntax { extension VariableDeclSyntax { /// Binding is a var: "`var` someVar = <...>" var isVar: Bool { - self.bindingSpecifier == .keyword(.var) + return self.bindingSpecifier.tokenKind == .keyword(.var) } var identifier: String? { - guard let identifierPattern = self.firstOf(IdentifierPatternSyntax.self), + guard let identifierPattern = self.firstPatternOf(IdentifierPatternSyntax.self), case .identifier(let name) = identifierPattern.identifier.tokenKind else { return nil } @@ -42,9 +42,12 @@ extension VariableDeclSyntax { let newIndex = self.bindings.index(after: index) guard newIndex >= self.bindings.startIndex && newIndex < self.bindings.endIndex else { return nil + /// Returns the first binding with a `pattern` of type + /// `type`. + func firstPatternOf(_ type: T.Type) -> T? { + let result = self.bindings.first { patternBinding in + return patternBinding.pattern.as(type) != nil } - return self.bindings[newIndex] - } func firstOf(_ type: T.Type) -> T? { self.bindings.first { patternBinding in @@ -56,5 +59,6 @@ extension VariableDeclSyntax { self.bindings.firstIndex(where: { patternBinding in patternBinding.pattern.as(type) != nil }) + return result?.pattern.as(type) } } From 27c71882bdfb4917e8d8a6189e02f04a5186b3e0 Mon Sep 17 00:00:00 2001 From: Alfons Hoogervorst Date: Mon, 12 May 2025 05:00:10 +0200 Subject: [PATCH 15/28] Clean up helpers --- .../ReduceIntoInsteadOfLoopRuleHelpers.swift | 37 ------------------- 1 file changed, 37 deletions(-) diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRuleHelpers.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRuleHelpers.swift index dc1b931f75..1f79aadf14 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRuleHelpers.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRuleHelpers.swift @@ -2,15 +2,6 @@ import SwiftSyntax struct ReduceIntoInsteadOfLoopRuleHelpers { } -extension PatternBindingListSyntax { - func optionalIndex(after: PatternBindingListSyntax.Index?) -> PatternBindingListSyntax.Index? { - guard let after else { - return nil - } - return self.index(after: after) - } -} - extension VariableDeclSyntax { /// Binding is a var: "`var` someVar = <...>" var isVar: Bool { @@ -25,40 +16,12 @@ extension VariableDeclSyntax { return name } - func next(after index: PatternBindingListSyntax.Index?) -> PatternBindingSyntax? { - guard let index = self.bindings.optionalIndex(after: index), - index >= self.bindings.startIndex && index < self.bindings.endIndex else { - return nil - } - return self.bindings[index] - } - - func next(after: PatternBindingSyntax?) -> PatternBindingSyntax? { - guard let after, let index = self.bindings.firstIndex(where: { patterBindingSyntax in - patterBindingSyntax == after - }) else { - return nil - } - let newIndex = self.bindings.index(after: index) - guard newIndex >= self.bindings.startIndex && newIndex < self.bindings.endIndex else { - return nil /// Returns the first binding with a `pattern` of type /// `type`. func firstPatternOf(_ type: T.Type) -> T? { let result = self.bindings.first { patternBinding in return patternBinding.pattern.as(type) != nil } - - func firstOf(_ type: T.Type) -> T? { - self.bindings.first { patternBinding in - patternBinding.pattern.as(type) != nil - } as? T - } - - func firstIndexOf(_ type: T.Type) -> PatternBindingListSyntax.Index? { - self.bindings.firstIndex(where: { patternBinding in - patternBinding.pattern.as(type) != nil - }) return result?.pattern.as(type) } } From 7ff5b30968513cd85f0cc7805fa7b0ea77b5e895 Mon Sep 17 00:00:00 2001 From: Alfons Hoogervorst Date: Mon, 12 May 2025 06:24:13 +0200 Subject: [PATCH 16/28] Support array initialisation expression --- .../Rules/Idiomatic/ReduceIntoInsteadOfLoopRule.swift | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRule.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRule.swift index cf35a7b7c3..2b43dbfca5 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRule.swift @@ -216,6 +216,7 @@ private extension InitializerClauseSyntax { /// Otherwise checks for `FunctionCallExpr`,`MemberAccessExpr`,`DictionaryExpr` and `ArrayExpr`: /// 1. `= Set(...)` | `Set(...)` | `.init(...)` | `[]` /// 2. `= Array(...)` | `Array(...)` | `.init(...)` | `[]` + /// 2b. `= [Type]()` /// 3. `= Dictionary()` | `Dictionary()` | `.init(..)` | `[:]` func isTypeInitializer(for collectionType: CollectionType? = nil) -> Bool { func isSupportedType(with name: String) -> Bool { @@ -238,6 +239,15 @@ private extension InitializerClauseSyntax { } if let memberAccessExpr = functionCallExpr.calledExpression.as(MemberAccessExprSyntax.self), memberAccessExpr.declName.baseName.tokenKind == .keyword(.`init`) { + // found a collection initialisation expression of `.init()` + // e.g. var array: [Int] = .init() + return true + } + if let arrayExpr = functionCallExpr.calledExpression.as(ArrayExprSyntax.self), + arrayExpr.elements.count == 1, + arrayExpr.elements.first?.expression.is(DeclReferenceExprSyntax.self) == true { + // found an array initialisation expression of `[type]`() + // e.g. var array = [Int]() return true } } else if collectionType == .dictionary, From 40c4ff6481c0cf0a4b71d6b09a957456bbf436b3 Mon Sep 17 00:00:00 2001 From: Alfons Hoogervorst Date: Mon, 12 May 2025 06:31:47 +0200 Subject: [PATCH 17/28] Regenerate built-in rules to include our rule --- Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift | 2 +- Tests/GeneratedTests/GeneratedTests.swift | 6 ++++++ Tests/IntegrationTests/default_rule_configurations.yml | 5 +++++ 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift b/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift index c5b719ac94..3301b2c216 100644 --- a/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift +++ b/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift @@ -169,8 +169,8 @@ public let builtInRules: [any Rule.Type] = [ QuickDiscouragedPendingTestRule.self, RawValueForCamelCasedCodableEnumRule.self, ReduceBooleanRule.self, + ReduceIntoInsteadOfLoopRule.self, ReduceIntoRule.self, - ReduceIntoInsteadOfLoop.self, RedundantDiscardableLetRule.self, RedundantNilCoalescingRule.self, RedundantObjcAttributeRule.self, diff --git a/Tests/GeneratedTests/GeneratedTests.swift b/Tests/GeneratedTests/GeneratedTests.swift index 1b65b9e47b..5c33727384 100644 --- a/Tests/GeneratedTests/GeneratedTests.swift +++ b/Tests/GeneratedTests/GeneratedTests.swift @@ -1009,6 +1009,12 @@ final class ReduceBooleanRuleGeneratedTests: SwiftLintTestCase { } } +final class ReduceIntoInsteadOfLoopRuleGeneratedTests: SwiftLintTestCase { + func testWithDefaultConfiguration() { + verifyRule(ReduceIntoInsteadOfLoopRule.description) + } +} + final class ReduceIntoRuleGeneratedTests: SwiftLintTestCase { func testWithDefaultConfiguration() { verifyRule(ReduceIntoRule.description) diff --git a/Tests/IntegrationTests/default_rule_configurations.yml b/Tests/IntegrationTests/default_rule_configurations.yml index 981d667680..48d88da061 100644 --- a/Tests/IntegrationTests/default_rule_configurations.yml +++ b/Tests/IntegrationTests/default_rule_configurations.yml @@ -964,6 +964,11 @@ reduce_into: meta: opt-in: true correctable: false +reduce_into_instead_of_loop: + severity: warning + meta: + opt-in: false + correctable: false redundant_discardable_let: severity: warning ignore_swiftui_view_bodies: false From 68229014de1b109de14e5c9424394eb2d5ece956 Mon Sep 17 00:00:00 2001 From: Alfons Hoogervorst Date: Mon, 12 May 2025 08:02:46 +0200 Subject: [PATCH 18/28] Handle assignment expression `varName = <...>` --- .../ReduceIntoInsteadOfLoopRule.swift | 48 +++++++++++++------ 1 file changed, 33 insertions(+), 15 deletions(-) diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRule.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRule.swift index 2b43dbfca5..22f4dc7bfe 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRule.swift @@ -262,6 +262,11 @@ private extension InitializerClauseSyntax { } private extension ForStmtSyntax { + /// Checks whether any of the collection variables in scope are referenced + /// in the forStmt. + /// Note: When a varDecl is referenced, that's already a good trigger for + /// the warning: detecting which functions are mutating is not possible, + /// other than checking for a lhs-assignment. func referencedVariables(for variables: [VariableDeclSyntax]?) -> Set? { guard let variables, !variables.isEmpty else { return nil @@ -319,27 +324,40 @@ private extension FunctionCallExprSyntax { } private extension SequenceExprSyntax { - /// varName[xxx] = ... + /// Detect assignment expression: + /// varName[xxx] = ... // index + /// varName = ... func referencedVariable(for varName: String) -> ReferencedVariable? { let exprList = self.elements guard exprList.count >= 2 else { return nil } - let first = exprList.startIndex - let second = exprList.index(after: first) - guard let subscrExpr = exprList[first].as(SubscriptCallExprSyntax.self), - let assignmentExpr = exprList[second].as(AssignmentExprSyntax.self), - assignmentExpr.equal.text == "=", - subscrExpr.leftSquare.tokenKind == .leftSquare, - subscrExpr.rightSquare.tokenKind == .rightSquare, - let identifierExpr = subscrExpr.calledExpression.as(DeclReferenceExprSyntax.self), - identifierExpr.baseName.tokenKind == .identifier(varName) else { + let firstExpr = exprList[exprList.startIndex] + let secondExpr = exprList[exprList.index(after: exprList.startIndex)] + guard let assignmentExpr = secondExpr.as(AssignmentExprSyntax.self), + assignmentExpr.equal.tokenKind == .equal else { + // no assignment expression + return nil + } + if let subscrExpr = firstExpr.as(SubscriptCallExprSyntax.self), + subscrExpr.leftSquare.tokenKind == .leftSquare, + subscrExpr.rightSquare.tokenKind == .rightSquare, + let identifierExpr = subscrExpr.calledExpression.as(DeclReferenceExprSyntax.self), + identifierExpr.baseName.tokenKind == .identifier(varName) { + return .init( + name: varName, + position: exprList.positionAfterSkippingLeadingTrivia, + kind: .assignment(subscript: true) + ) + } else if let declExpr = firstExpr.as(DeclReferenceExprSyntax.self), + declExpr.baseName.tokenKind == .identifier(varName) { + return .init( + name: varName, + position: exprList.positionAfterSkippingLeadingTrivia, + kind: .assignment(subscript: false) + ) + } else { return nil } - return .init( - name: varName, - position: exprList.positionAfterSkippingLeadingTrivia, - kind: .assignment(subscript: true) - ) } } From b92bce3b7841f278737a8e3b347dd14119fccaf6 Mon Sep 17 00:00:00 2001 From: Alfons Hoogervorst Date: Mon, 12 May 2025 08:24:25 +0200 Subject: [PATCH 19/28] Clean up --- .../Rules/Idiomatic/ReduceIntoInsteadOfLoopRule.swift | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRule.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRule.swift index 22f4dc7bfe..14cf79e12d 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRule.swift @@ -10,7 +10,7 @@ struct ReduceIntoInsteadOfLoopRule: Rule { static let description = RuleDescription( identifier: "reduce_into_instead_of_loop", name: "Reduce Into Instead Of Loop", - description: "Prefer using reduce(into:) instead of a loop", + description: "Prefer using `reduce(into:)` instead of mutating a sequence in a `for _ in ...` loop", kind: .idiomatic, nonTriggeringExamples: ReduceIntoInsteadOfLoopRuleExamples.nonTriggeringExamples, triggeringExamples: ReduceIntoInsteadOfLoopRuleExamples.triggeringExamples @@ -72,7 +72,6 @@ private extension CodeBlockItemListSyntax { // Collect all ForInStmts and track their index ranges let indexRangeForStatements: [IndexRangeForStmts] = self.reduce(into: [IndexRangeForStmts]()) { partialResult, codeBlockItem in guard let codeBlockItemIndex = self.index(of: codeBlockItem) else { - assertionFailure("Unreachable") return } guard codeBlockItem.item.kind == .forStmt, @@ -95,7 +94,6 @@ private extension CodeBlockItemListSyntax { // Only VariableDecls on same level of scope of the ForInStmt. let result = self.reduce(into: [ForStmtSyntax: [VariableDeclSyntax]]()) { partialResult, codeBlockItem in guard let codeBlockItemIndex = self.index(of: codeBlockItem) else { - assertionFailure("Unreachable") return } guard let variableDecl = codeBlockItem.item.as(VariableDeclSyntax.self) else { @@ -121,11 +119,11 @@ private extension VariableDeclSyntax { var isTypeAnnotatedAndInitializer: Bool { guard self.isVar, self.identifier != nil, - let identifiePatternSyntax = self.firstPatternOf(IdentifierPatternSyntax.self) else { + let identifierPatternSyntax = self.firstPatternOf(IdentifierPatternSyntax.self) else { return false } // Is type-annotated, and initialized? - guard let patternBindingSyntax = identifiePatternSyntax.parent?.as(PatternBindingSyntax.self), + guard let patternBindingSyntax = identifierPatternSyntax.parent?.as(PatternBindingSyntax.self), let typeAnnotation = patternBindingSyntax.typeAnnotation, let type = typeAnnotation.collectionDeclarationType(), let initializerClause = patternBindingSyntax.initializer else { From 5ea7e80123eec878f52a976f001758344c6e3b8a Mon Sep 17 00:00:00 2001 From: Alfons Hoogervorst Date: Mon, 12 May 2025 16:52:41 +0200 Subject: [PATCH 20/28] Clean up --- .../ReduceIntoInsteadOfLoopRule.swift | 37 ++++++++++--------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRule.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRule.swift index 14cf79e12d..32e0f28313 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRule.swift @@ -20,15 +20,15 @@ struct ReduceIntoInsteadOfLoopRule: Rule { internal extension ReduceIntoInsteadOfLoopRule { final class Visitor: ViolationsSyntaxVisitor { override func visitPost(_ node: CodeBlockItemListSyntax) { - // reduce into forInStmts and variableDecls map + // Collect all varDecls in the same scope as forInStmts guard let all = node.allVariableDeclsForInStatmts() else { return } - // reduce variableDecls into the ones we're interested in + // Select those varDecls that have initialisers let selected = all.reduce(into: [ForStmtSyntax: [VariableDeclSyntax]]()) { partialResult, element in // we're interested fully type declared and implicitly declared by initializer let interestingVariableDecls = element.value.filter { variableDecl in - variableDecl.isTypeAnnotatedAndInitializer + return variableDecl.isTypeAnnotatedAndInitializer || variableDecl.isCollectionTypeInitializer } guard !interestingVariableDecls.isEmpty else { @@ -39,6 +39,8 @@ internal extension ReduceIntoInsteadOfLoopRule { guard !selected.isEmpty else { return } + // Collect all varDecls that are referenced (and in our case possibly + // mutate) in the forInStmt let referencedVars = selected.reduce(into: Set()) { partialResult, keyValue in let (forInStmt, variableDecls) = keyValue if let allReferencedVars = forInStmt.referencedVariables(for: variableDecls) { @@ -48,6 +50,7 @@ internal extension ReduceIntoInsteadOfLoopRule { guard !referencedVars.isEmpty else { return } + // If there are referenced varDecls, then report violations self.violations.append(contentsOf: referencedVars.map { $0.position }) } } @@ -59,7 +62,7 @@ internal extension ReduceIntoInsteadOfLoopRule { ] static let collectionNames: [String: CollectionType] = - ReduceIntoInsteadOfLoopRule.collectionTypes.reduce(into: [String: CollectionType]()) { partialResult, type in + ReduceIntoInsteadOfLoopRule.collectionTypes.reduce(into: [String: CollectionType]()) { partialResult, type in partialResult[type.name] = type } } @@ -125,7 +128,7 @@ private extension VariableDeclSyntax { // Is type-annotated, and initialized? guard let patternBindingSyntax = identifierPatternSyntax.parent?.as(PatternBindingSyntax.self), let typeAnnotation = patternBindingSyntax.typeAnnotation, - let type = typeAnnotation.collectionDeclarationType(), + let type = typeAnnotation.collectionDeclarationType, let initializerClause = patternBindingSyntax.initializer else { return false } @@ -139,7 +142,7 @@ private extension VariableDeclSyntax { return false } guard let initializerClausePatternBinding = self.bindings.first(where: { patternBindingSyntax in - patternBindingSyntax.initializer != nil + return patternBindingSyntax.initializer != nil }) else { return false } @@ -155,21 +158,21 @@ private extension VariableDeclSyntax { private extension TypeAnnotationSyntax { /// Returns one of the collection types we define - func collectionDeclarationType() -> CollectionType? { - if let genericTypeName = self.genericCollectionDeclarationType() { + var collectionDeclarationType: CollectionType? { + if let genericTypeName = self.genericCollectionDeclarationType { return genericTypeName } - if let array = self.arrayDeclarationType() { + if let array = self.arrayDeclarationType { return array } - if let dictionary = self.dictionaryDeclarationType() { + if let dictionary = self.dictionaryDeclarationType { return dictionary } return nil } /// var x: Set<>, var x: Array<>, var x: Dictionary<> - func genericCollectionDeclarationType() -> CollectionType? { + var genericCollectionDeclarationType: CollectionType? { guard let simpleTypeIdentifier = self.type.as(IdentifierTypeSyntax.self), let genericArgumentClause = simpleTypeIdentifier.genericArgumentClause, genericArgumentClause.leftAngle.tokenKind == .leftAngle, @@ -183,7 +186,7 @@ private extension TypeAnnotationSyntax { } /// var x: [Y] - func arrayDeclarationType() -> CollectionType? { + var arrayDeclarationType: CollectionType? { guard let arrayType = self.type.as(ArrayTypeSyntax.self), case .leftSquare = arrayType.leftSquare.tokenKind, case .rightSquare = arrayType.rightSquare.tokenKind, @@ -194,7 +197,7 @@ private extension TypeAnnotationSyntax { } /// var x: [K: V] - func dictionaryDeclarationType() -> CollectionType? { + var dictionaryDeclarationType: CollectionType? { guard let dictionaryType = self.type.as(DictionaryTypeSyntax.self), case .leftSquare = dictionaryType.leftSquare.tokenKind, case .rightSquare = dictionaryType.rightSquare.tokenKind, @@ -270,7 +273,7 @@ private extension ForStmtSyntax { return nil } let codeBlockItemList = self.body.statements - let references: Set = codeBlockItemList.reduce(into: .init(), { partialResult, codeBlock in + let references: Set = codeBlockItemList.reduce(into: [], { partialResult, codeBlock in // no need to cover one liner: someMutation(); someOtherMutation() variables.forEach { variableDecl in if let referenced = codeBlock.referencedVariable(for: variableDecl) { @@ -313,7 +316,7 @@ private extension FunctionCallExprSyntax { case .identifier(let name) = memberAccessExpr.declName.baseName.tokenKind else { return nil } - return .init( + return ReferencedVariable( name: varName, position: memberAccessExpr.positionAfterSkippingLeadingTrivia, kind: .method(name: name, arguments: self.arguments.count) @@ -342,14 +345,14 @@ private extension SequenceExprSyntax { subscrExpr.rightSquare.tokenKind == .rightSquare, let identifierExpr = subscrExpr.calledExpression.as(DeclReferenceExprSyntax.self), identifierExpr.baseName.tokenKind == .identifier(varName) { - return .init( + return ReferencedVariable( name: varName, position: exprList.positionAfterSkippingLeadingTrivia, kind: .assignment(subscript: true) ) } else if let declExpr = firstExpr.as(DeclReferenceExprSyntax.self), declExpr.baseName.tokenKind == .identifier(varName) { - return .init( + return ReferencedVariable( name: varName, position: exprList.positionAfterSkippingLeadingTrivia, kind: .assignment(subscript: false) From f91b8e94e1d255ad0874f6e1a4e9f49114efe14f Mon Sep 17 00:00:00 2001 From: Alfons Hoogervorst Date: Mon, 12 May 2025 20:48:51 +0200 Subject: [PATCH 21/28] Add triggering/non-triggering examples for the unit tests --- .../ReduceIntoInsteadOfLoopRuleExamples.swift | 183 +++++++++++------- 1 file changed, 113 insertions(+), 70 deletions(-) diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRuleExamples.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRuleExamples.swift index 575ce41a64..e191a0bcf3 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRuleExamples.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRuleExamples.swift @@ -1,78 +1,121 @@ -internal struct ReduceIntoInsteadOfLoopRuleExamples { +struct ReduceIntoInsteadOfLoopRuleExamples { static let nonTriggeringExamples: [Example] = [ -// Example(""" -// class Foo { -// static let constant: Int = 1 -// var variable: Int = 2 -// } -// """), -// Example(""" -// struct Foo { -// static let constant: Int = 1 -// } -// """), -// Example(""" -// enum InstFooance { -// static let constant = 1 -// } -// """), -// Example(""" -// struct Foo { -// let property1 -// let property2 -// init(property1: Int, property2: String) { -// self.property1 = property1 -// self.property2 = property2 -// } -// } -// """) - Example(""" - let encountered: Set = someArray.reduce(into: Set(), { result, eachN in - result.insert(eachN) - }) + Example(""" + let encountered: Array = someSequence.reduce(into: Array(), { result, eachN in + result.insert(eachN) + }) + """), + Example(""" + let encountered: Set = someSequence.reduce(into: Set(), { result, eachN in + result.insert(eachN) + }) + """), + Example(""" + let encountered: Dictionary = someSequence.reduce(into: Dictionary(), { result, eachN in + result[SomeType1Value] = SomeType2Value + }) + """), + ] + + static let triggeringExamples: [Example] = + triggeringArrayExamples + + triggeringSetExamples + + triggeringDictionaryExamples +} + +extension ReduceIntoInsteadOfLoopRuleExamples { + + private static let triggeringDictionaryExamples: [Example] = [ + Example(""" + var result: Dictionary = [:] + for eachN in someSequence { + ↓result[SomeType1Value] = SomeType2Value + eachN + } + """), + Example(""" + var result: Dictionary = [:] + for eachN in someSequence { + ↓result.someMethod(eachN) + } + """), + Example(""" + var result: Dictionary = .init() + for eachN in someSequence { + ↓result.someMethod(eachN) + } + """), + Example(""" + var result = Dictionary() + for eachN in someSequence { + ↓result.someMethod(eachN) + } """), ] - static let triggeringExamples: [Example] = [ -// Example(""" -// class Foo { -// static let one = 32 -// ↓let constant: Int = 1 -// } -// """), -// Example(""" -// struct Foo { -// ↓let constant: Int = 1 -// } -// """), -// Example(""" -// enum Foo { -// ↓let constant: Int = 1 -// } -// """), - Example(""" - var encountered: Set = [] - for eachN in someArray { - ↓encountered.insert(eachN) - } + private static let triggeringSetExamples: [Example] = [ + Example(""" + var result: Set = [] + for eachN in someSequence { + ↓result = result + [eachN] + } + """), + Example(""" + var result: Set = [] + for eachN in someSequence { + ↓result.someMethod(eachN) + } + """), + Example(""" + var result: Set = .init() + for eachN in someSequence { + ↓result.someMethod(eachN) + } + """), + Example(""" + var result = Set() + for eachN in someSequence { + ↓result.someMethod(eachN) + } + """), + ] + + private static let triggeringArrayExamples: [Example] = [ + Example(""" + var result: [SomeType] = [] + for eachN in someSequence { + ↓result[5] = eachN + } + """), + Example(""" + var result: [SomeType] = [] + for eachN in someSequence { + ↓result = result + [eachN] + } + """), + Example(""" + var result: [SomeType] = [] + for eachN in someSequence { + ↓result.someMethod(eachN) + } + """), + Example(""" + var result: [SomeType] = .init() + for eachN in someSequence { + ↓result.someMethod(eachN) + } + """), + Example(""" + var result = Array() + for eachN in someSequence { + ↓result.someMethod(eachN) + } + """), + Example(""" + var result = [SomeType]() + for eachN in someSequence { + ↓result.someMethod(eachN) + } """), ] } -// var encountered1: Set = [] -// var encountered1a = Set() -// var encountered1b: Set = Set() -// var encountered1c: Set = .init() -// var encountered2: [String] = [] -// var encountered2a = [String]() -// var encountered2b: [String] = [1, 2, 3, 4] -// var encountered2c: [String] = Array(contentsOf: other) -// var encountered3: Array = [] -// var encountered4: Dictionary = [] -// var encountered4b: [String: Int] = [:] -// var encountered4c: [String: Int] = ["2": 2, "3": 3] -// for eachN in someArray { -// encountered.insert(eachN) -// encountered1[2] = 45 -// let newSet = encountered.popFirst() -// } From 1443cb7dc9fe87228c2a24faf66c950e9352e1d9 Mon Sep 17 00:00:00 2001 From: Alfons Hoogervorst Date: Mon, 12 May 2025 21:50:02 +0200 Subject: [PATCH 22/28] Auto applied fixes --- .../Idiomatic/ReduceIntoInsteadOfLoopRule.swift | 14 +++++++------- .../ReduceIntoInsteadOfLoopRuleExamples.swift | 2 -- .../ReduceIntoInsteadOfLoopRuleHelpers.swift | 4 ++-- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRule.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRule.swift index 32e0f28313..b5f6431bf9 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRule.swift @@ -3,7 +3,7 @@ import SwiftSyntax typealias ReferencedVariable = ReduceIntoInsteadOfLoopRule.ReferencedVariable typealias CollectionType = ReduceIntoInsteadOfLoopRule.CollectionType -@SwiftSyntaxRule +@SwiftSyntaxRule(optIn: true) struct ReduceIntoInsteadOfLoopRule: Rule { var configuration = SeverityConfiguration(.warning) @@ -28,7 +28,7 @@ internal extension ReduceIntoInsteadOfLoopRule { let selected = all.reduce(into: [ForStmtSyntax: [VariableDeclSyntax]]()) { partialResult, element in // we're interested fully type declared and implicitly declared by initializer let interestingVariableDecls = element.value.filter { variableDecl in - return variableDecl.isTypeAnnotatedAndInitializer + variableDecl.isTypeAnnotatedAndInitializer || variableDecl.isCollectionTypeInitializer } guard !interestingVariableDecls.isEmpty else { @@ -142,7 +142,7 @@ private extension VariableDeclSyntax { return false } guard let initializerClausePatternBinding = self.bindings.first(where: { patternBindingSyntax in - return patternBindingSyntax.initializer != nil + patternBindingSyntax.initializer != nil }) else { return false } @@ -326,7 +326,7 @@ private extension FunctionCallExprSyntax { private extension SequenceExprSyntax { /// Detect assignment expression: - /// varName[xxx] = ... // index + /// varName[xxx] = ... // index /// varName = ... func referencedVariable(for varName: String) -> ReferencedVariable? { let exprList = self.elements @@ -350,15 +350,15 @@ private extension SequenceExprSyntax { position: exprList.positionAfterSkippingLeadingTrivia, kind: .assignment(subscript: true) ) - } else if let declExpr = firstExpr.as(DeclReferenceExprSyntax.self), + } + if let declExpr = firstExpr.as(DeclReferenceExprSyntax.self), declExpr.baseName.tokenKind == .identifier(varName) { return ReferencedVariable( name: varName, position: exprList.positionAfterSkippingLeadingTrivia, kind: .assignment(subscript: false) ) - } else { - return nil } + return nil } } diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRuleExamples.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRuleExamples.swift index e191a0bcf3..ddfa1f31fd 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRuleExamples.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRuleExamples.swift @@ -24,7 +24,6 @@ struct ReduceIntoInsteadOfLoopRuleExamples { } extension ReduceIntoInsteadOfLoopRuleExamples { - private static let triggeringDictionaryExamples: [Example] = [ Example(""" var result: Dictionary = [:] @@ -118,4 +117,3 @@ extension ReduceIntoInsteadOfLoopRuleExamples { """), ] } - diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRuleHelpers.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRuleHelpers.swift index 1f79aadf14..fbcfbccd2f 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRuleHelpers.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRuleHelpers.swift @@ -5,7 +5,7 @@ struct ReduceIntoInsteadOfLoopRuleHelpers { } extension VariableDeclSyntax { /// Binding is a var: "`var` someVar = <...>" var isVar: Bool { - return self.bindingSpecifier.tokenKind == .keyword(.var) + self.bindingSpecifier.tokenKind == .keyword(.var) } var identifier: String? { @@ -20,7 +20,7 @@ extension VariableDeclSyntax { /// `type`. func firstPatternOf(_ type: T.Type) -> T? { let result = self.bindings.first { patternBinding in - return patternBinding.pattern.as(type) != nil + patternBinding.pattern.as(type) != nil } return result?.pattern.as(type) } From 4a67268a0ad582944c16e07c158c0db7e1ec1c0c Mon Sep 17 00:00:00 2001 From: Alfons Hoogervorst Date: Mon, 12 May 2025 21:50:15 +0200 Subject: [PATCH 23/28] Make opt-in --- Tests/IntegrationTests/default_rule_configurations.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/IntegrationTests/default_rule_configurations.yml b/Tests/IntegrationTests/default_rule_configurations.yml index 48d88da061..16d6198547 100644 --- a/Tests/IntegrationTests/default_rule_configurations.yml +++ b/Tests/IntegrationTests/default_rule_configurations.yml @@ -967,7 +967,7 @@ reduce_into: reduce_into_instead_of_loop: severity: warning meta: - opt-in: false + opt-in: true correctable: false redundant_discardable_let: severity: warning From e773996eb6fabb934c88e444d533ec1ef04cf8f5 Mon Sep 17 00:00:00 2001 From: Alfons Hoogervorst Date: Mon, 12 May 2025 21:50:30 +0200 Subject: [PATCH 24/28] Add credit note --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d61ae13a07..3cce37279d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,8 @@ * Add new `allowed_numbers` option to the `no_magic_numbers` rule. [Martin Redington](https://github.com/mildm8nnered) +* Add new `reduce_into_instead_of_loop` opt-in rule that triggers when a `for ... in ` is used to update a sequence where a `reduce(into:)` is preferred. + [Alfons Hoogervorst](https://github.com/snofla/SwiftLint) ### Bug Fixes From a42d66b32f8c9747fd2dee12a2137ea2317c7b67 Mon Sep 17 00:00:00 2001 From: Alfons Hoogervorst Date: Mon, 12 May 2025 21:53:15 +0200 Subject: [PATCH 25/28] Fix ChangeLog line len --- CHANGELOG.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3cce37279d..3c27046de5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,7 +36,9 @@ * Add new `allowed_numbers` option to the `no_magic_numbers` rule. [Martin Redington](https://github.com/mildm8nnered) -* Add new `reduce_into_instead_of_loop` opt-in rule that triggers when a `for ... in ` is used to update a sequence where a `reduce(into:)` is preferred. +* Add new `reduce_into_instead_of_loop` opt-in rule that triggers when a + `for ... in ` is used to update a sequence where a `reduce(into:)` is + preferred. [Alfons Hoogervorst](https://github.com/snofla/SwiftLint) ### Bug Fixes From 9e8fbbcbe241b862a4f5dfd0065bf694f5d314a7 Mon Sep 17 00:00:00 2001 From: Alfons Hoogervorst Date: Mon, 12 May 2025 21:55:05 +0200 Subject: [PATCH 26/28] Add extra trailing space --- CHANGELOG.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3c27046de5..9c70d89a42 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,10 +36,10 @@ * Add new `allowed_numbers` option to the `no_magic_numbers` rule. [Martin Redington](https://github.com/mildm8nnered) -* Add new `reduce_into_instead_of_loop` opt-in rule that triggers when a - `for ... in ` is used to update a sequence where a `reduce(into:)` is +* Add new `reduce_into_instead_of_loop` opt-in rule that triggers when a + `for ... in ` is used to update a sequence where a `reduce(into:)` is preferred. - [Alfons Hoogervorst](https://github.com/snofla/SwiftLint) + [Alfons Hoogervorst](https://github.com/snofla/SwiftLint) ### Bug Fixes From ca19f7d92c9f2e0d436444540b4d213a4e0d9a66 Mon Sep 17 00:00:00 2001 From: Alfons Hoogervorst Date: Mon, 12 May 2025 22:16:33 +0200 Subject: [PATCH 27/28] Remove old files --- .../Models/BuiltInRules.swift | 1 - .../Idiomatic/ReduceIntoInsteadOfLoop.swift | 327 ------------------ .../ReduceIntoInsteadOfLoopExamples.swift | 78 ----- .../ReduceIntoInsteadOfLoopHelpers.swift | 67 ---- .../ReduceIntoInsteadOfLoopModels.swift | 55 --- 5 files changed, 528 deletions(-) delete mode 100644 Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift delete mode 100644 Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopExamples.swift delete mode 100644 Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopHelpers.swift delete mode 100644 Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopModels.swift diff --git a/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift b/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift index 1042a47394..3301b2c216 100644 --- a/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift +++ b/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift @@ -171,7 +171,6 @@ public let builtInRules: [any Rule.Type] = [ ReduceBooleanRule.self, ReduceIntoInsteadOfLoopRule.self, ReduceIntoRule.self, - ReduceIntoInsteadOfLoop.self, RedundantDiscardableLetRule.self, RedundantNilCoalescingRule.self, RedundantObjcAttributeRule.self, diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift deleted file mode 100644 index 363418e1e5..0000000000 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoop.swift +++ /dev/null @@ -1,327 +0,0 @@ -import SwiftSyntax - -typealias ReferencedVariable = ReduceIntoInsteadOfLoop.ReferencedVariable -typealias CollectionType = ReduceIntoInsteadOfLoop.CollectionType - -struct ReduceIntoInsteadOfLoop: ConfigurationProviderRule, SwiftSyntaxRule, OptInRule { - var configuration = SeverityConfiguration(.warning) - - static let description = RuleDescription( - identifier: "reduce_into_instead_of_loop", - name: "Reduce Into Instead Of Loop", - description: "Prefer using reduce(into:) instead of a loop", - kind: .idiomatic, - nonTriggeringExamples: ReduceIntoInsteadOfLoopExamples.nonTriggeringExamples, - triggeringExamples: ReduceIntoInsteadOfLoopExamples.triggeringExamples - ) - - init() {} - - func makeVisitor(file: SwiftLintFile) -> ViolationsSyntaxVisitor { - Visitor(viewMode: .sourceAccurate) - } -} - -internal extension ReduceIntoInsteadOfLoop { - final class Visitor: ViolationsSyntaxVisitor { - override func visitPost(_ node: CodeBlockItemListSyntax) { - // reduce into forInStmts and variableDecls map - guard let all = node.allVariableDeclsForInStatmts() else { - return - } - // reduce variableDecls into the ones we're interested in - let selected = all.reduce(into: [ForInStmtSyntax: [VariableDeclSyntax]]()) { partialResult, element in - // we're interested fully type declared and implicitly declared by initializer - let interestingVariableDecls = element.value.filter { variableDecl in - return variableDecl.isTypeAnnotatedAndInitializer - || variableDecl.isCollectionTypeInitializer - } - guard !interestingVariableDecls.isEmpty else { - return - } - partialResult[element.key] = interestingVariableDecls - } - guard !selected.isEmpty else { - return - } - let referencedVars = selected.reduce(into: Set()) { partialResult, keyValue in - let (forInStmt, variableDecls) = keyValue - if let allReferencedVars = forInStmt.referencedVariables(for: variableDecls) { - partialResult.formUnion(allReferencedVars) - } - } - guard !referencedVars.isEmpty else { - return - } - self.violations.append(contentsOf: referencedVars.map { $0.position }) - } - } - - static let collectionTypes: [CollectionType] = [ - CollectionType(name: "Set", genericArguments: 1), - CollectionType(name: "Array", genericArguments: 1), - CollectionType(name: "Dictionary", genericArguments: 2) - ] - - static let collectionNames: [String: CollectionType] = - ReduceIntoInsteadOfLoop.collectionTypes.reduce(into: [String: CollectionType]()) { partialResult, type in - return partialResult[type.name] = type - } -} - -private extension CodeBlockItemListSyntax { - /// Returns a dictionary with all VariableDecls preceding a ForInStmt, at the same scope level - func allVariableDeclsForInStatmts() -> [ForInStmtSyntax: [VariableDeclSyntax]]? { - typealias IndexRange = Range - typealias IndexRangeForStmts = (IndexRange, ForInStmtSyntax) - typealias IndexRangeStmts = (IndexRange, CodeBlockItemSyntax) - // collect all ForInStmts and track their index ranges - let indexed: [IndexRangeForStmts] = self.reduce(into: [IndexRangeStmts]()) { partialResult, codeBlockItem in - guard codeBlockItem.is(ForInStmtSyntax.self) else { - return - } - // Not sure whether ForInStmtSyntax.index == CodeBlockItem.index - guard let last = partialResult.last else { - partialResult.append((self.startIndex.. = []`, `: Array<> = []`, or `: Dictionary<> = [:]` - var isTypeAnnotatedAndInitializer: Bool { - guard self.isVar && self.identifier != nil, - let idIndex = self.firstIndexOf(IdentifierPatternSyntax.self) else { - return false - } - // Is type-annotated, and initialized? - guard let typeAnnotationIndex = self.next(after: idIndex), - let typeAnnotation = typeAnnotationIndex.as(TypeAnnotationSyntax.self), - let type = typeAnnotation.collectionDeclarationType(), - let initializerClause = self.next(after: typeAnnotationIndex)?.as(InitializerClauseSyntax.self) else { - return false - } - return initializerClause.isTypeInitializer(for: type) - } - - /// Is initialized with empty collection: `= Set(), = Array(), = Dictionary[:]` - /// but a couple of more, see `InitializerClauseExprSyntax.isCollectionInitializer` - var isCollectionTypeInitializer: Bool { - guard self.isVar && self.identifier != nil, - let idIndex = self.firstIndexOf(IdentifierPatternSyntax.self) else { - return false - } - let initializerClause = self.next(after: idIndex, of: InitializerClauseSyntax.self) - guard initializerClause?.isTypeInitializer() ?? false else { - return false - } - return true - } -} - -private extension TypeAnnotationSyntax { - func collectionDeclarationType() -> CollectionType? { - if let genericTypeName = self.genericCollectionDeclarationType() { - return genericTypeName - } else if let array = self.arrayDeclarationType() { - return array - } else if let dictionary = self.dictionaryDeclarationType() { - return dictionary - } else { - return nil - } - } - - /// var x: Set<>, var x: Array<>, var x: Dictionary<> - func genericCollectionDeclarationType() -> CollectionType? { - guard let simpleTypeIdentifier = self.type.as(SimpleTypeIdentifierSyntax.self), - let genericArgumentClause = simpleTypeIdentifier.genericArgumentClause, - genericArgumentClause.leftAngleBracket.tokenKind == .leftAngle, - genericArgumentClause.rightAngleBracket.tokenKind == .rightAngle, - case .identifier(let name) = simpleTypeIdentifier.name.tokenKind, - let collectionType = CollectionType.names[name], - genericArgumentClause.arguments.count == collectionType.genericArguments else { - return nil - } - return collectionType - } - - /// var x: [Y] - func arrayDeclarationType() -> CollectionType? { - guard let arrayType = self.type.as(ArrayTypeSyntax.self), - case .leftSquareBracket = arrayType.leftSquareBracket.tokenKind, - case .rightSquareBracket = arrayType.rightSquareBracket.tokenKind, - arrayType.elementType.kind == .simpleTypeIdentifier else { - return nil - } - return .array - } - - /// var x: [K: V] - func dictionaryDeclarationType() -> CollectionType? { - guard let dictionaryType = self.type.as(DictionaryTypeSyntax.self), - case .leftSquareBracket = dictionaryType.leftSquareBracket.tokenKind, - case .rightSquareBracket = dictionaryType.rightSquareBracket.tokenKind, - case .colon = dictionaryType.colon.tokenKind else { - return nil - } - return .dictionary - } -} - -private extension InitializerClauseSyntax { - /// --- - /// If `nil` we don't know the type and investigate the following, which is a`FunctionCallExpr`: - /// var x = Set(...) - /// var y = Array(...) - /// var z = Dictionary(...) - /// Otherwise checks for `FunctionCallExpr`,`MemberAccessExpr`,`DictionaryExpr` and `ArrayExpr`: - /// 1. `= Set(...)` | `Set(...)` | `.init(...)` | `[]` - /// 2. `= Array(...)` | `Array(...)` | `.init(...)` | `[]` - /// 3. `= Dictionary()` | `Dictionary()` | `.init(..)` | `[:]` - func isTypeInitializer(for collectionType: CollectionType? = nil) -> Bool { - func isSupportedType(with name: String) -> Bool { - if let collectionType { - return collectionType.name == name - } else { - return CollectionType.names[name] != nil - } - } - guard self.equal.tokenKind == .equal else { return false } - if let functionCallExpr = self.value.as(FunctionCallExprSyntax.self) { - // either construction using explicit specialisation, or general construction - if let specializeExpr = functionCallExpr.calledExpression.as(SpecializeExprSyntax.self), - let identifierExpr = specializeExpr.expression.as(IdentifierExprSyntax.self), - case .identifier(let typename) = identifierExpr.identifier.tokenKind { - return isSupportedType(with: typename) - } else if let identifierExpr = functionCallExpr.calledExpression.as(IdentifierExprSyntax.self), - case .identifier(let typename) = identifierExpr.identifier.tokenKind { - return isSupportedType(with: typename) - } else if let memberAccessExpr = functionCallExpr.calledExpression.as(MemberAccessExprSyntax.self), - memberAccessExpr.name.tokenKind == .keyword(.`init`) { - return true - } - } else if collectionType == .dictionary, - self.value.as(DictionaryExprSyntax.self) != nil { - return true - } else if collectionType == .array || collectionType == .set, - self.value.as(ArrayExprSyntax.self) != nil { - return true - } - return false - } -} - -private extension ForInStmtSyntax { - func referencedVariables(for variables: [VariableDeclSyntax]?) -> Set? { - guard let variables, !variables.isEmpty, - let codeBlock = self.body.as(CodeBlockSyntax.self), - let codeBlockItemList = codeBlock.statements.as(CodeBlockItemListSyntax.self) else { - return nil - } - let references: Set = codeBlockItemList.reduce(into: .init(), { partialResult, codeBlock in - // no need to cover one liner: someMutation(); someOtherMutation() - variables.forEach { variableDecl in - if let referenced = codeBlock.referencedVariable(for: variableDecl) { - partialResult.insert(referenced) - } - } - }) - return references.isEmpty ? nil : references - } -} - -private extension CodeBlockItemSyntax { - func referencedVariable(for variableDecl: VariableDeclSyntax?) -> ReferencedVariable? { - guard let identifier = variableDecl?.identifier else { - return nil - } - return self.referencedVariable(for: identifier) - } - - func referencedVariable(for varName: String) -> ReferencedVariable? { - if let functionCallExpr = self.item.as(FunctionCallExprSyntax.self) { - return functionCallExpr.referencedVariable(for: varName) - } - if let sequenceExpr = self.item.as(SequenceExprSyntax.self) { - return sequenceExpr.referencedVariable(for: varName) - } - return nil - } -} - -private extension FunctionCallExprSyntax { - /// varName.method(x, y, n) - func referencedVariable(for varName: String) -> ReferencedVariable? { - guard self.leftParen?.tokenKind == .leftParen, - self.rightParen?.tokenKind == .rightParen, - let memberAccessExpr = self.calledExpression.as(MemberAccessExprSyntax.self), - memberAccessExpr.dot.tokenKind == .period, - let arguments = self.argumentList.as(TupleExprElementListSyntax.self)?.count, - let identifierExpr = memberAccessExpr.base?.as(IdentifierExprSyntax.self), - identifierExpr.identifier.tokenKind == .identifier(varName), - case .identifier(let name) = memberAccessExpr.name.tokenKind else { - return nil - } - return .init( - name: varName, - position: memberAccessExpr.positionAfterSkippingLeadingTrivia, - kind: .method(name: name, arguments: arguments) - ) - } -} - -private extension SequenceExprSyntax { - /// varName[xxx] = ... - func referencedVariable(for varName: String) -> ReferencedVariable? { - guard let exprList = self.as(ExprListSyntax.self), exprList.count >= 2 else { - return nil - } - let first = exprList.startIndex - let second = exprList.index(after: first) - guard let subscrExpr = exprList[first].as(SubscriptExprSyntax.self), - let assignmentExpr = exprList[second].as(AssignmentExprSyntax.self), - assignmentExpr.assignToken.text == "=", - subscrExpr.leftBracket.tokenKind == .leftSquareBracket, - subscrExpr.rightBracket.tokenKind == .rightSquareBracket, - subscrExpr.argumentList.is(TupleExprElementListSyntax.self), - let identifierExpr = subscrExpr.calledExpression.as(IdentifierExprSyntax.self), - identifierExpr.identifier.tokenKind == .identifier(varName) else { - return nil - } - return .init( - name: varName, - position: exprList.positionAfterSkippingLeadingTrivia, - kind: .assignment(subscript: true) - ) - } -} diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopExamples.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopExamples.swift deleted file mode 100644 index 4498998c22..0000000000 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopExamples.swift +++ /dev/null @@ -1,78 +0,0 @@ -internal struct ReduceIntoInsteadOfLoopExamples { - static let nonTriggeringExamples: [Example] = [ -// Example(""" -// class Foo { -// static let constant: Int = 1 -// var variable: Int = 2 -// } -// """), -// Example(""" -// struct Foo { -// static let constant: Int = 1 -// } -// """), -// Example(""" -// enum InstFooance { -// static let constant = 1 -// } -// """), -// Example(""" -// struct Foo { -// let property1 -// let property2 -// init(property1: Int, property2: String) { -// self.property1 = property1 -// self.property2 = property2 -// } -// } -// """) - Example(""" - let encountered: Set = someArray.reduce(into: Set(), { result, eachN in - result.insert(eachN) - }) - """) - ] - - static let triggeringExamples: [Example] = [ -// Example(""" -// class Foo { -// static let one = 32 -// ↓let constant: Int = 1 -// } -// """), -// Example(""" -// struct Foo { -// ↓let constant: Int = 1 -// } -// """), -// Example(""" -// enum Foo { -// ↓let constant: Int = 1 -// } -// """), - Example(""" - var encountered: Set = [] - for eachN in someArray { - ↓encountered.insert(eachN) - } - """) - ] -} - -// var encountered1: Set = [] -// var encountered1a = Set() -// var encountered1b: Set = Set() -// var encountered1c: Set = .init() -// var encountered2: [String] = [] -// var encountered2a = [String]() -// var encountered2b: [String] = [1, 2, 3, 4] -// var encountered2c: [String] = Array(contentsOf: other) -// var encountered3: Array = [] -// var encountered4: Dictionary = [] -// var encountered4b: [String: Int] = [:] -// var encountered4c: [String: Int] = ["2": 2, "3": 3] -// for eachN in someArray { -// encountered.insert(eachN) -// encountered1[2] = 45 -// let newSet = encountered.popFirst() -// } diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopHelpers.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopHelpers.swift deleted file mode 100644 index 39686dbfaa..0000000000 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopHelpers.swift +++ /dev/null @@ -1,67 +0,0 @@ -import SwiftSyntax - -struct ReduceIntoInsteadOfLoopHelpers { } - -extension PatternBindingListSyntax { - func optionalIndex(after: PatternBindingListSyntax.Index?) -> PatternBindingListSyntax.Index? { - guard let after else { - return nil - } - return self.index(after: after) - } -} - -extension VariableDeclSyntax { - var isVar: Bool { - return self.bindingKeyword.tokenKind == .keyword(.var) - } - - var identifier: String? { - guard let identifierPattern = self.firstOf(IdentifierPatternSyntax.self), - case .identifier(let name) = identifierPattern.identifier.tokenKind else { - return nil - } - return name - } - - func next(after index: PatternBindingListSyntax.Index?) -> PatternBindingSyntax? { - guard let index = self.bindings.optionalIndex(after: index), - index >= self.bindings.startIndex && index < self.bindings.endIndex else { - return nil - } - return self.bindings[index] - } - - func next(after index: PatternBindingListSyntax.Index?, of type: T.Type) -> T? { - guard let index = self.bindings.optionalIndex(after: index), - index >= self.bindings.startIndex && index < self.bindings.endIndex else { - return nil - } - return self.bindings[index].as(type) - } - - func next(after: PatternBindingSyntax?) -> PatternBindingSyntax? { - guard let after, let index = self.bindings.firstIndex(where: { patterBindingSyntax in - return patterBindingSyntax == after - }) else { - return nil - } - let newIndex = self.bindings.index(after: index) - guard newIndex >= self.bindings.startIndex && newIndex < self.bindings.endIndex else { - return nil - } - return self.bindings[newIndex] - } - - func firstOf(_ type: T.Type) -> T? { - return self.bindings.first { patternBinding in - return patternBinding.as(type) != nil - } as? T - } - - func firstIndexOf(_ type: T.Type) -> PatternBindingListSyntax.Index? { - return self.bindings.firstIndex(where: { patternBinding in - return patternBinding.as(type) != nil - }) - } -} diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopModels.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopModels.swift deleted file mode 100644 index 9581d076f7..0000000000 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopModels.swift +++ /dev/null @@ -1,55 +0,0 @@ -import SwiftSyntax - -typealias ReduceIntoInsteadOfLoopModels = ReduceIntoInsteadOfLoop - -internal extension ReduceIntoInsteadOfLoop { - struct ReferencedVariable: Hashable { - let name: String - let position: AbsolutePosition - let kind: Kind - - func hash(into hasher: inout Hasher) { - hasher.combine(self.name) - hasher.combine(self.position.utf8Offset) - hasher.combine(self.kind) - } - } - - enum Kind: Hashable { - case method(name: String, arguments: Int) - case assignment(subscript: Bool) - - func hash(into hasher: inout Hasher) { - switch self { - case let .method(name, arguments): - hasher.combine("method") - hasher.combine(name) - hasher.combine(arguments) - case let .assignment(`subscript`): - hasher.combine("assignment") - hasher.combine(`subscript`) - } - } - } - - struct CollectionType: Hashable { - let name: String - let genericArguments: Int - - static let types: [CollectionType] = [ - .set, - .array, - .dictionary - ] - - static let array = CollectionType(name: "Array", genericArguments: 1) - static let set = CollectionType(name: "Set", genericArguments: 1) - static let dictionary = CollectionType(name: "Dictionary", genericArguments: 2) - - static let names: [String: CollectionType] = { - return CollectionType.types.reduce(into: [String: CollectionType]()) { partialResult, collectionType in - partialResult[collectionType.name] = collectionType - } - }() - } -} From 21446f1c92b790b52b007b625152488b599139c3 Mon Sep 17 00:00:00 2001 From: Alfons Hoogervorst Date: Tue, 13 May 2025 06:02:10 +0200 Subject: [PATCH 28/28] Clean up merge warnings --- .../Rules/Idiomatic/ReduceIntoInsteadOfLoopRule.swift | 4 ++-- .../Idiomatic/ReduceIntoInsteadOfLoopRuleExamples.swift | 6 +++--- .../Rules/Idiomatic/ReduceIntoInsteadOfLoopRuleModels.swift | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRule.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRule.swift index b5f6431bf9..762d9ee3a8 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRule.swift @@ -62,7 +62,7 @@ internal extension ReduceIntoInsteadOfLoopRule { ] static let collectionNames: [String: CollectionType] = - ReduceIntoInsteadOfLoopRule.collectionTypes.reduce(into: [String: CollectionType]()) { partialResult, type in + ReduceIntoInsteadOfLoopRule.collectionTypes.reduce(into: [:]) { partialResult, type in partialResult[type.name] = type } } @@ -73,7 +73,7 @@ private extension CodeBlockItemListSyntax { typealias IndexRange = Range typealias IndexRangeForStmts = (range: IndexRange, forStmt: ForStmtSyntax) // Collect all ForInStmts and track their index ranges - let indexRangeForStatements: [IndexRangeForStmts] = self.reduce(into: [IndexRangeForStmts]()) { partialResult, codeBlockItem in + let indexRangeForStatements: [IndexRangeForStmts] = self.reduce(into: []) { partialResult, codeBlockItem in guard let codeBlockItemIndex = self.index(of: codeBlockItem) else { return } diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRuleExamples.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRuleExamples.swift index ddfa1f31fd..d5641bdbb8 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRuleExamples.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRuleExamples.swift @@ -1,17 +1,17 @@ struct ReduceIntoInsteadOfLoopRuleExamples { static let nonTriggeringExamples: [Example] = [ Example(""" - let encountered: Array = someSequence.reduce(into: Array(), { result, eachN in + let result: [SomeType] = someSequence.reduce(into: [], { result, eachN in result.insert(eachN) }) """), Example(""" - let encountered: Set = someSequence.reduce(into: Set(), { result, eachN in + let result: Set = someSequence.reduce(into: []], { result, eachN in result.insert(eachN) }) """), Example(""" - let encountered: Dictionary = someSequence.reduce(into: Dictionary(), { result, eachN in + let result: [SomeType1: SomeType2] = someSequence.reduce(into: [:], { result, eachN in result[SomeType1Value] = SomeType2Value }) """), diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRuleModels.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRuleModels.swift index 99f2c70ab7..1a3523b8c3 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRuleModels.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/ReduceIntoInsteadOfLoopRuleModels.swift @@ -1,6 +1,6 @@ import SwiftSyntax -typealias ReduceIntoInsteadOfLoopModels = ReduceIntoInsteadOfLoopRule +struct ReduceIntoInsteadOfLoopRuleModels {} internal extension ReduceIntoInsteadOfLoopRule { struct ReferencedVariable: Hashable {