From a7c58c14d469f4f44a2bd4ad60626a76ba391dae Mon Sep 17 00:00:00 2001 From: Marcelo Fabri Date: Thu, 8 Dec 2016 18:22:11 -0200 Subject: [PATCH 1/2] Add RedundantStringEnumValueRule Fixes #946 --- CHANGELOG.md | 5 + .../Models/MasterRuleList.swift | 1 + .../Rules/RedundantStringEnumValueRule.swift | 153 ++++++++++++++++++ SwiftLint.xcodeproj/project.pbxproj | 4 + .../SwiftLintFrameworkTests/RulesTests.swift | 4 + 5 files changed, 167 insertions(+) create mode 100644 Source/SwiftLintFramework/Rules/RedundantStringEnumValueRule.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 27a0abcf20..232d52572b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,11 @@ * Add `EmojiReporter`: a human friendly reporter. [Michał Kałużny](https://github.com/justMaku) + +* Add `redundant_string_enum_value` rule that warns against String enums + with redundant value assignments. + [Marcelo Fabri](https://github.com/marcelofabri) + [#946](https://github.com/realm/SwiftLint/issues/946) ##### Bug Fixes diff --git a/Source/SwiftLintFramework/Models/MasterRuleList.swift b/Source/SwiftLintFramework/Models/MasterRuleList.swift index 5ce0b54078..69541516e5 100644 --- a/Source/SwiftLintFramework/Models/MasterRuleList.swift +++ b/Source/SwiftLintFramework/Models/MasterRuleList.swift @@ -75,6 +75,7 @@ public let masterRuleList = RuleList(rules: PrivateOutletRule.self, PrivateUnitTestRule.self, RedundantNilCoalescingRule.self, + RedundantStringEnumValueRule.self, ReturnArrowWhitespaceRule.self, StatementPositionRule.self, SwitchCaseOnNewlineRule.self, diff --git a/Source/SwiftLintFramework/Rules/RedundantStringEnumValueRule.swift b/Source/SwiftLintFramework/Rules/RedundantStringEnumValueRule.swift new file mode 100644 index 0000000000..ddd2ddb844 --- /dev/null +++ b/Source/SwiftLintFramework/Rules/RedundantStringEnumValueRule.swift @@ -0,0 +1,153 @@ +// +// RedundantStringEnumValueRule.swift +// SwiftLint +// +// Created by Marcelo Fabri on 08/12/16. +// Copyright © 2016 Realm. All rights reserved. +// + +import Foundation +import SourceKittenFramework + +public struct RedundantStringEnumValueRule: ASTRule, ConfigurationProviderRule { + public var configuration = SeverityConfiguration(.warning) + + public init() {} + + public static let description = RuleDescription( + identifier: "redundant_string_enum_value", + name: "Redudant String Enum Value", + description: "String enum values can be ommited when they are equal to the enumcase name.", + nonTriggeringExamples: [ + "enum Numbers: String {\n case one\n case two\n}\n", + "enum Numbers: Int {\n case one = 1\n case two = 2\n}\n", + "enum Numbers: String {\n case one = \"ONE\"\n case two = \"TWO\"\n}\n", + "enum Numbers: String {\n case one = \"ONE\"\n case two = \"two\"\n}\n", + "enum Numbers: String {\n case one, two\n}\n" + ], + triggeringExamples: [ + "enum Numbers: String {\n case one = ↓\"one\"\n case two = ↓\"two\"\n}\n", + "enum Numbers: String {\n case one = ↓\"one\", two = ↓\"two\"\n}\n", + "enum Numbers: String {\n case one, two = ↓\"two\"\n}\n" + ] + ) + + public func validateFile(_ file: File, + kind: SwiftDeclarationKind, + dictionary: [String: SourceKitRepresentable]) -> [StyleViolation] { + guard kind == .enum else { + return [] + } + + // Check if it's a String enum + let inheritedTypes = (dictionary["key.inheritedtypes"] as? [SourceKitRepresentable])? + .flatMap({ ($0 as? [String: SourceKitRepresentable]) as? [String: String] }) + .flatMap({ $0["key.name"] }) ?? [] + guard inheritedTypes.contains("String") else { + return [] + } + + let violations = violatingOffsetsForEnum(dictionary: dictionary, file: file) + return violations.map { + StyleViolation(ruleDescription: type(of: self).description, + severity: configuration.severity, + location: Location(file: file, byteOffset: $0)) + } + } + + private func violatingOffsetsForEnum(dictionary: [String: SourceKitRepresentable], + file: File) -> [Int] { + let substructure = dictionary["key.substructure"] as? [SourceKitRepresentable] ?? [] + var enumCases = 0 + + let violations = substructure.flatMap { subItem -> [Int] in + guard let subDict = subItem as? [String: SourceKitRepresentable], + let kindString = subDict["key.kind"] as? String, + SwiftDeclarationKind(rawValue: kindString) == .enumcase else { + return [] + } + + enumCases += enumElementsCount(dictionary: subDict) + return violatingOffsetsForEnumCase(dictionary: subDict, file: file) + } + + guard violations.count == enumCases else { + return [] + } + + return violations + } + + private func enumElementsCount(dictionary: [String: SourceKitRepresentable]) -> Int { + let enumSubstructure = dictionary["key.substructure"] as? [SourceKitRepresentable] ?? [] + return enumSubstructure.filter { item -> Bool in + guard let subDict = item as? [String: SourceKitRepresentable], + let kindString = subDict["key.kind"] as? String, + SwiftDeclarationKind(rawValue: kindString) == .enumelement else { + return false + } + + guard !filterEnumInits(dictionary: subDict).isEmpty else { + return false + } + + return true + }.count + } + + private func violatingOffsetsForEnumCase(dictionary: [String: SourceKitRepresentable], + file: File) -> [Int] { + let enumSubstructure = dictionary["key.substructure"] as? [SourceKitRepresentable] ?? [] + let violations = enumSubstructure.flatMap { item -> [Int] in + guard let subDict = item as? [String: SourceKitRepresentable], + let kindString = subDict["key.kind"] as? String, + SwiftDeclarationKind(rawValue: kindString) == .enumelement, + let name = subDict["key.name"] as? String else { + return [] + } + + return violatingOffsetsForEnumElement(dictionary: subDict, name: name, file: file) + } + + return violations + } + + private func violatingOffsetsForEnumElement(dictionary: [String: SourceKitRepresentable], + name: String, + file: File) -> [Int] { + let enumInits = filterEnumInits(dictionary: dictionary) + + return enumInits.flatMap { dictionary -> Int? in + guard let offset = (dictionary["key.offset"] as? Int64).flatMap({ Int($0) }), + let length = (dictionary["key.length"] as? Int64).flatMap({ Int($0) }) else { + return nil + } + + // the string would be quoted if offset and length were used directly + let enumCaseName = file.contents.substringWithByteRange(start: offset + 1, + length: length - 2) ?? "" + guard enumCaseName == name else { + return nil + } + + return offset + } + } + + private func filterEnumInits(dictionary: [String: SourceKitRepresentable]) -> + [[String: SourceKitRepresentable]] { + guard let elements = dictionary["key.elements"] as? [SourceKitRepresentable] else { + return [] + } + + let enumInitKind = "source.lang.swift.structure.elem.init_expr" + return elements.flatMap { element in + guard let dict = element as? [String: SourceKitRepresentable], + dict["key.kind"] as? String == enumInitKind else { + return nil + } + + return dict + } + } +} diff --git a/SwiftLint.xcodeproj/project.pbxproj b/SwiftLint.xcodeproj/project.pbxproj index ff89206510..82cba26d1d 100644 --- a/SwiftLint.xcodeproj/project.pbxproj +++ b/SwiftLint.xcodeproj/project.pbxproj @@ -73,6 +73,7 @@ D0E7B65319E9C6AD00EDBA4D /* SwiftLintFramework.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = D0D1216D19E87B05005E4BAA /* SwiftLintFramework.framework */; }; D0E7B65619E9C76900EDBA4D /* main.swift in Sources */ = {isa = PBXBuildFile; fileRef = D0D1211B19E87861005E4BAA /* main.swift */; }; D40F83881DE9179200524C62 /* TrailingCommaConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = D40F83871DE9179200524C62 /* TrailingCommaConfiguration.swift */; }; + D41E7E0B1DF9DABB0065259A /* RedundantStringEnumValueRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D41E7E0A1DF9DABB0065259A /* RedundantStringEnumValueRule.swift */; }; D4348EEA1C46122C007707FB /* FunctionBodyLengthRuleTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4348EE91C46122C007707FB /* FunctionBodyLengthRuleTests.swift */; }; D43DB1081DC573DA00281215 /* ImplicitGetterRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D43DB1071DC573DA00281215 /* ImplicitGetterRule.swift */; }; D44254201DB87CA200492EA4 /* ValidIBInspectableRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D442541E1DB87C3D00492EA4 /* ValidIBInspectableRule.swift */; }; @@ -277,6 +278,7 @@ D0D1217D19E87B05005E4BAA /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = ""; }; D0E7B63219E9C64500EDBA4D /* swiftlint.app */ = {isa = PBXFileReference; explicitFileType = wrapper.application; includeInIndex = 0; path = swiftlint.app; sourceTree = BUILT_PRODUCTS_DIR; }; D40F83871DE9179200524C62 /* TrailingCommaConfiguration.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = TrailingCommaConfiguration.swift; sourceTree = ""; }; + D41E7E0A1DF9DABB0065259A /* RedundantStringEnumValueRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = RedundantStringEnumValueRule.swift; sourceTree = ""; }; D4348EE91C46122C007707FB /* FunctionBodyLengthRuleTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = FunctionBodyLengthRuleTests.swift; sourceTree = ""; }; D43DB1071DC573DA00281215 /* ImplicitGetterRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ImplicitGetterRule.swift; sourceTree = ""; }; D442541E1DB87C3D00492EA4 /* ValidIBInspectableRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ValidIBInspectableRule.swift; sourceTree = ""; }; @@ -664,6 +666,7 @@ B2902A0B1D66815600BFCCF7 /* PrivateUnitTestRule.swift */, 094385021D5D4F78009168CF /* PrivateOutletRule.swift */, 24B4DF0B1D6DFA370097803B /* RedundantNilCoalescingRule.swift */, + D41E7E0A1DF9DABB0065259A /* RedundantStringEnumValueRule.swift */, E57B23C01B1D8BF000DEA512 /* ReturnArrowWhitespaceRule.swift */, 3BCC04CE1C4F56D3006073C3 /* RuleConfigurations */, 692B60AB1BD8F2E700C7AA22 /* StatementPositionRule.swift */, @@ -1014,6 +1017,7 @@ 3BB47D851C51D80000AE6A10 /* NSRegularExpression+SwiftLint.swift in Sources */, E881985B1BEA974E00333A11 /* StatementPositionRule.swift in Sources */, B58AEED61C492C7B00E901FD /* ForceUnwrappingRule.swift in Sources */, + D41E7E0B1DF9DABB0065259A /* RedundantStringEnumValueRule.swift in Sources */, E88DEA711B09847500A66CB0 /* ViolationSeverity.swift in Sources */, B2902A0C1D66815600BFCCF7 /* PrivateUnitTestRule.swift in Sources */, 3BD9CD3D1C37175B009A5D25 /* YamlParser.swift in Sources */, diff --git a/Tests/SwiftLintFrameworkTests/RulesTests.swift b/Tests/SwiftLintFrameworkTests/RulesTests.swift index 669349c9d1..17b2f2877b 100644 --- a/Tests/SwiftLintFrameworkTests/RulesTests.swift +++ b/Tests/SwiftLintFrameworkTests/RulesTests.swift @@ -229,6 +229,10 @@ class RulesTests: XCTestCase { verifyRule(RedundantNilCoalescingRule.description) } + func testRedundantStringEnumValue() { + verifyRule(RedundantStringEnumValueRule.description) + } + func testReturnArrowWhitespace() { verifyRule(ReturnArrowWhitespaceRule.description) } From 43380cefe061fb6eb2a2b84647573c975910b413 Mon Sep 17 00:00:00 2001 From: JP Simard Date: Thu, 8 Dec 2016 13:33:05 -0800 Subject: [PATCH 2/2] refactor RedundantStringEnumValueRule.swift to consolidate duplicate code and generally simplify the code --- .../Rules/RedundantStringEnumValueRule.swift | 64 ++++++++----------- 1 file changed, 26 insertions(+), 38 deletions(-) diff --git a/Source/SwiftLintFramework/Rules/RedundantStringEnumValueRule.swift b/Source/SwiftLintFramework/Rules/RedundantStringEnumValueRule.swift index ddd2ddb844..9e917103db 100644 --- a/Source/SwiftLintFramework/Rules/RedundantStringEnumValueRule.swift +++ b/Source/SwiftLintFramework/Rules/RedundantStringEnumValueRule.swift @@ -9,6 +9,18 @@ import Foundation import SourceKittenFramework +private func children(of dict: [String: SourceKitRepresentable], + matching kind: SwiftDeclarationKind) -> [[String: SourceKitRepresentable]] { + return (dict["key.substructure"] as? [SourceKitRepresentable] ?? []).flatMap { item in + if let subDict = item as? [String: SourceKitRepresentable], + let kindString = subDict["key.kind"] as? String, + SwiftDeclarationKind(rawValue: kindString) == kind { + return subDict + } + return nil + } +} + public struct RedundantStringEnumValueRule: ASTRule, ConfigurationProviderRule { public var configuration = SeverityConfiguration(.warning) @@ -17,7 +29,7 @@ public struct RedundantStringEnumValueRule: ASTRule, ConfigurationProviderRule { public static let description = RuleDescription( identifier: "redundant_string_enum_value", name: "Redudant String Enum Value", - description: "String enum values can be ommited when they are equal to the enumcase name.", + description: "String enum values can be omitted when they are equal to the enumcase name.", nonTriggeringExamples: [ "enum Numbers: String {\n case one\n case two\n}\n", "enum Numbers: Int {\n case one = 1\n case two = 2\n}\n", @@ -57,21 +69,15 @@ public struct RedundantStringEnumValueRule: ASTRule, ConfigurationProviderRule { private func violatingOffsetsForEnum(dictionary: [String: SourceKitRepresentable], file: File) -> [Int] { - let substructure = dictionary["key.substructure"] as? [SourceKitRepresentable] ?? [] - var enumCases = 0 - - let violations = substructure.flatMap { subItem -> [Int] in - guard let subDict = subItem as? [String: SourceKitRepresentable], - let kindString = subDict["key.kind"] as? String, - SwiftDeclarationKind(rawValue: kindString) == .enumcase else { - return [] - } + var caseCount = 0 + var violations = [Int]() - enumCases += enumElementsCount(dictionary: subDict) - return violatingOffsetsForEnumCase(dictionary: subDict, file: file) + for enumCase in children(of: dictionary, matching: .enumcase) { + caseCount += enumElementsCount(dictionary: enumCase) + violations += violatingOffsetsForEnumCase(dictionary: enumCase, file: file) } - guard violations.count == enumCases else { + guard violations.count == caseCount else { return [] } @@ -79,37 +85,19 @@ public struct RedundantStringEnumValueRule: ASTRule, ConfigurationProviderRule { } private func enumElementsCount(dictionary: [String: SourceKitRepresentable]) -> Int { - let enumSubstructure = dictionary["key.substructure"] as? [SourceKitRepresentable] ?? [] - return enumSubstructure.filter { item -> Bool in - guard let subDict = item as? [String: SourceKitRepresentable], - let kindString = subDict["key.kind"] as? String, - SwiftDeclarationKind(rawValue: kindString) == .enumelement else { - return false - } - - guard !filterEnumInits(dictionary: subDict).isEmpty else { - return false - } - - return true - }.count + return children(of: dictionary, matching: .enumelement).filter({ element in + return !filterEnumInits(dictionary: element).isEmpty + }).count } private func violatingOffsetsForEnumCase(dictionary: [String: SourceKitRepresentable], file: File) -> [Int] { - let enumSubstructure = dictionary["key.substructure"] as? [SourceKitRepresentable] ?? [] - let violations = enumSubstructure.flatMap { item -> [Int] in - guard let subDict = item as? [String: SourceKitRepresentable], - let kindString = subDict["key.kind"] as? String, - SwiftDeclarationKind(rawValue: kindString) == .enumelement, - let name = subDict["key.name"] as? String else { - return [] + return children(of: dictionary, matching: .enumelement).flatMap { element -> [Int] in + guard let name = element["key.name"] as? String else { + return [] } - - return violatingOffsetsForEnumElement(dictionary: subDict, name: name, file: file) + return violatingOffsetsForEnumElement(dictionary: element, name: name, file: file) } - - return violations } private func violatingOffsetsForEnumElement(dictionary: [String: SourceKitRepresentable],