Move `excludedPaths` out of iteration loop to speed up lint times (#4955)

This commit is contained in:
Andyy Hope 2023-05-07 04:43:33 +08:00 committed by GitHub
parent 7756793356
commit 5e15039554
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 98 additions and 37 deletions

View File

@ -96,6 +96,11 @@
[keith](https://github.com/keith)
[#4782](https://github.com/realm/SwiftLint/issues/4782)
* Improve lint times of SwiftLintPlugin by moving the
`excludedPaths(fileManager:)` operation out of the linting iterations.
[andyyhope](https://github.com/andyyhope)
[#4844](https://github.com/realm/SwiftLint/issues/4844)
## 0.51.0: bzllint
#### Breaking

View File

@ -1,6 +1,11 @@
import Foundation
extension Configuration {
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.
///
@ -12,8 +17,8 @@ extension Configuration {
///
/// - returns: Files to lint.
public func lintableFiles(inPath path: String, forceExclude: Bool,
excludeByPrefix: Bool = false) -> [SwiftLintFile] {
return lintablePaths(inPath: path, forceExclude: forceExclude, excludeByPrefix: excludeByPrefix)
excludeBy: ExcludeBy) -> [SwiftLintFile] {
return lintablePaths(inPath: path, forceExclude: forceExclude, excludeBy: excludeBy)
.compactMap(SwiftLintFile.init(pathDeferringReading:))
}
@ -30,14 +35,17 @@ extension Configuration {
internal func lintablePaths(
inPath path: String,
forceExclude: Bool,
excludeByPrefix: Bool = false,
excludeBy: ExcludeBy,
fileManager: LintableFileManager = FileManager.default
) -> [String] {
if fileManager.isFile(atPath: path) {
if forceExclude {
return excludeByPrefix
? filterExcludedPathsByPrefix(in: [path.absolutePathStandardized()])
: filterExcludedPaths(fileManager: fileManager, in: [path.absolutePathStandardized()])
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]
@ -48,9 +56,12 @@ extension Configuration {
.flatMap(Glob.resolveGlob)
.parallelFlatMap { fileManager.filesToLint(inPath: $0, rootDirectory: rootDirectory) }
return excludeByPrefix
? filterExcludedPathsByPrefix(in: pathsForPath, includedPaths)
: filterExcludedPaths(fileManager: fileManager, 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.
@ -60,7 +71,7 @@ extension Configuration {
///
/// - returns: The input paths after removing the excluded paths.
public func filterExcludedPaths(
fileManager: LintableFileManager = FileManager.default,
_ excludedPaths: [String],
in paths: [String]...
) -> [String] {
let allPaths = paths.flatMap { $0 }
@ -71,7 +82,6 @@ extension Configuration {
let result = NSMutableOrderedSet(array: allPaths)
#endif
let excludedPaths = self.excludedPaths(fileManager: fileManager)
result.minusSet(Set(excludedPaths))
// swiftlint:disable:next force_cast
return result.map { $0 as! String }
@ -98,7 +108,7 @@ extension Configuration {
/// - parameter fileManager: The file manager to get child paths in a given parent location.
///
/// - returns: The expanded excluded file paths.
private func excludedPaths(fileManager: LintableFileManager) -> [String] {
public func excludedPaths(fileManager: LintableFileManager = FileManager.default) -> [String] {
return excludedPaths
.flatMap(Glob.resolveGlob)
.parallelFlatMap { fileManager.filesToLint(inPath: $0, rootDirectory: rootDirectory) }

View File

@ -222,10 +222,14 @@ extension Configuration {
}
let scriptInputPaths = files.compactMap { $0.path }
let filesToLint = visitor.useExcludingByPrefix ?
filterExcludedPathsByPrefix(in: scriptInputPaths) :
filterExcludedPaths(in: scriptInputPaths)
return filesToLint.map(SwiftLintFile.init(pathDeferringReading:))
if visitor.useExcludingByPrefix {
return filterExcludedPathsByPrefix(in: scriptInputPaths)
.map(SwiftLintFile.init(pathDeferringReading:))
} else {
return filterExcludedPaths(excludedPaths(), in: scriptInputPaths)
.map(SwiftLintFile.init(pathDeferringReading:))
}
}
if !visitor.quiet {
let filesInfo: String
@ -238,8 +242,12 @@ extension Configuration {
queuedPrintError("\(visitor.action) Swift files \(filesInfo)")
}
return visitor.paths.flatMap {
self.lintableFiles(inPath: $0, forceExclude: visitor.forceExclude,
excludeByPrefix: visitor.useExcludingByPrefix)
self.lintableFiles(
inPath: $0,
forceExclude: visitor.forceExclude,
excludeBy: visitor.useExcludingByPrefix
? .prefix
: .paths(excludedPaths: excludedPaths()))
}
}

View File

@ -17,7 +17,10 @@ private let config: Configuration = {
class IntegrationTests: SwiftLintTestCase {
func testSwiftLintLints() {
// This is as close as we're ever going to get to a self-hosting linter.
let swiftFiles = config.lintableFiles(inPath: "", forceExclude: false)
let swiftFiles = config.lintableFiles(
inPath: "",
forceExclude: false,
excludeBy: .paths(excludedPaths: config.excludedPaths()))
XCTAssert(
swiftFiles.contains(where: { #file.bridge().absolutePathRepresentation() == $0.path }),
"current file should be included"
@ -35,7 +38,10 @@ class IntegrationTests: SwiftLintTestCase {
}
func testSwiftLintAutoCorrects() {
let swiftFiles = config.lintableFiles(inPath: "", forceExclude: false)
let swiftFiles = config.lintableFiles(
inPath: "",
forceExclude: false,
excludeBy: .paths(excludedPaths: config.excludedPaths()))
let storage = RuleStorage()
let corrections = swiftFiles.parallelFlatMap {
Linter(file: $0, configuration: config).collect(into: storage).correct(using: storage)

View File

@ -251,43 +251,69 @@ class ConfigurationTests: SwiftLintTestCase {
}
func testExcludedPaths() {
let fileManager = TestFileManager()
let configuration = Configuration(includedPaths: ["directory"],
excludedPaths: ["directory/excluded",
"directory/ExcludedFile.swift"])
let paths = configuration.lintablePaths(inPath: "", forceExclude: false, fileManager: TestFileManager())
let excludedPaths = configuration.excludedPaths(fileManager: fileManager)
let paths = configuration.lintablePaths(inPath: "",
forceExclude: false,
excludeBy: .paths(excludedPaths: excludedPaths),
fileManager: fileManager)
XCTAssertEqual(["directory/File1.swift", "directory/File2.swift"].absolutePathsStandardized(), paths)
}
func testForceExcludesFile() {
let fileManager = TestFileManager()
let configuration = Configuration(excludedPaths: ["directory/ExcludedFile.swift"])
let paths = configuration.lintablePaths(inPath: "directory/ExcludedFile.swift", forceExclude: true,
fileManager: TestFileManager())
let excludedPaths = configuration.excludedPaths(fileManager: fileManager)
let paths = configuration.lintablePaths(inPath: "directory/ExcludedFile.swift",
forceExclude: true,
excludeBy: .paths(excludedPaths: excludedPaths),
fileManager: fileManager)
XCTAssertEqual([], paths)
}
func testForceExcludesFileNotPresentInExcluded() {
let fileManager = TestFileManager()
let configuration = Configuration(includedPaths: ["directory"],
excludedPaths: ["directory/ExcludedFile.swift", "directory/excluded"])
let paths = configuration.lintablePaths(inPath: "", forceExclude: true, fileManager: TestFileManager())
let excludedPaths = configuration.excludedPaths(fileManager: fileManager)
let paths = configuration.lintablePaths(inPath: "",
forceExclude: true,
excludeBy: .paths(excludedPaths: excludedPaths),
fileManager: fileManager)
XCTAssertEqual(["directory/File1.swift", "directory/File2.swift"].absolutePathsStandardized(), paths)
}
func testForceExcludesDirectory() {
let fileManager = TestFileManager()
let configuration = Configuration(excludedPaths: ["directory/excluded", "directory/ExcludedFile.swift"])
let paths = configuration.lintablePaths(inPath: "directory", forceExclude: true,
fileManager: TestFileManager())
let excludedPaths = configuration.excludedPaths(fileManager: fileManager)
let paths = configuration.lintablePaths(inPath: "directory",
forceExclude: true,
excludeBy: .paths(excludedPaths: excludedPaths),
fileManager: fileManager)
XCTAssertEqual(["directory/File1.swift", "directory/File2.swift"].absolutePathsStandardized(), paths)
}
func testForceExcludesDirectoryThatIsNotInExcludedButHasChildrenThatAre() {
let fileManager = TestFileManager()
let configuration = Configuration(excludedPaths: ["directory/excluded", "directory/ExcludedFile.swift"])
let paths = configuration.lintablePaths(inPath: "directory", forceExclude: true,
fileManager: TestFileManager())
let excludedPaths = configuration.excludedPaths(fileManager: fileManager)
let paths = configuration.lintablePaths(inPath: "directory",
forceExclude: true,
excludeBy: .paths(excludedPaths: excludedPaths),
fileManager: fileManager)
XCTAssertEqual(["directory/File1.swift", "directory/File2.swift"].absolutePathsStandardized(), paths)
}
func testLintablePaths() {
let paths = Configuration.default.lintablePaths(inPath: Mock.Dir.level0, forceExclude: false)
let excluded = Configuration.default.excludedPaths(fileManager: TestFileManager())
let paths = Configuration.default.lintablePaths(inPath: Mock.Dir.level0,
forceExclude: false,
excludeBy: .paths(excludedPaths: excluded))
let filenames = paths.map { $0.bridge().lastPathComponent }.sorted()
let expectedFilenames = [
"DirectoryLevel1.swift",
@ -301,7 +327,9 @@ class ConfigurationTests: SwiftLintTestCase {
func testGlobIncludePaths() {
FileManager.default.changeCurrentDirectoryPath(Mock.Dir.level0)
let configuration = Configuration(includedPaths: ["**/Level2"])
let paths = configuration.lintablePaths(inPath: Mock.Dir.level0, forceExclude: true)
let paths = configuration.lintablePaths(inPath: Mock.Dir.level0,
forceExclude: true,
excludeBy: .paths(excludedPaths: configuration.excludedPaths))
let filenames = paths.map { $0.bridge().lastPathComponent }.sorted()
let expectedFilenames = ["Level2.swift", "Level3.swift"]
@ -314,7 +342,11 @@ class ConfigurationTests: SwiftLintTestCase {
excludedPaths: [Mock.Dir.level3.stringByAppendingPathComponent("*.swift")]
)
XCTAssertEqual(configuration.lintablePaths(inPath: "", forceExclude: false), [])
let excludedPaths = configuration.excludedPaths()
let lintablePaths = configuration.lintablePaths(inPath: "",
forceExclude: false,
excludeBy: .paths(excludedPaths: excludedPaths))
XCTAssertEqual(lintablePaths, [])
}
// MARK: - Testing Configuration Equality
@ -400,7 +432,7 @@ extension ConfigurationTests {
"Level1/Level2/Level3"])
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"])
}
@ -410,7 +442,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)
}
@ -420,7 +452,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)
}
@ -434,7 +466,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)
}
@ -448,7 +480,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)
}
@ -460,7 +492,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"])
}