Skip to content

Commit

Permalink
Add void_function_in_ternary opt-in rule
Browse files Browse the repository at this point in the history
Fixes #2358
  • Loading branch information
marcelofabri committed Feb 8, 2020
1 parent 8d9c501 commit f8ac10e
Show file tree
Hide file tree
Showing 6 changed files with 132 additions and 0 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@
`return <expression>` in a function that is `Void`.
[Marcelo Fabri](https://github.com/marcelofabri)

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

#### Bug Fixes

* Fix `discarded_notification_center_observer` false positives when
Expand Down
1 change: 1 addition & 0 deletions Source/SwiftLintFramework/Models/MasterRuleList.swift
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ public let masterRuleList = 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,110 @@
import Foundation
import SourceKittenFramework
#if canImport(SwiftSyntax)
import SwiftSyntax
#endif

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

public init() {}

public static let description = RuleDescription(
identifier: "void_function_in_ternary",
name: "VoidFunctionInTernaryConditionRule",
description: "Returning values 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)
}
""")
],
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)
}
""")
]
)

public func validate(file: SwiftLintFile) -> [StyleViolation] {
#if canImport(SwiftSyntax)
return validate(file: file, visitor: TernaryVisitor())
#else
return []
#endif
}
}

#if canImport(SwiftSyntax)
private class TernaryVisitor: SyntaxRuleVisitor {
private var positions = [AbsolutePosition]()

func visit(_ node: TernaryExprSyntax) -> SyntaxVisitorContinueKind {
if node.firstChoice is FunctionCallExprSyntax, node.secondChoice is FunctionCallExprSyntax,
let parent = node.parent as? ExprListSyntax, !parent.containsAssignment,
parent.parent is SequenceExprSyntax,
let blockItem = parent.parent?.parent as? CodeBlockItemSyntax, !blockItem.isClosureImplictReturn {
positions.append(node.questionMark.positionAfterSkippingLeadingTrivia)
}

return .visitChildren
}

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.utf8Offset)))
}
}
}

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

private extension CodeBlockItemSyntax {
var isClosureImplictReturn: Bool {
guard let parent = parent as? CodeBlockItemListSyntax else {
return false
}

return Array(parent.children).count == 1 && parent.parent is ClosureExprSyntax
}
}
#endif
4 changes: 4 additions & 0 deletions SwiftLint.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,7 @@
D462021F1E15F52D0027AAD1 /* NumberSeparatorRuleExamples.swift in Sources */ = {isa = PBXBuildFile; fileRef = D462021E1E15F52D0027AAD1 /* NumberSeparatorRuleExamples.swift */; };
D46252541DF63FB200BE2CA1 /* NumberSeparatorRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D46252531DF63FB200BE2CA1 /* NumberSeparatorRule.swift */; };
D466B620233D229F0068190B /* FlatMapOverMapReduceRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D466B61F233D229F0068190B /* FlatMapOverMapReduceRule.swift */; };
D467275823DD971300DE73B6 /* VoidFunctionInTernaryConditionRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D467275723DD971300DE73B6 /* VoidFunctionInTernaryConditionRule.swift */; };
D46A317F1F1CEDCD00AF914A /* UnneededParenthesesInClosureArgumentRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D46A317E1F1CEDCD00AF914A /* UnneededParenthesesInClosureArgumentRule.swift */; };
D46C7C3E23BF2F6A007C517F /* PreferSelfTypeOverTypeOfSelfRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D46C7C3D23BF2F6A007C517F /* PreferSelfTypeOverTypeOfSelfRule.swift */; };
D46E041D1DE3712C00728374 /* TrailingCommaRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D46E041C1DE3712C00728374 /* TrailingCommaRule.swift */; };
Expand Down Expand Up @@ -836,6 +837,7 @@
D462021E1E15F52D0027AAD1 /* NumberSeparatorRuleExamples.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = NumberSeparatorRuleExamples.swift; sourceTree = "<group>"; };
D46252531DF63FB200BE2CA1 /* NumberSeparatorRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = NumberSeparatorRule.swift; sourceTree = "<group>"; };
D466B61F233D229F0068190B /* FlatMapOverMapReduceRule.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FlatMapOverMapReduceRule.swift; sourceTree = "<group>"; };
D467275723DD971300DE73B6 /* VoidFunctionInTernaryConditionRule.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = VoidFunctionInTernaryConditionRule.swift; sourceTree = "<group>"; };
D46A317E1F1CEDCD00AF914A /* UnneededParenthesesInClosureArgumentRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = UnneededParenthesesInClosureArgumentRule.swift; sourceTree = "<group>"; };
D46C7C3D23BF2F6A007C517F /* PreferSelfTypeOverTypeOfSelfRule.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PreferSelfTypeOverTypeOfSelfRule.swift; sourceTree = "<group>"; };
D46E041C1DE3712C00728374 /* TrailingCommaRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = TrailingCommaRule.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1381,6 +1383,7 @@
D4EA77C71F817FD200C315FB /* UnneededBreakInSwitchRule.swift */,
181D9E162038343D001F6887 /* UntypedErrorInCatchRule.swift */,
D43B04631E0620AB004016AF /* UnusedEnumeratedRule.swift */,
D467275723DD971300DE73B6 /* VoidFunctionInTernaryConditionRule.swift */,
41715DE023BEA08E00544BDF /* FileNameNoSpaceRule.swift */,
626D02961F31CBCC0054788D /* XCTFailMessageRule.swift */,
622AD7FF216ACE6200A002C6 /* XCTSpecificMatcherRule.swift */,
Expand Down Expand Up @@ -2336,6 +2339,7 @@
6C15818D237026AC00F582A2 /* GitHubActionsLoggingReporter.swift in Sources */,
62FE5D32200CABDD00F68793 /* DiscouragedOptionalCollectionExamples.swift in Sources */,
D41C09BD23C1B99D00F105C4 /* OrphanedDocCommentRule.swift in Sources */,
D467275823DD971300DE73B6 /* VoidFunctionInTernaryConditionRule.swift in Sources */,
D49896F12026B36C00814A83 /* RedundantSetAccessControlRule.swift in Sources */,
E4A6CF752363CBFB00DD5B18 /* RandomAccessCollection+Swiftlint.swift in Sources */,
29FFC37A1F15764D007E4825 /* FileLengthRuleConfiguration.swift in Sources */,
Expand Down
6 changes: 6 additions & 0 deletions Tests/LinuxMain.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1597,6 +1597,12 @@ extension VerticalWhitespaceRuleTests {
]
}

extension VoidFunctionInTernaryConditionRuleTests {
static var allTests: [(String, (VoidFunctionInTernaryConditionRuleTests) -> () throws -> Void)] = [
("testWithDefaultConfiguration", testWithDefaultConfiguration)
]
}

extension VoidReturnRuleTests {
static var allTests: [(String, (VoidReturnRuleTests) -> () throws -> Void)] = [
("testWithDefaultConfiguration", testWithDefaultConfiguration)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -834,6 +834,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 f8ac10e

Please sign in to comment.