From 27aa8e338381179e61986a98aa1509908a63d7eb Mon Sep 17 00:00:00 2001 From: Cristian Filipov Date: Thu, 18 Aug 2016 16:44:21 -0700 Subject: [PATCH 01/27] Use path arg as root for include/exclude paths --- .../Extensions/NSFileManager+SwiftLint.swift | 4 ++-- Source/SwiftLintFramework/Models/Configuration.swift | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Source/SwiftLintFramework/Extensions/NSFileManager+SwiftLint.swift b/Source/SwiftLintFramework/Extensions/NSFileManager+SwiftLint.swift index effe133030..9d47da2ff0 100644 --- a/Source/SwiftLintFramework/Extensions/NSFileManager+SwiftLint.swift +++ b/Source/SwiftLintFramework/Extensions/NSFileManager+SwiftLint.swift @@ -9,8 +9,8 @@ import Foundation extension NSFileManager { - internal func filesToLintAtPath(path: String) -> [String] { - let absolutePath = path.absolutePathStandardized() + internal func filesToLintAtPath(path: String, rootDirectory: String? = nil) -> [String] { + let absolutePath = (path.absolutePathRepresentation(rootDirectory ?? NSFileManager.defaultManager().currentDirectoryPath) as NSString).stringByStandardizingPath var isDirectory: ObjCBool = false guard fileExistsAtPath(absolutePath, isDirectory: &isDirectory) else { return [] diff --git a/Source/SwiftLintFramework/Models/Configuration.swift b/Source/SwiftLintFramework/Models/Configuration.swift index 504071b416..809e3ba721 100644 --- a/Source/SwiftLintFramework/Models/Configuration.swift +++ b/Source/SwiftLintFramework/Models/Configuration.swift @@ -181,8 +181,8 @@ public struct Configuration: Equatable { public func lintablePathsForPath(path: String, fileManager: NSFileManager = fileManager) -> [String] { let pathsForPath = included.isEmpty ? fileManager.filesToLintAtPath(path) : [] - let excludedPaths = excluded.flatMap(fileManager.filesToLintAtPath) - let includedPaths = included.flatMap(fileManager.filesToLintAtPath) + let excludedPaths = excluded.flatMap{fileManager.filesToLintAtPath($0, rootDirectory: self.rootPath)} + let includedPaths = included.flatMap{fileManager.filesToLintAtPath($0, rootDirectory: self.rootPath)} return (pathsForPath + includedPaths).filter({ !excludedPaths.contains($0) }) } From ea592e0e98dc67b702563e7dd73cb6f9fcaad6f3 Mon Sep 17 00:00:00 2001 From: Cristian Filipov Date: Thu, 18 Aug 2016 17:17:03 -0700 Subject: [PATCH 02/27] Add rule to check for private unit tests --- .../Models/MasterRuleList.swift | 1 + .../Rules/MissingDocsRule.swift | 16 +- .../Rules/PrivateUnitTestRule.swift | 149 ++++++++++++++++++ .../PrivateUnitTestConfiguration.swift | 67 ++++++++ SwiftLint.xcodeproj/project.pbxproj | 10 +- Tests/SwiftLintFramework/RulesTests.swift | 4 + 6 files changed, 234 insertions(+), 13 deletions(-) create mode 100644 Source/SwiftLintFramework/Rules/PrivateUnitTestRule.swift create mode 100644 Source/SwiftLintFramework/Rules/RuleConfigurations/PrivateUnitTestConfiguration.swift diff --git a/Source/SwiftLintFramework/Models/MasterRuleList.swift b/Source/SwiftLintFramework/Models/MasterRuleList.swift index 2414c3e7a2..2faea4009f 100644 --- a/Source/SwiftLintFramework/Models/MasterRuleList.swift +++ b/Source/SwiftLintFramework/Models/MasterRuleList.swift @@ -64,6 +64,7 @@ public let masterRuleList = RuleList(rules: NestingRule.self, OpeningBraceRule.self, OperatorFunctionWhitespaceRule.self, + PrivateUnitTestRule.self, ReturnArrowWhitespaceRule.self, StatementPositionRule.self, TodoRule.self, diff --git a/Source/SwiftLintFramework/Rules/MissingDocsRule.swift b/Source/SwiftLintFramework/Rules/MissingDocsRule.swift index ca68266d31..86d7b339a1 100644 --- a/Source/SwiftLintFramework/Rules/MissingDocsRule.swift +++ b/Source/SwiftLintFramework/Rules/MissingDocsRule.swift @@ -43,7 +43,7 @@ extension File { guard let _ = (dictionary["key.kind"] as? String).flatMap(SwiftDeclarationKind.init), offset = dictionary["key.offset"] as? Int64, accessibility = dictionary["key.accessibility"] as? String - where acl.map({ $0.sourcekitValue() }).contains(accessibility) else { + where acl.map({ $0.rawValue }).contains(accessibility) else { return substructureOffsets } if getDocumentationCommentBody(dictionary, syntaxMap: syntaxMap) != nil { @@ -54,17 +54,9 @@ extension File { } public enum AccessControlLevel: String { - case Private = "private" - case Internal = "internal" - case Public = "public" - - private func sourcekitValue() -> String { - switch self { - case Private: return "source.lang.swift.accessibility.private" - case Internal: return "source.lang.swift.accessibility.internal" - case Public: return "source.lang.swift.accessibility.public" - } - } + case Private = "source.lang.swift.accessibility.private" + case Internal = "source.lang.swift.accessibility.internal" + case Public = "source.lang.swift.accessibility.public" } public struct MissingDocsRule: OptInRule { diff --git a/Source/SwiftLintFramework/Rules/PrivateUnitTestRule.swift b/Source/SwiftLintFramework/Rules/PrivateUnitTestRule.swift new file mode 100644 index 0000000000..078f0d9c6e --- /dev/null +++ b/Source/SwiftLintFramework/Rules/PrivateUnitTestRule.swift @@ -0,0 +1,149 @@ +// +// ClassVisibilityRule.swift +// SwiftLint +// +// Created by Cristian Filipov on 8/3/16. +// Copyright © 2016 Realm. All rights reserved. +// + +import Foundation +import SourceKittenFramework + +public extension AccessControlLevel { + init?(_ dictionary: [String: SourceKitRepresentable]) { + guard let + accessibility = dictionary["key.accessibility"] as? String, + acl = AccessControlLevel(rawValue: accessibility) + else { return nil } + self = acl + } +} + +public func superclass(dictionary: [String: SourceKitRepresentable]) -> String? { + typealias SKArray = [SourceKitRepresentable] + typealias SKDict = [String: SourceKitRepresentable] + guard let + kindString = dictionary["key.kind"] as? String, + kind = SwiftDeclarationKind(rawValue: kindString) + where kind == .Class + else { return nil } + guard let + inheritedTypes = dictionary["key.inheritedtypes"] as? SKArray, + className = (inheritedTypes[0] as? SKDict)?["key.name"] as? String + else { return nil } + return className +} + +public struct PrivateUnitTestRule: ASTRule, ConfigurationProviderRule { + + public var configuration: PrivateUnitTestConfiguration = { + var configuration = PrivateUnitTestConfiguration(identifier: "private_unit_test") + configuration.message = "Unit test marked `private` will be silently skipped." + configuration.regex = regex("XCTestCase") + return configuration + }() + + public init() {} + + public static let description = RuleDescription( + identifier: "private_unit_test", + name: "Private Unit Test", + description: "Unit tests marked private are silently skipped.", + nonTriggeringExamples: [ + "func testFoo() {}", + "internal func testFoo() {}", + "public func testFoo() {}", + "class FooTest: XCTestCase {}", + "internal class FooTest: XCTestCase {}", + "public class FooTest: XCTestCase {}" + ], + triggeringExamples: [ + "private func testFoo() {}", + "private class FooTest: XCTestCase {}", + ] + ) + + public func validateFile( + file: File, + kind: SwiftDeclarationKind, + dictionary: [String: SourceKitRepresentable]) + -> [StyleViolation] { + + guard kind == .Class else { return [] } + let classViolations = validateClass(file, kind: kind, dictionary: dictionary) + guard classViolations.isEmpty else { return classViolations } + + let substructure = dictionary["key.substructure"] as? [SourceKitRepresentable] ?? [] + return substructure.flatMap { subItem -> [StyleViolation] in + guard + let subDict = subItem as? [String: SourceKitRepresentable], + kindString = subDict["key.kind"] as? String, + kind = KindType(rawValue: kindString) + where kind == .FunctionMethodInstance + else { return [] } + return self.validateFunction(file, kind: kind, dictionary: subDict) + } + + } + + /* It's not strictly necessary to check for `private` on classes because a + private class will result in `private` on all its members in the AST. + However, it's still useful to check the class explicitly because this + gives us a more clear error message. If we check only methods, the line + number of the error will be that of the function, which may not + necessarily be marked `private` but inhereted it from the class access + modifier. By checking the class we ensure the line nuber we report for + the violation will match the line that must be edited. + */ + private func validateClass( + file: File, + kind: SwiftDeclarationKind, + dictionary: [String: SourceKitRepresentable]) + -> [StyleViolation] { + + assert(kind == .Class) + guard let superclass = superclass(dictionary) else { return [] } + let pathMatch = configuration.regex.matchesInString( + superclass, + options: [], + range: NSRange(location: 0, length: (superclass as NSString).length)) + guard !pathMatch.isEmpty else { return [] } + return validateAccessControlLevel(file, dictionary: dictionary) + + } + + private func validateFunction( + file: File, + kind: SwiftDeclarationKind, + dictionary: [String: SourceKitRepresentable]) + -> [StyleViolation] { + + assert(kind == .FunctionMethodInstance) + guard + let name = dictionary["key.name"] as? NSString + where name.hasPrefix("test") + else { return [] } + return validateAccessControlLevel(file, dictionary: dictionary) + + } + + private func validateAccessControlLevel( + file: File, + dictionary: [String: SourceKitRepresentable]) + -> [StyleViolation] { + + guard let acl = AccessControlLevel(dictionary) else { return [] } + switch acl { + case .Private: + let nameOffset = Int(dictionary["key.nameoffset"] as? Int64 ?? 0) + return [StyleViolation( + ruleDescription: self.dynamicType.description, + severity: configuration.severityConfiguration.severity, + location: Location(file: file, byteOffset: nameOffset), + reason: configuration.message)] + default: return [] + } + + } + +} diff --git a/Source/SwiftLintFramework/Rules/RuleConfigurations/PrivateUnitTestConfiguration.swift b/Source/SwiftLintFramework/Rules/RuleConfigurations/PrivateUnitTestConfiguration.swift new file mode 100644 index 0000000000..151b2b1ae0 --- /dev/null +++ b/Source/SwiftLintFramework/Rules/RuleConfigurations/PrivateUnitTestConfiguration.swift @@ -0,0 +1,67 @@ +// +// PrivateUnitTestConfiguration.swift +// SwiftLint +// +// Created by Cristian Filipov on 8/5/16. +// Copyright © 2016 Realm. All rights reserved. +// + +import Foundation +import SourceKittenFramework + +public struct PrivateUnitTestConfiguration: RuleConfiguration, Equatable { + public let identifier: String + public var name: String? + public var message = "Regex matched." + public var regex = NSRegularExpression() + public var included = NSRegularExpression() + public var severityConfiguration = SeverityConfiguration(.Warning) + + public var severity: ViolationSeverity { + return severityConfiguration.severity + } + + public var consoleDescription: String { + return "\(severity.rawValue.lowercaseString): \(regex.pattern)" + } + + public var description: RuleDescription { + return RuleDescription(identifier: identifier, + name: name ?? identifier, + description: "") + } + + public init(identifier: String) { + self.identifier = identifier + } + + public mutating func applyConfiguration(configuration: AnyObject) throws { + guard let configurationDict = configuration as? [String: AnyObject], + regexString = configurationDict["regex"] as? String else { + throw ConfigurationError.UnknownConfiguration + } + + regex = try NSRegularExpression.cached(pattern: regexString) + + if let includedString = configurationDict["included"] as? String { + included = try NSRegularExpression.cached(pattern: includedString) + } + if let name = configurationDict["name"] as? String { + self.name = name + } + if let message = configurationDict["message"] as? String { + self.message = message + } + if let severityString = configurationDict["severity"] as? String { + try severityConfiguration.applyConfiguration(severityString) + } + } +} + +public func == (lhs: PrivateUnitTestConfiguration, rhs: PrivateUnitTestConfiguration) -> Bool { + return lhs.identifier == rhs.identifier && + lhs.message == rhs.message && + lhs.regex == rhs.regex && + lhs.included.pattern == rhs.included.pattern && + lhs.severity == rhs.severity +} diff --git a/SwiftLint.xcodeproj/project.pbxproj b/SwiftLint.xcodeproj/project.pbxproj index b89854b471..db8225b90f 100644 --- a/SwiftLint.xcodeproj/project.pbxproj +++ b/SwiftLint.xcodeproj/project.pbxproj @@ -52,6 +52,8 @@ 7250948A1D0859260039B353 /* StatementPositionConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = 725094881D0855760039B353 /* StatementPositionConfiguration.swift */; }; 83894F221B0C928A006214E1 /* RulesCommand.swift in Sources */ = {isa = PBXBuildFile; fileRef = 83894F211B0C928A006214E1 /* RulesCommand.swift */; }; 83D71E281B131ECE000395DE /* RuleDescription.swift in Sources */ = {isa = PBXBuildFile; fileRef = 83D71E261B131EB5000395DE /* RuleDescription.swift */; }; + B2902A0C1D66815600BFCCF7 /* PrivateUnitTestRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = B2902A0B1D66815600BFCCF7 /* PrivateUnitTestRule.swift */; }; + B2902A0E1D6681F700BFCCF7 /* PrivateUnitTestConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = B2902A0D1D6681F700BFCCF7 /* PrivateUnitTestConfiguration.swift */; }; B58AEED61C492C7B00E901FD /* ForceUnwrappingRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = B58AEED51C492C7B00E901FD /* ForceUnwrappingRule.swift */; }; BFF028AE1CBCF8A500B38A9D /* TrailingWhitespaceConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = BF48D2D61CBCCA5F0080BDAE /* TrailingWhitespaceConfiguration.swift */; }; D0AAAB5019FB0960007B24B3 /* SwiftLintFramework.framework in Embed Frameworks */ = {isa = PBXBuildFile; fileRef = D0D1216D19E87B05005E4BAA /* SwiftLintFramework.framework */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; }; @@ -213,6 +215,8 @@ 725094881D0855760039B353 /* StatementPositionConfiguration.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = StatementPositionConfiguration.swift; sourceTree = ""; }; 83894F211B0C928A006214E1 /* RulesCommand.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = RulesCommand.swift; sourceTree = ""; }; 83D71E261B131EB5000395DE /* RuleDescription.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = RuleDescription.swift; sourceTree = ""; }; + B2902A0B1D66815600BFCCF7 /* PrivateUnitTestRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = PrivateUnitTestRule.swift; sourceTree = ""; }; + B2902A0D1D6681F700BFCCF7 /* PrivateUnitTestConfiguration.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = PrivateUnitTestConfiguration.swift; sourceTree = ""; }; B58AEED51C492C7B00E901FD /* ForceUnwrappingRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ForceUnwrappingRule.swift; sourceTree = ""; }; BF48D2D61CBCCA5F0080BDAE /* TrailingWhitespaceConfiguration.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = TrailingWhitespaceConfiguration.swift; sourceTree = ""; }; D0D1211B19E87861005E4BAA /* main.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = main.swift; sourceTree = ""; usesTabs = 0; }; @@ -352,11 +356,12 @@ isa = PBXGroup; children = ( 3BCC04D01C4F56D3006073C3 /* NameConfiguration.swift */, + B2902A0D1D6681F700BFCCF7 /* PrivateUnitTestConfiguration.swift */, 3BB47D821C514E8100AE6A10 /* RegexConfiguration.swift */, 3B0B14531C505D6300BE82F7 /* SeverityConfiguration.swift */, 3BCC04CF1C4F56D3006073C3 /* SeverityLevelsConfiguration.swift */, - BF48D2D61CBCCA5F0080BDAE /* TrailingWhitespaceConfiguration.swift */, 725094881D0855760039B353 /* StatementPositionConfiguration.swift */, + BF48D2D61CBCCA5F0080BDAE /* TrailingWhitespaceConfiguration.swift */, ); path = RuleConfigurations; sourceTree = ""; @@ -599,6 +604,7 @@ E88DEA951B099CF200A66CB0 /* NestingRule.swift */, 692B1EB11BD7E00F00EAABFF /* OpeningBraceRule.swift */, E5A167C81B25A0B000CF2D03 /* OperatorFunctionWhitespaceRule.swift */, + B2902A0B1D66815600BFCCF7 /* PrivateUnitTestRule.swift */, E57B23C01B1D8BF000DEA512 /* ReturnArrowWhitespaceRule.swift */, 3BCC04CE1C4F56D3006073C3 /* RuleConfigurations */, 692B60AB1BD8F2E700C7AA22 /* StatementPositionRule.swift */, @@ -914,6 +920,7 @@ E86396C91BADB2B9002C9E88 /* JSONReporter.swift in Sources */, E881985A1BEA96EA00333A11 /* OperatorFunctionWhitespaceRule.swift in Sources */, 3BCC04D21C4F56D3006073C3 /* NameConfiguration.swift in Sources */, + B2902A0E1D6681F700BFCCF7 /* PrivateUnitTestConfiguration.swift in Sources */, E88DEA6F1B09843F00A66CB0 /* Location.swift in Sources */, E88DEA771B098D0C00A66CB0 /* Rule.swift in Sources */, 7250948A1D0859260039B353 /* StatementPositionConfiguration.swift in Sources */, @@ -924,6 +931,7 @@ E881985B1BEA974E00333A11 /* StatementPositionRule.swift in Sources */, B58AEED61C492C7B00E901FD /* ForceUnwrappingRule.swift in Sources */, E88DEA711B09847500A66CB0 /* ViolationSeverity.swift in Sources */, + B2902A0C1D66815600BFCCF7 /* PrivateUnitTestRule.swift in Sources */, 3BD9CD3D1C37175B009A5D25 /* YamlParser.swift in Sources */, F22314B01D4FA4D7009AD165 /* LegacyNSGeometryFunctionsRule.swift in Sources */, E88DEA8C1B0999A000A66CB0 /* ASTRule.swift in Sources */, diff --git a/Tests/SwiftLintFramework/RulesTests.swift b/Tests/SwiftLintFramework/RulesTests.swift index 7933ac3935..f13ad93d25 100644 --- a/Tests/SwiftLintFramework/RulesTests.swift +++ b/Tests/SwiftLintFramework/RulesTests.swift @@ -170,6 +170,10 @@ class RulesTests: XCTestCase { verifyRule(OperatorFunctionWhitespaceRule.description) } + func testPrivateUnitTest() { + verifyRule(PrivateUnitTestRule.description) + } + func testReturnArrowWhitespace() { verifyRule(ReturnArrowWhitespaceRule.description) } From 0a684da88539a8305827223b23dec994034e4233 Mon Sep 17 00:00:00 2001 From: Cristian Filipov Date: Thu, 18 Aug 2016 22:36:06 -0700 Subject: [PATCH 03/27] Fix rule description --- .../Rules/PrivateUnitTestRule.swift | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/Source/SwiftLintFramework/Rules/PrivateUnitTestRule.swift b/Source/SwiftLintFramework/Rules/PrivateUnitTestRule.swift index 078f0d9c6e..d0fb7412c1 100644 --- a/Source/SwiftLintFramework/Rules/PrivateUnitTestRule.swift +++ b/Source/SwiftLintFramework/Rules/PrivateUnitTestRule.swift @@ -9,7 +9,7 @@ import Foundation import SourceKittenFramework -public extension AccessControlLevel { +extension AccessControlLevel { init?(_ dictionary: [String: SourceKitRepresentable]) { guard let accessibility = dictionary["key.accessibility"] as? String, @@ -19,7 +19,7 @@ public extension AccessControlLevel { } } -public func superclass(dictionary: [String: SourceKitRepresentable]) -> String? { +func superclass(dictionary: [String: SourceKitRepresentable]) -> String? { typealias SKArray = [SourceKitRepresentable] typealias SKDict = [String: SourceKitRepresentable] guard let @@ -34,6 +34,8 @@ public func superclass(dictionary: [String: SourceKitRepresentable]) -> String? return className } +public class FooTest: NSObject { } + public struct PrivateUnitTestRule: ASTRule, ConfigurationProviderRule { public var configuration: PrivateUnitTestConfiguration = { @@ -50,16 +52,15 @@ public struct PrivateUnitTestRule: ASTRule, ConfigurationProviderRule { name: "Private Unit Test", description: "Unit tests marked private are silently skipped.", nonTriggeringExamples: [ - "func testFoo() {}", - "internal func testFoo() {}", - "public func testFoo() {}", - "class FooTest: XCTestCase {}", - "internal class FooTest: XCTestCase {}", - "public class FooTest: XCTestCase {}" + "class FooTest: XCTestCase { func test1() {}; internal func test2() {}; public func test3() {}; }", + "internal class FooTest: XCTestCase { func test1() {}; internal func test2() {}; public func test3() {}; }", + "public class FooTest: XCTestCase { func test1() {}; internal func test2() {}; public func test3() {}; }" ], triggeringExamples: [ - "private func testFoo() {}", - "private class FooTest: XCTestCase {}", + "private ↓class FooTest: XCTestCase { func test1() {}; internal func test2() {}; public func test3() {}; private func test4() {}; }", + "class FooTest: XCTestCase { func test1() {}; internal func test2() {}; public func test3() {}; private ↓func test4() {}; }", + "internal class FooTest: XCTestCase { func test1() {}; internal func test2() {}; public func test3() {}; private ↓func test4() {}; }", + "public class FooTest: XCTestCase { func test1() {}; internal func test2() {}; public func test3() {}; private ↓func test4() {}; }", ] ) @@ -135,11 +136,11 @@ public struct PrivateUnitTestRule: ASTRule, ConfigurationProviderRule { guard let acl = AccessControlLevel(dictionary) else { return [] } switch acl { case .Private: - let nameOffset = Int(dictionary["key.nameoffset"] as? Int64 ?? 0) + let offset = Int(dictionary["key.offset"] as? Int64 ?? 0) return [StyleViolation( ruleDescription: self.dynamicType.description, severity: configuration.severityConfiguration.severity, - location: Location(file: file, byteOffset: nameOffset), + location: Location(file: file, byteOffset: offset), reason: configuration.message)] default: return [] } From 809ca17e001abf04e403da41d6234d591c03568d Mon Sep 17 00:00:00 2001 From: Cristian Filipov Date: Thu, 18 Aug 2016 22:50:07 -0700 Subject: [PATCH 04/27] Make lines <=100 characters --- .../Rules/PrivateUnitTestRule.swift | 46 ++++++++++++++++--- 1 file changed, 39 insertions(+), 7 deletions(-) diff --git a/Source/SwiftLintFramework/Rules/PrivateUnitTestRule.swift b/Source/SwiftLintFramework/Rules/PrivateUnitTestRule.swift index d0fb7412c1..6f86a532f4 100644 --- a/Source/SwiftLintFramework/Rules/PrivateUnitTestRule.swift +++ b/Source/SwiftLintFramework/Rules/PrivateUnitTestRule.swift @@ -52,15 +52,47 @@ public struct PrivateUnitTestRule: ASTRule, ConfigurationProviderRule { name: "Private Unit Test", description: "Unit tests marked private are silently skipped.", nonTriggeringExamples: [ - "class FooTest: XCTestCase { func test1() {}; internal func test2() {}; public func test3() {}; }", - "internal class FooTest: XCTestCase { func test1() {}; internal func test2() {}; public func test3() {}; }", - "public class FooTest: XCTestCase { func test1() {}; internal func test2() {}; public func test3() {}; }" + "class FooTest: XCTestCase { " + + "func test1() {}; " + + "internal func test2() {}; " + + "public func test3() {}; " + + "}", + "internal class FooTest: XCTestCase { " + + "func test1() {}; " + + "internal func test2() {}; " + + "public func test3() {}; " + + "}", + "public class FooTest: XCTestCase { " + + "func test1() {}; " + + "internal func test2() {}; " + + "public func test3() {}; " + + "}" ], triggeringExamples: [ - "private ↓class FooTest: XCTestCase { func test1() {}; internal func test2() {}; public func test3() {}; private func test4() {}; }", - "class FooTest: XCTestCase { func test1() {}; internal func test2() {}; public func test3() {}; private ↓func test4() {}; }", - "internal class FooTest: XCTestCase { func test1() {}; internal func test2() {}; public func test3() {}; private ↓func test4() {}; }", - "public class FooTest: XCTestCase { func test1() {}; internal func test2() {}; public func test3() {}; private ↓func test4() {}; }", + "private ↓class FooTest: XCTestCase { " + + "func test1() {}; " + + "internal func test2() {}; " + + "public func test3() {}; " + + "private func test4() {}; " + + "}", + "class FooTest: XCTestCase { " + + "func test1() {}; " + + "internal func test2() {}; " + + "public func test3() {}; " + + "private ↓func test4() {}; " + + "}", + "internal class FooTest: XCTestCase { " + + "func test1() {}; " + + "internal func test2() {}; " + + "public func test3() {}; " + + "private ↓func test4() {}; " + + "}", + "public class FooTest: XCTestCase { " + + "func test1() {}; " + + "internal func test2() {}; " + + "public func test3() {}; " + + "private ↓func test4() {}; " + + "}", ] ) From 1970c74c5782390b4091c317d35cf422724e4f61 Mon Sep 17 00:00:00 2001 From: Cristian Filipov Date: Sat, 20 Aug 2016 20:43:05 -0700 Subject: [PATCH 05/27] Update CHANGELOG and a couple minor changes --- CHANGELOG.md | 4 ++ .../Rules/PrivateUnitTestRule.swift | 54 +++++++++---------- 2 files changed, 31 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c6f22324b9..9ccceb2377 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,10 @@ [bootstraponline](https://github.com/bootstraponline) [#689](https://github.com/realm/SwiftLint/issues/689) +* Add rule to check for private unit tests (private unit tests don't get run by +XCTest). + [Cristian Filipov](https://github.com/cfilipov) + ##### Bug Fixes * Fix LegacyConstructorRule when using variables instead of numbers. diff --git a/Source/SwiftLintFramework/Rules/PrivateUnitTestRule.swift b/Source/SwiftLintFramework/Rules/PrivateUnitTestRule.swift index 6f86a532f4..aa80cf71b8 100644 --- a/Source/SwiftLintFramework/Rules/PrivateUnitTestRule.swift +++ b/Source/SwiftLintFramework/Rules/PrivateUnitTestRule.swift @@ -9,7 +9,7 @@ import Foundation import SourceKittenFramework -extension AccessControlLevel { +private extension AccessControlLevel { init?(_ dictionary: [String: SourceKitRepresentable]) { guard let accessibility = dictionary["key.accessibility"] as? String, @@ -40,7 +40,7 @@ public struct PrivateUnitTestRule: ASTRule, ConfigurationProviderRule { public var configuration: PrivateUnitTestConfiguration = { var configuration = PrivateUnitTestConfiguration(identifier: "private_unit_test") - configuration.message = "Unit test marked `private` will be silently skipped." + configuration.message = "Unit test marked `private` will not be run by XCTest." configuration.regex = regex("XCTestCase") return configuration }() @@ -53,45 +53,45 @@ public struct PrivateUnitTestRule: ASTRule, ConfigurationProviderRule { description: "Unit tests marked private are silently skipped.", nonTriggeringExamples: [ "class FooTest: XCTestCase { " + - "func test1() {}; " + - "internal func test2() {}; " + - "public func test3() {}; " + + "func test1() {}\n " + + "internal func test2() {}\n " + + "public func test3() {}\n " + "}", "internal class FooTest: XCTestCase { " + - "func test1() {}; " + - "internal func test2() {}; " + - "public func test3() {}; " + + "func test1() {}\n " + + "internal func test2() {}\n " + + "public func test3() {}\n " + "}", "public class FooTest: XCTestCase { " + - "func test1() {}; " + - "internal func test2() {}; " + - "public func test3() {}; " + + "func test1() {}\n " + + "internal func test2() {}\n " + + "public func test3() {}\n " + "}" ], triggeringExamples: [ "private ↓class FooTest: XCTestCase { " + - "func test1() {}; " + - "internal func test2() {}; " + - "public func test3() {}; " + - "private func test4() {}; " + + "func test1() {}\n " + + "internal func test2() {}\n " + + "public func test3() {}\n " + + "private func test4() {}\n " + "}", "class FooTest: XCTestCase { " + - "func test1() {}; " + - "internal func test2() {}; " + - "public func test3() {}; " + - "private ↓func test4() {}; " + + "func test1() {}\n " + + "internal func test2() {}\n " + + "public func test3() {}\n " + + "private ↓func test4() {}\n " + "}", "internal class FooTest: XCTestCase { " + - "func test1() {}; " + - "internal func test2() {}; " + - "public func test3() {}; " + - "private ↓func test4() {}; " + + "func test1() {}\n " + + "internal func test2() {}\n " + + "public func test3() {}\n " + + "private ↓func test4() {}\n " + "}", "public class FooTest: XCTestCase { " + - "func test1() {}; " + - "internal func test2() {}; " + - "public func test3() {}; " + - "private ↓func test4() {}; " + + "func test1() {}\n " + + "internal func test2() {}\n " + + "public func test3() {}\n " + + "private ↓func test4() {}\n " + "}", ] ) From dc3c41e627aaafd19c299e196fe8fbf95c022798 Mon Sep 17 00:00:00 2001 From: Cristian Filipov Date: Sun, 21 Aug 2016 22:53:56 -0700 Subject: [PATCH 06/27] Add `description` to AccessControlLevel --- .../Rules/MissingDocsRule.swift | 21 +++++++++++++++++-- .../Rules/PrivateUnitTestRule.swift | 2 +- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/Source/SwiftLintFramework/Rules/MissingDocsRule.swift b/Source/SwiftLintFramework/Rules/MissingDocsRule.swift index 86d7b339a1..46276523cc 100644 --- a/Source/SwiftLintFramework/Rules/MissingDocsRule.swift +++ b/Source/SwiftLintFramework/Rules/MissingDocsRule.swift @@ -53,10 +53,27 @@ extension File { } } -public enum AccessControlLevel: String { +public enum AccessControlLevel: String, CustomStringConvertible { case Private = "source.lang.swift.accessibility.private" case Internal = "source.lang.swift.accessibility.internal" case Public = "source.lang.swift.accessibility.public" + + internal init?(description value: String) { + switch value { + case "private": self = .Private + case "internal": self = .Internal + case "public": self = .Public + default: return nil + } + } + + public var description: String { + switch self { + case Private: return "private" + case Internal: return "internal" + case Public: return "public" + } + } } public struct MissingDocsRule: OptInRule { @@ -64,7 +81,7 @@ public struct MissingDocsRule: OptInRule { guard let array = [String].arrayOf(configuration) else { throw ConfigurationError.UnknownConfiguration } - let acl = array.flatMap(AccessControlLevel.init) + let acl = array.flatMap(AccessControlLevel.init(description:)) parameters = zip([.Warning, .Error], acl).map(RuleParameter.init) } diff --git a/Source/SwiftLintFramework/Rules/PrivateUnitTestRule.swift b/Source/SwiftLintFramework/Rules/PrivateUnitTestRule.swift index aa80cf71b8..c6f83a651f 100644 --- a/Source/SwiftLintFramework/Rules/PrivateUnitTestRule.swift +++ b/Source/SwiftLintFramework/Rules/PrivateUnitTestRule.swift @@ -124,7 +124,7 @@ public struct PrivateUnitTestRule: ASTRule, ConfigurationProviderRule { However, it's still useful to check the class explicitly because this gives us a more clear error message. If we check only methods, the line number of the error will be that of the function, which may not - necessarily be marked `private` but inhereted it from the class access + necessarily be marked `private` but inherited it from the class access modifier. By checking the class we ensure the line nuber we report for the violation will match the line that must be edited. */ From e543b54a172ba7eba6b2f72228debede090b6305 Mon Sep 17 00:00:00 2001 From: Cristian Filipov Date: Sun, 21 Aug 2016 23:15:01 -0700 Subject: [PATCH 07/27] Update CHANGELOG & code spacing --- CHANGELOG.md | 4 ++++ Source/SwiftLintFramework/Models/Configuration.swift | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c6f22324b9..5e4daf28c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,10 @@ [bootstraponline](https://github.com/bootstraponline) [#689](https://github.com/realm/SwiftLint/issues/689) +* Fixed: SwiftLint assumes paths in the YAML config file are relative to the +current directory even when `--path` is passed as an argument. + [Cristian Filipov](https://github.com/cfilipov) + ##### Bug Fixes * Fix LegacyConstructorRule when using variables instead of numbers. diff --git a/Source/SwiftLintFramework/Models/Configuration.swift b/Source/SwiftLintFramework/Models/Configuration.swift index 809e3ba721..ab49e94763 100644 --- a/Source/SwiftLintFramework/Models/Configuration.swift +++ b/Source/SwiftLintFramework/Models/Configuration.swift @@ -181,8 +181,8 @@ public struct Configuration: Equatable { public func lintablePathsForPath(path: String, fileManager: NSFileManager = fileManager) -> [String] { let pathsForPath = included.isEmpty ? fileManager.filesToLintAtPath(path) : [] - let excludedPaths = excluded.flatMap{fileManager.filesToLintAtPath($0, rootDirectory: self.rootPath)} - let includedPaths = included.flatMap{fileManager.filesToLintAtPath($0, rootDirectory: self.rootPath)} + let excludedPaths = excluded.flatMap { fileManager.filesToLintAtPath($0, rootDirectory: self.rootPath) } + let includedPaths = included.flatMap { fileManager.filesToLintAtPath($0, rootDirectory: self.rootPath) } return (pathsForPath + includedPaths).filter({ !excludedPaths.contains($0) }) } From 53a0010bb1940893cb32d6fda0784579cc5320c5 Mon Sep 17 00:00:00 2001 From: Cristian Filipov Date: Sun, 21 Aug 2016 23:21:25 -0700 Subject: [PATCH 08/27] Move CHANGELOG item to Breaking --- CHANGELOG.md | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e4daf28c4..e83be300c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,9 @@ ##### Breaking -* None. +* Fixed: SwiftLint assumes paths in the YAML config file are relative to the +current directory even when `--path` is passed as an argument. + [Cristian Filipov](https://github.com/cfilipov) ##### Enhancements @@ -28,10 +30,6 @@ [bootstraponline](https://github.com/bootstraponline) [#689](https://github.com/realm/SwiftLint/issues/689) -* Fixed: SwiftLint assumes paths in the YAML config file are relative to the -current directory even when `--path` is passed as an argument. - [Cristian Filipov](https://github.com/cfilipov) - ##### Bug Fixes * Fix LegacyConstructorRule when using variables instead of numbers. From c7d0cf04eb30d9713f04ac718e65e3f275030107 Mon Sep 17 00:00:00 2001 From: "M. Porooshani" Date: Tue, 2 Aug 2016 10:47:47 +0430 Subject: [PATCH 09/27] Fixed returns doc for init methods issue-557 Signed-off-by: M. Porooshani --- CHANGELOG.md | 4 ++++ Source/SwiftLintFramework/Rules/ValidDocsRule.swift | 13 ++++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 121e9cedee..d1891e9462 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,10 @@ * Adds 'ConditionalReturnsOnNewLineRule' rule. [Rohan Dhaimade](https://github.com/HaloZero) + +* Made `- returns:` doc optional for initializers. + [Mohpor](https://github.com/mohpor) + [#557](https://github.com/realm/SwiftLint/issues/557) ##### Bug Fixes diff --git a/Source/SwiftLintFramework/Rules/ValidDocsRule.swift b/Source/SwiftLintFramework/Rules/ValidDocsRule.swift index d8e17a9b73..84f4507908 100644 --- a/Source/SwiftLintFramework/Rules/ValidDocsRule.swift +++ b/Source/SwiftLintFramework/Rules/ValidDocsRule.swift @@ -51,7 +51,6 @@ func declarationReturns(declaration: String, kind: SwiftDeclarationKind? = nil) guard let outsideBracesMatch = matchOutsideBraces(declaration) else { return false } - return outsideBracesMatch.containsString("->") } @@ -66,6 +65,12 @@ func matchOutsideBraces(declaration: String) -> NSString? { return NSString(string: declaration).substringWithRange(outsideBracesMatch.range) } +func declarationIsInitializer(declaration: String) -> Bool { + return !regex("^((.+)?\\s+)?init\\?*\\(.*\\)") + .matchesInString(declaration, options: [], + range: NSRange(location: 0, length: declaration.characters.count)).isEmpty +} + func commentHasBatchedParameters(comment: String) -> Bool { return comment.lowercaseString.containsString("- parameters:") } @@ -76,11 +81,17 @@ func commentReturns(comment: String) -> Bool { } func missingReturnDocumentation(declaration: String, comment: String) -> Bool { + guard !declarationIsInitializer(declaration) else { + return false + } return declarationReturns(declaration) && !commentReturns(comment) } func superfluousReturnDocumentation(declaration: String, comment: String, kind: SwiftDeclarationKind) -> Bool { + guard !declarationIsInitializer(declaration) else { + return false + } return !declarationReturns(declaration, kind: kind) && commentReturns(comment) } From ed192f35bdbbe19a0a116b240058225cb3d6ecf6 Mon Sep 17 00:00:00 2001 From: Cristian Filipov Date: Mon, 22 Aug 2016 23:17:50 -0700 Subject: [PATCH 10/27] Fix long lines and unit test --- CHANGELOG.md | 2 +- .../Extensions/NSFileManager+SwiftLint.swift | 4 +++- Source/SwiftLintFramework/Models/Configuration.swift | 8 ++++++-- Tests/SwiftLintFramework/ConfigurationTests.swift | 5 ++++- 4 files changed, 14 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e7a348f41b..7c7fc5e450 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,7 @@ ##### Breaking * Fixed: SwiftLint assumes paths in the YAML config file are relative to the -current directory even when `--path` is passed as an argument. + current directory even when `--path` is passed as an argument. [Cristian Filipov](https://github.com/cfilipov) ##### Enhancements diff --git a/Source/SwiftLintFramework/Extensions/NSFileManager+SwiftLint.swift b/Source/SwiftLintFramework/Extensions/NSFileManager+SwiftLint.swift index 9d47da2ff0..0fa8b11d37 100644 --- a/Source/SwiftLintFramework/Extensions/NSFileManager+SwiftLint.swift +++ b/Source/SwiftLintFramework/Extensions/NSFileManager+SwiftLint.swift @@ -10,7 +10,9 @@ import Foundation extension NSFileManager { internal func filesToLintAtPath(path: String, rootDirectory: String? = nil) -> [String] { - let absolutePath = (path.absolutePathRepresentation(rootDirectory ?? NSFileManager.defaultManager().currentDirectoryPath) as NSString).stringByStandardizingPath + let rootPath = rootDirectory ?? NSFileManager.defaultManager().currentDirectoryPath + let absolutePath = (path.absolutePathRepresentation(rootPath) as NSString) + .stringByStandardizingPath var isDirectory: ObjCBool = false guard fileExistsAtPath(absolutePath, isDirectory: &isDirectory) else { return [] diff --git a/Source/SwiftLintFramework/Models/Configuration.swift b/Source/SwiftLintFramework/Models/Configuration.swift index 149c5e189e..cef283e3c5 100644 --- a/Source/SwiftLintFramework/Models/Configuration.swift +++ b/Source/SwiftLintFramework/Models/Configuration.swift @@ -189,8 +189,12 @@ public struct Configuration: Equatable { public func lintablePathsForPath(path: String, fileManager: NSFileManager = fileManager) -> [String] { let pathsForPath = included.isEmpty ? fileManager.filesToLintAtPath(path) : [] - let excludedPaths = excluded.flatMap { fileManager.filesToLintAtPath($0, rootDirectory: self.rootPath) } - let includedPaths = included.flatMap { fileManager.filesToLintAtPath($0, rootDirectory: self.rootPath) } + let excludedPaths = excluded.flatMap { + fileManager.filesToLintAtPath($0, rootDirectory: self.rootPath) + } + let includedPaths = included.flatMap { + fileManager.filesToLintAtPath($0, rootDirectory: self.rootPath) + } return (pathsForPath + includedPaths).filter({ !excludedPaths.contains($0) }) } diff --git a/Tests/SwiftLintFramework/ConfigurationTests.swift b/Tests/SwiftLintFramework/ConfigurationTests.swift index 4b1e864d35..a1df230ca8 100644 --- a/Tests/SwiftLintFramework/ConfigurationTests.swift +++ b/Tests/SwiftLintFramework/ConfigurationTests.swift @@ -119,7 +119,10 @@ class ConfigurationTests: XCTestCase { } private class TestFileManager: NSFileManager { - private override func filesToLintAtPath(path: String) -> [String] { + private override func filesToLintAtPath( + path: String, + rootDirectory: String? = nil) + -> [String] { switch path { case "directory": return ["directory/File1.swift", "directory/File2.swift", "directory/excluded/Excluded.swift", From 0026f301b6995660e8dd20305eb92435e620f092 Mon Sep 17 00:00:00 2001 From: JP Simard Date: Tue, 23 Aug 2016 11:52:45 -0700 Subject: [PATCH 11/27] Release 0.11.2 --- CHANGELOG.md | 4 +++- Source/SwiftLintFramework/Supporting Files/Info.plist | 2 +- Source/swiftlint/Supporting Files/Info.plist | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fc0ea1275d..24755d1894 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,6 @@ -## Master +## 0.11.2: Communal Clothesline + +This release has seen a phenomenal uptake in community contributions! ##### Breaking diff --git a/Source/SwiftLintFramework/Supporting Files/Info.plist b/Source/SwiftLintFramework/Supporting Files/Info.plist index 21ddaf9668..6ba067ba27 100644 --- a/Source/SwiftLintFramework/Supporting Files/Info.plist +++ b/Source/SwiftLintFramework/Supporting Files/Info.plist @@ -15,7 +15,7 @@ CFBundlePackageType FMWK CFBundleShortVersionString - 0.11.1 + 0.11.2 CFBundleSignature ???? CFBundleVersion diff --git a/Source/swiftlint/Supporting Files/Info.plist b/Source/swiftlint/Supporting Files/Info.plist index c1bcc2ab45..be21efada7 100644 --- a/Source/swiftlint/Supporting Files/Info.plist +++ b/Source/swiftlint/Supporting Files/Info.plist @@ -17,7 +17,7 @@ CFBundlePackageType APPL CFBundleShortVersionString - 0.11.1 + 0.11.2 CFBundleSignature ???? CFBundleVersion From 2410f6bcb80178faf3bd44b020cd645bc24a2fa9 Mon Sep 17 00:00:00 2001 From: JP Simard Date: Tue, 23 Aug 2016 12:07:56 -0700 Subject: [PATCH 12/27] move changelog entry to appropriate section --- CHANGELOG.md | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d01dfd2a3..9cddc94bab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,4 @@ -## 0.11.2: Communal Clothesline - -This release has seen a phenomenal uptake in community contributions! +## Master ##### Breaking @@ -10,6 +8,22 @@ This release has seen a phenomenal uptake in community contributions! ##### Enhancements +* None. + +##### Bug Fixes + +* None. + +## 0.11.2: Communal Clothesline + +This release has seen a phenomenal uptake in community contributions! + +##### Breaking + +* None. + +##### Enhancements + * Add `MarkRule` rule to enforce `// MARK` syntax. [Krzysztof Rodak](https://github.com/krodak) [#749](https://github.com/realm/SwiftLint/issues/749) From f559abb2d5d34c6566ea8ef4a3c00e9c5b056de3 Mon Sep 17 00:00:00 2001 From: JP Simard Date: Wed, 24 Aug 2016 12:29:24 -0700 Subject: [PATCH 13/27] make Vertical Whitespace rule opt-in fixes #772 --- CHANGELOG.md | 5 ++++- Source/SwiftLintFramework/Rules/VerticalWhitespaceRule.swift | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9cddc94bab..5e5be5b1cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,10 @@ ##### Bug Fixes -* None. +* Made Vertical Whitespace Rule added in 0.11.2 opt-in due to performance + issues. + [JP Simard](https://github.com/jpsim) + [#772](https://github.com/realm/SwiftLint/issues/772) ## 0.11.2: Communal Clothesline diff --git a/Source/SwiftLintFramework/Rules/VerticalWhitespaceRule.swift b/Source/SwiftLintFramework/Rules/VerticalWhitespaceRule.swift index f967acfa66..c8ac18a6fd 100644 --- a/Source/SwiftLintFramework/Rules/VerticalWhitespaceRule.swift +++ b/Source/SwiftLintFramework/Rules/VerticalWhitespaceRule.swift @@ -12,7 +12,7 @@ import SourceKittenFramework private let descriptionReason = "Limit vertical whitespace to a single empty line." public struct VerticalWhitespaceRule: CorrectableRule, - ConfigurationProviderRule { + ConfigurationProviderRule, OptInRule { public var configuration = SeverityConfiguration(.Warning) From 0c605fb98dc94c913a1f716e1a8bcaa357f477c5 Mon Sep 17 00:00:00 2001 From: JP Simard Date: Wed, 24 Aug 2016 12:30:57 -0700 Subject: [PATCH 14/27] release 0.12.0 --- CHANGELOG.md | 2 +- Source/SwiftLintFramework/Supporting Files/Info.plist | 2 +- Source/swiftlint/Supporting Files/Info.plist | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e5be5b1cb..449dfc557f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,4 @@ -## Master +## 0.12.0: Vertical Laundry ##### Breaking diff --git a/Source/SwiftLintFramework/Supporting Files/Info.plist b/Source/SwiftLintFramework/Supporting Files/Info.plist index 6ba067ba27..846fce99a8 100644 --- a/Source/SwiftLintFramework/Supporting Files/Info.plist +++ b/Source/SwiftLintFramework/Supporting Files/Info.plist @@ -15,7 +15,7 @@ CFBundlePackageType FMWK CFBundleShortVersionString - 0.11.2 + 0.12.0 CFBundleSignature ???? CFBundleVersion diff --git a/Source/swiftlint/Supporting Files/Info.plist b/Source/swiftlint/Supporting Files/Info.plist index be21efada7..4999a6660b 100644 --- a/Source/swiftlint/Supporting Files/Info.plist +++ b/Source/swiftlint/Supporting Files/Info.plist @@ -17,7 +17,7 @@ CFBundlePackageType APPL CFBundleShortVersionString - 0.11.2 + 0.12.0 CFBundleSignature ???? CFBundleVersion From 4cb44013ebb4142363977cfb93d305d3b3bf30ba Mon Sep 17 00:00:00 2001 From: dbeard Date: Wed, 24 Aug 2016 09:21:16 -0700 Subject: [PATCH 15/27] Add redundant nil coalesing operator rule --- CHANGELOG.md | 16 +++++++ .../Models/MasterRuleList.swift | 1 + .../Rules/RedundantNilCoalesingRule.swift | 48 +++++++++++++++++++ SwiftLint.xcodeproj/project.pbxproj | 8 +++- Tests/SwiftLintFramework/RulesTests.swift | 4 ++ 5 files changed, 75 insertions(+), 2 deletions(-) create mode 100644 Source/SwiftLintFramework/Rules/RedundantNilCoalesingRule.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 449dfc557f..610d4336f8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,19 @@ +## Master + +##### Breaking + +* None. + +##### Enhancements + +* Add `RedundantNilCoalesingRule` Opt-In rule that warns against `?? nil`. + [Daniel Beard](https://github.com/daniel-beard) + [#764](https://github.com/realm/SwiftLint/issues/764) + +##### Bug Fixes + +* None. + ## 0.12.0: Vertical Laundry ##### Breaking diff --git a/Source/SwiftLintFramework/Models/MasterRuleList.swift b/Source/SwiftLintFramework/Models/MasterRuleList.swift index 9c3d69f6f2..2143aa665b 100644 --- a/Source/SwiftLintFramework/Models/MasterRuleList.swift +++ b/Source/SwiftLintFramework/Models/MasterRuleList.swift @@ -68,6 +68,7 @@ public let masterRuleList = RuleList(rules: OperatorFunctionWhitespaceRule.self, PrivateOutletRule.self, PrivateUnitTestRule.self, + RedundantNilCoalesingRule.self, ReturnArrowWhitespaceRule.self, StatementPositionRule.self, TodoRule.self, diff --git a/Source/SwiftLintFramework/Rules/RedundantNilCoalesingRule.swift b/Source/SwiftLintFramework/Rules/RedundantNilCoalesingRule.swift new file mode 100644 index 0000000000..e10a878b4a --- /dev/null +++ b/Source/SwiftLintFramework/Rules/RedundantNilCoalesingRule.swift @@ -0,0 +1,48 @@ +// +// RedundantNilCoalesingRule.swift +// SwiftLint +// +// Created by Daniel Beard on 8/24/16. +// Copyright © 2016 Realm. All rights reserved. +// + +import Foundation +import SourceKittenFramework + +extension File { + private func violatingRedundantNilCoalesingRanges() -> [NSRange] { + return matchPattern( + "\\?\\?\\s*nil\\b", // ?? {whitespace} nil {word boundary} + excludingSyntaxKinds: SyntaxKind.commentAndStringKinds() + ) + } +} + +public struct RedundantNilCoalesingRule: OptInRule, ConfigurationProviderRule { + + public var configuration = SeverityConfiguration(.Warning) + + public init() {} + + public static let description = RuleDescription( + identifier: "redundant_nil_coalesing", + name: "Redundant Nil Coalesing", + description: "nil coalescing operator is only evaluated if the lhs is nil " + + ", coalesing operator with nil as rhs is redundant", + nonTriggeringExamples: [ + "var myVar: Int?; myVar ?? 0" + ], + triggeringExamples: [ + "var myVar = nil; myVar ↓?? nil" + ] + ) + + public func validateFile(file: File) -> [StyleViolation] { + return file.violatingRedundantNilCoalesingRanges().map { + StyleViolation(ruleDescription: self.dynamicType.description, + severity: configuration.severity, + location: Location(file: file, characterOffset: $0.location)) + } + } + +} diff --git a/SwiftLint.xcodeproj/project.pbxproj b/SwiftLint.xcodeproj/project.pbxproj index 0d7472441c..2dde1948a5 100644 --- a/SwiftLint.xcodeproj/project.pbxproj +++ b/SwiftLint.xcodeproj/project.pbxproj @@ -9,9 +9,10 @@ /* Begin PBXBuildFile section */ 006ECFC41C44E99E00EF6364 /* LegacyConstantRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 006ECFC31C44E99E00EF6364 /* LegacyConstantRule.swift */; }; 02FD8AEF1BFC18D60014BFFB /* ExtendedNSStringTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 02FD8AEE1BFC18D60014BFFB /* ExtendedNSStringTests.swift */; }; - 1EC163521D5992D900DD2928 /* VerticalWhitespaceRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1EC163511D5992D900DD2928 /* VerticalWhitespaceRule.swift */; }; 094385041D5D4F7C009168CF /* PrivateOutletRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 094385021D5D4F78009168CF /* PrivateOutletRule.swift */; }; + 1EC163521D5992D900DD2928 /* VerticalWhitespaceRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1EC163511D5992D900DD2928 /* VerticalWhitespaceRule.swift */; }; 1F11B3CF1C252F23002E8FA8 /* ClosingBraceRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1F11B3CE1C252F23002E8FA8 /* ClosingBraceRule.swift */; }; + 24B4DF0D1D6DFDE90097803B /* RedundantNilCoalesingRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 24B4DF0B1D6DFA370097803B /* RedundantNilCoalesingRule.swift */; }; 24E17F721B14BB3F008195BE /* File+Cache.swift in Sources */ = {isa = PBXBuildFile; fileRef = 24E17F701B1481FF008195BE /* File+Cache.swift */; }; 2E02005F1C54BF680024D09D /* CyclomaticComplexityRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2E02005E1C54BF680024D09D /* CyclomaticComplexityRule.swift */; }; 2E5761AA1C573B83003271AF /* FunctionParameterCountRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2E5761A91C573B83003271AF /* FunctionParameterCountRule.swift */; }; @@ -179,9 +180,10 @@ /* Begin PBXFileReference section */ 006ECFC31C44E99E00EF6364 /* LegacyConstantRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = LegacyConstantRule.swift; sourceTree = ""; }; 02FD8AEE1BFC18D60014BFFB /* ExtendedNSStringTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ExtendedNSStringTests.swift; sourceTree = ""; }; - 1EC163511D5992D900DD2928 /* VerticalWhitespaceRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = VerticalWhitespaceRule.swift; sourceTree = ""; }; 094385021D5D4F78009168CF /* PrivateOutletRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = PrivateOutletRule.swift; sourceTree = ""; }; + 1EC163511D5992D900DD2928 /* VerticalWhitespaceRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = VerticalWhitespaceRule.swift; sourceTree = ""; }; 1F11B3CE1C252F23002E8FA8 /* ClosingBraceRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ClosingBraceRule.swift; sourceTree = ""; }; + 24B4DF0B1D6DFA370097803B /* RedundantNilCoalesingRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = RedundantNilCoalesingRule.swift; sourceTree = ""; }; 24E17F701B1481FF008195BE /* File+Cache.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "File+Cache.swift"; sourceTree = ""; }; 2E02005E1C54BF680024D09D /* CyclomaticComplexityRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = CyclomaticComplexityRule.swift; sourceTree = ""; }; 2E5761A91C573B83003271AF /* FunctionParameterCountRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = FunctionParameterCountRule.swift; sourceTree = ""; }; @@ -618,6 +620,7 @@ E5A167C81B25A0B000CF2D03 /* OperatorFunctionWhitespaceRule.swift */, B2902A0B1D66815600BFCCF7 /* PrivateUnitTestRule.swift */, 094385021D5D4F78009168CF /* PrivateOutletRule.swift */, + 24B4DF0B1D6DFA370097803B /* RedundantNilCoalesingRule.swift */, E57B23C01B1D8BF000DEA512 /* ReturnArrowWhitespaceRule.swift */, 3BCC04CE1C4F56D3006073C3 /* RuleConfigurations */, 692B60AB1BD8F2E700C7AA22 /* StatementPositionRule.swift */, @@ -940,6 +943,7 @@ E88DEA6F1B09843F00A66CB0 /* Location.swift in Sources */, 93E0C3CE1D67BD7F007FA25D /* ConditionalReturnsOnNewline.swift in Sources */, E88DEA771B098D0C00A66CB0 /* Rule.swift in Sources */, + 24B4DF0D1D6DFDE90097803B /* RedundantNilCoalesingRule.swift in Sources */, 7250948A1D0859260039B353 /* StatementPositionConfiguration.swift in Sources */, E81619531BFC162C00946723 /* QueuedPrint.swift in Sources */, E87E4A051BFB927C00FCFE46 /* TrailingSemicolonRule.swift in Sources */, diff --git a/Tests/SwiftLintFramework/RulesTests.swift b/Tests/SwiftLintFramework/RulesTests.swift index 548523c709..75c57c15e9 100644 --- a/Tests/SwiftLintFramework/RulesTests.swift +++ b/Tests/SwiftLintFramework/RulesTests.swift @@ -190,6 +190,10 @@ class RulesTests: XCTestCase { verifyRule(PrivateUnitTestRule.description) } + func testRedundantNilCoalesing() { + verifyRule(RedundantNilCoalesingRule.description) + } + func testReturnArrowWhitespace() { verifyRule(ReturnArrowWhitespaceRule.description) } From 26daddcf0ddec346035f891e76d1ef3405d743c7 Mon Sep 17 00:00:00 2001 From: rohan Date: Wed, 24 Aug 2016 23:43:25 -0700 Subject: [PATCH 16/27] Adding new configuration for private outlet rules to allow private(set) --- CHANGELOG.md | 3 ++ .../Rules/PrivateOutletRule.swift | 11 ++++-- .../PrivateOutletRuleConfiguration.swift | 39 +++++++++++++++++++ SwiftLint.xcodeproj/project.pbxproj | 4 ++ Tests/SwiftLintFramework/RulesTests.swift | 14 +++++++ Tests/SwiftLintFramework/TestHelpers.swift | 2 +- 6 files changed, 69 insertions(+), 4 deletions(-) create mode 100644 Source/SwiftLintFramework/Rules/RuleConfigurations/PrivateOutletRuleConfiguration.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 610d4336f8..6cf954f4fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,9 @@ [Daniel Beard](https://github.com/daniel-beard) [#764](https://github.com/realm/SwiftLint/issues/764) +* Adds `allow_private_set` configuration for the `private_outlet` rule. + [Rohan Dhaimade](https://github.com/HaloZero) + ##### Bug Fixes * None. diff --git a/Source/SwiftLintFramework/Rules/PrivateOutletRule.swift b/Source/SwiftLintFramework/Rules/PrivateOutletRule.swift index bd1e9d5f40..6e41538aef 100644 --- a/Source/SwiftLintFramework/Rules/PrivateOutletRule.swift +++ b/Source/SwiftLintFramework/Rules/PrivateOutletRule.swift @@ -10,7 +10,7 @@ import Foundation import SourceKittenFramework public struct PrivateOutletRule: ASTRule, OptInRule, ConfigurationProviderRule { - public var configuration = SeverityConfiguration(.Warning) + public var configuration = PrivateOutletRuleConfiguration(allowPrivateSet: false) public init() {} @@ -47,8 +47,13 @@ public struct PrivateOutletRule: ASTRule, OptInRule, ConfigurationProviderRule { // Check if private let accessibility = (dictionary["key.accessibility"] as? String) ?? "" + let setterAccesiblity = (dictionary["key.setter_accessibility"] as? String) ?? "" let isPrivate = accessibility == "source.lang.swift.accessibility.private" - guard !isPrivate else { return [] } + let isPrivateSet = setterAccesiblity == "source.lang.swift.accessibility.private" + + if isPrivate || (self.configuration.allowPrivateSet && isPrivateSet) { + return [] + } // Violation found! let location: Location @@ -60,7 +65,7 @@ public struct PrivateOutletRule: ASTRule, OptInRule, ConfigurationProviderRule { return [ StyleViolation(ruleDescription: self.dynamicType.description, - severity: configuration.severity, + severity: configuration.severityConfiguration.severity, location: location ) ] diff --git a/Source/SwiftLintFramework/Rules/RuleConfigurations/PrivateOutletRuleConfiguration.swift b/Source/SwiftLintFramework/Rules/RuleConfigurations/PrivateOutletRuleConfiguration.swift new file mode 100644 index 0000000000..076914f63c --- /dev/null +++ b/Source/SwiftLintFramework/Rules/RuleConfigurations/PrivateOutletRuleConfiguration.swift @@ -0,0 +1,39 @@ +// +// PrivateOutletRuleConfiguration +// SwiftLint +// +// Created by Rohan Dhaimade on 24/08/2016. +// Copyright © 2016 Realm. All rights reserved. +// + +import Foundation + +public struct PrivateOutletRuleConfiguration: RuleConfiguration, Equatable { + var severityConfiguration = SeverityConfiguration(.Warning) + var allowPrivateSet = false + + public var consoleDescription: String { + return severityConfiguration.consoleDescription + ", allow_private_set: \(allowPrivateSet)" + } + + public init(allowPrivateSet: Bool) { + self.allowPrivateSet = allowPrivateSet + } + + public mutating func applyConfiguration(configuration: AnyObject) throws { + guard let configuration = configuration as? [String: AnyObject] else { + throw ConfigurationError.UnknownConfiguration + } + + allowPrivateSet = (configuration["allow_private_set"] as? Bool == true) + + if let severityString = configuration["severity"] as? String { + try severityConfiguration.applyConfiguration(severityString) + } + } +} + +public func == (lhs: PrivateOutletRuleConfiguration, + rhs: PrivateOutletRuleConfiguration) -> Bool { + return lhs.allowPrivateSet == rhs.allowPrivateSet +} diff --git a/SwiftLint.xcodeproj/project.pbxproj b/SwiftLint.xcodeproj/project.pbxproj index 2dde1948a5..f64e416b6a 100644 --- a/SwiftLint.xcodeproj/project.pbxproj +++ b/SwiftLint.xcodeproj/project.pbxproj @@ -67,6 +67,7 @@ D0E7B65619E9C76900EDBA4D /* main.swift in Sources */ = {isa = PBXBuildFile; fileRef = D0D1211B19E87861005E4BAA /* main.swift */; }; D4348EEA1C46122C007707FB /* FunctionBodyLengthRuleTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4348EE91C46122C007707FB /* FunctionBodyLengthRuleTests.swift */; }; D44AD2761C0AA5350048F7B0 /* LegacyConstructorRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = D44AD2741C0AA3730048F7B0 /* LegacyConstructorRule.swift */; }; + DAD3BE4A1D6ECD9500660239 /* PrivateOutletRuleConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = DAD3BE491D6ECD9500660239 /* PrivateOutletRuleConfiguration.swift */; }; E57B23C11B1D8BF000DEA512 /* ReturnArrowWhitespaceRule.swift in Sources */ = {isa = PBXBuildFile; fileRef = E57B23C01B1D8BF000DEA512 /* ReturnArrowWhitespaceRule.swift */; }; E802ED001C56A56000A35AE1 /* Benchmark.swift in Sources */ = {isa = PBXBuildFile; fileRef = E802ECFF1C56A56000A35AE1 /* Benchmark.swift */; }; E809EDA11B8A71DF00399043 /* Configuration.swift in Sources */ = {isa = PBXBuildFile; fileRef = E809EDA01B8A71DF00399043 /* Configuration.swift */; }; @@ -255,6 +256,7 @@ D0E7B63219E9C64500EDBA4D /* swiftlint.app */ = {isa = PBXFileReference; explicitFileType = wrapper.application; includeInIndex = 0; path = swiftlint.app; sourceTree = BUILT_PRODUCTS_DIR; }; D4348EE91C46122C007707FB /* FunctionBodyLengthRuleTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = FunctionBodyLengthRuleTests.swift; sourceTree = ""; }; D44AD2741C0AA3730048F7B0 /* LegacyConstructorRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = LegacyConstructorRule.swift; sourceTree = ""; }; + DAD3BE491D6ECD9500660239 /* PrivateOutletRuleConfiguration.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = PrivateOutletRuleConfiguration.swift; sourceTree = ""; }; E57B23C01B1D8BF000DEA512 /* ReturnArrowWhitespaceRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ReturnArrowWhitespaceRule.swift; sourceTree = ""; }; E5A167C81B25A0B000CF2D03 /* OperatorFunctionWhitespaceRule.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = OperatorFunctionWhitespaceRule.swift; sourceTree = ""; }; E802ECFF1C56A56000A35AE1 /* Benchmark.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = Benchmark.swift; sourceTree = ""; }; @@ -366,6 +368,7 @@ isa = PBXGroup; children = ( 3BCC04D01C4F56D3006073C3 /* NameConfiguration.swift */, + DAD3BE491D6ECD9500660239 /* PrivateOutletRuleConfiguration.swift */, B2902A0D1D6681F700BFCCF7 /* PrivateUnitTestConfiguration.swift */, 3BB47D821C514E8100AE6A10 /* RegexConfiguration.swift */, 3B0B14531C505D6300BE82F7 /* SeverityConfiguration.swift */, @@ -904,6 +907,7 @@ E812249C1B04FADC001783D2 /* Linter.swift in Sources */, 1F11B3CF1C252F23002E8FA8 /* ClosingBraceRule.swift in Sources */, E81CDE711C00FEAA00B430F6 /* ValidDocsRule.swift in Sources */, + DAD3BE4A1D6ECD9500660239 /* PrivateOutletRuleConfiguration.swift in Sources */, 2E02005F1C54BF680024D09D /* CyclomaticComplexityRule.swift in Sources */, E87E4A091BFB9CAE00FCFE46 /* SyntaxKind+SwiftLint.swift in Sources */, 3B0B14541C505D6300BE82F7 /* SeverityConfiguration.swift in Sources */, diff --git a/Tests/SwiftLintFramework/RulesTests.swift b/Tests/SwiftLintFramework/RulesTests.swift index 75c57c15e9..b25fad748e 100644 --- a/Tests/SwiftLintFramework/RulesTests.swift +++ b/Tests/SwiftLintFramework/RulesTests.swift @@ -184,6 +184,20 @@ class RulesTests: XCTestCase { func testPrivateOutlet() { verifyRule(PrivateOutletRule.description) + + let baseDescription = PrivateOutletRule.description + let nonTriggeringExamples = baseDescription.nonTriggeringExamples + [ + "class Foo {\n @IBOutlet private(set) var label: UILabel?\n}\n", + "class Foo {\n @IBOutlet private(set) var label: UILabel!\n}\n", + "class Foo {\n @IBOutlet weak private(set) var label: UILabel?\n}\n", + "class Foo {\n @IBOutlet private(set) weak var label: UILabel?\n}\n" + ] + let description = RuleDescription(identifier: baseDescription.identifier, + name: baseDescription.name, + description: baseDescription.description, + nonTriggeringExamples: nonTriggeringExamples, + triggeringExamples: baseDescription.triggeringExamples) + verifyRule(description, ruleConfiguration: ["allow_private_set": true]) } func testPrivateUnitTest() { diff --git a/Tests/SwiftLintFramework/TestHelpers.swift b/Tests/SwiftLintFramework/TestHelpers.swift index d97ffe25e7..de7d3d1a8f 100644 --- a/Tests/SwiftLintFramework/TestHelpers.swift +++ b/Tests/SwiftLintFramework/TestHelpers.swift @@ -85,7 +85,7 @@ func makeConfig(ruleConfiguration: AnyObject?, _ identifier: String) -> Configur if let ruleConfiguration = ruleConfiguration, ruleType = masterRuleList.list[identifier] { // The caller has provided a custom configuration for the rule under test return (try? ruleType.init(configuration: ruleConfiguration)).flatMap { configuredRule in - return Configuration(configuredRules: [configuredRule]) + return Configuration(whitelistRules: [identifier], configuredRules: [configuredRule]) } } return Configuration(whitelistRules: [identifier]) From 1ed079e8146beb351037cd40a10d254e3595509f Mon Sep 17 00:00:00 2001 From: JP Simard Date: Thu, 25 Aug 2016 14:36:35 -0700 Subject: [PATCH 17/27] fix setterAccesiblity typo --- Source/SwiftLintFramework/Rules/PrivateOutletRule.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Source/SwiftLintFramework/Rules/PrivateOutletRule.swift b/Source/SwiftLintFramework/Rules/PrivateOutletRule.swift index 6e41538aef..a9ce695d11 100644 --- a/Source/SwiftLintFramework/Rules/PrivateOutletRule.swift +++ b/Source/SwiftLintFramework/Rules/PrivateOutletRule.swift @@ -47,9 +47,9 @@ public struct PrivateOutletRule: ASTRule, OptInRule, ConfigurationProviderRule { // Check if private let accessibility = (dictionary["key.accessibility"] as? String) ?? "" - let setterAccesiblity = (dictionary["key.setter_accessibility"] as? String) ?? "" + let setterAccessiblity = (dictionary["key.setter_accessibility"] as? String) ?? "" let isPrivate = accessibility == "source.lang.swift.accessibility.private" - let isPrivateSet = setterAccesiblity == "source.lang.swift.accessibility.private" + let isPrivateSet = setterAccessiblity == "source.lang.swift.accessibility.private" if isPrivate || (self.configuration.allowPrivateSet && isPrivateSet) { return [] From 651351e3bdb3595eaa8cb75ed7d1fa2e41e83a60 Mon Sep 17 00:00:00 2001 From: J Cheyo Jimenez Date: Sun, 28 Aug 2016 21:43:42 -0700 Subject: [PATCH 18/27] fix for verticalspace regex bug --- CHANGELOG.md | 4 +- .../Extensions/File+SwiftLint.swift | 2 +- .../SwiftLintFramework/Rules/ColonRule.swift | 2 +- .../Rules/VerticalWhitespaceRule.swift | 56 +++++++------------ 4 files changed, 24 insertions(+), 40 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6cf954f4fa..0907ba1fa9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,9 @@ ##### Bug Fixes -* None. +* Fixed regex bug in Vertical Whitespace Rule by using SourceKitten instead. + [J. Cheyo Jimenez](https://github.com/masters3d) + [#772](https://github.com/realm/SwiftLint/issues/772) ## 0.12.0: Vertical Laundry diff --git a/Source/SwiftLintFramework/Extensions/File+SwiftLint.swift b/Source/SwiftLintFramework/Extensions/File+SwiftLint.swift index 9d16947275..9d467a21a8 100644 --- a/Source/SwiftLintFramework/Extensions/File+SwiftLint.swift +++ b/Source/SwiftLintFramework/Extensions/File+SwiftLint.swift @@ -99,7 +99,7 @@ extension File { internal func matchPattern(regex: NSRegularExpression) -> [(NSRange, [SyntaxKind])] { return rangesAndTokensMatching(regex).map { range, tokens in - (range, tokens.map({ $0.type }).flatMap(SyntaxKind.init)) + (range, tokens.flatMap { SyntaxKind(rawValue: $0.type) }) } } diff --git a/Source/SwiftLintFramework/Rules/ColonRule.swift b/Source/SwiftLintFramework/Rules/ColonRule.swift index 43f3bfb898..68606379c1 100644 --- a/Source/SwiftLintFramework/Rules/ColonRule.swift +++ b/Source/SwiftLintFramework/Rules/ColonRule.swift @@ -143,7 +143,7 @@ public struct ColonRule: CorrectableRule, ConfigurationProviderRule { let nsstring = file.contents as NSString let commentAndStringKindsSet = Set(SyntaxKind.commentAndStringKinds()) return file.rangesAndTokensMatching(pattern).filter { range, syntaxTokens in - let syntaxKinds = syntaxTokens.map({ $0.type }).flatMap(SyntaxKind.init) + let syntaxKinds = syntaxTokens.flatMap { SyntaxKind(rawValue: $0.type) } if !syntaxKinds.startsWith([.Identifier, .Typeidentifier]) { return false } diff --git a/Source/SwiftLintFramework/Rules/VerticalWhitespaceRule.swift b/Source/SwiftLintFramework/Rules/VerticalWhitespaceRule.swift index c8ac18a6fd..47c5eed859 100644 --- a/Source/SwiftLintFramework/Rules/VerticalWhitespaceRule.swift +++ b/Source/SwiftLintFramework/Rules/VerticalWhitespaceRule.swift @@ -12,7 +12,7 @@ import SourceKittenFramework private let descriptionReason = "Limit vertical whitespace to a single empty line." public struct VerticalWhitespaceRule: CorrectableRule, - ConfigurationProviderRule, OptInRule { + ConfigurationProviderRule { public var configuration = SeverityConfiguration(.Warning) @@ -37,7 +37,7 @@ public struct VerticalWhitespaceRule: CorrectableRule, "let b = 0\n\n\nclass AAA {}\n": "let b = 0\n\nclass AAA {}\n", "let c = 0\n\n\nlet num = 1\n": "let c = 0\n\nlet num = 1\n", "// bca \n\n\n": "// bca \n\n", - ] // End of line autocorrections are handeled by Trailing Newline Rule. + ] // End of line autocorrections are handled by Trailing Newline Rule. ) public func validateFile(file: File) -> [StyleViolation] { @@ -48,23 +48,17 @@ public struct VerticalWhitespaceRule: CorrectableRule, var violations = [StyleViolation]() for (eachLastLine, eachSectionCount) in linesSections { - // Skips violation for areas where the rule is disabled - let region = file.regions().filter { - $0.contains(Location(file: file.path, line: eachLastLine.index, character: 0)) - }.first - if region?.isRuleDisabled(self) == true { - continue - } - - let violation = StyleViolation(ruleDescription: self.dynamicType.description, + // Skips violations for areas where the rule is disabled + if !file.ruleEnabledViolatingRanges([eachLastLine.range], forRule: self).isEmpty { + let violation = StyleViolation(ruleDescription: self.dynamicType.description, severity: configuration.severity, location: Location(file: file.path, line: eachLastLine.index ), reason: descriptionReason + " Currently \(eachSectionCount + 1)." ) - violations.append(violation) + violations.append(violation) + } } - return violations } @@ -93,22 +87,15 @@ public struct VerticalWhitespaceRule: CorrectableRule, blankLinesSections.append(lineSection) } - // matching all accurrences of /* */ - let matchMultilineComments = "/\\*(.|[\\r\\n])*?\\*/" - let comments = file.matchPattern(matchMultilineComments) - + // filtering out violations in comments and strings + let stringAndComments = Set(SyntaxKind.commentAndStringKinds()) + let syntaxMap = file.syntaxMap var result = [(lastLine: Line, linesToRemove: Int)]() for eachSection in blankLinesSections { guard let lastLine = eachSection.last else { continue } - - // filtering out violations within a multiple comment block - let isSectionInComment = !comments.filter { - (eachRange, _ ) in eachRange.intersectsRange(lastLine.range) - }.isEmpty - - if isSectionInComment { - continue // skipping the lines found in multiline comment - } else { + let kindInSection = syntaxMap.tokensIn(lastLine.byteRange) + .flatMap { SyntaxKind(rawValue: $0.type) } + if stringAndComments.isDisjointWith(kindInSection) { result.append((lastLine, eachSection.count)) } } @@ -130,33 +117,28 @@ public struct VerticalWhitespaceRule: CorrectableRule, var correctedLines = [String]() var corrections = [Correction]() - let fileRegions = file.regions() - - forLoopCounter: for currentLine in file.lines { + for currentLine in file.lines { // Doesnt correct lines where rule is disabled - let region = fileRegions.filter { - $0.contains(Location(file: file.path, line: currentLine.index, character: 0)) - }.first - if region?.isRuleDisabled(self) == true { + if file.ruleEnabledViolatingRanges([currentLine.range], forRule: self).isEmpty { correctedLines.append(currentLine.content) - continue forLoopCounter + continue } - // by not incling lines in correctedLines, it removes them + // removes lines by skipping them from correctedLines if Set(indexOfLinesToDelete).contains(currentLine.index) { let description = self.dynamicType.description let location = Location(file: file.path, line: currentLine.index) //reports every line that is being deleted corrections.append(Correction(ruleDescription: description, location: location)) - continue forLoopCounter + continue // skips line } // all lines that pass get added to final output file correctedLines.append(currentLine.content) } - // converts lines back to file, add trailing line + // converts lines back to file and adds trailing line if !corrections.isEmpty { file.write(correctedLines.joinWithSeparator("\n") + "\n") return corrections From 2eaac54c51af8acc72014c60f68f41858d6ec766 Mon Sep 17 00:00:00 2001 From: JP Simard Date: Mon, 29 Aug 2016 10:04:11 -0700 Subject: [PATCH 19/27] clarify that vertical_whitespace is on by default again This is a followup commit from #775 --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0907ba1fa9..c4ffc16782 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,8 @@ ##### Bug Fixes -* Fixed regex bug in Vertical Whitespace Rule by using SourceKitten instead. +* Fixed regex bug in Vertical Whitespace Rule by using SourceKitten instead. + The rule now enabled by default again (no longer opt-in). [J. Cheyo Jimenez](https://github.com/masters3d) [#772](https://github.com/realm/SwiftLint/issues/772) From a60bcddc8f1f07e86c04ae3c62c6fc76ad523d68 Mon Sep 17 00:00:00 2001 From: Cristian Filipov Date: Mon, 29 Aug 2016 20:48:52 -0700 Subject: [PATCH 20/27] Fix #786: Private unit test rule not scoped to tests --- .../Rules/PrivateUnitTestRule.swift | 48 ++++++++----------- .../PrivateUnitTestConfiguration.swift | 11 ++--- 2 files changed, 26 insertions(+), 33 deletions(-) diff --git a/Source/SwiftLintFramework/Rules/PrivateUnitTestRule.swift b/Source/SwiftLintFramework/Rules/PrivateUnitTestRule.swift index c6f83a651f..9eb60b58ca 100644 --- a/Source/SwiftLintFramework/Rules/PrivateUnitTestRule.swift +++ b/Source/SwiftLintFramework/Rules/PrivateUnitTestRule.swift @@ -103,7 +103,19 @@ public struct PrivateUnitTestRule: ASTRule, ConfigurationProviderRule { -> [StyleViolation] { guard kind == .Class else { return [] } - let classViolations = validateClass(file, kind: kind, dictionary: dictionary) + guard isTestClass(dictionary) == true else { return [] } + + /* It's not strictly necessary to check for `private` on classes because a + private class will result in `private` on all its members in the AST. + However, it's still useful to check the class explicitly because this + gives us a more clear error message. If we check only methods, the line + number of the error will be that of the function, which may not + necessarily be marked `private` but inherited it from the class access + modifier. By checking the class we ensure the line nuber we report for + the violation will match the line that must be edited. + */ + + let classViolations = validateAccessControlLevel(file, dictionary: dictionary) guard classViolations.isEmpty else { return classViolations } let substructure = dictionary["key.substructure"] as? [SourceKitRepresentable] ?? [] @@ -119,30 +131,14 @@ public struct PrivateUnitTestRule: ASTRule, ConfigurationProviderRule { } - /* It's not strictly necessary to check for `private` on classes because a - private class will result in `private` on all its members in the AST. - However, it's still useful to check the class explicitly because this - gives us a more clear error message. If we check only methods, the line - number of the error will be that of the function, which may not - necessarily be marked `private` but inherited it from the class access - modifier. By checking the class we ensure the line nuber we report for - the violation will match the line that must be edited. - */ - private func validateClass( - file: File, - kind: SwiftDeclarationKind, - dictionary: [String: SourceKitRepresentable]) - -> [StyleViolation] { - - assert(kind == .Class) - guard let superclass = superclass(dictionary) else { return [] } - let pathMatch = configuration.regex.matchesInString( - superclass, - options: [], - range: NSRange(location: 0, length: (superclass as NSString).length)) - guard !pathMatch.isEmpty else { return [] } - return validateAccessControlLevel(file, dictionary: dictionary) - + private func isTestClass(dictionary: [String: SourceKitRepresentable]) -> Bool { + guard let superclass = superclass(dictionary) else { return false } + let pathMatch = configuration.regex.matchesInString( + superclass, + options: [], + range: NSRange(location: 0, length: (superclass as NSString).length)) + guard !pathMatch.isEmpty else { return false } + return true } private func validateFunction( @@ -176,7 +172,5 @@ public struct PrivateUnitTestRule: ASTRule, ConfigurationProviderRule { reason: configuration.message)] default: return [] } - } - } diff --git a/Source/SwiftLintFramework/Rules/RuleConfigurations/PrivateUnitTestConfiguration.swift b/Source/SwiftLintFramework/Rules/RuleConfigurations/PrivateUnitTestConfiguration.swift index 151b2b1ae0..7510682989 100644 --- a/Source/SwiftLintFramework/Rules/RuleConfigurations/PrivateUnitTestConfiguration.swift +++ b/Source/SwiftLintFramework/Rules/RuleConfigurations/PrivateUnitTestConfiguration.swift @@ -36,13 +36,12 @@ public struct PrivateUnitTestConfiguration: RuleConfiguration, Equatable { } public mutating func applyConfiguration(configuration: AnyObject) throws { - guard let configurationDict = configuration as? [String: AnyObject], - regexString = configurationDict["regex"] as? String else { - throw ConfigurationError.UnknownConfiguration + guard let configurationDict = configuration as? [String: AnyObject] else { + throw ConfigurationError.UnknownConfiguration + } + if let regexString = configurationDict["regex"] as? String { + regex = try NSRegularExpression.cached(pattern: regexString) } - - regex = try NSRegularExpression.cached(pattern: regexString) - if let includedString = configurationDict["included"] as? String { included = try NSRegularExpression.cached(pattern: includedString) } From e402df4b73dadd86b987f71a3ab0f59dc9d43570 Mon Sep 17 00:00:00 2001 From: Cristian Filipov Date: Mon, 29 Aug 2016 21:17:37 -0700 Subject: [PATCH 21/27] Add unit test for issue #786 --- .../Rules/PrivateUnitTestRule.swift | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/Source/SwiftLintFramework/Rules/PrivateUnitTestRule.swift b/Source/SwiftLintFramework/Rules/PrivateUnitTestRule.swift index 9eb60b58ca..70d94bd91d 100644 --- a/Source/SwiftLintFramework/Rules/PrivateUnitTestRule.swift +++ b/Source/SwiftLintFramework/Rules/PrivateUnitTestRule.swift @@ -66,6 +66,17 @@ public struct PrivateUnitTestRule: ASTRule, ConfigurationProviderRule { "func test1() {}\n " + "internal func test2() {}\n " + "public func test3() {}\n " + + "}", + // Non-test classes + "private class Foo: NSObject { " + + "func test1() {}\n " + + "internal func test2() {}\n " + + "public func test3() {}\n " + + "}", + "private class Foo { " + + "func test1() {}\n " + + "internal func test2() {}\n " + + "public func test3() {}\n " + "}" ], triggeringExamples: [ From 5fd99cd1d77f0ef1f7df1481eb675d6a13ae3f84 Mon Sep 17 00:00:00 2001 From: Cristian Filipov Date: Mon, 29 Aug 2016 23:21:43 -0700 Subject: [PATCH 22/27] Update CHANGELOG --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c4ffc16782..4c384834e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,10 @@ [J. Cheyo Jimenez](https://github.com/masters3d) [#772](https://github.com/realm/SwiftLint/issues/772) +* Fixed: Private unit test rule not scoped to test classes + [Cristian Filipov](https://github.com/cfilipov) + [#786](https://github.com/realm/SwiftLint/issues/786) + ## 0.12.0: Vertical Laundry ##### Breaking From c9e09e419026029813a272be7374bb173ee9d88e Mon Sep 17 00:00:00 2001 From: Ruotger Deecke Date: Tue, 30 Aug 2016 17:09:28 +0200 Subject: [PATCH 23/27] fix Mark rule case `// MARK: -` see https://github.com/realm/SwiftLint/issues/778 --- Source/SwiftLintFramework/Rules/MarkRule.swift | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Source/SwiftLintFramework/Rules/MarkRule.swift b/Source/SwiftLintFramework/Rules/MarkRule.swift index bc2bdc5384..52099894d2 100644 --- a/Source/SwiftLintFramework/Rules/MarkRule.swift +++ b/Source/SwiftLintFramework/Rules/MarkRule.swift @@ -20,7 +20,8 @@ public struct MarkRule: ConfigurationProviderRule { description: "MARK comment should be in valid format.", nonTriggeringExamples: [ "// MARK: good\n", - "// MARK: - good\n" + "// MARK: - good\n", + "// MARK: -\n" ], triggeringExamples: [ "//MARK: bad", @@ -32,7 +33,7 @@ public struct MarkRule: ConfigurationProviderRule { ) public func validateFile(file: File) -> [StyleViolation] { - let options = ["MARK:[^ ]", "[^ ]MARK: [^-]", "\\sMARK:[^ ]", "MARK:[ ][-][^ ]"] + let options = ["MARK:[^ ]", "[^ ]MARK: [^-]", "\\sMARK:[^ ]", "MARK:[ ][-][^\\s ]"] let pattern = "(" + options.joinWithSeparator("|") + ")" return file.matchPattern(pattern, withSyntaxKinds: [.Comment]).flatMap { range in From 11cb911386b390e9f23453075ee38ef7ea94c93e Mon Sep 17 00:00:00 2001 From: Ruotger Deecke Date: Tue, 30 Aug 2016 17:20:59 +0200 Subject: [PATCH 24/27] add fix to CHANGELOG.md --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c4ffc16782..35faaa13cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,10 @@ The rule now enabled by default again (no longer opt-in). [J. Cheyo Jimenez](https://github.com/masters3d) [#772](https://github.com/realm/SwiftLint/issues/772) +* Fixed regex bug in Mark Rule where MARK could not be used with only a hyphen + but no descriptive text: `// MARK: -` + [Ruotger Deecke](https://github.com/roddi) + [#778](https://github.com/realm/SwiftLint/issues/778) ## 0.12.0: Vertical Laundry From f0573e7821eab67e79ffe8acd978c9faffed037d Mon Sep 17 00:00:00 2001 From: JP Simard Date: Tue, 30 Aug 2016 10:23:18 -0700 Subject: [PATCH 25/27] fixup changelog entry from #789 --- CHANGELOG.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 35faaa13cf..3e2242597f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,8 +19,9 @@ The rule now enabled by default again (no longer opt-in). [J. Cheyo Jimenez](https://github.com/masters3d) [#772](https://github.com/realm/SwiftLint/issues/772) -* Fixed regex bug in Mark Rule where MARK could not be used with only a hyphen - but no descriptive text: `// MARK: -` + +* Fixed regex bug in Mark Rule where MARK could not be used with only a hyphen + but no descriptive text: `// MARK: -`. [Ruotger Deecke](https://github.com/roddi) [#778](https://github.com/realm/SwiftLint/issues/778) From 8af3ea4af8eaa07158c4a223e3b9b22ee60c4251 Mon Sep 17 00:00:00 2001 From: Cristian Filipov Date: Tue, 30 Aug 2016 21:50:39 -0700 Subject: [PATCH 26/27] Update formatting --- CHANGELOG.md | 3 ++- Source/SwiftLintFramework/Rules/PrivateUnitTestRule.swift | 6 ++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c384834e2..ccd4c1eadf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,7 +20,8 @@ [J. Cheyo Jimenez](https://github.com/masters3d) [#772](https://github.com/realm/SwiftLint/issues/772) -* Fixed: Private unit test rule not scoped to test classes +* Fixed: Private unit test rule not scoped to test classes. + Fixed: Private unit test rule config is ignored if regex is missing. [Cristian Filipov](https://github.com/cfilipov) [#786](https://github.com/realm/SwiftLint/issues/786) diff --git a/Source/SwiftLintFramework/Rules/PrivateUnitTestRule.swift b/Source/SwiftLintFramework/Rules/PrivateUnitTestRule.swift index 70d94bd91d..7b8efed8c5 100644 --- a/Source/SwiftLintFramework/Rules/PrivateUnitTestRule.swift +++ b/Source/SwiftLintFramework/Rules/PrivateUnitTestRule.swift @@ -113,8 +113,7 @@ public struct PrivateUnitTestRule: ASTRule, ConfigurationProviderRule { dictionary: [String: SourceKitRepresentable]) -> [StyleViolation] { - guard kind == .Class else { return [] } - guard isTestClass(dictionary) == true else { return [] } + guard kind == .Class && isTestClass(dictionary) else { return [] } /* It's not strictly necessary to check for `private` on classes because a private class will result in `private` on all its members in the AST. @@ -148,8 +147,7 @@ public struct PrivateUnitTestRule: ASTRule, ConfigurationProviderRule { superclass, options: [], range: NSRange(location: 0, length: (superclass as NSString).length)) - guard !pathMatch.isEmpty else { return false } - return true + return !pathMatch.isEmpty } private func validateFunction( From d00220e6e14d8d8b46623084d48b3beddd745157 Mon Sep 17 00:00:00 2001 From: Norio Nomura Date: Thu, 1 Sep 2016 09:45:09 +0900 Subject: [PATCH 27/27] Change `included` to `include` Fix #794 --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 46e7c8df47..9c722d7e9c 100644 --- a/README.md +++ b/README.md @@ -211,7 +211,7 @@ following syntax: ```yaml custom_rules: pirates_beat_ninjas: # rule identifier - included: "*.swift" # regex that defines paths to include during linting. optional. + include: "*.swift" # regex that defines paths to include during linting. optional. name: "Pirates Beat Ninjas" # rule name. optional. regex: "([n,N]inja)" # matching pattern match_kinds: # SyntaxKinds to match. optional.