Skip to content

Commit

Permalink
Merge pull request #2308 from Dschee/conditional-returns-on-newline
Browse files Browse the repository at this point in the history
Add `if_only` option to rule conditional_returns_on_newline
  • Loading branch information
marcelofabri authored Jul 24, 2018
2 parents c7c0ac8 + 1452da4 commit e9c6d6a
Show file tree
Hide file tree
Showing 8 changed files with 98 additions and 20 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@
[Ornithologist Coder](https://github.com/ornithocoder)
[#2283](https://github.com/realm/SwiftLint/issues/2283)

* Add new bool config option `if_only` to rule `conditional_returns_on_newline`
to specify that the rule should only be applied to `if` statements.
[Cihat Gündüz](https://github.com/Dschee)
[#2307](https://github.com/realm/SwiftLint/issues/2307)

#### Bug Fixes

* Fix an issue with `control_statement` where commas in clauses prevented the
Expand Down
2 changes: 1 addition & 1 deletion Source/SwiftLintFramework/Models/MasterRuleList.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Generated using Sourcery 0.11.2 — https://github.com/krzysztofzablocki/Sourcery
// Generated using Sourcery 0.13.1 — https://github.com/krzysztofzablocki/Sourcery
// DO NOT EDIT

public let masterRuleList = RuleList(rules: [
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import Foundation
import SourceKittenFramework

public struct ConditionalReturnsOnNewlineRule: ConfigurationProviderRule, Rule, OptInRule, AutomaticTestableRule {
public var configuration = SeverityConfiguration(.warning)
public struct ConditionalReturnsOnNewlineRule: ConfigurationProviderRule, Rule, OptInRule {
public var configuration = ConditionalReturnsOnNewlineConfiguration()

public init() {}

Expand Down Expand Up @@ -30,19 +30,21 @@ public struct ConditionalReturnsOnNewlineRule: ConfigurationProviderRule, Rule,
)

public func validate(file: File) -> [StyleViolation] {
let pattern = "(guard|if)[^\n]*return"
let pattern = configuration.ifOnly ? "(if)[^\n]*return" : "(guard|if)[^\n]*return"

return file.rangesAndTokens(matching: pattern).filter { _, tokens in
guard let firstToken = tokens.first, let lastToken = tokens.last,
SyntaxKind(rawValue: firstToken.type) == .keyword &&
SyntaxKind(rawValue: lastToken.type) == .keyword else {
return false
}

return ["if", "guard"].contains(content(for: firstToken, file: file)) &&
let searchTokens = configuration.ifOnly ? ["if"] : ["if", "guard"]
return searchTokens.contains(content(for: firstToken, file: file)) &&
content(for: lastToken, file: file) == "return"
}.map {
StyleViolation(ruleDescription: type(of: self).description,
severity: configuration.severity,
severity: configuration.severityConfiguration.severity,
location: Location(file: file, characterOffset: $0.0.location))
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import Foundation

public struct ConditionalReturnsOnNewlineConfiguration: RuleConfiguration {
private(set) var severityConfiguration = SeverityConfiguration(.warning)
private(set) var ifOnly = false

public var consoleDescription: String {
return [severityConfiguration.consoleDescription, "if_only: \(ifOnly)"].joined(separator: ", ")
}

public mutating func apply(configuration: Any) throws {
guard let configuration = configuration as? [String: Any] else {
throw ConfigurationError.unknownConfiguration
}

ifOnly = configuration["if_only"] as? Bool ?? false

if let severityString = configuration["severity"] as? String {
try severityConfiguration.apply(configuration: severityString)
}
}
}

extension ConditionalReturnsOnNewlineConfiguration: Equatable {
public static func == (lhs: ConditionalReturnsOnNewlineConfiguration,
rhs: ConditionalReturnsOnNewlineConfiguration) -> Bool {
return lhs.severityConfiguration == rhs.severityConfiguration &&
lhs.ifOnly == rhs.ifOnly
}
}
16 changes: 12 additions & 4 deletions SwiftLint.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,8 @@
78F032461D7C877E00BE709A /* OverriddenSuperCallRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 78F032441D7C877800BE709A /* OverriddenSuperCallRule.swift */; };
78F032481D7D614300BE709A /* OverridenSuperCallConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = 78F032471D7D614300BE709A /* OverridenSuperCallConfiguration.swift */; };
7C0C2E7A1D2866CB0076435A /* ExplicitInitRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7C0C2E791D2866CB0076435A /* ExplicitInitRule.swift */; };
820F451E21073D7200AA056A /* ConditionalReturnsOnNewlineRuleTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 820F451D21073D7200AA056A /* ConditionalReturnsOnNewlineRuleTests.swift */; };
824AB64D2105C39F004B5A8F /* ConditionalReturnsOnNewlineConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = 824AB64C2105C39F004B5A8F /* ConditionalReturnsOnNewlineConfiguration.swift */; };
825F19D11EEFF19700969EF1 /* ObjectLiteralRuleTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 825F19D01EEFF19700969EF1 /* ObjectLiteralRuleTests.swift */; };
827169B31F488181003FB9AF /* ExplicitEnumRawValueRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 827169B21F488181003FB9AF /* ExplicitEnumRawValueRule.swift */; };
827169B51F48D712003FB9AF /* NoGroupingExtensionRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 827169B41F48D712003FB9AF /* NoGroupingExtensionRule.swift */; };
Expand Down Expand Up @@ -515,6 +517,8 @@
78F032441D7C877800BE709A /* OverriddenSuperCallRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = OverriddenSuperCallRule.swift; sourceTree = "<group>"; };
78F032471D7D614300BE709A /* OverridenSuperCallConfiguration.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = OverridenSuperCallConfiguration.swift; sourceTree = "<group>"; };
7C0C2E791D2866CB0076435A /* ExplicitInitRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ExplicitInitRule.swift; sourceTree = "<group>"; };
820F451D21073D7200AA056A /* ConditionalReturnsOnNewlineRuleTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ConditionalReturnsOnNewlineRuleTests.swift; sourceTree = "<group>"; };
824AB64C2105C39F004B5A8F /* ConditionalReturnsOnNewlineConfiguration.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ConditionalReturnsOnNewlineConfiguration.swift; sourceTree = "<group>"; };
825F19D01EEFF19700969EF1 /* ObjectLiteralRuleTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ObjectLiteralRuleTests.swift; sourceTree = "<group>"; };
827169B21F488181003FB9AF /* ExplicitEnumRawValueRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ExplicitEnumRawValueRule.swift; sourceTree = "<group>"; };
827169B41F48D712003FB9AF /* NoGroupingExtensionRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = NoGroupingExtensionRule.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -811,35 +815,36 @@
children = (
D4C4A34A1DEA4FD700E0E04C /* AttributesConfiguration.swift */,
D43B04671E07228D004016AF /* ColonConfiguration.swift */,
824AB64C2105C39F004B5A8F /* ConditionalReturnsOnNewlineConfiguration.swift */,
67EB4DF81E4CC101004E9ACD /* CyclomaticComplexityConfiguration.swift */,
62A498551F306A7700D766E4 /* DiscouragedDirectInitConfiguration.swift */,
125AAC77203AA82D0004BCE0 /* ExplicitTypeInterfaceConfiguration.swift */,
D4C4A3511DEFBBB700E0E04C /* FileHeaderConfiguration.swift */,
29FFC3781F1574FD007E4825 /* FileLengthRuleConfiguration.swift */,
8F2CC1CA20A6A070006ED34F /* FileNameConfiguration.swift */,
8B01E4FB20A4183C00C9233E /* FunctionParameterCountConfiguration.swift */,
47ACC8971E7DC74E0088EEB2 /* ImplicitlyUnwrappedOptionalConfiguration.swift */,
3B034B6C1E0BE544005D49A9 /* LineLengthConfiguration.swift */,
3BCC04D01C4F56D3006073C3 /* NameConfiguration.swift */,
188B3FF3207D61230073C2D6 /* ModifierOrderConfiguration.swift */,
3BCC04D01C4F56D3006073C3 /* NameConfiguration.swift */,
D93DA3CF1E699E4E00809827 /* NestingConfiguration.swift */,
D4DA1DFD1E1A10DB0037413D /* NumberSeparatorConfiguration.swift */,
A1A6F3F11EE319ED00A9F9E2 /* ObjectLiteralConfiguration.swift */,
78F032471D7D614300BE709A /* OverridenSuperCallConfiguration.swift */,
D4246D6C1F30D8620097E658 /* PrivateOverFilePrivateRuleConfiguration.swift */,
DAD3BE491D6ECD9500660239 /* PrivateOutletRuleConfiguration.swift */,
D4246D6C1F30D8620097E658 /* PrivateOverFilePrivateRuleConfiguration.swift */,
B2902A0D1D6681F700BFCCF7 /* PrivateUnitTestConfiguration.swift */,
009E09291DFEE4DD00B588A7 /* ProhibitedSuperConfiguration.swift */,
3BB47D821C514E8100AE6A10 /* RegexConfiguration.swift */,
B89F3BC71FD5ED7D00931E59 /* RequiredEnumCaseRuleConfiguration.swift */,
3B0B14531C505D6300BE82F7 /* SeverityConfiguration.swift */,
3BCC04CF1C4F56D3006073C3 /* SeverityLevelsConfiguration.swift */,
725094881D0855760039B353 /* StatementModeConfiguration.swift */,
787CDE38208E7D41005F3D2F /* SwitchCaseAlignmentConfiguration.swift */,
D40F83871DE9179200524C62 /* TrailingCommaConfiguration.swift */,
BF48D2D61CBCCA5F0080BDAE /* TrailingWhitespaceConfiguration.swift */,
CE8178EB1EAC02CD0063186E /* UnusedOptionalBindingConfiguration.swift */,
006204DA1E1E48F900FFFBE1 /* VerticalWhitespaceConfiguration.swift */,
787CDE38208E7D41005F3D2F /* SwitchCaseAlignmentConfiguration.swift */,
8F2CC1CA20A6A070006ED34F /* FileNameConfiguration.swift */,
);
path = RuleConfigurations;
sourceTree = "<group>";
Expand Down Expand Up @@ -1004,6 +1009,7 @@
D4EAB3A320E9948D0051C09A /* AutomaticRuleTests.generated.swift */,
D43B04651E071ED3004016AF /* ColonRuleTests.swift */,
E81ADD731ED6052F000CD451 /* CommandTests.swift */,
820F451D21073D7200AA056A /* ConditionalReturnsOnNewlineRuleTests.swift */,
E809EDA21B8A73FB00399043 /* ConfigurationTests.swift */,
F480DC7E1F26090000099465 /* ConfigurationTests+Nested.swift */,
F480DC821F2609D700099465 /* ConfigurationTests+ProjectMock.swift */,
Expand Down Expand Up @@ -1767,6 +1773,7 @@
C328A2F71E6759AE00A9E4D7 /* ExplicitTypeInterfaceRule.swift in Sources */,
93E0C3CE1D67BD7F007FA25D /* ConditionalReturnsOnNewlineRule.swift in Sources */,
D43DB1081DC573DA00281215 /* ImplicitGetterRule.swift in Sources */,
824AB64D2105C39F004B5A8F /* ConditionalReturnsOnNewlineConfiguration.swift in Sources */,
62A6E7931F3317E3003A0479 /* JoinedDefaultParameterRule.swift in Sources */,
D4FD4C851F2A260A00DD8AA8 /* BlockBasedKVORule.swift in Sources */,
7C0C2E7A1D2866CB0076435A /* ExplicitInitRule.swift in Sources */,
Expand Down Expand Up @@ -1884,6 +1891,7 @@
F480DC831F2609D700099465 /* ConfigurationTests+ProjectMock.swift in Sources */,
3B63D46D1E1F05160057BE35 /* LineLengthConfigurationTests.swift in Sources */,
6C7045441C6ADA450003F15A /* SourceKitCrashTests.swift in Sources */,
820F451E21073D7200AA056A /* ConditionalReturnsOnNewlineRuleTests.swift in Sources */,
D4246D6F1F30DB260097E658 /* PrivateOverFilePrivateRuleTests.swift in Sources */,
B25DCD101F7EF6DC0028A199 /* MultilineArgumentsRuleTests.swift in Sources */,
3BB47D871C51DE6E00AE6A10 /* CustomRulesTests.swift in Sources */,
Expand Down
8 changes: 5 additions & 3 deletions Tests/LinuxMain.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Generated using Sourcery 0.11.2 — https://github.com/krzysztofzablocki/Sourcery
// Generated using Sourcery 0.13.1 — https://github.com/krzysztofzablocki/Sourcery
// DO NOT EDIT

