Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@

### Bug Fixes

* Fix `overridden_super_call` false positives when `super` is called once per mutually exclusive branch.
[leno23](https://github.com/leno23)
[#5655](https://github.com/realm/SwiftLint/issues/5655)

* Avoid false positives in `prefer_self_in_static_references` for generic
constraints and generic parameter bounds such as `where A: P` and `<A: P>`
in classes and extensions.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,21 @@ struct OverriddenSuperCallRule: Rule {
}
}
"""),
Example("""
class VC: UIViewController {
override func viewWillAppear(_ animated: Bool) {
if animated {
UIView.transition(with: view, duration: 0.25, options: .curveEaseInOut, animations: {
self.view.transform = .identity
}, completion: { _ in
super.viewWillAppear(true)
})
} else {
super.viewWillAppear(false)
}
}
}
"""),
],
triggeringExamples: [
Example("""
Expand Down Expand Up @@ -95,7 +110,7 @@ private extension OverriddenSuperCallRule {
position: body.leftBrace.endPositionBeforeTrailingTrivia,
reason: "Method '\(name)' should call to super function"
))
} else if superCallsCount > 1 {
} else if superCallsCount > 1, !node.superCallsAreOnlyInMutuallyExclusiveBranches() {
violations.append(ReasonedRuleViolation(
position: body.leftBrace.endPositionBeforeTrailingTrivia,
reason: "Method '\(name)' should call to super only once"
Expand Down
94 changes: 90 additions & 4 deletions Source/SwiftLintCore/Extensions/SwiftSyntax+SwiftLint.swift
Original file line number Diff line number Diff line change
Expand Up @@ -235,12 +235,34 @@ public extension FunctionDeclSyntax {
/// How many times this function calls the `super` implementation in its body.
/// Returns 0 if the function has no body.
func numberOfCallsToSuper() -> Int {
superCallExpressions().count
}

/// All `super.<name>(...)` call expressions in the function body.
func superCallExpressions() -> [FunctionCallExprSyntax] {
guard let body else {
return 0
return []
}

return SuperCallVisitor(expectedFunctionName: name.text)
.walk(tree: body, handler: \.superCallsCount)
.walk(tree: body, handler: \.superCalls)
}

/// True when every `super` call sits in mutually exclusive control-flow branches (e.g. if/else).
func superCallsAreOnlyInMutuallyExclusiveBranches() -> Bool {
let calls = superCallExpressions()
guard calls.count > 1 else {
return true
}

for index in calls.indices {
for otherIndex in calls.index(after: index)..<calls.count {
if !calls[index].isInMutuallyExclusiveBranch(comparedTo: calls[otherIndex]) {
return false
}
}
}
return true
}
}

Expand Down Expand Up @@ -406,7 +428,7 @@ private extension String {

private class SuperCallVisitor: SyntaxVisitor {
private let expectedFunctionName: String
private(set) var superCallsCount = 0
private(set) var superCalls = [FunctionCallExprSyntax]()

init(expectedFunctionName: String) {
self.expectedFunctionName = expectedFunctionName
Expand All @@ -420,6 +442,70 @@ private class SuperCallVisitor: SyntaxVisitor {
return
}

superCallsCount += 1
superCalls.append(node)
}
}

private enum SuperCallControlFlowMarker: Equatable {
case ifThen(SyntaxIdentifier)
case ifElse(SyntaxIdentifier)
case switchCase(SyntaxIdentifier)

func isMutuallyExclusive(with other: SuperCallControlFlowMarker) -> Bool {
switch (self, other) {
case let (.ifThen(id), .ifElse(otherId)), let (.ifElse(id), .ifThen(otherId)):
return id == otherId
case let (.switchCase(id), .switchCase(otherId)):
return id != otherId
default:
return false
}
}
}

private extension FunctionCallExprSyntax {
func isInMutuallyExclusiveBranch(comparedTo other: FunctionCallExprSyntax) -> Bool {
let path = controlFlowPath()
let otherPath = other.controlFlowPath()
let sharedLength = min(path.count, otherPath.count)

for index in 0..<sharedLength where path[index] != otherPath[index] {
return path[index].isMutuallyExclusive(with: otherPath[index])
}
return false
}

func controlFlowPath() -> [SuperCallControlFlowMarker] {
var markers = [SuperCallControlFlowMarker]()
var current = Syntax(self).parent

while let parent = current {
if let ifExpr = parent.as(IfExprSyntax.self) {
let nodeSyntax = Syntax(self)
if ifExpr.body.containsDescendant(nodeSyntax) {
markers.append(.ifThen(ifExpr.id))
} else if let elseBody = ifExpr.elseBody, elseBody.containsDescendant(nodeSyntax) {
markers.append(.ifElse(ifExpr.id))
}
} else if let switchCase = parent.as(SwitchCaseSyntax.self) {
markers.append(.switchCase(switchCase.id))
}
current = parent.parent
}

return markers.reversed()
}
}

private extension SyntaxProtocol {
func containsDescendant(_ descendant: Syntax) -> Bool {
var current: Syntax? = descendant
while let node = current {
if node.id == id {
return true
}
current = node.parent
}
return false
}
}