Skip to content

Commit 3e8317b

Browse files
fredpifredpi
committed
Fix root directory determination for remote configs
1 parent d8afaae commit 3e8317b

File tree

4 files changed

+123
-93
lines changed

4 files changed

+123
-93
lines changed

‎Source/SwiftLintFramework/Extensions/Configuration+FileGraph.swift

+6-90
Original file line numberDiff line numberDiff line change
@@ -2,83 +2,6 @@ import Foundation
22

33
internal extension Configuration {
44
struct FileGraph: Hashable {
5-
// MARK: - Subtypes
6-
public enum FilePath: Hashable { // swiftlint:disable:this nesting
7-
case promised(urlString: String)
8-
case existing(path: String)
9-
}
10-
11-
private class Vertix: Hashable { // swiftlint:disable:this nesting
12-
internal var originatesFromRemote: Bool { return originalRemoteString != nil }
13-
internal let originalRemoteString: String?
14-
15-
private(set) var filePath: FilePath
16-
17-
private(set) var configurationString: String = ""
18-
private(set) var configurationDict: [String: Any] = [:]
19-
20-
init(string: String, rootDirectory: String) {
21-
if string.hasPrefix("http://") || string.hasPrefix("https://") {
22-
originalRemoteString = string
23-
filePath = .promised(urlString: string)
24-
} else {
25-
originalRemoteString = nil
26-
filePath = .existing(
27-
path: string.bridge().absolutePathRepresentation(rootDirectory: rootDirectory)
28-
)
29-
}
30-
}
31-
32-
internal func build(
33-
remoteConfigTimeout: TimeInterval,
34-
remoteConfigTimeoutIfCached: TimeInterval
35-
) throws {
36-
let path = try filePath.resolve(
37-
remoteConfigTimeout: remoteConfigTimeout,
38-
remoteConfigTimeoutIfCached: remoteConfigTimeoutIfCached
39-
)
40-
41-
filePath = .existing(path: path)
42-
configurationString = try read(at: path)
43-
configurationDict = try YamlParser.parse(configurationString)
44-
}
45-
46-
private func read(at path: String) throws -> String {
47-
guard !path.isEmpty && FileManager.default.fileExists(atPath: path) else {
48-
throw ConfigurationError.generic("File \(path) can't be found.")
49-
}
50-
51-
return try String(contentsOfFile: path, encoding: .utf8)
52-
}
53-
54-
internal static func == (lhs: Vertix, rhs: Vertix) -> Bool {
55-
return lhs.filePath == rhs.filePath
56-
}
57-
58-
internal func hash(into hasher: inout Hasher) {
59-
hasher.combine(filePath)
60-
}
61-
}
62-
63-
private struct Edge: Hashable { // swiftlint:disable:this nesting
64-
var parent: Vertix!
65-
var child: Vertix!
66-
67-
internal static func == (lhs: Edge, rhs: Edge) -> Bool {
68-
return lhs.parent == rhs.parent && lhs.child == rhs.child
69-
}
70-
71-
internal func hash(into hasher: inout Hasher) {
72-
hasher.combine(parent)
73-
hasher.combine(child)
74-
}
75-
}
76-
77-
private enum EdgeType: Hashable { // swiftlint:disable:this nesting
78-
case childConfig
79-
case parentConfig
80-
}
81-
825
// MARK: - Properties
836
private static let defaultRemoteConfigTimeout: TimeInterval = 2
847
private static let defaultRemoteConfigTimeoutIfCached: TimeInterval = 1
@@ -209,11 +132,12 @@ internal extension Configuration {
209132
throw ConfigurationError.generic("Remote configs are not allowed to reference local configs.")
210133
} else {
211134
let existingVertix = findPossiblyExistingVertix(sameAs: referencedVertix)
135+
let existingVertixCopy = existingVertix.map { $0.copy(withNewRootDirectory: rootDirectory) }
212136

213137
edges.insert(
214138
type == .childConfig
215-
? Edge(parent: vertix, child: existingVertix ?? referencedVertix)
216-
: Edge(parent: existingVertix ?? referencedVertix, child: vertix)
139+
? Edge(parent: vertix, child: existingVertixCopy ?? referencedVertix)
140+
: Edge(parent: existingVertixCopy ?? referencedVertix, child: vertix)
217141
)
218142

219143
if existingVertix == nil {
@@ -251,6 +175,7 @@ internal extension Configuration {
251175
private func validate() throws -> [(configurationDict: [String: Any], rootDirectory: String)] {
252176
// Detect cycles via back-edge detection during DFS
253177
func walkDown(stack: [Vertix]) throws {
178+
// Please note that the equality check (`==`), not the identity check (`===`) is used
254179
let neighbours = edges.filter { $0.parent == stack.last }.map { $0.child! }
255180
if stack.contains(where: neighbours.contains) {
256181
throw ConfigurationError.generic("There's a cycle of child / parent config references. "
@@ -296,19 +221,10 @@ internal extension Configuration {
296221
verticesToMerge.append(vertix)
297222
}
298223

299-
return try verticesToMerge.map {
300-
var rootDirectory = ""
301-
if case let .existing(path) = $0.filePath {
302-
rootDirectory = path.bridge().deletingLastPathComponent
303-
} else {
304-
throw ConfigurationError.generic(
305-
"Internal error: Processing promised config that doesn't exist yet"
306-
)
307-
}
308-
224+
return verticesToMerge.map {
309225
return (
310226
configurationDict: $0.configurationDict,
311-
rootDirectory: rootDirectory
227+
rootDirectory: $0.rootDirectory
312228
)
313229
}
314230
}

‎Source/SwiftLintFramework/Extensions/Configuration+FileGraphSubtypes.swift

+114
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
import Foundation // swiftlint:disable:this file_name
2+
3+
internal extension Configuration.FileGraph {
4+
// MARK: - FilePath
5+
enum FilePath: Hashable {
6+
case promised(urlString: String)
7+
case existing(path: String)
8+
}
9+
10+
// MARK: - Vertix
11+
class Vertix: Hashable {
12+
internal let originalRemoteString: String?
13+
internal var originatesFromRemote: Bool { originalRemoteString != nil }
14+
internal var rootDirectory: String {
15+
if !originatesFromRemote, case let .existing(path) = filePath {
16+
// This is a local file, so its root directory is its containing directory
17+
return path.bridge().deletingLastPathComponent
18+
} else {
19+
// This is a remote file, so its root directory is the directory where it was referenced from
20+
return originalRootDirectory
21+
}
22+
}
23+
24+
private let originalRootDirectory: String
25+
private(set) var filePath: FilePath
26+
private(set) var configurationString: String = ""
27+
private(set) var configurationDict: [String: Any] = [:]
28+
29+
init(string: String, rootDirectory: String) {
30+
originalRootDirectory = rootDirectory
31+
if string.hasPrefix("http://") || string.hasPrefix("https://") {
32+
originalRemoteString = string
33+
filePath = .promised(urlString: string)
34+
} else {
35+
originalRemoteString = nil
36+
filePath = .existing(
37+
path: string.bridge().absolutePathRepresentation(rootDirectory: rootDirectory)
38+
)
39+
}
40+
}
41+
42+
init(originalRemoteString: String?, originalRootDirectory: String, filePath: FilePath) {
43+
self.originalRemoteString = originalRemoteString
44+
self.originalRootDirectory = originalRootDirectory
45+
self.filePath = filePath
46+
}
47+
48+
internal func copy(withNewRootDirectory rootDirectory: String) -> Vertix {
49+
let vertix = Vertix(
50+
originalRemoteString: originalRemoteString,
51+
originalRootDirectory: rootDirectory,
52+
filePath: filePath
53+
)
54+
vertix.configurationString = configurationString
55+
vertix.configurationDict = configurationDict
56+
return vertix
57+
}
58+
59+
internal func build(
60+
remoteConfigTimeout: TimeInterval,
61+
remoteConfigTimeoutIfCached: TimeInterval
62+
) throws {
63+
let path = try filePath.resolve(
64+
remoteConfigTimeout: remoteConfigTimeout,
65+
remoteConfigTimeoutIfCached: remoteConfigTimeoutIfCached
66+
)
67+
68+
filePath = .existing(path: path)
69+
configurationString = try read(at: path)
70+
configurationDict = try YamlParser.parse(configurationString)
71+
}
72+
73+
private func read(at path: String) throws -> String {
74+
guard !path.isEmpty && FileManager.default.fileExists(atPath: path) else {
75+
throw ConfigurationError.generic("File \(path) can't be found.")
76+
}
77+
78+
return try String(contentsOfFile: path, encoding: .utf8)
79+
}
80+
81+
internal static func == (lhs: Vertix, rhs: Vertix) -> Bool {
82+
return lhs.filePath == rhs.filePath
83+
&& lhs.originalRemoteString == rhs.originalRemoteString
84+
&& lhs.originalRootDirectory == rhs.originalRootDirectory
85+
}
86+
87+
internal func hash(into hasher: inout Hasher) {
88+
hasher.combine(filePath)
89+
hasher.combine(originalRemoteString)
90+
hasher.combine(originalRootDirectory)
91+
}
92+
}
93+
94+
// MARK: - Edge
95+
struct Edge: Hashable {
96+
var parent: Vertix!
97+
var child: Vertix!
98+
99+
internal static func == (lhs: Edge, rhs: Edge) -> Bool {
100+
return lhs.parent == rhs.parent && lhs.child == rhs.child
101+
}
102+
103+
internal func hash(into hasher: inout Hasher) {
104+
hasher.combine(parent)
105+
hasher.combine(child)
106+
}
107+
}
108+
109+
// MARK: - EdgeType
110+
enum EdgeType: Hashable {
111+
case childConfig
112+
case parentConfig
113+
}
114+
}

‎Tests/LinuxMain.swift

+3-1
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,7 @@ extension ConfigurationAliasesTests {
180180
extension ConfigurationTests {
181181
static var allTests: [(String, (ConfigurationTests) -> () throws -> Void)] = [
182182
("testInit", testInit),
183+
("testNoConfiguration", testNoConfiguration),
183184
("testEmptyConfiguration", testEmptyConfiguration),
184185
("testInitWithRelativePathAndRootPath", testInitWithRelativePathAndRootPath),
185186
("testEnableAllRulesConfiguration", testEnableAllRulesConfiguration),
@@ -199,7 +200,6 @@ extension ConfigurationTests {
199200
("testGlobExcludePaths", testGlobExcludePaths),
200201
("testIsEqualTo", testIsEqualTo),
201202
("testIsNotEqualTo", testIsNotEqualTo),
202-
("testNoConfiguration", testNoConfiguration),
203203
("testCustomConfiguration", testCustomConfiguration),
204204
("testConfigurationWithSwiftFileAsRoot", testConfigurationWithSwiftFileAsRoot),
205205
("testConfigurationWithSwiftFileAsRootAndCustomConfiguration", testConfigurationWithSwiftFileAsRootAndCustomConfiguration),
@@ -218,6 +218,8 @@ extension ConfigurationTests {
218218
("testExcludeByPrefixForceExcludesDirectory", testExcludeByPrefixForceExcludesDirectory),
219219
("testExcludeByPrefixForceExcludesDirectoryThatIsNotInExcludedButHasChildrenThatAre", testExcludeByPrefixForceExcludesDirectoryThatIsNotInExcludedButHasChildrenThatAre),
220220
("testExcludeByPrefixGlobExcludePaths", testExcludeByPrefixGlobExcludePaths),
221+
("testValidRemoteChildConfig", testValidRemoteChildConfig),
222+
("testValidRemoteParentConfig", testValidRemoteParentConfig),
221223
("testMerge", testMerge),
222224
("testWarningThresholdMerging", testWarningThresholdMerging),
223225
("testOnlyRulesMerging", testOnlyRulesMerging),

‎Tests/SwiftLintFrameworkTests/Resources/ProjectMock/RemoteChildConfig/child_config_expected.yml

-2
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,5 @@ included:
55
- Test/Test2/Test
66
- Test/Test1/Test/Test
77
- Test/Test2/Test/Test
8-
- Folder/SomeFile
9-
- ../SomeFile
108

119
line_length: 80

0 commit comments

Comments
 (0)