@testable import SwiftLintFrameworkTests
Expand All @@ -22,7 +22,8 @@ extension AttributesRuleTests {
static var allTests: [(String, (AttributesRuleTests) -> () throws -> Void)] = [
("testAttributesWithDefaultConfiguration", testAttributesWithDefaultConfiguration),
("testAttributesWithAlwaysOnSameLine", testAttributesWithAlwaysOnSameLine),
("testAttributesWithAlwaysOnLineAbove", testAttributesWithAlwaysOnLineAbove)
("testAttributesWithAlwaysOnLineAbove", testAttributesWithAlwaysOnLineAbove),
("testAttributesWithAttributesOnLineAboveButOnOtherDeclaration", testAttributesWithAttributesOnLineAboveButOnOtherDeclaration)
]
}

Expand Down Expand Up @@ -107,7 +108,8 @@ extension CompilerProtocolInitRuleTests {

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

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Generated using Sourcery 0.11.2 — https://github.com/krzysztofzablocki/Sourcery
// Generated using Sourcery 0.13.1 — https://github.com/krzysztofzablocki/Sourcery
// DO NOT EDIT

import SwiftLintFramework
Expand Down Expand Up @@ -66,12 +66,6 @@ class CompilerProtocolInitRuleTests: XCTestCase {
}
}

