diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d7c05e1b..de115b722 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -164,6 +164,12 @@ [phlippieb](https://github.com/phlippieb) [#5471](https://github.com/realm/SwiftLint/issues/5471) +* Extends `unused_enumerated` rule to cover closure parameters, to + detect cases like `list.enumerated().map { idx, _ in idx }` and + `list.enumerated().map { $1 }`. + [Martin Redington](https://github.com/mildm8nnered) + [#5470](https://github.com/realm/SwiftLint/issues/5470) + #### Bug Fixes * Silence `discarded_notification_center_observer` rule in closures. Furthermore, diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/UnusedEnumeratedRule.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/UnusedEnumeratedRule.swift index f3a2de8d9..55d94c785 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/UnusedEnumeratedRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/UnusedEnumeratedRule.swift @@ -18,57 +18,223 @@ struct UnusedEnumeratedRule: Rule { Example("for (idx, _) in bar.enumerated().something() { }"), Example("for (idx, _) in bar.something() { }"), Example("for idx in bar.indices { }"), - Example("for (section, (event, _)) in data.enumerated() {}") + Example("for (section, (event, _)) in data.enumerated() {}"), + Example("list.enumerated().map { idx, elem in \"\\(idx): \\(elem)\" }"), + Example("list.enumerated().map { $0 + $1 }"), + Example("list.enumerated().something().map { _, elem in elem }"), + Example("list.enumerated().map { ($0.offset, $0.element) }"), + Example("list.enumerated().map { ($0.0, $0.1) }"), + Example(""" + list.enumerated().map { + $1.enumerated().forEach { print($0, $1) } + return $0 + } + """) ], triggeringExamples: [ Example("for (↓_, foo) in bar.enumerated() { }"), Example("for (↓_, foo) in abc.bar.enumerated() { }"), Example("for (↓_, foo) in abc.something().enumerated() { }"), - Example("for (idx, ↓_) in bar.enumerated() { }") + Example("for (idx, ↓_) in bar.enumerated() { }"), + Example("list.enumerated().map { idx, ↓_ in idx }"), + Example("list.enumerated().map { ↓_, elem in elem }"), + Example("list.↓enumerated().forEach { print($0) }"), + Example("list.↓enumerated().map { $1 }"), + Example(""" + list.enumerated().map { + $1.↓enumerated().forEach { print($1) } + return $0 + } + """), + Example(""" + list.↓enumerated().map { + $1.enumerated().forEach { print($0, $1) } + return 1 + } + """), + Example(""" + list.enumerated().map { + $1.enumerated().filter { + print($0, $1) + $1.↓enumerated().forEach { + if $1 == 2 { + return true + } + } + return false + } + return $0 + } + """, excludeFromDocumentation: true) + , + Example(""" + list.↓enumerated().map { + $1.forEach { print($0) } + return $1 + } + """, excludeFromDocumentation: true) ] ) } private extension UnusedEnumeratedRule { + private struct Closure { + let enumeratedPosition: AbsolutePosition? + var zeroPosition: AbsolutePosition? + var onePosition: AbsolutePosition? + + init(enumeratedPosition: AbsolutePosition? = nil) { + self.enumeratedPosition = enumeratedPosition + } + } + final class Visitor: ViolationsSyntaxVisitor { + private var nextClosureId: SyntaxIdentifier? + private var lastEnumeratedPosition: AbsolutePosition? + private var closures = Stack() + override func visitPost(_ node: ForStmtSyntax) { guard let tuplePattern = node.pattern.as(TuplePatternSyntax.self), tuplePattern.elements.count == 2, let functionCall = node.sequence.asFunctionCall, functionCall.isEnumerated, let firstElement = tuplePattern.elements.first, - let secondElement = tuplePattern.elements.last, - case let firstTokenIsUnderscore = firstElement.isUnderscore, - case let lastTokenIsUnderscore = secondElement.isUnderscore, - firstTokenIsUnderscore || lastTokenIsUnderscore else { + let secondElement = tuplePattern.elements.last + else { return } - let position: AbsolutePosition - let reason: String - if firstTokenIsUnderscore { - position = firstElement.positionAfterSkippingLeadingTrivia - reason = "When the index is not used, `.enumerated()` can be removed" + let firstTokenIsUnderscore = firstElement.isUnderscore + let lastTokenIsUnderscore = secondElement.isUnderscore + guard firstTokenIsUnderscore || lastTokenIsUnderscore else { + return + } + + addViolation( + zeroPosition: firstTokenIsUnderscore ? firstElement.positionAfterSkippingLeadingTrivia : nil, + onePosition: firstTokenIsUnderscore ? nil : secondElement.positionAfterSkippingLeadingTrivia + ) + } + + override func visit(_ node: FunctionCallExprSyntax) -> SyntaxVisitorContinueKind { + guard node.isEnumerated, + let parent = node.parent, + parent.as(MemberAccessExprSyntax.self)?.declName.baseName.text != "filter", + let trailingClosure = parent.parent?.as(FunctionCallExprSyntax.self)?.trailingClosure + else { + return .visitChildren + } + + if let parameterClause = trailingClosure.signature?.parameterClause { + guard let parameterClause = parameterClause.as(ClosureShorthandParameterListSyntax.self), + parameterClause.count == 2, + let firstElement = parameterClause.first, + let secondElement = parameterClause.last + else { + return .visitChildren + } + + let firstTokenIsUnderscore = firstElement.isUnderscore + let lastTokenIsUnderscore = secondElement.isUnderscore + guard firstTokenIsUnderscore || lastTokenIsUnderscore else { + return .visitChildren + } + + addViolation( + zeroPosition: firstTokenIsUnderscore ? firstElement.positionAfterSkippingLeadingTrivia : nil, + onePosition: firstTokenIsUnderscore ? nil : secondElement.positionAfterSkippingLeadingTrivia + ) } else { - position = secondElement.positionAfterSkippingLeadingTrivia + nextClosureId = trailingClosure.id + lastEnumeratedPosition = node.enumeratedPosition + } + + return .visitChildren + } + + override func visit(_ node: ClosureExprSyntax) -> SyntaxVisitorContinueKind { + if let nextClosureId, nextClosureId == node.id, let lastEnumeratedPosition { + closures.push(Closure(enumeratedPosition: lastEnumeratedPosition)) + self.nextClosureId = nil + self.lastEnumeratedPosition = nil + } else { + closures.push(Closure()) + } + return .visitChildren + } + + override func visitPost(_ node: ClosureExprSyntax) { + if let closure = closures.pop(), (closure.zeroPosition != nil) != (closure.onePosition != nil) { + addViolation( + zeroPosition: closure.onePosition, + onePosition: closure.zeroPosition, + enumeratedPosition: closure.enumeratedPosition + ) + } + } + + override func visitPost(_ node: DeclReferenceExprSyntax) { + guard + let closure = closures.peek(), + closure.enumeratedPosition != nil, + node.baseName.text == "$0" || node.baseName.text == "$1" + else { + return + } + closures.modifyLast { + if node.baseName.text == "$0" { + let member = node.parent?.as(MemberAccessExprSyntax.self)?.declName.baseName.text + if member == "element" || member == "1" { + $0.onePosition = node.positionAfterSkippingLeadingTrivia + } else { + $0.zeroPosition = node.positionAfterSkippingLeadingTrivia + } + } else { + $0.onePosition = node.positionAfterSkippingLeadingTrivia + } + } + } + + private func addViolation( + zeroPosition: AbsolutePosition?, + onePosition: AbsolutePosition?, + enumeratedPosition: AbsolutePosition? = nil + ) { + var position: AbsolutePosition? + var reason: String? + if let zeroPosition { + position = zeroPosition + reason = "When the index is not used, `.enumerated()` can be removed" + } else if let onePosition { + position = onePosition reason = "When the item is not used, `.indices` should be used instead of `.enumerated()`" } - violations.append(ReasonedRuleViolation(position: position, reason: reason)) + if let enumeratedPosition { + position = enumeratedPosition + } + + if let position, let reason { + violations.append(ReasonedRuleViolation(position: position, reason: reason)) + } } } } private extension FunctionCallExprSyntax { var isEnumerated: Bool { - guard let memberAccess = calledExpression.as(MemberAccessExprSyntax.self), + enumeratedPosition != nil + } + + var enumeratedPosition: AbsolutePosition? { + if let memberAccess = calledExpression.as(MemberAccessExprSyntax.self), memberAccess.base != nil, memberAccess.declName.baseName.text == "enumerated", - hasNoArguments else { - return false + hasNoArguments { + return memberAccess.declName.positionAfterSkippingLeadingTrivia } - return true + return nil } var hasNoArguments: Bool { @@ -83,3 +249,9 @@ private extension TuplePatternElementSyntax { pattern.is(WildcardPatternSyntax.self) } } + +private extension ClosureShorthandParameterSyntax { + var isUnderscore: Bool { + name.tokenKind == .wildcard + } +}