From 85c2c7edaf1c5c669919c55083822893753053aa Mon Sep 17 00:00:00 2001 From: Dan Federman Date: Thu, 3 Oct 2024 09:27:57 -0700 Subject: [PATCH] Reduce main thread usage in SafeDITool (#114) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Use complete argument list * Reduce main thread usage in SafeDITool * Improve code coverage * More simplification * Remove Requirements section from README – defer instead to Package.swift --- README.md | 8 -- .../Extensions/CollectionExtensions.swift | 6 ++ .../Generators/DependencyTreeGenerator.swift | 4 +- .../SafeDICore/Models/TypeDescription.swift | 16 +--- Sources/SafeDITool/SafeDITool.swift | 89 +++++++++++-------- .../Helpers/SafeDIToolTestExecution.swift | 3 +- .../SafeDIToolCodeGenerationErrorTests.swift | 2 + 7 files changed, 68 insertions(+), 60 deletions(-) diff --git a/README.md b/README.md index a302a23..ccaa3f9 100644 --- a/README.md +++ b/README.md @@ -484,14 +484,6 @@ The `SafeDITool` utility is designed to able to be integrated into projects of a The `SafeDITool` can parse all of your Swift files at once, or for even better performance, the tool can be run on each dependent module as part of the build. Running this tool on each dependent module is currently left as an exercise to the reader. -### Requirements - -* Xcode 16.0 or later -* iOS 13 or later -* tvOS 13 or later -* watchOS 6 or later -* macOS 10.13 or later - ## Under the hood SafeDI has a `SafeDITool` executable that the `SafeDIGenerator` plugin utilizes to read code and generate a dependency tree. The tool utilizes Apple‘s [SwiftSyntax](https://github.com/apple/swift-syntax) library to parse your code and find your `@Instantiable` types‘ initializers and dependencies. With this information, SafeDI creates a graph of your project‘s dependencies. This graph is validated as part of the `SafeDITool`‘s execution, and the tool emits human-readible errors if the dependency graph is not valid. Source code is only generated if the dependency graph is valid. diff --git a/Sources/SafeDICore/Extensions/CollectionExtensions.swift b/Sources/SafeDICore/Extensions/CollectionExtensions.swift index e885a20..6da5c13 100644 --- a/Sources/SafeDICore/Extensions/CollectionExtensions.swift +++ b/Sources/SafeDICore/Extensions/CollectionExtensions.swift @@ -68,3 +68,9 @@ extension Collection { return try arrayToTransform.map { try transform($0) } + [lastItem] } } + +extension Collection { + public func removingEmpty() -> [Element] { + filter { !$0.isEmpty } + } +} diff --git a/Sources/SafeDICore/Generators/DependencyTreeGenerator.swift b/Sources/SafeDICore/Generators/DependencyTreeGenerator.swift index 75342c2..f41c8b5 100644 --- a/Sources/SafeDICore/Generators/DependencyTreeGenerator.swift +++ b/Sources/SafeDICore/Generators/DependencyTreeGenerator.swift @@ -47,7 +47,7 @@ public actor DependencyTreeGenerator { for try await generatedRoot in taskGroup { generatedRoots.append(generatedRoot) } - return generatedRoots.filter { !$0.isEmpty }.sorted().joined(separator: "\n\n") + return generatedRoots.removingEmpty().sorted().joined(separator: "\n\n") } let importsWhitespace = imports.isEmpty ? "" : "\n" @@ -74,7 +74,7 @@ public actor DependencyTreeGenerator { for try await generatedRoot in taskGroup { generatedRoots.append(generatedRoot) } - return generatedRoots.filter { !$0.isEmpty }.sorted().joined(separator: "\n\n") + return generatedRoots.removingEmpty().sorted().joined(separator: "\n\n") } return """ diff --git a/Sources/SafeDICore/Models/TypeDescription.swift b/Sources/SafeDICore/Models/TypeDescription.swift index ec3527f..d74ead1 100644 --- a/Sources/SafeDICore/Models/TypeDescription.swift +++ b/Sources/SafeDICore/Models/TypeDescription.swift @@ -131,7 +131,7 @@ public enum TypeDescription: Codable, Hashable, Comparable, Sendable { }.joined(separator: ", "))) """ case let .closure(arguments, isAsync, doesThrow, returnType): - return "(\(arguments.map(\.asSource).joined(separator: ", ")))\([isAsync ? " async" : "", doesThrow ? " throws" : ""].filter { !$0.isEmpty }.joined()) -> \(returnType.asSource)" + return "(\(arguments.map(\.asSource).joined(separator: ", ")))\([isAsync ? " async" : "", doesThrow ? " throws" : ""].removingEmpty().joined()) -> \(returnType.asSource)" case let .unknown(text): return text } @@ -519,18 +519,10 @@ private final class GenericArgumentVisitor: SyntaxVisitor { extension TypeSpecifierListSyntax { fileprivate var textRepresentation: [String]? { - let specifiers = compactMap { specifier in - if case let .simpleTypeSpecifier(simpleTypeSpecifierSyntax) = specifier { - simpleTypeSpecifierSyntax.specifier.text - } else { - // lifetimeTypeSpecifier is SPI, so we ignore it. - nil - } - } - if specifiers.isEmpty { - return nil + if isEmpty { + nil } else { - return specifiers + compactMap(\.trimmedDescription) } } } diff --git a/Sources/SafeDITool/SafeDITool.swift b/Sources/SafeDITool/SafeDITool.swift index f33764e..ea492c2 100644 --- a/Sources/SafeDITool/SafeDITool.swift +++ b/Sources/SafeDITool/SafeDITool.swift @@ -46,7 +46,6 @@ struct SafeDITool: AsyncParsableCommand, Sendable { // MARK: Internal - @MainActor func run() async throws { if swiftSourcesFilePath == nil, include.isEmpty { throw ValidationError("Must provide either 'swift-sources-file-path' or '--include'.") @@ -105,44 +104,58 @@ struct SafeDITool: AsyncParsableCommand, Sendable { // MARK: Private - @MainActor private func findSwiftFiles() async throws -> Set { - var swiftFiles = Set() - if let swiftSourcesFilePath { - try swiftFiles.formUnion( - String(contentsOfFile: swiftSourcesFilePath) - .components(separatedBy: CharacterSet(arrayLiteral: ",")) - .filter { !$0.isEmpty } - ) - } - for included in include { - let includedURL = included.asFileURL - let includedFileEnumerator = fileFinder - .enumerator( - at: includedURL, - includingPropertiesForKeys: nil, - options: [.skipsHiddenFiles], - errorHandler: nil - ) - guard let files = includedFileEnumerator?.compactMap({ $0 as? URL }) else { - struct CouldNotEnumerateDirectoryError: Error, CustomStringConvertible { - let directory: String - - var description: String { - "Could not create file enumerator for directory '\(directory)'" - } + try await withThrowingTaskGroup( + of: [String].self, + returning: Set.self + ) { taskGroup in + taskGroup.addTask { + if let swiftSourcesFilePath { + try String(contentsOfFile: swiftSourcesFilePath) + .components(separatedBy: CharacterSet(arrayLiteral: ",")) + .removingEmpty() + } else { + [] } - throw CouldNotEnumerateDirectoryError(directory: included) } - swiftFiles.formUnion((files + [includedURL]).compactMap { - if $0.pathExtension == "swift" { - $0.standardizedFileURL.relativePath - } else { - nil + let fileFinder = await fileFinder + for included in include { + taskGroup.addTask { + let includedURL = included.asFileURL + let includedFileEnumerator = fileFinder + .enumerator( + at: includedURL, + includingPropertiesForKeys: nil, + options: [.skipsHiddenFiles], + errorHandler: nil + ) + guard let files = includedFileEnumerator?.compactMap({ $0 as? URL }) else { + struct CouldNotEnumerateDirectoryError: Error, CustomStringConvertible { + let directory: String + + var description: String { + "Could not create file enumerator for directory '\(directory)'" + } + } + throw CouldNotEnumerateDirectoryError(directory: included) + } + return (files + [includedURL]).compactMap { + if $0.pathExtension == "swift" { + $0.standardizedFileURL.relativePath + } else { + nil + } + } } - }) + } + + var swiftFiles = Set() + for try await includedFiles in taskGroup { + swiftFiles.formUnion(includedFiles) + } + + return swiftFiles } - return swiftFiles } private func loadSwiftFiles() async throws -> [String] { @@ -201,7 +214,7 @@ struct SafeDITool: AsyncParsableCommand, Sendable { try .init( String(contentsOfFile: dependentModuleInfoFilePath) .components(separatedBy: CharacterSet(arrayLiteral: ",")) - .filter { !$0.isEmpty } + .removingEmpty() .map(\.asFileURL) ) } else { @@ -304,7 +317,7 @@ extension String { } } -protocol FileFinder { +protocol FileFinder: Sendable { func enumerator( at url: URL, includingPropertiesForKeys keys: [URLResourceKey]?, @@ -314,5 +327,9 @@ protocol FileFinder { } extension FileManager: FileFinder {} +extension FileManager: @retroactive @unchecked Sendable { + // FileManager is thread safe: + // https://developer.apple.com/documentation/foundation/nsfilemanager#1651181 +} @MainActor var fileFinder: FileFinder = FileManager.default diff --git a/Tests/SafeDIToolTests/Helpers/SafeDIToolTestExecution.swift b/Tests/SafeDIToolTests/Helpers/SafeDIToolTestExecution.swift index 8dae66d..80177eb 100644 --- a/Tests/SafeDIToolTests/Helpers/SafeDIToolTestExecution.swift +++ b/Tests/SafeDIToolTests/Helpers/SafeDIToolTestExecution.swift @@ -133,11 +133,10 @@ struct StubFileFinder: FileFinder { let files: [URL] } -@MainActor func assertThrowsError( _ errorDescription: String, line: UInt = #line, - block: @MainActor () async throws -> some Any + block: @MainActor () async throws -> some Sendable ) async { do { _ = try await block() diff --git a/Tests/SafeDIToolTests/SafeDIToolCodeGenerationErrorTests.swift b/Tests/SafeDIToolTests/SafeDIToolCodeGenerationErrorTests.swift index 940151d..bc14691 100644 --- a/Tests/SafeDIToolTests/SafeDIToolCodeGenerationErrorTests.swift +++ b/Tests/SafeDIToolTests/SafeDIToolCodeGenerationErrorTests.swift @@ -1733,6 +1733,7 @@ final class SafeDIToolCodeGenerationErrorTests: XCTestCase { tool.moduleInfoOutput = nil tool.dependentModuleInfoFilePath = nil tool.dependencyTreeOutput = nil + tool.dotFileOutput = nil await assertThrowsError("Could not create file enumerator for directory 'Fake'") { try await tool.run() } @@ -1746,6 +1747,7 @@ final class SafeDIToolCodeGenerationErrorTests: XCTestCase { tool.moduleInfoOutput = nil tool.dependentModuleInfoFilePath = nil tool.dependencyTreeOutput = nil + tool.dotFileOutput = nil await assertThrowsError("Must provide either 'swift-sources-file-path' or '--include'.") { try await tool.run() }