Add cycle detection & remote-local-reference tests; fix multiple issues

Source/SwiftLintFramework/Extensions/Configuration+FileGraph.swift

@@ -115,16 +115,7 @@ internal extension Configuration {
                 : Configuration.Key.parentConfig.rawValue
 
             if let reference = vertix.configurationDict[key] as? String {
-                var rootDirectory: String = ""
-                if case let .existing(path) = vertix.filePath {
-                    rootDirectory = path.bridge().deletingLastPathComponent
-                } else {
-                    throw ConfigurationError.generic(
-                        "Internal error: Processing promised config that doesn't exist yet"
-                    )
-                }
-
-                let referencedVertix = Vertix(string: reference, rootDirectory: rootDirectory)
+                let referencedVertix = Vertix(string: reference, rootDirectory: vertix.rootDirectory)
 
                 // Local vertices are allowed to have local / remote references
                 // Remote vertices are only allowed to have remote references
@@ -176,13 +167,13 @@ internal extension Configuration {
             // Detect cycles via back-edge detection during DFS
             func walkDown(stack: [Vertix]) throws {
                 // Please note that the equality check (`==`), not the identity check (`===`) is used
-                let neighbours = edges.filter { $0.parent == stack.last }.map { $0.child! }
-                if stack.contains(where: neighbours.contains) {
+                let children = edges.filter { $0.parent == stack.last }.map { $0.child! }
+                if stack.contains(where: children.contains) {
                     throw ConfigurationError.generic("There's a cycle of child / parent config references. "
                         + "Please check the hierarchy of configuration files passed via the command line "
                         + "and the childConfig / parentConfig entries within them.")
                 }
-                try neighbours.forEach { try walkDown(stack: stack + [$0]) }
+                try children.forEach { try walkDown(stack: stack + [$0]) }
             }
 
             try vertices.forEach { try walkDown(stack: [$0]) }

Source/SwiftLintFramework/Extensions/Configuration+FileGraphSubtypes.swift

@@ -81,7 +81,7 @@ internal extension Configuration.FileGraph {
         internal static func == (lhs: Vertix, rhs: Vertix) -> Bool {
             return lhs.filePath == rhs.filePath
                 && lhs.originalRemoteString == rhs.originalRemoteString
-                && lhs.originalRootDirectory == rhs.originalRootDirectory
+                && lhs.rootDirectory == rhs.rootDirectory
         }
 
         internal func hash(into hasher: inout Hasher) {

Source/SwiftLintFramework/Extensions/Configuration+Remote.swift

@@ -6,8 +6,11 @@ import FoundationNetworking
 
 internal extension Configuration.FileGraph.FilePath {
     // MARK: - Properties: Remote Cache
+    /// This should never be touched.
+    private static let swiftlintPath: String = ".swiftlint"
+
     /// This should never be touched. Change the version number for changes to the cache format
-    private static let remoteCachePath: String = ".swiftlint/RemoteConfigCache"
+    private static let remoteCachePath: String = "\(swiftlintPath)/RemoteConfigCache"
 
     /// If the format of the caching is changed in the future, change this version number
     private static let remoteCacheVersionNumber: String = "v1"
@@ -222,6 +225,10 @@ internal extension Configuration.FileGraph.FilePath {
 
         try? FileManager.default.removeItem(atPath: gitignorePath)
         try? FileManager.default.removeItem(atPath: remoteCachePath)
+
+        if (try? FileManager.default.contentsOfDirectory(atPath: swiftlintPath))?.isEmpty == true {
+            try? FileManager.default.removeItem(atPath: swiftlintPath)
+        }
     }
 
     private func maintainRemoteConfigCache(rootDirectory: String) throws {

Source/SwiftLintFramework/Models/Configuration.swift

@@ -163,17 +163,19 @@ public struct Configuration {
     /// - parameter configurationFiles:         The path on disk to one or multiple configuration files. If this array
     ///                                         is empty, the default `.swiftlint.yml` file will be used.
     /// - parameter enableAllRules:             Enable all available rules.
-    /// - parameter cachePath:                  The location of the persisted cache to
-    ///                                         use whith this configuration.
+    /// - parameter cachePath:                  The location of the persisted cache to use whith this configuration.
     /// - parameter ignoreParentAndChildConfigs:If `true`, child and parent config references will be ignored.
     /// - parameter mockedNetworkResults:       For testing purposes only. Instead of loading the specified urls,
     ///                                         the mocked value will be used. Example: ["http://mock.com": "content"]
+    /// - parameter useDefaultConfigOnFailure:  If this value is specified, it will override the normal behavior.
+    ///                                         This is only intended for tests checking whether invalid configs fail.
     public init(
         configurationFiles: [String], // No default value here to avoid ambiguous Configuration() initializer
         enableAllRules: Bool = false,
         cachePath: String? = nil,
         ignoreParentAndChildConfigs: Bool = false,
-        mockedNetworkResults: [String: String] = [:]
+        mockedNetworkResults: [String: String] = [:],
+        useDefaultConfigOnFailure: Bool? = nil
     ) {
         // Handle mocked network results if needed
         Configuration.FileGraph.FilePath.mockedNetworkResults = mockedNetworkResults
@@ -223,14 +225,14 @@ public struct Configuration {
                 errorString = "Unknown Error"
             }
 
-            if hasCustomConfigurationFiles {
-                // Files that were explicitly specified could not be loaded -> fail
-                queuedPrintError("error: \(errorString)")
-                queuedFatalError("Could not read configuration")
-            } else {
+            if useDefaultConfigOnFailure ?? !hasCustomConfigurationFiles {
                 // No files were explicitly specified, so maybe the user doesn't want a config at all -> warn
                 queuedPrintError("warning: \(errorString) – Falling back to default configuration")
                 self.init(rulesMode: rulesMode, cachePath: cachePath)
+            } else {
+                // Files that were explicitly specified could not be loaded -> fail
+                queuedPrintError("error: \(errorString)")
+                queuedFatalError("Could not read configuration")
             }
         }
     }

Tests/LinuxMain.swift

@@ -209,17 +209,12 @@ extension ConfigurationTests {
         ("testConfiguresCorrectlyFromDict", testConfiguresCorrectlyFromDict),
         ("testConfigureFallsBackCorrectly", testConfigureFallsBackCorrectly),
         ("testAllowZeroLintableFiles", testAllowZeroLintableFiles),
-        ("testValidChildConfig", testValidChildConfig),
-        ("testValidParentConfig", testValidParentConfig),
-        ("testCommandLineChildConfigs", testCommandLineChildConfigs),
         ("testExcludeByPrefixExcludedPaths", testExcludeByPrefixExcludedPaths),
         ("testExcludeByPrefixForceExcludesFile", testExcludeByPrefixForceExcludesFile),
         ("testExcludeByPrefixForceExcludesFileNotPresentInExcluded", testExcludeByPrefixForceExcludesFileNotPresentInExcluded),
         ("testExcludeByPrefixForceExcludesDirectory", testExcludeByPrefixForceExcludesDirectory),
         ("testExcludeByPrefixForceExcludesDirectoryThatIsNotInExcludedButHasChildrenThatAre", testExcludeByPrefixForceExcludesDirectoryThatIsNotInExcludedButHasChildrenThatAre),
         ("testExcludeByPrefixGlobExcludePaths", testExcludeByPrefixGlobExcludePaths),
-        ("testValidRemoteChildConfig", testValidRemoteChildConfig),
-        ("testValidRemoteParentConfig", testValidRemoteParentConfig),
         ("testMerge", testMerge),
         ("testWarningThresholdMerging", testWarningThresholdMerging),
         ("testOnlyRulesMerging", testOnlyRulesMerging),
@@ -230,7 +225,16 @@ extension ConfigurationTests {
         ("testLevel2", testLevel2),
         ("testLevel3", testLevel3),
         ("testNestedConfigurationForOnePathPassedIn", testNestedConfigurationForOnePathPassedIn),
-        ("testParentConfigIsIgnoredAsNestedConfiguration", testParentConfigIsIgnoredAsNestedConfiguration)
+        ("testParentConfigIsIgnoredAsNestedConfiguration", testParentConfigIsIgnoredAsNestedConfiguration),
+        ("testValidChildConfig", testValidChildConfig),
+        ("testValidParentConfig", testValidParentConfig),
+        ("testCommandLineChildConfigs", testCommandLineChildConfigs),
+        ("testConfigCycleDetection", testConfigCycleDetection),
+        ("testCommandLineConfigsCycleDetection", testCommandLineConfigsCycleDetection),
+        ("testValidRemoteChildConfig", testValidRemoteChildConfig),
+        ("testValidRemoteParentConfig", testValidRemoteParentConfig),
+        ("testsRemoteConfigNotAllowedToReferenceLocalConfig", testsRemoteConfigNotAllowedToReferenceLocalConfig),
+        ("testRemoteConfigCycleDetection", testRemoteConfigCycleDetection)
     ]
 }
 

Tests/SwiftLintFrameworkTests/ConfigurationTests+Mock.swift

@@ -1,6 +1,8 @@
 @testable import SwiftLintFramework
 import XCTest
 
+// swiftlint:disable nesting identifier_name
+
 internal extension ConfigurationTests {
     enum Mock {
         // MARK: Test Resources Path
@@ -16,10 +18,19 @@ internal extension ConfigurationTests {
             static var nestedSub: String { nested.stringByAppendingPathComponent("Sub") }
             static var childConfigTest1: String { level0.stringByAppendingPathComponent("ChildConfig/Test1/Main") }
             static var childConfigTest2: String { level0.stringByAppendingPathComponent("ChildConfig/Test2") }
+            static var childConfigCycle1: String { level0.stringByAppendingPathComponent("ChildConfig/Cycle1") }
+            static var childConfigCycle2: String { level0.stringByAppendingPathComponent("ChildConfig/Cycle2") }
+            static var childConfigCycle3: String { level0.stringByAppendingPathComponent("ChildConfig/Cycle3/Main") }
+            static var childConfigCycle4: String { level0.stringByAppendingPathComponent("ChildConfig/Cycle4") }
             static var parentConfigTest1: String { level0.stringByAppendingPathComponent("ParentConfig/Test1") }
             static var parentConfigTest2: String { level0.stringByAppendingPathComponent("ParentConfig/Test2") }
-            static var remoteChildConfig: String { level0.stringByAppendingPathComponent("RemoteChildConfig") }
-            static var remoteParentConfig: String { level0.stringByAppendingPathComponent("RemoteParentConfig") }
+            static var parentConfigCycle1: String { level0.stringByAppendingPathComponent("ParentConfig/Cycle1") }
+            static var parentConfigCycle2: String { level0.stringByAppendingPathComponent("ParentConfig/Cycle2") }
+            static var parentConfigCycle3: String { level0.stringByAppendingPathComponent("ParentConfig/Cycle3") }
+            static var remoteConfigChild: String { level0.stringByAppendingPathComponent("RemoteConfig/Child") }
+            static var remoteConfigParent: String { level0.stringByAppendingPathComponent("RemoteConfig/Parent") }
+            static var remoteConfigLocalRef: String { level0.stringByAppendingPathComponent("RemoteConfig/LocalRef") }
+            static var remoteConfigCycle: String { level0.stringByAppendingPathComponent("RemoteConfig/Cycle") }
             static var emptyFolder: String { level0.stringByAppendingPathComponent("EmptyFolder") }
         }
 
@@ -33,7 +44,6 @@ internal extension ConfigurationTests {
             static var _2CustomRulesDisabled: String {
                 Dir.level2.stringByAppendingPathComponent("custom_rules_disabled.yml")
             }
-
             static var _3: String { Dir.level3.stringByAppendingPathComponent(Configuration.defaultFileName) }
             static var nested: String { Dir.nested.stringByAppendingPathComponent(Configuration.defaultFileName) }
         }
@@ -57,7 +67,6 @@ internal extension ConfigurationTests {
             static var _2CustomRulesDisabled: Configuration {
                 Configuration(configurationFiles: [Yml._2CustomRulesDisabled])
             }
-
             static var _3: Configuration { Configuration(configurationFiles: [Yml._3]) }
             static var nested: Configuration { Configuration(configurationFiles: [Yml.nested]) }
         }