Skip to content

Commit 428bd06

Browse files
committed
Improve performance of excluded files filter
The current algorithm is like "collect all included files and subtract all excluded files". Collecting all included and all excluded files relies on the file system. This can become slow when the patterns used to exclude files resolve to a large number of files. The new approach only collects all lintable files and checks them against the exclude patterns. This can be done by in-memory string-regex-match and does therefore not require file system accesses. (cherry picked from commit 152355e)
1 parent e451f8b commit 428bd06

File tree

14 files changed

+138
-96
lines changed

14 files changed

+138
-96
lines changed

BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ swift_library(
7070
visibility = ["//visibility:public"],
7171
deps = [
7272
":Yams.wrapper",
73+
"@FilenameMatcher",
7374
"@SourceKittenFramework",
7475
"@SwiftSyntax//:SwiftIDEUtils_opt",
7576
"@SwiftSyntax//:SwiftOperators_opt",

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -689,6 +689,7 @@
689689
* Ignore TipKit's `#Rule` macro in `empty_count` rule.
690690
[Ueeek](https://github.com/Ueeek)
691691
[#5883](https://github.com/realm/SwiftLint/issues/5883)
692+
#### Bug Fixes
692693

693694
* Ignore super calls with trailing closures in `unneeded_override` rule.
694695
[SimplyDanny](https://github.com/SimplyDanny)

MODULE.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ bazel_dep(name = "rules_swift", version = "3.1.2", max_compatibility_level = 3,
1515

1616
bazel_dep(name = "sourcekitten", version = "0.37.2", repo_name = "SourceKittenFramework")
1717
bazel_dep(name = "swift_argument_parser", version = "1.6.2", repo_name = "SwiftArgumentParser")
18+
bazel_dep(name = "swift-filename-matcher", version = "2.0.1", repo_name = "FilenameMatcher")
1819
bazel_dep(name = "yams", version = "6.2.0", repo_name = "Yams")
1920

2021
swiftlint_repos = use_extension("//bazel:repos.bzl", "swiftlint_repos_bzlmod")

Package.resolved

Lines changed: 9 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Package.swift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ let package = Package(
4242
.package(url: "https://github.com/scottrhoyt/SwiftyTextTable.git", .upToNextMajor(from: "0.9.0")),
4343
.package(url: "https://github.com/JohnSundell/CollectionConcurrencyKit.git", .upToNextMajor(from: "0.2.0")),
4444
.package(url: "https://github.com/krzyzanowskim/CryptoSwift.git", .upToNextMajor(from: "1.9.0")),
45+
.package(url: "https://github.com/ileitch/swift-filename-matcher", .upToNextMinor(from: "2.0.1")),
4546
],
4647
targets: [
4748
.executableTarget(
@@ -65,6 +66,7 @@ let package = Package(
6566
.target(
6667
name: "SwiftLintFramework",
6768
dependencies: [
69+
.product(name: "FilenameMatcher", package: "swift-filename-matcher"),
6870
"SwiftLintBuiltInRules",
6971
"SwiftLintCore",
7072
"SwiftLintExtraRules",
@@ -96,6 +98,7 @@ let package = Package(
9698
dependencies: [
9799
.product(name: "CryptoSwift", package: "CryptoSwift", condition: .when(platforms: [.linux])),
98100
.target(name: "DyldWarningWorkaround", condition: .when(platforms: [.macOS])),
101+
.product(name: "FilenameMatcher", package: "swift-filename-matcher"),
99102
.product(name: "SourceKittenFramework", package: "SourceKitten"),
100103
.product(name: "SwiftIDEUtils", package: "swift-syntax"),
101104
.product(name: "SwiftOperators", package: "swift-syntax"),
@@ -167,6 +170,7 @@ let package = Package(
167170
.testTarget(
168171
name: "FileSystemAccessTests",
169172
dependencies: [
173+
.product(name: "FilenameMatcher", package: "swift-filename-matcher"),
170174
"SwiftLintFramework",
171175
"TestHelpers",
172176
"SwiftLintCoreMacros",

Source/SwiftLintCore/Extensions/String+SwiftLint.swift

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,9 @@ public extension String {
6868

6969
/// Returns a new string, converting the path to a canonical absolute path.
7070
///
71+
/// > Important: This method might use an incorrect working directory internally. This can cause test failures
72+
/// in Bazel builds but does not seem to cause trouble in production.
73+
///
7174
/// - returns: A new `String`.
7275
func absolutePathStandardized() -> String {
7376
bridge().absolutePathRepresentation().bridge().standardizingPath

Source/SwiftLintFramework/Configuration+CommandLine.swift

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -252,15 +252,12 @@ extension Configuration {
252252
guard options.forceExclude else {
253253
return files
254254
}
255-
256255
let scriptInputPaths = files.compactMap(\.path)
257-
258-
if options.useExcludingByPrefix {
259-
return filterExcludedPathsByPrefix(in: scriptInputPaths)
260-
.map(SwiftLintFile.init(pathDeferringReading:))
261-
}
262-
return filterExcludedPaths(excludedPaths(), in: scriptInputPaths)
263-
.map(SwiftLintFile.init(pathDeferringReading:))
256+
return (
257+
visitor.options.useExcludingByPrefix
258+
? filterExcludedPathsByPrefix(in: scriptInputPaths)
259+
: filterExcludedPaths(in: scriptInputPaths)
260+
).map(SwiftLintFile.init(pathDeferringReading:))
264261
}
265262
if !options.quiet {
266263
let filesInfo: String
@@ -272,14 +269,12 @@ extension Configuration {
272269

273270
queuedPrintError("\(options.capitalizedVerb) Swift files \(filesInfo)")
274271
}
275-
let excludeLintableFilesBy = options.useExcludingByPrefix
276-
? Configuration.ExcludeBy.prefix
277-
: .paths(excludedPaths: excludedPaths())
278-
return options.paths.flatMap {
272+
return visitor.options.paths.flatMap {
279273
self.lintableFiles(
280274
inPath: $0,
281-
forceExclude: options.forceExclude,
282-
excludeBy: excludeLintableFilesBy)
275+
forceExclude: visitor.options.forceExclude,
276+
excludeByPrefix: visitor.options.useExcludingByPrefix
277+
)
283278
}
284279
}
285280

Lines changed: 29 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,22 @@
1+
import FilenameMatcher
12
import Foundation
23

34
extension Configuration {
4-
public enum ExcludeBy {
5-
case prefix
6-
case paths(excludedPaths: [String])
7-
}
8-
95
// MARK: Lintable Paths
6+
107
/// Returns the files that can be linted by SwiftLint in the specified parent path.
118
///
129
/// - parameter path: The parent path in which to search for lintable files. Can be a directory or a
1310
/// file.
1411
/// - parameter forceExclude: Whether or not excludes defined in this configuration should be applied even if
1512
/// `path` is an exact match.
16-
/// - parameter excludeByPrefix: Whether or not uses excluding by prefix algorithm.
13+
/// - parameter excludeByPrefix: Whether or not it uses the exclude-by-prefix algorithm.
1714
///
1815
/// - returns: Files to lint.
1916
public func lintableFiles(inPath path: String,
2017
forceExclude: Bool,
21-
excludeBy: ExcludeBy) -> [SwiftLintFile] {
22-
lintablePaths(inPath: path, forceExclude: forceExclude, excludeBy: excludeBy)
18+
excludeByPrefix: Bool) -> [SwiftLintFile] {
19+
lintablePaths(inPath: path, forceExclude: forceExclude, excludeByPrefix: excludeByPrefix)
2320
.parallelCompactMap {
2421
SwiftLintFile(pathDeferringReading: $0)
2522
}
@@ -31,63 +28,55 @@ extension Configuration {
3128
/// file.
3229
/// - parameter forceExclude: Whether or not excludes defined in this configuration should be applied even if
3330
/// `path` is an exact match.
34-
/// - parameter excludeByPrefix: Whether or not uses excluding by prefix algorithm.
31+
/// - parameter excludeByPrefix: Whether or not it uses the exclude-by-prefix algorithm.
3532
/// - parameter fileManager: The lintable file manager to use to search for lintable files.
3633
///
3734
/// - returns: Paths for files to lint.
3835
internal func lintablePaths(
3936
inPath path: String,
4037
forceExclude: Bool,
41-
excludeBy: ExcludeBy,
38+
excludeByPrefix: Bool,
4239
fileManager: some LintableFileManager = FileManager.default
4340
) -> [String] {
4441
if fileManager.isFile(atPath: path) {
42+
let file = fileManager.filesToLint(inPath: path, rootDirectory: nil)
4543
if forceExclude {
46-
switch excludeBy {
47-
case .prefix:
48-
return filterExcludedPathsByPrefix(in: [path.absolutePathStandardized()])
49-
case .paths(let excludedPaths):
50-
return filterExcludedPaths(excludedPaths, in: [path.absolutePathStandardized()])
51-
}
44+
return excludeByPrefix
45+
? filterExcludedPathsByPrefix(in: file)
46+
: filterExcludedPaths(in: file)
5247
}
5348
// If path is a file and we're not forcing excludes, skip filtering with excluded/included paths
54-
return [path]
49+
return file
5550
}
5651

5752
let pathsForPath = includedPaths.isEmpty ? fileManager.filesToLint(inPath: path, rootDirectory: nil) : []
5853
let includedPaths = self.includedPaths
5954
.flatMap(Glob.resolveGlob)
6055
.parallelFlatMap { fileManager.filesToLint(inPath: $0, rootDirectory: rootDirectory) }
6156

62-
switch excludeBy {
63-
case .prefix:
64-
return filterExcludedPathsByPrefix(in: pathsForPath, includedPaths)
65-
case .paths(let excludedPaths):
66-
return filterExcludedPaths(excludedPaths, in: pathsForPath, includedPaths)
67-
}
57+
return excludeByPrefix
58+
? filterExcludedPathsByPrefix(in: pathsForPath + includedPaths)
59+
: filterExcludedPaths(in: pathsForPath + includedPaths)
6860
}
6961

7062
/// Returns an array of file paths after removing the excluded paths as defined by this configuration.
7163
///
72-
/// - parameter fileManager: The lintable file manager to use to expand the excluded paths into all matching paths.
73-
/// - parameter paths: The input paths to filter.
64+
/// - parameter paths: The input paths to filter.
7465
///
7566
/// - returns: The input paths after removing the excluded paths.
76-
public func filterExcludedPaths(
77-
_ excludedPaths: [String],
78-
in paths: [String]...
79-
) -> [String] {
80-
let allPaths = paths.flatMap(\.self)
67+
public func filterExcludedPaths(in paths: [String]) -> [String] {
8168
#if os(Linux)
82-
let result = NSMutableOrderedSet(capacity: allPaths.count)
83-
result.addObjects(from: allPaths)
69+
let result = NSMutableOrderedSet(capacity: paths.count)
70+
result.addObjects(from: paths)
8471
#else
85-
let result = NSMutableOrderedSet(array: allPaths)
72+
let result = NSOrderedSet(array: paths)
8673
#endif
87-
88-
result.minusSet(Set(excludedPaths))
89-
// swiftlint:disable:next force_cast
90-
return result.map { $0 as! String }
74+
let exclusionPatterns = self.excludedPaths.flatMap {
75+
Glob.createFilenameMatchers(root: rootDirectory, pattern: $0)
76+
}
77+
return result.array
78+
.parallelCompactMap { exclusionPatterns.anyMatch(filename: $0 as! String) ? nil : $0 as? String }
79+
// swiftlint:disable:previous force_cast
9180
}
9281

9382
/// Returns the file paths that are excluded by this configuration using filtering by absolute path prefix.
@@ -96,25 +85,12 @@ extension Configuration {
9685
/// algorithm `filterExcludedPaths`.
9786
///
9887
/// - returns: The input paths after removing the excluded paths.
99-
public func filterExcludedPathsByPrefix(in paths: [String]...) -> [String] {
100-
let allPaths = paths.flatMap(\.self)
88+
public func filterExcludedPathsByPrefix(in paths: [String]) -> [String] {
10189
let excludedPaths = self.excludedPaths
102-
.parallelFlatMap { @Sendable in Glob.resolveGlob($0) }
90+
.parallelFlatMap { Glob.resolveGlob($0) }
10391
.map { $0.absolutePathStandardized() }
104-
return allPaths.filter { path in
92+
return paths.filter { path in
10593
!excludedPaths.contains { path.hasPrefix($0) }
10694
}
10795
}
108-
109-
/// Returns the file paths that are excluded by this configuration after expanding them using the specified file
110-
/// manager.
111-
///
112-
/// - parameter fileManager: The file manager to get child paths in a given parent location.
113-
///
114-
/// - returns: The expanded excluded file paths.
115-
public func excludedPaths(fileManager: some LintableFileManager = FileManager.default) -> [String] {
116-
excludedPaths
117-
.flatMap(Glob.resolveGlob)
118-
.parallelFlatMap { fileManager.filesToLint(inPath: $0, rootDirectory: rootDirectory) }
119-
}
12096
}

Source/SwiftLintFramework/Helpers/Glob.swift

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import FilenameMatcher
12
import Foundation
23
import SourceKittenFramework
34

@@ -37,6 +38,29 @@ struct Glob {
3738
.map { $0.absolutePathStandardized() }
3839
}
3940

41+
static func createFilenameMatchers(root: String, pattern: String) -> [FilenameMatcher] {
42+
var absolutPathPattern = pattern
43+
if !pattern.starts(with: root) {
44+
// If the root is not already part of the pattern, prepend it.
45+
absolutPathPattern = root + (root.hasSuffix("/") ? "" : "/") + absolutPathPattern
46+
}
47+
absolutPathPattern = absolutPathPattern.absolutePathStandardized()
48+
if pattern.hasSuffix(".swift") || pattern.hasSuffix("/**") {
49+
// Suffix is already well defined.
50+
return [FilenameMatcher(pattern: absolutPathPattern)]
51+
}
52+
if pattern.hasSuffix("/") {
53+
// Matching all files in the folder.
54+
return [FilenameMatcher(pattern: absolutPathPattern + "**")]
55+
}
56+
// The pattern could match files in the last folder in the path or all contained files if the last component
57+
// represents folders.
58+
return [
59+
FilenameMatcher(pattern: absolutPathPattern),
60+
FilenameMatcher(pattern: absolutPathPattern + "/**"),
61+
]
62+
}
63+
4064
// MARK: Private
4165

4266
private static func expandGlobstar(pattern: String) -> [String] {

0 commit comments

Comments
 (0)