Skip to content

Commit 25f3c0c

Browse files
committed
Use built-in URL modifying methods instead of string concat when creating paths.
The `FileIterator` class was using string concat to combine strings into file paths. This isn't very resilient to different types of valid input. The built-in file URL modifying methods (e.g. appendingPathComponent...) are more resilient. This manifested as an error when trying to format a directory, where I included a trailing `/` in the directory path passed to swift-format. That `/` resulted in `FileIterator` working on paths that included 2 `/` characters. Later on, `deleteLastPathComponent` handles the paths with 2 `/` characters by appending `../`. Eventually, this leads to an infinite loop of appending `../` to the search path when searching for configuration files. I compared the performance of this implementation with the previous implementation. There wasn't a notable difference. I tested by recursively formatting the swift-protobuf repo: - Original: 56.7 seconds - Revised: 56.1 seconds
1 parent 52626da commit 25f3c0c

File tree

1 file changed

+9
-8
lines changed

1 file changed

+9
-8
lines changed

Sources/swift-format/Utilities/FileIterator.swift

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ struct FileIterator: Sequence, IteratorProtocol {
2626
var dirIterator: FileManager.DirectoryEnumerator? = nil
2727

2828
/// Keep track of the current directory we're recursing through.
29-
var currentDirectory: String = ""
29+
var currentDirectory = URL(fileURLWithPath: "")
3030

3131
/// Keep track of paths we have visited to prevent duplicates.
3232
var visited: Set<String> = []
@@ -54,7 +54,7 @@ struct FileIterator: Sequence, IteratorProtocol {
5454
if FileManager.default.fileExists(atPath: next, isDirectory: &isDir) {
5555
if isDir.boolValue {
5656
dirIterator = FileManager.default.enumerator(atPath: next)
57-
currentDirectory = next
57+
currentDirectory = URL(fileURLWithPath: next, isDirectory: true)
5858
} else { output = next }
5959
} else {
6060
// If a path doesn't exist, allow it pass down into the SwiftFormat API so it can throw
@@ -74,12 +74,13 @@ struct FileIterator: Sequence, IteratorProtocol {
7474
while output == nil {
7575
var isDir: ObjCBool = false
7676
if let item = dirIterator?.nextObject() as? String {
77-
if item.hasSuffix(fileSuffix)
78-
&& FileManager.default.fileExists(
79-
atPath: currentDirectory + "/" + item, isDirectory: &isDir)
80-
&& !isDir.boolValue
81-
{
82-
output = currentDirectory + "/" + item
77+
if item.hasSuffix(fileSuffix) {
78+
let absoluteItemPath = currentDirectory.appendingPathComponent(item).path
79+
if FileManager.default.fileExists(atPath: absoluteItemPath, isDirectory: &isDir)
80+
&& !isDir.boolValue
81+
{
82+
output = absoluteItemPath
83+
}
8384
}
8485
} else { break }
8586
}

0 commit comments

Comments
 (0)