diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a8debd45..abaabfc18 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/Source/SwiftLintCore/Extensions/Configuration+LintableFiles.swift b/Source/SwiftLintCore/Extensions/Configuration+LintableFiles.swift index d1c6dd649..3c070a10d 100644 --- a/Source/SwiftLintCore/Extensions/Configuration+LintableFiles.swift +++ b/Source/SwiftLintCore/Extensions/Configuration+LintableFiles.swift @@ -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) } diff --git a/Source/swiftlint/Extensions/Configuration+CommandLine.swift b/Source/swiftlint/Extensions/Configuration+CommandLine.swift index 18a9b8753..45a357077 100644 --- a/Source/swiftlint/Extensions/Configuration+CommandLine.swift +++ b/Source/swiftlint/Extensions/Configuration+CommandLine.swift @@ -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())) } } diff --git a/Tests/IntegrationTests/IntegrationTests.swift b/Tests/IntegrationTests/IntegrationTests.swift index f1bcc162c..bca4c5ace 100644 --- a/Tests/IntegrationTests/IntegrationTests.swift +++ b/Tests/IntegrationTests/IntegrationTests.swift @@ -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) diff --git a/Tests/SwiftLintFrameworkTests/ConfigurationTests.swift b/Tests/SwiftLintFrameworkTests/ConfigurationTests.swift index b701fe0a8..81b10d002 100644 --- a/Tests/SwiftLintFrameworkTests/ConfigurationTests.swift +++ b/Tests/SwiftLintFrameworkTests/ConfigurationTests.swift @@ -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"]) }