From 1d39071dfd822c86c096c872eb4e357bcf7df9e2 Mon Sep 17 00:00:00 2001 From: Marcelo Fabri Date: Sat, 8 Feb 2020 13:23:02 -0800 Subject: [PATCH] Add `void_function_in_ternary` opt-in rule (#3085) * Add `void_function_in_ternary` opt-in rule Fixes #2358 * Remove unused import and improve description --- CHANGELOG.md | 5 + .../Models/MasterRuleList.swift | 1 + .../VoidFunctionInTernaryConditionRule.swift | 109 ++++++++++++++++++ SwiftLint.xcodeproj/project.pbxproj | 4 + Tests/LinuxMain.swift | 7 ++ .../AutomaticRuleTests.generated.swift | 6 + 6 files changed, 132 insertions(+) create mode 100644 Source/SwiftLintFramework/Rules/Idiomatic/VoidFunctionInTernaryConditionRule.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index bc5115eb4..3cba71996 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,11 @@ `return ` 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 diff --git a/Source/SwiftLintFramework/Models/MasterRuleList.swift b/Source/SwiftLintFramework/Models/MasterRuleList.swift index 670c10671..3b4c42e6d 100644 --- a/Source/SwiftLintFramework/Models/MasterRuleList.swift +++ b/Source/SwiftLintFramework/Models/MasterRuleList.swift @@ -190,6 +190,7 @@ public let masterRuleList = RuleList(rules: [ VerticalWhitespaceClosingBracesRule.self, VerticalWhitespaceOpeningBracesRule.self, VerticalWhitespaceRule.self, + VoidFunctionInTernaryConditionRule.self, VoidReturnRule.self, WeakDelegateRule.self, XCTFailMessageRule.self, diff --git a/Source/SwiftLintFramework/Rules/Idiomatic/VoidFunctionInTernaryConditionRule.swift b/Source/SwiftLintFramework/Rules/Idiomatic/VoidFunctionInTernaryConditionRule.swift new file mode 100644 index 000000000..106054958 --- /dev/null +++ b/Source/SwiftLintFramework/Rules/Idiomatic/VoidFunctionInTernaryConditionRule.swift @@ -0,0 +1,109 @@ +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: "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) + } + """) + ], + 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 diff --git a/SwiftLint.xcodeproj/project.pbxproj b/SwiftLint.xcodeproj/project.pbxproj index 03b04d812..1db531758 100644 --- a/SwiftLint.xcodeproj/project.pbxproj +++ b/SwiftLint.xcodeproj/project.pbxproj @@ -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 */; }; @@ -836,6 +837,7 @@ D462021E1E15F52D0027AAD1 /* NumberSeparatorRuleExamples.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = NumberSeparatorRuleExamples.swift; sourceTree = ""; }; D46252531DF63FB200BE2CA1 /* NumberSeparatorRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = NumberSeparatorRule.swift; sourceTree = ""; }; D466B61F233D229F0068190B /* FlatMapOverMapReduceRule.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FlatMapOverMapReduceRule.swift; sourceTree = ""; }; + D467275723DD971300DE73B6 /* VoidFunctionInTernaryConditionRule.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = VoidFunctionInTernaryConditionRule.swift; sourceTree = ""; }; D46A317E1F1CEDCD00AF914A /* UnneededParenthesesInClosureArgumentRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = UnneededParenthesesInClosureArgumentRule.swift; sourceTree = ""; }; D46C7C3D23BF2F6A007C517F /* PreferSelfTypeOverTypeOfSelfRule.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PreferSelfTypeOverTypeOfSelfRule.swift; sourceTree = ""; }; D46E041C1DE3712C00728374 /* TrailingCommaRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = TrailingCommaRule.swift; sourceTree = ""; }; @@ -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 */, @@ -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 */, diff --git a/Tests/LinuxMain.swift b/Tests/LinuxMain.swift index 822682e4a..f3c9551f9 100644 --- a/Tests/LinuxMain.swift +++ b/Tests/LinuxMain.swift @@ -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) @@ -1868,6 +1874,7 @@ XCTMain([ testCase(VerticalWhitespaceClosingBracesRuleTests.allTests), testCase(VerticalWhitespaceOpeningBracesRuleTests.allTests), testCase(VerticalWhitespaceRuleTests.allTests), + testCase(VoidFunctionInTernaryConditionRuleTests.allTests), testCase(VoidReturnRuleTests.allTests), testCase(WeakDelegateRuleTests.allTests), testCase(XCTFailMessageRuleTests.allTests), diff --git a/Tests/SwiftLintFrameworkTests/AutomaticRuleTests.generated.swift b/Tests/SwiftLintFrameworkTests/AutomaticRuleTests.generated.swift index 4a56fc58f..f740ae939 100644 --- a/Tests/SwiftLintFrameworkTests/AutomaticRuleTests.generated.swift +++ b/Tests/SwiftLintFrameworkTests/AutomaticRuleTests.generated.swift @@ -834,6 +834,12 @@ class VerticalWhitespaceOpeningBracesRuleTests: XCTestCase { } } +class VoidFunctionInTernaryConditionRuleTests: XCTestCase { + func testWithDefaultConfiguration() { + verifyRule(VoidFunctionInTernaryConditionRule.description) + } +} + class VoidReturnRuleTests: XCTestCase { func testWithDefaultConfiguration() { verifyRule(VoidReturnRule.description)