diff --git a/CHANGELOG.md b/CHANGELOG.md index 1596786b4..ed72e7b6b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,8 +23,8 @@ [Marcelo Fabri](https://github.com/marcelofabri) [#1061](https://github.com/realm/SwiftLint/issues/1061) -* Add 'ignores_function_declarations' and 'ignores_comments' as options - to LineLengthRule. +* Add `ignores_function_declarations` and `ignores_comments` as options + to LineLengthRule. [Michael L. Welles](https://github.com/mlwelles) [#598](https://github.com/realm/SwiftLint/issues/598) [#975](https://github.com/realm/SwiftLint/issues/975) @@ -1710,8 +1710,7 @@ This release has seen a phenomenal uptake in community contributions! * The following rules now conform to `ASTRule`: FunctionBodyLength, Nesting, TypeBodyLength, TypeName, VariableName. - [JP Simard](https://github.com/jpsim) - + [JP Simard](https://github.com/jpsim) ##### Bug Fixes * Trailing newline and file length violations are now displayed in Xcode. diff --git a/Source/SwiftLintFramework/Extensions/File+SwiftLint.swift b/Source/SwiftLintFramework/Extensions/File+SwiftLint.swift index 8b319309c..372f823ad 100644 --- a/Source/SwiftLintFramework/Extensions/File+SwiftLint.swift +++ b/Source/SwiftLintFramework/Extensions/File+SwiftLint.swift @@ -106,8 +106,7 @@ extension File { var maybeLine = lineIterator.next() var maybeStructure = structureIterator.next() while let line = maybeLine, let structure = maybeStructure { - if NSLocationInRange(structure.byteRange.location, line.byteRange) || - NSLocationInRange(line.byteRange.location, structure.byteRange) { + if NSLocationInRange(structure.byteRange.location, line.byteRange) { if let swiftDeclarationKind = SwiftDeclarationKind(rawValue:structure.kind) { results[line.index].append(swiftDeclarationKind) } diff --git a/Source/SwiftLintFramework/Rules/LineLengthRule.swift b/Source/SwiftLintFramework/Rules/LineLengthRule.swift index 404baa596..ace5668c7 100644 --- a/Source/SwiftLintFramework/Rules/LineLengthRule.swift +++ b/Source/SwiftLintFramework/Rules/LineLengthRule.swift @@ -9,11 +9,15 @@ import Foundation import SourceKittenFramework -public struct LineLengthRule: ConfigurationProviderRule, SourceKitFreeRule { +public struct LineLengthRule: ConfigurationProviderRule { public var configuration = LineLengthConfiguration(warning: 120, error: 200) public init() {} + private let commentKinds = Set(SyntaxKind.commentKinds()) + private let nonCommentKinds = Set(SyntaxKind.allKinds()).subtracting(Set(SyntaxKind.commentKinds())) + private let functionKinds = Set(SwiftDeclarationKind.functionKinds()) + public static let description = RuleDescription( identifier: "line_length", name: "Line Length", @@ -31,7 +35,7 @@ public struct LineLengthRule: ConfigurationProviderRule, SourceKitFreeRule { ) public func validate(file: File) -> [StyleViolation] { - let minValue = configuration.params.map({ $0.value }).min(by: <) ?? Int.max + let minValue = configuration.params.map({ $0.value }).min() ?? Int.max let swiftDeclarationKindsByLine: [[SwiftDeclarationKind]] = file.swiftDeclarationKindsByLine() ?? [] let syntaxKindsByLine: [[SyntaxKind]] = file.syntaxKindsByLine() ?? [] return file.lines.flatMap { line in @@ -43,23 +47,20 @@ public struct LineLengthRule: ConfigurationProviderRule, SourceKitFreeRule { } if configuration.ignoresFunctionDeclarations && - line.index < swiftDeclarationKindsByLine.count { - let functionKinds = swiftDeclarationKindsByLine[line.index].filter { kind in - SwiftDeclarationKind.functionKinds().contains(kind) - } - if !functionKinds.isEmpty { - return nil - } + lineHasKinds(line: line, + kinds: functionKinds, + kindsByLine: swiftDeclarationKindsByLine) { + return nil } if configuration.ignoresComments && - line.index < syntaxKindsByLine.count { - let lineCommentKinds = syntaxKindsByLine[line.index].filter { - return SyntaxKind.commentKinds().contains($0) - } - if !lineCommentKinds.isEmpty { - return nil - } + lineHasKinds(line: line, + kinds: commentKinds, + kindsByLine: syntaxKindsByLine) && + !lineHasKinds(line: line, + kinds: nonCommentKinds, + kindsByLine: syntaxKindsByLine) { + return nil } var strippedString = line.content @@ -113,8 +114,17 @@ public struct LineLengthRule: ConfigurationProviderRule, SourceKitFreeRule { return modifiedString } + private func lineHasKinds(line: Line, kinds: Set, kindsByLine: [[Kind]]) -> Bool { + let index = line.index + if index >= kindsByLine.count { + return false + } + return !kinds.intersection(Set(kindsByLine[index])).isEmpty + } + } + fileprivate extension String { var strippingURLs: String { let range = NSRange(location: 0, length: bridge().length) diff --git a/Source/SwiftLintFramework/Rules/RuleConfigurations/LineLengthConfiguration.swift b/Source/SwiftLintFramework/Rules/RuleConfigurations/LineLengthConfiguration.swift index e5d108e6f..316c37137 100644 --- a/Source/SwiftLintFramework/Rules/RuleConfigurations/LineLengthConfiguration.swift +++ b/Source/SwiftLintFramework/Rules/RuleConfigurations/LineLengthConfiguration.swift @@ -12,12 +12,30 @@ public struct LineLengthRuleOptions: OptionSet { public let rawValue: Int public init(rawValue: Int) { self.rawValue = rawValue } public init() { self.rawValue = 0 } + public static let ignoreUrls = LineLengthRuleOptions(rawValue: 1 << 0) + public static let ignoreFunctionDeclarations = LineLengthRuleOptions(rawValue: 1 << 1) + public static let ignoreComments = LineLengthRuleOptions(rawValue: 1 << 2) - static let ignoreUrls = LineLengthRuleOptions(rawValue: 1 << 0) - static let ignoreFunctionDeclarations = LineLengthRuleOptions(rawValue: 1 << 1) - static let ignoreComments = LineLengthRuleOptions(rawValue: 1 << 2) + public static let all: LineLengthRuleOptions = [.ignoreUrls, .ignoreFunctionDeclarations, .ignoreComments] +} + +fileprivate enum ConfigurationKey: String { + case warning = "warning" + case error = "error" + case ignoresURLs = "ignores_urls" + case ignoresFunctionDeclarations = "ignores_function_declarations" + case ignoresComments = "ignores_comments" + static func all() -> [ConfigurationKey] { + return [.warning, + .error, + .ignoresURLs, + .ignoresFunctionDeclarations, + .ignoresComments] + } + static func allValues() -> [String] { + return all().map{ $0.rawValue } + } - static let all: LineLengthRuleOptions = [.ignoreUrls, .ignoreFunctionDeclarations, .ignoreComments] } public struct LineLengthConfiguration: RuleConfiguration, Equatable { @@ -29,22 +47,16 @@ public struct LineLengthConfiguration: RuleConfiguration, Equatable { var ignoresURLs: Bool var ignoresFunctionDeclarations: Bool var ignoresComments: Bool - + var params: [RuleParameter] { return length.params } - public init(warning: Int, error: Int?, options: LineLengthRuleOptions? = []) { - length = SeverityLevelsConfiguration(warning: warning, error: error) - if let options = options { - self.ignoresURLs = options.contains(.ignoreUrls) - self.ignoresFunctionDeclarations = options.contains(.ignoreFunctionDeclarations) - self.ignoresComments = options.contains(.ignoreComments) - } else { - self.ignoresURLs = false - self.ignoresFunctionDeclarations = false - self.ignoresComments = false - } + public init(warning: Int, error: Int?, options: LineLengthRuleOptions = []) { + self.length = SeverityLevelsConfiguration(warning: warning, error: error) + self.ignoresURLs = options.contains(.ignoreUrls) + self.ignoresFunctionDeclarations = options.contains(.ignoreFunctionDeclarations) + self.ignoresComments = options.contains(.ignoreComments) } public mutating func apply(configuration: Any) throws { @@ -53,20 +65,26 @@ public struct LineLengthConfiguration: RuleConfiguration, Equatable { let warning = configurationArray[0] let error = (configurationArray.count > 1) ? configurationArray[1] : nil length = SeverityLevelsConfiguration(warning: warning, error: error) - } else if let configDict = configuration as? [String: Any], !configDict.isEmpty - && Set(configDict.keys).isSubset(of: ["warning", - "error", - "ignores_urls", - "ignores_function_declarations", - "ignores_comments"]) { - let warning = configDict["warning"] as? Int ?? length.warning - let error = configDict["error"] as? Int - length = SeverityLevelsConfiguration(warning: warning, error: error) - ignoresURLs = configDict["ignores_urls"] as? Bool ?? ignoresURLs - if let funcDec = - configDict["ignores_function_declarations"] as? Bool { ignoresFunctionDeclarations = funcDec + } else if let configDict = configuration as? [String: Any], !configDict.isEmpty { + for (string, value) in configDict { + guard let key = ConfigurationKey(rawValue:string) else { + throw ConfigurationError.unknownConfiguration + } + switch (key, value) { + case (.error, let intValue as Int): + length.error = intValue + case (.warning, let intValue as Int): + length.warning = intValue + case (.ignoresFunctionDeclarations, let boolValue as Bool): + ignoresFunctionDeclarations = boolValue + case (.ignoresComments, let boolValue as Bool): + ignoresComments = boolValue + case (.ignoresURLs, let boolValue as Bool): + ignoresURLs = boolValue + default: + throw ConfigurationError.unknownConfiguration + } } - ignoresComments = configDict["ignores_comments"] as? Bool ?? ignoresComments } else { throw ConfigurationError.unknownConfiguration } diff --git a/Tests/SwiftLintFrameworkTests/LineLengthConfigurationTests.swift b/Tests/SwiftLintFrameworkTests/LineLengthConfigurationTests.swift index c0d675e14..9d5df6103 100644 --- a/Tests/SwiftLintFrameworkTests/LineLengthConfigurationTests.swift +++ b/Tests/SwiftLintFrameworkTests/LineLengthConfigurationTests.swift @@ -11,20 +11,17 @@ import SourceKittenFramework import XCTest class LineLengthConfigurationTests: XCTestCase { - let allFlags = LineLengthRuleOptions.all func testLineLengthConfigurationInitializerSetsLength() { let warning = 100 let error = 150 let length1 = SeverityLevelsConfiguration(warning: warning, error: error) let configuration1 = LineLengthConfiguration(warning: warning, - error: error, - options: allFlags) + error: error) XCTAssertEqual(configuration1.length, length1) let length2 = SeverityLevelsConfiguration(warning: warning, error: nil) let configuration2 = LineLengthConfiguration(warning: warning, - error: nil, - options: allFlags) + error: nil) XCTAssertEqual(configuration2.length, length2) } @@ -36,17 +33,41 @@ class LineLengthConfigurationTests: XCTestCase { XCTAssertTrue(configuration1.ignoresURLs) let configuration2 = LineLengthConfiguration(warning: 100, - error: 150, - options: nil) + error: 150) XCTAssertFalse(configuration2.ignoresURLs) } + + func testLineLengthConfigurationInitialiserSetsIgnoresFunctionDeclarations() { + let configuration1 = LineLengthConfiguration(warning: 100, + error: 150, + options: [.ignoreFunctionDeclarations]) + + XCTAssertTrue(configuration1.ignoresFunctionDeclarations) + + let configuration2 = LineLengthConfiguration(warning: 100, + error: 150) + XCTAssertFalse(configuration2.ignoresFunctionDeclarations) + } + + func testLineLengthConfigurationInitialiserSetsIgnoresComments() { + let configuration1 = LineLengthConfiguration(warning: 100, + error: 150, + options: [.ignoreComments]) + + XCTAssertTrue(configuration1.ignoresComments) + + let configuration2 = LineLengthConfiguration(warning: 100, + error: 150) + XCTAssertFalse(configuration2.ignoresComments) + } + + func testLineLengthConfigurationParams() { let warning = 13 let error = 10 let configuration = LineLengthConfiguration(warning: warning, - error: error, - options: [.ignoreFunctionDeclarations]) + error: error) let params = [RuleParameter(severity: .error, value: error), RuleParameter(severity: .warning, value: warning)] XCTAssertEqual(configuration.params, params) } @@ -54,21 +75,36 @@ class LineLengthConfigurationTests: XCTestCase { func testLineLengthConfigurationPartialParams() { let warning = 13 let configuration = LineLengthConfiguration(warning: warning, - error: nil, - options: [.ignoreFunctionDeclarations]) + error: nil) XCTAssertEqual(configuration.params, [RuleParameter(severity: .warning, value: 13)]) } func testLineLengthConfigurationThrowsOnBadConfig() { let config = "unknown" - var configuration = LineLengthConfiguration(warning: 100, error: 150, options: allFlags) + var configuration = LineLengthConfiguration(warning: 100, error: 150) checkError(ConfigurationError.unknownConfiguration) { try configuration.apply(configuration: config) } } + + func testLineLengthConfigurationThrowsOnBadConfigValues() { + let badConfigs: [[String: Any]] = [ + ["warning": true], + ["ignores_function_declarations": 300], + ["unsupported_key": "unsupported key is unsupported"] + ] + + for badConfig in badConfigs { + var configuration = LineLengthConfiguration(warning: 100, error: 150) + checkError(ConfigurationError.unknownConfiguration) { + try configuration.apply(configuration: badConfig) + } + } + } + func testLineLengthConfigurationApplyConfigurationWithArray() { - var configuration = LineLengthConfiguration(warning: 0, error: 0, options: nil) + var configuration = LineLengthConfiguration(warning: 0, error: 0) let warning1 = 100 let error1 = 100 @@ -107,7 +143,7 @@ class LineLengthConfigurationTests: XCTestCase { let length2 = SeverityLevelsConfiguration(warning: warning2, error: error2) let config2: [String: Int] = ["warning": warning2, "error": error2] - let length3 = SeverityLevelsConfiguration(warning: warning2, error: nil) + let length3 = SeverityLevelsConfiguration(warning: warning2, error: error2) let config3: [String: Bool] = ["ignores_urls": false, "ignores_function_declarations": false, "ignores_comments": false] @@ -136,20 +172,20 @@ class LineLengthConfigurationTests: XCTestCase { } func testLineLengthConfigurationCompares() { - let configuration1 = LineLengthConfiguration(warning: 100, error: 100, options: allFlags) + let configuration1 = LineLengthConfiguration(warning: 100, error: 100) let configuration2 = LineLengthConfiguration(warning: 100, error: 100, options: [.ignoreFunctionDeclarations, .ignoreComments]) XCTAssertFalse(configuration1 == configuration2) - let configuration3 = LineLengthConfiguration(warning: 100, error: 200, options: allFlags) + let configuration3 = LineLengthConfiguration(warning: 100, error: 200) XCTAssertFalse(configuration1 == configuration3) - let configuration4 = LineLengthConfiguration(warning: 200, error: 100, options: allFlags) + let configuration4 = LineLengthConfiguration(warning: 200, error: 100) XCTAssertFalse(configuration1 == configuration4) - let configuration5 = LineLengthConfiguration(warning: 100, error: 100, options: allFlags) + let configuration5 = LineLengthConfiguration(warning: 100, error: 100) XCTAssertTrue(configuration1 == configuration5) let configuration6 = LineLengthConfiguration(warning: 100, diff --git a/Tests/SwiftLintFrameworkTests/LineLengthRuleTests.swift b/Tests/SwiftLintFrameworkTests/LineLengthRuleTests.swift index 7e7c80dc8..2343f5148 100644 --- a/Tests/SwiftLintFrameworkTests/LineLengthRuleTests.swift +++ b/Tests/SwiftLintFrameworkTests/LineLengthRuleTests.swift @@ -12,20 +12,20 @@ import XCTest class LineLengthRuleTests: XCTestCase { - let longFunctionDeclaration = "public func superDuperLongFunctionDeclaration(a: String, b: String, " + + private let longFunctionDeclaration = "public func superDuperLongFunctionDeclaration(a: String, b: String, " + "c: String, d: String, e: String, f: String, g: String, h: String, i: String, " + "j: String, k: String, l: String, m: String, n: String, o: String, p: String, " + "q: String, r: String, s: String, t: String, u: String, v: String, w: String, " + "x: String, y: String, z: String) {\n" - let longComment = String(repeating: "/", count: 121) + "\n" - let longBlockComment = "/*" + String(repeating: " ", count: 121) + "*/\n" - + private let longComment = String(repeating: "/", count: 121) + "\n" + private let longBlockComment = "/*" + String(repeating: " ", count: 121) + "*/\n" + private let declarationWithTrailingLongComment = "let foo = 1 " + String(repeating: "/", count: 121) + "\n" func testLineLength() { verifyRule(LineLengthRule.description, commentDoesntViolate: false, stringDoesntViolate: false) } - func testLineLengthWithIgnoreFunctionDeclaraionsEnabled() { + func testLineLengthWithIgnoreFunctionDeclarationsEnabled() { let baseDescription = LineLengthRule.description let triggeringExamples = baseDescription.triggeringExamples let nonTriggeringExamples = baseDescription.nonTriggeringExamples + [longFunctionDeclaration] @@ -41,7 +41,7 @@ class LineLengthRuleTests: XCTestCase { func testLineLengthWithIgnoreCommentsEnabled() { let baseDescription = LineLengthRule.description - let triggeringExamples = [longFunctionDeclaration] + let triggeringExamples = [longFunctionDeclaration, declarationWithTrailingLongComment] let nonTriggeringExamples = [longComment, longBlockComment] let description = RuleDescription(identifier: baseDescription.identifier, name: baseDescription.name,