Skip to content

Commit

Permalink
Add back void_function_in_ternary rule (#3956)
Browse files Browse the repository at this point in the history
  • Loading branch information
marcelofabri authored Jun 27, 2022
1 parent a19ffdd commit 45a03de
Show file tree
Hide file tree
Showing 4 changed files with 239 additions and 0 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@
a custom rule.
[Marcelo Fabri](https://github.com/marcelofabri)

* Add back `void_function_in_ternary` opt-in rule to warn against using
a ternary operator to call `Void` functions.
[Marcelo Fabri](https://github.com/marcelofabri)

#### Bug Fixes

* Ignore array types in `syntactic_sugar` rule if their associated `Index` is
Expand Down
1 change: 1 addition & 0 deletions Source/SwiftLintFramework/Models/PrimaryRuleList.swift
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,7 @@ public let primaryRuleList = RuleList(rules: [
VerticalWhitespaceClosingBracesRule.self,
VerticalWhitespaceOpeningBracesRule.self,
VerticalWhitespaceRule.self,
VoidFunctionInTernaryConditionRule.self,
VoidReturnRule.self,
WeakDelegateRule.self,
XCTFailMessageRule.self,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,228 @@
import SourceKittenFramework
import SwiftSyntax

public struct VoidFunctionInTernaryConditionRule: ConfigurationProviderRule, AutomaticTestableRule {
public var configuration = SeverityConfiguration(.warning)

public init() {}

public static let description = RuleDescription(
identifier: "void_function_in_ternary",
name: "Void Function in Ternary",
description: "Using ternary to call Void functions should be avoided.",
kind: .idiomatic,
minSwiftVersion: .fiveDotOne,
nonTriggeringExamples: [
Example("let result = success ? foo() : bar()"),
Example("""
if success {
askQuestion()
} else {
exit()
}
"""),
Example("""
var price: Double {
return hasDiscount ? calculatePriceWithDiscount() : calculateRegularPrice()
}
"""),
Example("foo(x == 2 ? a() : b())"),
Example("""
chevronView.image = collapsed ? .icon(.mediumChevronDown) : .icon(.mediumChevronUp)
"""),
Example("""
array.map { elem in
elem.isEmpty() ? .emptyValue() : .number(elem)
}
"""),
Example("""
func compute(data: [Int]) -> Int {
data.isEmpty ? 0 : expensiveComputation(data)
}
"""),
Example("""
var value: Int {
mode == .fast ? fastComputation() : expensiveComputation()
}
"""),
Example("""
var value: Int {
get {
mode == .fast ? fastComputation() : expensiveComputation()
}
}
"""),
Example("""
subscript(index: Int) -> Int {
get {
index == 0 ? defaultValue() : compute(index)
}
"""),
Example("""
subscript(index: Int) -> Int {
index == 0 ? defaultValue() : compute(index)
""")
],
triggeringExamples: [
Example("success ↓? askQuestion() : exit()"),
Example("""
perform { elem in
elem.isEmpty() ↓? .emptyValue() : .number(elem)
return 1
}
"""),
Example("""
DispatchQueue.main.async {
self.sectionViewModels[section].collapsed.toggle()
self.sectionViewModels[section].collapsed
↓? self.tableView.deleteRows(at: [IndexPath(row: 0, section: section)], with: .automatic)
: self.tableView.insertRows(at: [IndexPath(row: 0, section: section)], with: .automatic)
self.tableView.scrollToRow(at: IndexPath(row: NSNotFound, section: section), at: .top, animated: true)
}
"""),
Example("""
subscript(index: Int) -> Int {
index == 0 ↓? something() : somethingElse(index)
return index
"""),
Example("""
var value: Int {
mode == .fast ↓? something() : somethingElse()
return 0
}
"""),
Example("""
var value: Int {
get {
mode == .fast ↓? something() : somethingElse()
return 0
}
}
"""),
Example("""
subscript(index: Int) -> Int {
get {
index == 0 ↓? something() : somethingElse(index)
return index
}
""")
]
)

public func validate(file: SwiftLintFile) -> [StyleViolation] {
let visitor = VoidFunctionInTernaryConditionVisitor()
return visitor.walk(file: file) { visitor in
visitor.violations(for: self, in: file)
}
}
}

private class VoidFunctionInTernaryConditionVisitor: SyntaxVisitor {
private var positions = [AbsolutePosition]()

override func visitPost(_ node: TernaryExprSyntax) {
guard node.firstChoice.is(FunctionCallExprSyntax.self),
node.secondChoice.is(FunctionCallExprSyntax.self),
let parent = node.parent?.as(ExprListSyntax.self),
!parent.containsAssignment,
let grandparent = parent.parent,
grandparent.is(SequenceExprSyntax.self),
let blockItem = grandparent.parent?.as(CodeBlockItemSyntax.self),
!blockItem.isImplicitReturn else {
return
}

positions.append(node.questionMark.positionAfterSkippingLeadingTrivia)
}

func violations(for rule: VoidFunctionInTernaryConditionRule, in file: SwiftLintFile) -> [StyleViolation] {
return positions.map { position in
StyleViolation(ruleDescription: type(of: rule).description,
severity: rule.configuration.severity,
location: Location(file: file, byteOffset: ByteCount(position)))
}
}
}

private extension ExprListSyntax {
var containsAssignment: Bool {
return children.contains(where: { $0.is(AssignmentExprSyntax.self) })
}
}

private extension CodeBlockItemSyntax {
var isImplicitReturn: Bool {
isClosureImplictReturn || isFunctionImplicitReturn ||
isVariableImplicitReturn || isSubscriptImplicitReturn ||
isAcessorImplicitReturn
}

var isClosureImplictReturn: Bool {
guard let parent = parent?.as(CodeBlockItemListSyntax.self),
let grandparent = parent.parent else {
return false
}

return parent.children.count == 1 && grandparent.is(ClosureExprSyntax.self)
}

var isFunctionImplicitReturn: Bool {
guard let parent = parent?.as(CodeBlockItemListSyntax.self),
let functionDecl = parent.parent?.parent?.as(FunctionDeclSyntax.self) else {
return false
}

return parent.children.count == 1 && functionDecl.signature.allowsImplicitReturns
}

var isVariableImplicitReturn: Bool {
guard let parent = parent?.as(CodeBlockItemListSyntax.self) else {
return false
}

let isVariableDecl = parent.parent?.parent?.as(PatternBindingSyntax.self) != nil
return parent.children.count == 1 && isVariableDecl
}

var isSubscriptImplicitReturn: Bool {
guard let parent = parent?.as(CodeBlockItemListSyntax.self),
let subscriptDecl = parent.parent?.parent?.as(SubscriptDeclSyntax.self) else {
return false
}

return parent.children.count == 1 && subscriptDecl.allowsImplicitReturns
}

var isAcessorImplicitReturn: Bool {
guard let parent = parent?.as(CodeBlockItemListSyntax.self),
parent.parent?.parent?.as(AccessorDeclSyntax.self) != nil else {
return false
}

return parent.children.count == 1
}
}

private extension FunctionSignatureSyntax {
var allowsImplicitReturns: Bool {
output?.allowsImplicitReturns ?? false
}
}

private extension SubscriptDeclSyntax {
var allowsImplicitReturns: Bool {
result.allowsImplicitReturns
}
}

private extension ReturnClauseSyntax {
var allowsImplicitReturns: Bool {
if let simpleType = returnType.as(SimpleTypeIdentifierSyntax.self) {
return simpleType.name.text != "Void" && simpleType.name.text != "Never"
} else if let tupleType = returnType.as(TupleTypeSyntax.self) {
return !tupleType.elements.isEmpty
} else {
return true
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -923,6 +923,12 @@ class VerticalWhitespaceOpeningBracesRuleTests: XCTestCase {
}
}

class VoidFunctionInTernaryConditionRuleTests: XCTestCase {
func testWithDefaultConfiguration() {
verifyRule(VoidFunctionInTernaryConditionRule.description)
}
}

class VoidReturnRuleTests: XCTestCase {
func testWithDefaultConfiguration() {
verifyRule(VoidReturnRule.description)
Expand Down

0 comments on commit 45a03de

Please sign in to comment.