diff --git a/CHANGELOG.md b/CHANGELOG.md index 612e577b77..f79e0075e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,11 @@ ### Bug Fixes +* Avoid false positives from `unused_enumerated` when higher-order calls on + `.enumerated()` use result members like `?.offset` after the closure. + [theamodhshetty](https://github.com/theamodhshetty) + [#5881](https://github.com/realm/SwiftLint/issues/5881) + * Add an `ignore_attributes` option to `implicit_optional_initialization` so wrappers/attributes that require explicit `= nil` can be excluded from style checks for both `style: always` and `style: never`. diff --git a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/UnusedEnumeratedRule.swift b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/UnusedEnumeratedRule.swift index 60e6b62956..83caa6b3e1 100644 --- a/Source/SwiftLintBuiltInRules/Rules/Idiomatic/UnusedEnumeratedRule.swift +++ b/Source/SwiftLintBuiltInRules/Rules/Idiomatic/UnusedEnumeratedRule.swift @@ -25,6 +25,18 @@ struct UnusedEnumeratedRule: Rule { Example("list.enumerated().map { ($0.offset, $0.element) }"), Example("list.enumerated().map { ($0.0, $0.1) }"), Example(""" + list.enumerated().first { + $0.element.0.isNumber && + $0.element.1.isNumber && + $0.element.0 != $0.element.1 + }?.offset + """), + Example(""" + list.enumerated().max { + $0.element < $1.element + }?.offset + """), + Example(""" list.enumerated().map { $1.enumerated().forEach { print($0, $1) } return $0 @@ -91,19 +103,21 @@ struct UnusedEnumeratedRule: Rule { private extension UnusedEnumeratedRule { private struct Closure { - let enumeratedPosition: AbsolutePosition? + let enumeratedPosition: AbsolutePosition + let usedEnumeratedResultMembers: (zero: Bool, one: Bool) var zeroPosition: AbsolutePosition? var onePosition: AbsolutePosition? + } - init(enumeratedPosition: AbsolutePosition? = nil) { - self.enumeratedPosition = enumeratedPosition - } + private struct PendingClosure { + let id: SyntaxIdentifier + let enumeratedPosition: AbsolutePosition + let usedEnumeratedResultMembers: (zero: Bool, one: Bool) } final class Visitor: ViolationsSyntaxVisitor { - private var nextClosureId: SyntaxIdentifier? - private var lastEnumeratedPosition: AbsolutePosition? - private var closures = Stack() + private var pendingClosure: PendingClosure? + private var closures = Stack() override func visitPost(_ node: ForStmtSyntax) { guard let tuplePattern = node.pattern.as(TuplePatternSyntax.self), @@ -132,7 +146,8 @@ private extension UnusedEnumeratedRule { guard node.isEnumerated, let parent = node.parent, parent.as(MemberAccessExprSyntax.self)?.declName.baseName.text != "filter", - let trailingClosure = parent.parent?.as(FunctionCallExprSyntax.self)?.trailingClosure + let parentCall = parent.parent?.as(FunctionCallExprSyntax.self), + let trailingClosure = parentCall.trailingClosure else { return .visitChildren } @@ -156,44 +171,55 @@ private extension UnusedEnumeratedRule { zeroPosition: firstTokenIsUnderscore ? firstElement.positionAfterSkippingLeadingTrivia : nil, onePosition: firstTokenIsUnderscore ? nil : secondElement.positionAfterSkippingLeadingTrivia ) - } else { - nextClosureId = trailingClosure.id - lastEnumeratedPosition = node.enumeratedPosition + } else if let enumeratedPosition = node.enumeratedPosition { + pendingClosure = PendingClosure( + id: trailingClosure.id, + enumeratedPosition: enumeratedPosition, + usedEnumeratedResultMembers: parentCall.usedEnumeratedResultMembers + ) } 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 + if let pendingClosure, pendingClosure.id == node.id { + closures.push(Closure( + enumeratedPosition: pendingClosure.enumeratedPosition, + usedEnumeratedResultMembers: pendingClosure.usedEnumeratedResultMembers + )) + self.pendingClosure = nil } else { - closures.push(Closure()) + closures.push(nil) } return .visitChildren } override func visitPost(_: ClosureExprSyntax) { - if let closure = closures.pop(), (closure.zeroPosition != nil) != (closure.onePosition != nil) { - addViolation( - zeroPosition: closure.onePosition, - onePosition: closure.zeroPosition, - enumeratedPosition: closure.enumeratedPosition - ) - } + guard let closure = popTrackedClosure() else { return } + + let zeroPosition = closure.zeroPosition + ?? (closure.usedEnumeratedResultMembers.zero ? closure.enumeratedPosition : nil) + let onePosition = closure.onePosition + ?? (closure.usedEnumeratedResultMembers.one ? closure.enumeratedPosition : nil) + guard (zeroPosition != nil) != (onePosition != nil) else { return } + + addViolation( + zeroPosition: onePosition, + onePosition: zeroPosition, + enumeratedPosition: closure.enumeratedPosition + ) } override func visitPost(_ node: DeclReferenceExprSyntax) { guard - let closure = closures.peek(), - closure.enumeratedPosition != nil, + currentTrackedClosure != nil, node.baseName.text == "$0" || node.baseName.text == "$1" else { return } - closures.modifyLast { + + modifyTrackedClosure { if node.baseName.text == "$0" { let member = node.parent?.as(MemberAccessExprSyntax.self)?.declName.baseName.text if member == "element" || member == "1" { @@ -210,6 +236,22 @@ private extension UnusedEnumeratedRule { } } + private var currentTrackedClosure: Closure? { + closures.peek().flatMap(\.self) + } + + private func popTrackedClosure() -> Closure? { + closures.pop().flatMap(\.self) + } + + private func modifyTrackedClosure(_ modifier: (inout Closure) -> Void) { + closures.modifyLast { + guard var closure = $0 else { return } + modifier(&closure) + $0 = closure + } + } + private func addViolation( zeroPosition: AbsolutePosition?, onePosition: AbsolutePosition?, @@ -257,6 +299,35 @@ private extension FunctionCallExprSyntax { && additionalTrailingClosures.isEmpty && arguments.isEmpty } + + var usedEnumeratedResultMembers: (zero: Bool, one: Bool) { + var currentNode: Syntax? = Syntax(self) + + while let node = currentNode, let parent = node.parent { + if let memberAccess = parent.as(MemberAccessExprSyntax.self), + memberAccess.base?.id == node.id { + switch memberAccess.declName.baseName.text { + case "offset", "0": + return (true, false) + case "element", "1": + return (false, true) + default: + break + } + } + + guard parent.is(OptionalChainingExprSyntax.self) + || parent.is(ForceUnwrapExprSyntax.self) + || parent.is(MemberAccessExprSyntax.self) + else { + return (false, false) + } + + currentNode = parent + } + + return (false, false) + } } private extension TuplePatternElementSyntax {