From ffd7be591971227796978ddff347c24667ed665d Mon Sep 17 00:00:00 2001 From: Marcelo Fabri Date: Thu, 3 Aug 2017 11:33:23 +0200 Subject: [PATCH 1/4] =?UTF-8?q?Fix=20picking=20the=20wrong=20configuration?= =?UTF-8?q?=20with=20=E2=80=94path?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #1744 --- CHANGELOG.md | 5 +++++ .../Extensions/Configuration+Merging.swift | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index abb1c2af86..1136e2d385 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,11 @@ [Hossam Ghareeb](https://github.com/hossamghareeb) [#1763](https://github.com/realm/SwiftLint/issues/1763) +* Fix using wrong configuration when using `--path` and when there is + a configuration in a parent directory. + [Marcelo Fabri](https://github.com/marcelofabri) + [#1744](https://github.com/realm/SwiftLint/issues/1744) + ## 0.21.0: Vintage Washboard ##### Breaking diff --git a/Source/SwiftLintFramework/Extensions/Configuration+Merging.swift b/Source/SwiftLintFramework/Extensions/Configuration+Merging.swift index d2bf97ff1c..e07ebfc0de 100644 --- a/Source/SwiftLintFramework/Extensions/Configuration+Merging.swift +++ b/Source/SwiftLintFramework/Extensions/Configuration+Merging.swift @@ -25,6 +25,10 @@ extension Configuration { let pathNSString = path.bridge() let configurationSearchPath = pathNSString.appendingPathComponent(Configuration.fileName) + if configurationSearchPath == configurationPath { + return self + } + // If a configuration exists and it isn't us, load and merge the configurations if configurationSearchPath != configurationPath && FileManager.default.fileExists(atPath: configurationSearchPath) { From 182915dffa2edfc62a32931f31458df561909e1c Mon Sep 17 00:00:00 2001 From: Marcelo Fabri Date: Sat, 5 Aug 2017 13:43:35 +0200 Subject: [PATCH 2/4] Add tests --- Tests/LinuxMain.swift | 2 ++ .../ConfigurationTests.swift | 16 ++++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/Tests/LinuxMain.swift b/Tests/LinuxMain.swift index fa102f94d5..4b7fc5f9a4 100644 --- a/Tests/LinuxMain.swift +++ b/Tests/LinuxMain.swift @@ -66,6 +66,8 @@ extension ConfigurationTests { ("testIsEqualTo", testIsEqualTo), ("testIsNotEqualTo", testIsNotEqualTo), ("testCustomConfiguration", testCustomConfiguration), + ("testConfigurationWithSwiftFileAsRoot", testConfigurationWithSwiftFileAsRoot), + ("testConfigurationWithSwiftFileAsRootAndCustomConfiguration", testConfigurationWithSwiftFileAsRootAndCustomConfiguration), ("testConfiguresCorrectlyFromDict", testConfiguresCorrectlyFromDict), ("testConfigureFallsBackCorrectly", testConfigureFallsBackCorrectly), ("testConfiguresCorrectlyFromDeprecatedAlias", testConfiguresCorrectlyFromDeprecatedAlias), diff --git a/Tests/SwiftLintFrameworkTests/ConfigurationTests.swift b/Tests/SwiftLintFrameworkTests/ConfigurationTests.swift index 78660e2455..06ef0a8607 100644 --- a/Tests/SwiftLintFrameworkTests/ConfigurationTests.swift +++ b/Tests/SwiftLintFrameworkTests/ConfigurationTests.swift @@ -195,6 +195,22 @@ class ConfigurationTests: XCTestCase { projectMockConfig0CustomPath.configuration(for: file)) } + func testConfigurationWithSwiftFileAsRoot() { + let configuration = Configuration(path: projectMockYAML0, + rootPath: projectMockSwift0, + optional: false, quiet: true) + let file = File(path: projectMockSwift0)! + XCTAssertEqual(configuration.configuration(for: file), configuration) + } + + func testConfigurationWithSwiftFileAsRootAndCustomConfiguration() { + let configuration = Configuration(path: projectMockYAML0CustomPath, + rootPath: projectMockSwift0, + optional: false, quiet: true) + let file = File(path: projectMockSwift0)! + XCTAssertEqual(configuration.configuration(for: file), configuration) + } + // MARK: - Testing Rules from config dictionary let testRuleList = RuleList(rules: RuleWithLevelsMock.self) From 843ca9c5358c480b9ead56abba3b026d28a77e4a Mon Sep 17 00:00:00 2001 From: Marcelo Fabri Date: Mon, 7 Aug 2017 14:04:55 +0200 Subject: [PATCH 3/4] Use configuration rootDirectory --- .../Extensions/Configuration+Merging.swift | 30 +++++++++++++++---- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/Source/SwiftLintFramework/Extensions/Configuration+Merging.swift b/Source/SwiftLintFramework/Extensions/Configuration+Merging.swift index e07ebfc0de..6b16b3cd71 100644 --- a/Source/SwiftLintFramework/Extensions/Configuration+Merging.swift +++ b/Source/SwiftLintFramework/Extensions/Configuration+Merging.swift @@ -18,17 +18,13 @@ extension Configuration { } private func configuration(forPath path: String) -> Configuration { - if path == rootPath { + if path == rootDirectory { return self } let pathNSString = path.bridge() let configurationSearchPath = pathNSString.appendingPathComponent(Configuration.fileName) - if configurationSearchPath == configurationPath { - return self - } - // If a configuration exists and it isn't us, load and merge the configurations if configurationSearchPath != configurationPath && FileManager.default.fileExists(atPath: configurationSearchPath) { @@ -47,6 +43,30 @@ extension Configuration { return self } + private var rootDirectory: String? { + guard let rootPath = rootPath else { + return nil + } + + var isDirectoryObjC: ObjCBool = false + guard FileManager.default.fileExists(atPath: rootPath, isDirectory: &isDirectoryObjC) else { + return nil + } + + let isDirectory: Bool + #if os(Linux) + isDirectory = isDirectoryObjC + #else + isDirectory = isDirectoryObjC.boolValue + #endif + + if isDirectory { + return rootPath + } else { + return rootPath.bridge().deletingLastPathComponent + } + } + private struct HashableRule: Hashable { fileprivate let rule: Rule From d4248e9b12e1432985335a1bbbd7c206645e211f Mon Sep 17 00:00:00 2001 From: Marcelo Fabri Date: Thu, 17 Aug 2017 20:35:12 +0200 Subject: [PATCH 4/4] =?UTF-8?q?Fix=20tests=20on=20Linux=20=E2=9C=85?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Source/SwiftLintFramework/Models/Configuration.swift | 12 ++++++------ .../SwiftLintFramework/Models/MasterRuleList.swift | 2 +- Tests/LinuxMain.swift | 7 +++++-- .../ConfigurationTests+ProjectMock.swift | 5 ++--- 4 files changed, 14 insertions(+), 12 deletions(-) diff --git a/Source/SwiftLintFramework/Models/Configuration.swift b/Source/SwiftLintFramework/Models/Configuration.swift index df13cbb791..04d46d1f6d 100644 --- a/Source/SwiftLintFramework/Models/Configuration.swift +++ b/Source/SwiftLintFramework/Models/Configuration.swift @@ -21,12 +21,12 @@ public struct Configuration: Equatable { public static let fileName = ".swiftlint.yml" - public let included: [String] // included - public let excluded: [String] // excluded - public let reporter: String // reporter (xcode, json, csv, checkstyle) - public var warningThreshold: Int? // warning threshold - public var rootPath: String? // the root path to search for nested configurations - public var configurationPath: String? // if successfully loaded from a path + public let included: [String] // included + public let excluded: [String] // excluded + public let reporter: String // reporter (xcode, json, csv, checkstyle) + public let warningThreshold: Int? // warning threshold + public private(set) var rootPath: String? // the root path to search for nested configurations + public private(set) var configurationPath: String? // if successfully loaded from a path public let cachePath: String? // MARK: Rules Properties diff --git a/Source/SwiftLintFramework/Models/MasterRuleList.swift b/Source/SwiftLintFramework/Models/MasterRuleList.swift index 765621d323..e9635d7c8b 100644 --- a/Source/SwiftLintFramework/Models/MasterRuleList.swift +++ b/Source/SwiftLintFramework/Models/MasterRuleList.swift @@ -75,7 +75,6 @@ public let masterRuleList = RuleList(rules: [ PrivateUnitTestRule.self, ProhibitedSuperRule.self, ProtocolPropertyAccessorsOrderRule.self, - SingleTestClassRule.self, RedundantDiscardableLetRule.self, RedundantNilCoalescingRule.self, RedundantOptionalInitializationRule.self, @@ -83,6 +82,7 @@ public let masterRuleList = RuleList(rules: [ RedundantVoidReturnRule.self, ReturnArrowWhitespaceRule.self, ShorthandOperatorRule.self, + SingleTestClassRule.self, SortedImportsRule.self, StatementPositionRule.self, StrictFilePrivateRule.self, diff --git a/Tests/LinuxMain.swift b/Tests/LinuxMain.swift index 4b7fc5f9a4..90e2581ed9 100644 --- a/Tests/LinuxMain.swift +++ b/Tests/LinuxMain.swift @@ -390,6 +390,7 @@ extension RulesTests { ("testPrivateUnitTest", testPrivateUnitTest), ("testProhibitedSuper", testProhibitedSuper), ("testProtocolPropertyAccessorsOrder", testProtocolPropertyAccessorsOrder), + ("testSingleTestClass", testSingleTestClass), ("testRedundantDiscardableLet", testRedundantDiscardableLet), ("testRedundantNilCoalescing", testRedundantNilCoalescing), ("testRedundantOptionalInitialization", testRedundantOptionalInitialization), @@ -397,7 +398,6 @@ extension RulesTests { ("testRedundantVoidReturn", testRedundantVoidReturn), ("testReturnArrowWhitespace", testReturnArrowWhitespace), ("testShorthandOperator", testShorthandOperator), - ("testSingleTestClass", testSingleTestClass), ("testSortedImports", testSortedImports), ("testStatementPosition", testStatementPosition), ("testStatementPositionUncuddled", testStatementPositionUncuddled), @@ -463,7 +463,10 @@ extension UnusedOptionalBindingRuleTests { extension VerticalWhitespaceRuleTests { static var allTests: [(String, (VerticalWhitespaceRuleTests) -> () throws -> Void)] = [ ("testVerticalWhitespaceWithDefaultConfiguration", testVerticalWhitespaceWithDefaultConfiguration), - ("testAttributesWithMaxEmptyLines", testAttributesWithMaxEmptyLines) + ("testAttributesWithMaxEmptyLines", testAttributesWithMaxEmptyLines), + ("testAutoCorrectionWithMaxEmptyLines", testAutoCorrectionWithMaxEmptyLines), + ("testViolationMessageWithMaxEmptyLines", testViolationMessageWithMaxEmptyLines), + ("testViolationMessageWithDefaultConfiguration", testViolationMessageWithDefaultConfiguration) ] } diff --git a/Tests/SwiftLintFrameworkTests/ConfigurationTests+ProjectMock.swift b/Tests/SwiftLintFrameworkTests/ConfigurationTests+ProjectMock.swift index 247364dcd1..7f3f8eb021 100644 --- a/Tests/SwiftLintFrameworkTests/ConfigurationTests+ProjectMock.swift +++ b/Tests/SwiftLintFrameworkTests/ConfigurationTests+ProjectMock.swift @@ -56,9 +56,8 @@ extension ConfigurationTests { } var projectMockConfig0: Configuration { - var configuration = Configuration(path: projectMockYAML0, optional: false, quiet: true) - configuration.rootPath = projectMockPathLevel0 - return configuration + return Configuration(path: projectMockYAML0, rootPath: projectMockPathLevel0, + optional: false, quiet: true) } var projectMockConfig0CustomPath: Configuration {