GitHub/SwiftLint
1
0
Fork 0
mirror of https://github.com/realm/SwiftLint.git synced 2026-05-07 20:12:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

Revert "Improve performance of excluded files filter" (#5962)

Browse Source 
This reverts commit 152355e36f from #5157.

# Conflicts:
#	tools/oss-check
This commit is contained in:
Danny Mösch
2025-01-15 19:15:44 +01:00
committed by GitHub
parent 9b22cda361
commit fcdc98a52d
13 changed files with 97 additions and 132 deletions
Download Patch File Download Diff File Expand all files Collapse all files
BUILD
-1
View File
@@ -94,7 +94,6 @@ swift_library(
":SourceKittenFramework.wrapper",
"@sourcekitten_com_github_jpsim_yams//:Yams",
"@swiftlint_com_github_scottrhoyt_swifty_text_table//:SwiftyTextTable",
"@com_github_ileitch_swift-filename-matcher//:FilenameMatcher"
] + select({
"@platforms//os:linux": ["@com_github_krzyzanowskim_cryptoswift//:CryptoSwift"],
"//conditions:default": [":DyldWarningWorkaround"],
CHANGELOG.md
+6
View File
@@ -19,6 +19,12 @@
[SimplyDanny](https://github.com/SimplyDanny)
[#5954](https://github.com/realm/SwiftLint/issues/5954)
* Revert changes to improve performance when exclude patterns resolve to a large set of files. While resolving files
indeed got much faster in certain setups, it leads to missed exclusions for nested configurations and when the linted
folder is not the current folder.
[SimplyDanny](https://github.com/SimplyDanny)
[#5953](https://github.com/realm/SwiftLint/issues/5953)
#### Experimental
* None.
MODULE.bazel
-1
View File
@@ -14,7 +14,6 @@ bazel_dep(name = "sourcekitten", version = "0.36.0", repo_name = "com_github_jps
bazel_dep(name = "swift-syntax", version = "600.0.0", repo_name = "SwiftSyntax")
bazel_dep(name = "swift_argument_parser", version = "1.3.1.1", repo_name = "sourcekitten_com_github_apple_swift_argument_parser")
bazel_dep(name = "yams", version = "5.1.3", repo_name = "sourcekitten_com_github_jpsim_yams")
bazel_dep(name = "swift-filename-matcher", version = "2.0.0", repo_name = "com_github_ileitch_swift-filename-matcher")
swiftlint_repos = use_extension("//bazel:repos.bzl", "swiftlint_repos_bzlmod")
use_repo(
Package.resolved
-9
View File
@@ -36,15 +36,6 @@
"version" : "1.3.1"
}
},
{
"identity" : "swift-filename-matcher",
"kind" : "remoteSourceControl",
"location" : "https://github.com/ileitch/swift-filename-matcher",
"state" : {
"revision" : "516ff95f6a06c7a9eff8e944e989c7af076c5cdb",
"version" : "2.0.0"
}
},
{
"identity" : "swift-syntax",
"kind" : "remoteSourceControl",
Package.swift
-2
View File
@@ -37,7 +37,6 @@ let package = Package(
.package(url: "https://github.com/scottrhoyt/SwiftyTextTable.git", from: "0.9.0"),
.package(url: "https://github.com/JohnSundell/CollectionConcurrencyKit.git", from: "0.2.0"),
.package(url: "https://github.com/krzyzanowskim/CryptoSwift.git", .upToNextMinor(from: "1.8.4")),
.package(url: "https://github.com/ileitch/swift-filename-matcher", .upToNextMinor(from: "2.0.0")),
],
targets: [
.executableTarget(
@@ -92,7 +91,6 @@ let package = Package(
.product(name: "SwiftSyntaxBuilder", package: "swift-syntax"),
.product(name: "SwiftyTextTable", package: "SwiftyTextTable"),
.product(name: "Yams", package: "Yams"),
.product(name: "FilenameMatcher", package: "swift-filename-matcher"),
"SwiftLintCoreMacros",
],
swiftSettings: swiftFeatures + strictConcurrency
Source/SwiftLintCore/Extensions/String+SwiftLint.swift
-9
View File
@@ -68,20 +68,11 @@ public extension String {
/// Returns a new string, converting the path to a canonical absolute path.
///
/// > Important: This method might use an incorrect working directory internally. This can cause test failures
/// in Bazel builds but does not seem to cause trouble in production.
///
/// - returns: A new `String`.
func absolutePathStandardized() -> String {
bridge().absolutePathRepresentation().bridge().standardizingPath
}
/// Like ``absolutePathStandardized()`` but with the working directory that's used everywhere else.
var normalized: String {
let cwd = FileManager.default.currentDirectoryPath.bridge().standardizingPath
return bridge().absolutePathRepresentation(rootDirectory: cwd)
}
var isFile: Bool {
if self.isEmpty {
return false
Source/SwiftLintFramework/Configuration+CommandLine.swift
+14 -9
View File
@@ -257,12 +257,15 @@ extension Configuration {
guard options.forceExclude else {
return files
}
let scriptInputPaths = files.compactMap(\.path)
return (
visitor.options.useExcludingByPrefix
? filterExcludedPathsByPrefix(in: scriptInputPaths)
: filterExcludedPaths(in: scriptInputPaths)
).map(SwiftLintFile.init(pathDeferringReading:))
if options.useExcludingByPrefix {
return filterExcludedPathsByPrefix(in: scriptInputPaths)
.map(SwiftLintFile.init(pathDeferringReading:))
}
return filterExcludedPaths(excludedPaths(), in: scriptInputPaths)
.map(SwiftLintFile.init(pathDeferringReading:))
}
if !options.quiet {
let filesInfo: String
@@ -274,12 +277,14 @@ extension Configuration {
queuedPrintError("\(options.capitalizedVerb) Swift files \(filesInfo)")
}
return visitor.options.paths.flatMap {
let excludeLintableFilesBy = options.useExcludingByPrefix
? Configuration.ExcludeBy.prefix
: .paths(excludedPaths: excludedPaths())
return options.paths.flatMap {
self.lintableFiles(
inPath: $0,
forceExclude: visitor.options.forceExclude,
excludeByPrefix: visitor.options.useExcludingByPrefix
)
forceExclude: options.forceExclude,
excludeBy: excludeLintableFilesBy)
}
}
Source/SwiftLintFramework/Configuration/Configuration+LintableFiles.swift
+53 -27
View File
@@ -1,21 +1,25 @@
import Foundation
extension Configuration {
// MARK: Lintable Paths
public enum ExcludeBy {
case prefix
case paths(excludedPaths: [String])
}
// MARK: Lintable Paths
/// Returns the files that can be linted by SwiftLint in the specified parent path.
///
/// - parameter path: The parent path in which to search for lintable files. Can be a directory or a
/// file.
/// - parameter forceExclude: Whether or not excludes defined in this configuration should be applied even if
/// `path` is an exact match.
/// - parameter excludeByPrefix: Whether or not it uses the exclude-by-prefix algorithm.
/// - parameter excludeByPrefix: Whether or not uses excluding by prefix algorithm.
///
/// - returns: Files to lint.
public func lintableFiles(inPath path: String,
forceExclude: Bool,
excludeByPrefix: Bool) -> [SwiftLintFile] {
lintablePaths(inPath: path, forceExclude: forceExclude, excludeByPrefix: excludeByPrefix)
excludeBy: ExcludeBy) -> [SwiftLintFile] {
lintablePaths(inPath: path, forceExclude: forceExclude, excludeBy: excludeBy)
.compactMap(SwiftLintFile.init(pathDeferringReading:))
}
@@ -25,21 +29,24 @@ extension Configuration {
/// file.
/// - parameter forceExclude: Whether or not excludes defined in this configuration should be applied even if
/// `path` is an exact match.
/// - parameter excludeByPrefix: Whether or not it uses the exclude-by-prefix algorithm.
/// - parameter excludeByPrefix: Whether or not uses excluding by prefix algorithm.
/// - parameter fileManager: The lintable file manager to use to search for lintable files.
///
/// - returns: Paths for files to lint.
internal func lintablePaths(
inPath path: String,
forceExclude: Bool,
excludeByPrefix: Bool,
excludeBy: ExcludeBy,
fileManager: some LintableFileManager = FileManager.default
) -> [String] {
if fileManager.isFile(atPath: path) {
if forceExclude {
return excludeByPrefix
? filterExcludedPathsByPrefix(in: [path.normalized])
: filterExcludedPaths(in: [path.normalized])
switch excludeBy {
case .prefix:
return filterExcludedPathsByPrefix(in: [path.absolutePathStandardized()])
case .paths(let excludedPaths):
return filterExcludedPaths(excludedPaths, in: [path.absolutePathStandardized()])
}
}
// If path is a file and we're not forcing excludes, skip filtering with excluded/included paths
return [path]
@@ -50,29 +57,35 @@ extension Configuration {
.flatMap(Glob.resolveGlob)
.parallelFlatMap { fileManager.filesToLint(inPath: $0, rootDirectory: rootDirectory) }
return excludeByPrefix
? filterExcludedPathsByPrefix(in: pathsForPath + includedPaths)
: filterExcludedPaths(in: pathsForPath + includedPaths)
switch excludeBy {
case .prefix:
return filterExcludedPathsByPrefix(in: pathsForPath, includedPaths)
case .paths(let excludedPaths):
return filterExcludedPaths(excludedPaths, in: pathsForPath, includedPaths)
}
}
/// Returns an array of file paths after removing the excluded paths as defined by this configuration.
///
/// - parameter paths: The input paths to filter.
/// - parameter fileManager: The lintable file manager to use to expand the excluded paths into all matching paths.
/// - parameter paths: The input paths to filter.
///
/// - returns: The input paths after removing the excluded paths.
public func filterExcludedPaths(in paths: [String]) -> [String] {
public func filterExcludedPaths(
_ excludedPaths: [String],
in paths: [String]...
) -> [String] {
let allPaths = paths.flatMap { $0 }
#if os(Linux)
let result = NSMutableOrderedSet(capacity: paths.count)
result.addObjects(from: paths)
let result = NSMutableOrderedSet(capacity: allPaths.count)
result.addObjects(from: allPaths)
#else
let result = NSMutableOrderedSet(array: paths)
let result = NSMutableOrderedSet(array: allPaths)
#endif
let exclusionPatterns = self.excludedPaths.flatMap {
Glob.createFilenameMatchers(root: rootDirectory, pattern: $0)
}
return result.array
.parallelCompactMap { exclusionPatterns.anyMatch(filename: $0 as! String) ? nil : $0 as? String }
// swiftlint:disable:previous force_cast
result.minusSet(Set(excludedPaths))
// swiftlint:disable:next force_cast
return result.map { $0 as! String }
}
/// Returns the file paths that are excluded by this configuration using filtering by absolute path prefix.
@@ -81,12 +94,25 @@ extension Configuration {
/// algorithm `filterExcludedPaths`.
///
/// - returns: The input paths after removing the excluded paths.
public func filterExcludedPathsByPrefix(in paths: [String]) -> [String] {
public func filterExcludedPathsByPrefix(in paths: [String]...) -> [String] {
let allPaths = paths.flatMap { $0 }
let excludedPaths = self.excludedPaths
.parallelFlatMap { Glob.resolveGlob($0) }
.map(\.normalized)
return paths.filter { path in
.parallelFlatMap { @Sendable in Glob.resolveGlob($0) }
.map { $0.absolutePathStandardized() }
return allPaths.filter { path in
!excludedPaths.contains { path.hasPrefix($0) }
}
}
/// Returns the file paths that are excluded by this configuration after expanding them using the specified file
/// manager.
///
/// - parameter fileManager: The file manager to get child paths in a given parent location.
///
/// - returns: The expanded excluded file paths.
public func excludedPaths(fileManager: some LintableFileManager = FileManager.default) -> [String] {
excludedPaths
.flatMap(Glob.resolveGlob)
.parallelFlatMap { fileManager.filesToLint(inPath: $0, rootDirectory: rootDirectory) }
}
}
Source/SwiftLintFramework/Helpers/Glob.swift
-23
View File
@@ -1,4 +1,3 @@
import FilenameMatcher
import Foundation
#if os(Linux)
@@ -37,28 +36,6 @@ struct Glob {
.map { $0.absolutePathStandardized() }
}
static func createFilenameMatchers(root: String, pattern: String) -> [FilenameMatcher] {
var absolutPathPattern = pattern
if !pattern.starts(with: root) {
// If the root is not already part of the pattern, prepend it.
absolutPathPattern = root + (root.hasSuffix("/") ? "" : "/") + absolutPathPattern
}
if pattern.hasSuffix(".swift") || pattern.hasSuffix("/**") {
// Suffix is already well defined.
return [FilenameMatcher(pattern: absolutPathPattern)]
}
if pattern.hasSuffix("/") {
// Matching all files in the folder.
return [FilenameMatcher(pattern: absolutPathPattern + "**")]
}
// The pattern could match files in the last folder in the path or all contained files if the last component
// represents folders.
return [
FilenameMatcher(pattern: absolutPathPattern),
FilenameMatcher(pattern: absolutPathPattern + "/**"),
]
}
// MARK: Private
private static func expandGlobstar(pattern: String) -> [String] {
Tests/FrameworkTests/ConfigurationTests.swift
+22 -15
View File
@@ -288,9 +288,10 @@ final class ConfigurationTests: SwiftLintTestCase {
excludedPaths: ["directory/excluded", "directory/ExcludedFile.swift"]
)
let excludedPaths = configuration.excludedPaths(fileManager: fileManager)
let paths = configuration.lintablePaths(inPath: "",
forceExclude: false,
excludeByPrefix: false,
excludeBy: .paths(excludedPaths: excludedPaths),
fileManager: fileManager)
XCTAssertEqual(["directory/File1.swift", "directory/File2.swift"].absolutePathsStandardized(), paths)
}
@@ -298,9 +299,10 @@ final class ConfigurationTests: SwiftLintTestCase {
func testForceExcludesFile() {
let fileManager = TestFileManager()
let configuration = Configuration(excludedPaths: ["directory/ExcludedFile.swift"])
let excludedPaths = configuration.excludedPaths(fileManager: fileManager)
let paths = configuration.lintablePaths(inPath: "directory/ExcludedFile.swift",
forceExclude: true,
excludeByPrefix: false,
excludeBy: .paths(excludedPaths: excludedPaths),
fileManager: fileManager)
XCTAssertEqual([], paths)
}
@@ -309,9 +311,10 @@ final class ConfigurationTests: SwiftLintTestCase {
let fileManager = TestFileManager()
let configuration = Configuration(includedPaths: ["directory"],
excludedPaths: ["directory/ExcludedFile.swift", "directory/excluded"])
let excludedPaths = configuration.excludedPaths(fileManager: fileManager)
let paths = configuration.lintablePaths(inPath: "",
forceExclude: true,
excludeByPrefix: false,
excludeBy: .paths(excludedPaths: excludedPaths),
fileManager: fileManager)
XCTAssertEqual(["directory/File1.swift", "directory/File2.swift"].absolutePathsStandardized(), paths)
}
@@ -319,9 +322,10 @@ final class ConfigurationTests: SwiftLintTestCase {
func testForceExcludesDirectory() {
let fileManager = TestFileManager()
let configuration = Configuration(excludedPaths: ["directory/excluded", "directory/ExcludedFile.swift"])
let excludedPaths = configuration.excludedPaths(fileManager: fileManager)
let paths = configuration.lintablePaths(inPath: "directory",
forceExclude: true,
excludeByPrefix: false,
excludeBy: .paths(excludedPaths: excludedPaths),
fileManager: fileManager)
XCTAssertEqual(["directory/File1.swift", "directory/File2.swift"].absolutePathsStandardized(), paths)
}
@@ -329,17 +333,19 @@ final class ConfigurationTests: SwiftLintTestCase {
func testForceExcludesDirectoryThatIsNotInExcludedButHasChildrenThatAre() {
let fileManager = TestFileManager()
let configuration = Configuration(excludedPaths: ["directory/excluded", "directory/ExcludedFile.swift"])
let excludedPaths = configuration.excludedPaths(fileManager: fileManager)
let paths = configuration.lintablePaths(inPath: "directory",
forceExclude: true,
excludeByPrefix: false,
excludeBy: .paths(excludedPaths: excludedPaths),
fileManager: fileManager)
XCTAssertEqual(["directory/File1.swift", "directory/File2.swift"].absolutePathsStandardized(), paths)
}
func testLintablePaths() {
let excluded = Configuration.default.excludedPaths(fileManager: TestFileManager())
let paths = Configuration.default.lintablePaths(inPath: Mock.Dir.level0,
forceExclude: false,
excludeByPrefix: false)
excludeBy: .paths(excludedPaths: excluded))
let filenames = paths.map { $0.bridge().lastPathComponent }.sorted()
let expectedFilenames = [
"DirectoryLevel1.swift",
@@ -355,7 +361,7 @@ final class ConfigurationTests: SwiftLintTestCase {
let configuration = Configuration(includedPaths: ["**/Level2"])
let paths = configuration.lintablePaths(inPath: Mock.Dir.level0,
forceExclude: true,
excludeByPrefix: false)
excludeBy: .paths(excludedPaths: configuration.excludedPaths))
let filenames = paths.map { $0.bridge().lastPathComponent }.sorted()
let expectedFilenames = ["Level2.swift", "Level3.swift"]
@@ -368,9 +374,10 @@ final class ConfigurationTests: SwiftLintTestCase {
excludedPaths: [Mock.Dir.level3.stringByAppendingPathComponent("*.swift")]
)
let excludedPaths = configuration.excludedPaths()
let lintablePaths = configuration.lintablePaths(inPath: "",
forceExclude: false,
excludeByPrefix: false)
excludeBy: .paths(excludedPaths: excludedPaths))
XCTAssertEqual(lintablePaths, [])
}
@@ -485,7 +492,7 @@ extension ConfigurationTests {
)
let paths = configuration.lintablePaths(inPath: Mock.Dir.level0,
forceExclude: false,
excludeByPrefix: true)
excludeBy: .prefix)
let filenames = paths.map { $0.bridge().lastPathComponent }
XCTAssertEqual(filenames, ["Level2.swift"])
}
@@ -495,7 +502,7 @@ extension ConfigurationTests {
let configuration = Configuration(excludedPaths: ["Level1/Level2/Level3/Level3.swift"])
let paths = configuration.lintablePaths(inPath: "Level1/Level2/Level3/Level3.swift",
forceExclude: true,
excludeByPrefix: true)
excludeBy: .prefix)
XCTAssertEqual([], paths)
}
@@ -505,7 +512,7 @@ extension ConfigurationTests {
excludedPaths: ["Level1/Level1.swift"])
let paths = configuration.lintablePaths(inPath: "Level1",
forceExclude: true,
excludeByPrefix: true)
excludeBy: .prefix)
let filenames = paths.map { $0.bridge().lastPathComponent }.sorted()
XCTAssertEqual(["Level2.swift", "Level3.swift"], filenames)
}
@@ -519,7 +526,7 @@ extension ConfigurationTests {
)
let paths = configuration.lintablePaths(inPath: ".",
forceExclude: true,
excludeByPrefix: true)
excludeBy: .prefix)
let filenames = paths.map { $0.bridge().lastPathComponent }.sorted()
XCTAssertEqual(["Level0.swift", "Level1.swift"], filenames)
}
@@ -533,7 +540,7 @@ extension ConfigurationTests {
)
let paths = configuration.lintablePaths(inPath: ".",
forceExclude: true,
excludeByPrefix: true)
excludeBy: .prefix)
let filenames = paths.map { $0.bridge().lastPathComponent }
XCTAssertEqual(["Level0.swift"], filenames)
}
@@ -545,7 +552,7 @@ extension ConfigurationTests {
excludedPaths: ["Level1/*/*.swift", "Level1/*/*/*.swift"])
let paths = configuration.lintablePaths(inPath: "Level1",
forceExclude: false,
excludeByPrefix: true)
excludeBy: .prefix)
let filenames = paths.map { $0.bridge().lastPathComponent }.sorted()
XCTAssertEqual(filenames, ["Level1.swift"])
}
@@ -599,7 +606,7 @@ extension ConfigurationTests {
private extension Sequence where Element == String {
func absolutePathsStandardized() -> [String] {
map(\.normalized)
map { $0.absolutePathStandardized() }
}
}
Tests/FrameworkTests/GlobTests.swift
-27
View File
@@ -83,31 +83,4 @@ final class GlobTests: SwiftLintTestCase {
let files = Glob.resolveGlob(mockPath.stringByAppendingPathComponent("**/*.swift"))
XCTAssertEqual(files.sorted(), expectedFiles.sorted())
}
func testCreateFilenameMatchers() {
func assertGlobMatch(root: String, pattern: String, filename: String) {
let matchers = Glob.createFilenameMatchers(root: root, pattern: pattern)
XCTAssert(matchers.anyMatch(filename: filename))
}
assertGlobMatch(root: "/a/b/", pattern: "c/*.swift", filename: "/a/b/c/d.swift")
assertGlobMatch(root: "/a", pattern: "**/*.swift", filename: "/a/b/c/d.swift")
assertGlobMatch(root: "/a", pattern: "**/*.swift", filename: "/a/b.swift")
assertGlobMatch(root: "", pattern: "**/*.swift", filename: "/a/b.swift")
assertGlobMatch(root: "", pattern: "a/**/b.swift", filename: "a/b.swift")
assertGlobMatch(root: "", pattern: "a/**/b.swift", filename: "a/c/b.swift")
assertGlobMatch(root: "", pattern: "**/*.swift", filename: "a.swift")
assertGlobMatch(root: "", pattern: "a/**/*.swift", filename: "a/b/c.swift")
assertGlobMatch(root: "", pattern: "a/**/*.swift", filename: "a/b.swift")
assertGlobMatch(root: "/a/b", pattern: "/a/b/c/*.swift", filename: "/a/b/c/d.swift")
assertGlobMatch(root: "/a/", pattern: "/a/b/c/*.swift", filename: "/a/b/c/d.swift")
assertGlobMatch(root: "", pattern: "/a/b/c", filename: "/a/b/c/d.swift")
assertGlobMatch(root: "", pattern: "/a/b/c/", filename: "/a/b/c/d.swift")
assertGlobMatch(root: "", pattern: "/a/b/c/*.swift", filename: "/a/b/c/d.swift")
assertGlobMatch(root: "", pattern: "/d.swift/*.swift", filename: "/d.swift/e.swift")
assertGlobMatch(root: "", pattern: "/a/**", filename: "/a/b/c/d.swift")
assertGlobMatch(root: "", pattern: "**/*Test*", filename: "/a/b/c/MyTest2.swift")
assertGlobMatch(root: "", pattern: "**/*Test*", filename: "/a/b/MyTests/c.swift")
}
}
Tests/IntegrationTests/IntegrationTests.swift
+2 -2
View File
@@ -23,7 +23,7 @@ final class IntegrationTests: SwiftLintTestCase {
let swiftFiles = config.lintableFiles(
inPath: "",
forceExclude: false,
excludeByPrefix: false)
excludeBy: .paths(excludedPaths: config.excludedPaths()))
XCTAssert(
swiftFiles.contains(where: { #filePath.bridge().absolutePathRepresentation() == $0.path }),
"current file should be included"
@@ -48,7 +48,7 @@ final class IntegrationTests: SwiftLintTestCase {
let swiftFiles = config.lintableFiles(
inPath: "",
forceExclude: false,
excludeByPrefix: false)
excludeBy: .paths(excludedPaths: config.excludedPaths()))
let storage = RuleStorage()
let corrections = swiftFiles.parallelFlatMap {
Linter(file: $0, configuration: config).collect(into: storage).correct(using: storage)
bazel/repos.bzl
-7
View File
@@ -64,13 +64,6 @@ def swiftlint_repos(bzlmod = False):
url = "https://github.com/krzyzanowskim/CryptoSwift/archive/refs/tags/1.8.4.tar.gz",
)
http_archive(
name = "com_github_ileitch_swift-filename-matcher",
sha256 = "1adbb1eb042910f996689827f7dee217bebf7c5178f34178bcfe468b5b3268a2",
strip_prefix = "swift-filename-matcher-2.0.0",
url = "https://github.com/ileitch/swift-filename-matcher/archive/refs/tags/2.0.0.tar.gz",
)
def _swiftlint_repos_bzlmod(_):
swiftlint_repos(bzlmod = True)