class ConditionalReturnsOnNewlineRuleTests: XCTestCase {
func testWithDefaultConfiguration() {
verifyRule(ConditionalReturnsOnNewlineRule.description)
}
}

class ContainsOverFirstNotNilRuleTests: XCTestCase {
func testWithDefaultConfiguration() {
verifyRule(ContainsOverFirstNotNilRule.description)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
@testable import SwiftLintFramework
import XCTest

class ConditionalReturnsOnNewlineRuleTests: XCTestCase {
func testConditionalReturnsOnNewlineWithDefaultConfiguration() {
// Test with default parameters
verifyRule(ConditionalReturnsOnNewlineRule.description)
}

func testConditionalReturnsOnNewlineWithIfOnly() {
// Test with `if_only` set to true
let nonTriggeringExamples = [
"guard true else {\n return true\n}",
"guard true,\n let x = true else {\n return true\n}",
"if true else {\n return true\n}",
"if true,\n let x = true else {\n return true\n}",
"if textField.returnKeyType == .Next {",
"if true { // return }",
"/*if true { */ return }",
"guard true else { return }"
]
let triggeringExamples = [
"↓if true { return }",
"↓if true { break } else { return }",
"↓if true { break } else { return }",
"↓if true { return \"YES\" } else { return \"NO\" }"
]

let alwaysOnSameLineDescription = ConditionalReturnsOnNewlineRule.description
.with(triggeringExamples: triggeringExamples)
.with(nonTriggeringExamples: nonTriggeringExamples)

verifyRule(alwaysOnSameLineDescription,
ruleConfiguration: ["if_only": true])

}
}

0 comments on commit e9c6d6a

Please sign in to comment.