Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Explicit Enum Raw Value Rule #1788

Merged
merged 12 commits into from
Aug 26, 2017
Merged
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,11 @@
* Invalidate cache when Swift version changes.
[Marcelo Fabri](https://github.com/marcelofabri)

* Add `explicit_enum_raw_value` opt-in rule to allow refactoring the
Swift API without breaking the API contract.
[Mazyod](https://github.com/mazyod)
[#1778](https://github.com/realm/SwiftLint/issues/1778)

##### Bug Fixes

* Fix false positive on `force_unwrapping` rule when declaring
Expand Down
98 changes: 98 additions & 0 deletions Rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
* [Empty Enum Arguments](#empty-enum-arguments)
* [Empty Parameters](#empty-parameters)
* [Empty Parentheses with Trailing Closure](#empty-parentheses-with-trailing-closure)
* [Explicit Enum Raw Value](#explicit-enum-raw-value)
* [Explicit Init](#explicit-init)
* [Explicit Top Level ACL](#explicit-top-level-acl)
* [Explicit Type Interface](#explicit-type-interface)
Expand Down Expand Up @@ -2090,6 +2091,103 @@ UIView.animateWithDuration(0.3, animations: {



## Explicit Enum Raw Value

Identifier | Enabled by default | Supports autocorrection | Kind
--- | --- | --- | ---
`explicit_enum_raw_value` | Disabled | No | idiomatic

Enums should be explicitly assigned their raw values.

### Examples

<details>
<summary>Non Triggering Examples</summary>

```swift
enum Numbers {
case int(Int)
case short(Int16)
}

```

```swift
enum Numbers: Int {
case one = 1
case two = 2
}

```

```swift
enum Numbers: Double {
case one = 1.1
case two = 2.2
}

```

```swift
enum Numbers: String {
case one = "one"
case two = "two"
}

```

```swift
protocol Algebra {}
enum Numbers: Algebra {
case one
}

```

</details>
<details>
<summary>Triggering Examples</summary>

```swift
enum Numbers: Int {
case one = 10, ↓two, three = 30
}

```

```swift
enum Numbers: NSInteger {
case ↓one
}

```

```swift
enum Numbers: String {
case ↓one
case ↓two
}

```

```swift
enum Numbers: String {
case ↓one, two = "two"
}

```

```swift
enum Numbers: Decimal {
case ↓one, ↓two
}

```

</details>



## Explicit Init

Identifier | Enabled by default | Supports autocorrection | Kind
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 @@ -30,6 +30,7 @@ public let masterRuleList = RuleList(rules: [
EmptyEnumArgumentsRule.self,
EmptyParametersRule.self,
EmptyParenthesesWithTrailingClosureRule.self,
ExplicitEnumRawValueRule.self,
ExplicitInitRule.self,
ExplicitTopLevelACLRule.self,
ExplicitTypeInterfaceRule.self,
Expand Down
89 changes: 89 additions & 0 deletions Source/SwiftLintFramework/Rules/ExplicitEnumRawValueRule.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
//
// ExplicitEnumRawValueRule.swift
// SwiftLint
//
// Created by Mazyad Alabduljaleel on 8/19/17.
// Copyright © 2017 Realm. All rights reserved.
//

import Foundation
import SourceKittenFramework

public struct ExplicitEnumRawValueRule: ASTRule, OptInRule, ConfigurationProviderRule {
public var configuration = SeverityConfiguration(.warning)

public init() {}

public static let description = RuleDescription(
identifier: "explicit_enum_raw_value",
name: "Explicit Enum Raw Value",
description: "Enums should be explicitly assigned their raw values.",
kind: .idiomatic,
nonTriggeringExamples: [
"enum Numbers {\n case int(Int)\n case short(Int16)\n}\n",
"enum Numbers: Int {\n case one = 1\n case two = 2\n}\n",
"enum Numbers: Double {\n case one = 1.1\n case two = 2.2\n}\n",
"enum Numbers: String {\n case one = \"one\"\n case two = \"two\"\n}\n",
"protocol Algebra {}\nenum Numbers: Algebra {\n case one\n}\n"
],
triggeringExamples: [
"enum Numbers: Int {\n case one = 10, ↓two, three = 30\n}\n",
"enum Numbers: NSInteger {\n case ↓one\n}\n",
"enum Numbers: String {\n case ↓one\n case ↓two\n}\n",
"enum Numbers: String {\n case ↓one, two = \"two\"\n}\n",
"enum Numbers: Decimal {\n case ↓one, ↓two\n}\n"
]
)

public func validate(file: File, kind: SwiftDeclarationKind,
dictionary: [String: SourceKitRepresentable]) -> [StyleViolation] {
guard kind == .enum else {
return []
}

// Check if it's an enum which supports raw values
let implicitRawValueTypes: [Any] = [
Int.self, Int8.self, Int16.self, Int32.self, Int64.self,
UInt.self, UInt8.self, UInt16.self, UInt32.self, UInt64.self,
Double.self, Float.self, Float80.self, Decimal.self, NSNumber.self,
NSDecimalNumber.self, "NSInteger", String.self
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL that you can use NSNumber and other types as raw values 😅

]

let implicitRawValueSet = Set(implicitRawValueTypes.map(String.init(describing:)))
let enumInheritedTypesSet = Set(dictionary.inheritedTypes)

guard !implicitRawValueSet.isDisjoint(with: enumInheritedTypesSet) else {
return []
}

let violations = violatingOffsetsForEnum(dictionary: dictionary)
return violations.map {
StyleViolation(ruleDescription: type(of: self).description,
severity: configuration.severity,
location: Location(file: file, byteOffset: $0))
}
}

private func violatingOffsetsForEnum(dictionary: [String: SourceKitRepresentable]) -> [Int] {

let locs = substructureElements(of: dictionary, matching: .enumcase)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please combine the 3 flatMaps? I'd also recommend not storing the result in an intermediate variable, since it's unused other than just being returned right away, so doesn't make things any clearer IMHO.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I can see this logic using a single loop with guards and continue

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind, I don't think my suggestion would improve things after all.

.flatMap { substructureElements(of: $0, matching: .enumelement) }
.flatMap(enumElementsMissingInitExpr)
.flatMap { $0.offset }

return locs
}

private func substructureElements(of dict: [String: SourceKitRepresentable],
matching kind: SwiftDeclarationKind) -> [[String: SourceKitRepresentable]] {
return dict.substructure
.filter { $0.kind.flatMap(SwiftDeclarationKind.init) == kind }
}

private func enumElementsMissingInitExpr(
_ enumElements: [[String: SourceKitRepresentable]]) -> [[String: SourceKitRepresentable]] {

return enumElements
.filter { !$0.elements.contains { $0.kind == "source.lang.swift.structure.elem.init_expr" } }
}
}
4 changes: 4 additions & 0 deletions SwiftLint.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@
78F032481D7D614300BE709A /* OverridenSuperCallConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = 78F032471D7D614300BE709A /* OverridenSuperCallConfiguration.swift */; };
7C0C2E7A1D2866CB0076435A /* ExplicitInitRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7C0C2E791D2866CB0076435A /* ExplicitInitRule.swift */; };
825F19D11EEFF19700969EF1 /* ObjectLiteralRuleTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 825F19D01EEFF19700969EF1 /* ObjectLiteralRuleTests.swift */; };
827169B31F488181003FB9AF /* ExplicitEnumRawValueRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 827169B21F488181003FB9AF /* ExplicitEnumRawValueRule.swift */; };
83894F221B0C928A006214E1 /* RulesCommand.swift in Sources */ = {isa = PBXBuildFile; fileRef = 83894F211B0C928A006214E1 /* RulesCommand.swift */; };
83D71E281B131ECE000395DE /* RuleDescription.swift in Sources */ = {isa = PBXBuildFile; fileRef = 83D71E261B131EB5000395DE /* RuleDescription.swift */; };
85DA81321D6B471000951BC4 /* MarkRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 856651A61D6B395F005E6B29 /* MarkRule.swift */; };
Expand Down Expand Up @@ -404,6 +405,7 @@
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>"; };
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>"; };
83894F211B0C928A006214E1 /* RulesCommand.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = RulesCommand.swift; sourceTree = "<group>"; };
83D71E261B131EB5000395DE /* RuleDescription.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = RuleDescription.swift; sourceTree = "<group>"; };
856651A61D6B395F005E6B29 /* MarkRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = MarkRule.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -956,6 +958,7 @@
D4470D581EB6B4D1008A1B2E /* EmptyEnumArgumentsRule.swift */,
D47079AC1DFE2FA700027086 /* EmptyParametersRule.swift */,
D47079A61DFCEB2D00027086 /* EmptyParenthesesWithTrailingClosureRule.swift */,
827169B21F488181003FB9AF /* ExplicitEnumRawValueRule.swift */,
7C0C2E791D2866CB0076435A /* ExplicitInitRule.swift */,
1EF115911EB2AD5900E30140 /* ExplicitTopLevelACLRule.swift */,
C328A2F51E67595500A9E4D7 /* ExplicitTypeInterfaceRule.swift */,
Expand Down Expand Up @@ -1501,6 +1504,7 @@
D4D1B9BB1EAC2C910028BE6A /* AccessControlLevel.swift in Sources */,
4A9A3A3A1DC1D75F00DF5183 /* HTMLReporter.swift in Sources */,
D40F83881DE9179200524C62 /* TrailingCommaConfiguration.swift in Sources */,
827169B31F488181003FB9AF /* ExplicitEnumRawValueRule.swift in Sources */,
29FFC37A1F15764D007E4825 /* FileLengthRuleConfiguration.swift in Sources */,
3B5B9FE11C444DA20009AD27 /* Array+SwiftLint.swift in Sources */,
D43B04641E0620AB004016AF /* UnusedEnumeratedRule.swift in Sources */,
Expand Down
1 change: 1 addition & 0 deletions Tests/LinuxMain.swift
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,7 @@ extension RulesTests {
("testEmptyEnumArguments", testEmptyEnumArguments),
("testEmptyParameters", testEmptyParameters),
("testEmptyParenthesesWithTrailingClosure", testEmptyParenthesesWithTrailingClosure),
("testExplicitEnumRawValue", testExplicitEnumRawValue),
("testExplicitInit", testExplicitInit),
("testExplicitTopLevelACL", testExplicitTopLevelACL),
("testExplicitTypeInterface", testExplicitTypeInterface),
Expand Down
4 changes: 4 additions & 0 deletions Tests/SwiftLintFrameworkTests/RulesTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,10 @@ class RulesTests: XCTestCase {
verifyRule(EmptyParenthesesWithTrailingClosureRule.description)
}

func testExplicitEnumRawValue() {
verifyRule(ExplicitEnumRawValueRule.description)
}

func testExplicitInit() {
verifyRule(ExplicitInitRule.description)
}
Expand Down