Changes from PR feedback. Long comments following code in a line will now trigger, configuration will now fail if invalud value types are set for options

This commit is contained in:
Michael Welles
2017-02-06 15:33:44 -05:00
parent d5c86936e4
commit c9ff0aa07f
6 changed files with 137 additions and 75 deletions
+3 -4
View File
@@ -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.
@@ -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)
}
@@ -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<Kind>(line: Line, kinds: Set<Kind>, 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)
@@ -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<Int>] {
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
}
@@ -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,
@@ -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,