From 167a9c05c1802d21000420b157654faa2a19ac98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= Date: Sat, 1 Feb 2025 10:37:30 +0100 Subject: [PATCH 1/3] Ensure that content is complete using a queue --- BUILD | 1 + MODULE.bazel | 1 + Package.resolved | 9 +++++++ Package.swift | 2 ++ Source/SwiftLintCore/Models/Issue.swift | 35 +++++++++++-------------- bazel/Queue.BUILD | 11 ++++++++ bazel/repos.bzl | 8 ++++++ 7 files changed, 48 insertions(+), 19 deletions(-) create mode 100644 bazel/Queue.BUILD diff --git a/BUILD b/BUILD index 1174d12b31..e669ddf584 100644 --- a/BUILD +++ b/BUILD @@ -94,6 +94,7 @@ swift_library( ":SourceKittenFramework.wrapper", "@sourcekitten_com_github_jpsim_yams//:Yams", "@swiftlint_com_github_scottrhoyt_swifty_text_table//:SwiftyTextTable", + "@com_github_mattmassicotte_queue//:Queue", ] + select({ "@platforms//os:linux": ["@com_github_krzyzanowskim_cryptoswift//:CryptoSwift"], "//conditions:default": [":DyldWarningWorkaround"], diff --git a/MODULE.bazel b/MODULE.bazel index 5ec8e0f4ea..b4dc104131 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -19,6 +19,7 @@ swiftlint_repos = use_extension("//bazel:repos.bzl", "swiftlint_repos_bzlmod") use_repo( swiftlint_repos, "com_github_johnsundell_collectionconcurrencykit", + "com_github_mattmassicotte_queue", "com_github_krzyzanowskim_cryptoswift", "swiftlint_com_github_scottrhoyt_swifty_text_table", ) diff --git a/Package.resolved b/Package.resolved index baa9895fe5..87d004d68d 100644 --- a/Package.resolved +++ b/Package.resolved @@ -18,6 +18,15 @@ "version" : "1.8.4" } }, + { + "identity" : "queue", + "kind" : "remoteSourceControl", + "location" : "https://github.com/mattmassicotte/Queue", + "state" : { + "revision" : "6adf359a705e3252742905b413bb8f56401043ca", + "version" : "0.2.0" + } + }, { "identity" : "sourcekitten", "kind" : "remoteSourceControl", diff --git a/Package.swift b/Package.swift index 57ab9dcf70..34d8f9c992 100644 --- a/Package.swift +++ b/Package.swift @@ -39,6 +39,7 @@ 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/mattmassicotte/Queue", from: "0.2.0"), ], targets: [ .executableTarget( @@ -93,6 +94,7 @@ let package = Package( .product(name: "SwiftSyntaxBuilder", package: "swift-syntax"), .product(name: "SwiftyTextTable", package: "SwiftyTextTable"), .product(name: "Yams", package: "Yams"), + .product(name: "Queue", package: "Queue"), "SwiftLintCoreMacros", ], swiftSettings: swiftFeatures + strictConcurrency diff --git a/Source/SwiftLintCore/Models/Issue.swift b/Source/SwiftLintCore/Models/Issue.swift index 830911c8a9..aea20fbda9 100644 --- a/Source/SwiftLintCore/Models/Issue.swift +++ b/Source/SwiftLintCore/Models/Issue.swift @@ -1,4 +1,5 @@ import Foundation +import Queue /// All possible SwiftLint issues which are printed as warnings by default. public enum Issue: LocalizedError, Equatable { @@ -84,7 +85,8 @@ public enum Issue: LocalizedError, Equatable { /// Flag to enable warnings for deprecations being printed to the console. Printing is enabled by default. package nonisolated(unsafe) static var printDeprecationWarnings = true - @TaskLocal private static var messageConsumer: (@Sendable (String) -> Void)? + @TaskLocal private static var messageConsumer: (@Sendable @MainActor (String) async -> Void)? + @TaskLocal private static var printQueue: AsyncQueue? /// Hook used to capture all messages normally printed to stdout and return them back to the caller. /// @@ -95,18 +97,18 @@ public enum Issue: LocalizedError, Equatable { /// - returns: The collected messages produced while running the code in the runner. @MainActor static func captureConsole(runner: @Sendable () throws -> Void) async rethrows -> String { - actor Console { - static var content = "" - } - defer { - Console.content = "" - } - try $messageConsumer.withValue( - { Console.content += (Console.content.isEmpty ? "" : "\n") + $0 }, - operation: runner + var content = [String]() + let queue = AsyncQueue() + try $printQueue.withValue( + queue, + operation: { + try $messageConsumer.withValue( + { content.append($0) }, + operation: runner + ) + } ) - await Task.yield() - return Console.content + return await queue.addBarrierOperation { content.joined(separator: "\n") }.value } /// Wraps any `Error` into a `SwiftLintError.genericWarning` if it is not already a `SwiftLintError`. @@ -140,13 +142,8 @@ public enum Issue: LocalizedError, Equatable { if case .ruleDeprecated = self, !Self.printDeprecationWarnings { return } - Task(priority: .high) { @MainActor in - if let consumer = Self.messageConsumer { - consumer(errorDescription) - } else { - queuedPrintError(errorDescription) - } - } + Self.printQueue?.addOperation { await Self.messageConsumer?(errorDescription) } + queuedPrintError(errorDescription) } private var message: String { diff --git a/bazel/Queue.BUILD b/bazel/Queue.BUILD new file mode 100644 index 0000000000..a7e9e849b3 --- /dev/null +++ b/bazel/Queue.BUILD @@ -0,0 +1,11 @@ +load( + "@build_bazel_rules_swift//swift:swift.bzl", + "swift_library", +) + +swift_library( + name = "Queue", + srcs = glob(["Sources/Queue/*.swift"]), + module_name = "Queue", + visibility = ["//visibility:public"], +) diff --git a/bazel/repos.bzl b/bazel/repos.bzl index 592acd82f2..920dc5930a 100644 --- a/bazel/repos.bzl +++ b/bazel/repos.bzl @@ -56,6 +56,14 @@ def swiftlint_repos(bzlmod = False): url = "https://github.com/JohnSundell/CollectionConcurrencyKit/archive/refs/tags/0.2.0.tar.gz", ) + http_archive( + name = "com_github_mattmassicotte_queue", + sha256 = "2c00cde22406d868b2341432c119c73d0c1a8ce40a663469b1b8cc5c39007737", + build_file = "@SwiftLint//bazel:Queue.BUILD", + strip_prefix = "Queue-0.2.0", + url = "https://github.com/mattmassicotte/Queue/archive/refs/tags/0.2.0.tar.gz", + ) + http_archive( name = "com_github_krzyzanowskim_cryptoswift", sha256 = "69b23102ff453990d03aff4d3fabd172d0667b2b3ed95730021d60a0f8d50d14", From 9fb4a0d260bb99d84af4827a41e375840ad4a02b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= Date: Sat, 1 Feb 2025 15:58:35 +0100 Subject: [PATCH 2/3] Avoid async assert methods --- ...licitTypeInterfaceConfigurationTests.swift | 11 ++--- .../IndentationWidthRuleTests.swift | 17 ++++---- ...ultilineParametersConfigurationTests.swift | 30 +++++++------ .../NoEmptyBlockConfigurationTests.swift | 11 ++--- .../RuleConfigurationDescriptionTests.swift | 43 ++++++++++--------- 5 files changed, 59 insertions(+), 53 deletions(-) diff --git a/Tests/BuiltInRulesTests/ExplicitTypeInterfaceConfigurationTests.swift b/Tests/BuiltInRulesTests/ExplicitTypeInterfaceConfigurationTests.swift index 6108180ba8..3fe7ace943 100644 --- a/Tests/BuiltInRulesTests/ExplicitTypeInterfaceConfigurationTests.swift +++ b/Tests/BuiltInRulesTests/ExplicitTypeInterfaceConfigurationTests.swift @@ -25,11 +25,12 @@ final class ExplicitTypeInterfaceConfigurationTests: SwiftLintTestCase { } func testInvalidKeyInCustomConfiguration() async throws { - try await AsyncAssertEqual( - try await Issue.captureConsole { - var config = ExplicitTypeInterfaceConfiguration() - try config.apply(configuration: ["invalidKey": "error"]) - }, + let console = try await Issue.captureConsole { + var config = ExplicitTypeInterfaceConfiguration() + try config.apply(configuration: ["invalidKey": "error"]) + } + XCTAssertEqual( + console, "warning: Configuration for 'explicit_type_interface' rule contains the invalid key(s) 'invalidKey'." ) } diff --git a/Tests/BuiltInRulesTests/IndentationWidthRuleTests.swift b/Tests/BuiltInRulesTests/IndentationWidthRuleTests.swift index 498745be78..936dd77bfc 100644 --- a/Tests/BuiltInRulesTests/IndentationWidthRuleTests.swift +++ b/Tests/BuiltInRulesTests/IndentationWidthRuleTests.swift @@ -8,14 +8,15 @@ final class IndentationWidthRuleTests: SwiftLintTestCase { let defaultValue = IndentationWidthConfiguration().indentationWidth for indentation in [0, -1, -5] { - try await AsyncAssertEqual( - try await Issue.captureConsole { - var testee = IndentationWidthConfiguration() - try testee.apply(configuration: ["indentation_width": indentation]) - - // Value remains the default. - XCTAssertEqual(testee.indentationWidth, defaultValue) - }, + let console = try await Issue.captureConsole { + var testee = IndentationWidthConfiguration() + try testee.apply(configuration: ["indentation_width": indentation]) + + // Value remains the default. + XCTAssertEqual(testee.indentationWidth, defaultValue) + } + XCTAssertEqual( + console, "warning: Invalid configuration for 'indentation_width' rule. Falling back to default." ) } diff --git a/Tests/BuiltInRulesTests/MultilineParametersConfigurationTests.swift b/Tests/BuiltInRulesTests/MultilineParametersConfigurationTests.swift index 6d19466b94..2915e344d4 100644 --- a/Tests/BuiltInRulesTests/MultilineParametersConfigurationTests.swift +++ b/Tests/BuiltInRulesTests/MultilineParametersConfigurationTests.swift @@ -6,13 +6,14 @@ import XCTest final class MultilineParametersConfigurationTests: SwiftLintTestCase { func testInvalidMaxNumberOfSingleLineParameters() async throws { for maxNumberOfSingleLineParameters in [0, -1] { - try await AsyncAssertEqual( - try await Issue.captureConsole { - var config = MultilineParametersConfiguration() - try config.apply( - configuration: ["max_number_of_single_line_parameters": maxNumberOfSingleLineParameters] - ) - }, + let console = try await Issue.captureConsole { + var config = MultilineParametersConfiguration() + try config.apply( + configuration: ["max_number_of_single_line_parameters": maxNumberOfSingleLineParameters] + ) + } + XCTAssertEqual( + console, """ warning: Inconsistent configuration for 'multiline_parameters' rule: Option \ 'max_number_of_single_line_parameters' should be >= 1. @@ -22,13 +23,14 @@ final class MultilineParametersConfigurationTests: SwiftLintTestCase { } func testInvalidMaxNumberOfSingleLineParametersWithSingleLineEnabled() async throws { - try await AsyncAssertEqual( - try await Issue.captureConsole { - var config = MultilineParametersConfiguration() - try config.apply( - configuration: ["max_number_of_single_line_parameters": 2, "allows_single_line": false] - ) - }, + let console = try await Issue.captureConsole { + var config = MultilineParametersConfiguration() + try config.apply( + configuration: ["max_number_of_single_line_parameters": 2, "allows_single_line": false] + ) + } + XCTAssertEqual( + console, """ warning: Inconsistent configuration for 'multiline_parameters' rule: Option \ 'max_number_of_single_line_parameters' has no effect when 'allows_single_line' is false. diff --git a/Tests/BuiltInRulesTests/NoEmptyBlockConfigurationTests.swift b/Tests/BuiltInRulesTests/NoEmptyBlockConfigurationTests.swift index f555bbbcb7..737172b732 100644 --- a/Tests/BuiltInRulesTests/NoEmptyBlockConfigurationTests.swift +++ b/Tests/BuiltInRulesTests/NoEmptyBlockConfigurationTests.swift @@ -23,11 +23,12 @@ final class NoEmptyBlockConfigurationTests: SwiftLintTestCase { } func testInvalidKeyInCustomConfiguration() async throws { - try await AsyncAssertEqual( - try await Issue.captureConsole { - var config = NoEmptyBlockConfiguration() - try config.apply(configuration: ["invalidKey": "error"]) - }, + let console = try await Issue.captureConsole { + var config = NoEmptyBlockConfiguration() + try config.apply(configuration: ["invalidKey": "error"]) + } + XCTAssertEqual( + console, "warning: Configuration for 'no_empty_block' rule contains the invalid key(s) 'invalidKey'." ) } diff --git a/Tests/FrameworkTests/RuleConfigurationDescriptionTests.swift b/Tests/FrameworkTests/RuleConfigurationDescriptionTests.swift index ddda2f436d..842c2ee3f9 100644 --- a/Tests/FrameworkTests/RuleConfigurationDescriptionTests.swift +++ b/Tests/FrameworkTests/RuleConfigurationDescriptionTests.swift @@ -491,35 +491,36 @@ final class RuleConfigurationDescriptionTests: SwiftLintTestCase { } func testDeprecationWarning() async throws { - try await AsyncAssertEqual( - try await Issue.captureConsole { - var configuration = TestConfiguration() - try configuration.apply(configuration: ["set": [6, 7]]) - }, + let console = try await Issue.captureConsole { + var configuration = TestConfiguration() + try configuration.apply(configuration: ["set": [6, 7]]) + } + XCTAssertEqual( + console, "warning: Configuration option 'set' in 'my_rule' rule is deprecated. Use the option 'other_opt' instead." ) } func testNoDeprecationWarningIfNoDeprecatedPropertySet() async throws { - try await AsyncAssertTrue( - try await Issue.captureConsole { - var configuration = TestConfiguration() - try configuration.apply(configuration: ["flag": false]) - }.isEmpty - ) + let console = try await Issue.captureConsole { + var configuration = TestConfiguration() + try configuration.apply(configuration: ["flag": false]) + } + XCTAssertTrue(console.isEmpty) } func testInvalidKeys() async throws { - try await AsyncAssertEqual( - try await Issue.captureConsole { - var configuration = TestConfiguration() - try configuration.apply(configuration: [ - "severity": "error", - "warning": 3, - "unknown": 1, - "unsupported": true, - ]) - }, + let console = try await Issue.captureConsole { + var configuration = TestConfiguration() + try configuration.apply(configuration: [ + "severity": "error", + "warning": 3, + "unknown": 1, + "unsupported": true, + ]) + } + XCTAssertEqual( + console, "warning: Configuration for 'RuleMock' rule contains the invalid key(s) 'unknown', 'unsupported'." ) } From a9c8e0ce1bbc9d199083e75a7ddd013319143d1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Danny=20M=C3=B6sch?= Date: Mon, 10 Feb 2025 19:32:32 +0100 Subject: [PATCH 3/3] Make use of AsyncThrowingStream --- BUILD | 1 - MODULE.bazel | 1 - Package.resolved | 9 --------- Package.swift | 2 -- Source/SwiftLintCore/Models/Issue.swift | 22 ++++++---------------- bazel/Queue.BUILD | 11 ----------- bazel/repos.bzl | 8 -------- 7 files changed, 6 insertions(+), 48 deletions(-) delete mode 100644 bazel/Queue.BUILD diff --git a/BUILD b/BUILD index e669ddf584..1174d12b31 100644 --- a/BUILD +++ b/BUILD @@ -94,7 +94,6 @@ swift_library( ":SourceKittenFramework.wrapper", "@sourcekitten_com_github_jpsim_yams//:Yams", "@swiftlint_com_github_scottrhoyt_swifty_text_table//:SwiftyTextTable", - "@com_github_mattmassicotte_queue//:Queue", ] + select({ "@platforms//os:linux": ["@com_github_krzyzanowskim_cryptoswift//:CryptoSwift"], "//conditions:default": [":DyldWarningWorkaround"], diff --git a/MODULE.bazel b/MODULE.bazel index b4dc104131..5ec8e0f4ea 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -19,7 +19,6 @@ swiftlint_repos = use_extension("//bazel:repos.bzl", "swiftlint_repos_bzlmod") use_repo( swiftlint_repos, "com_github_johnsundell_collectionconcurrencykit", - "com_github_mattmassicotte_queue", "com_github_krzyzanowskim_cryptoswift", "swiftlint_com_github_scottrhoyt_swifty_text_table", ) diff --git a/Package.resolved b/Package.resolved index 87d004d68d..baa9895fe5 100644 --- a/Package.resolved +++ b/Package.resolved @@ -18,15 +18,6 @@ "version" : "1.8.4" } }, - { - "identity" : "queue", - "kind" : "remoteSourceControl", - "location" : "https://github.com/mattmassicotte/Queue", - "state" : { - "revision" : "6adf359a705e3252742905b413bb8f56401043ca", - "version" : "0.2.0" - } - }, { "identity" : "sourcekitten", "kind" : "remoteSourceControl", diff --git a/Package.swift b/Package.swift index 34d8f9c992..57ab9dcf70 100644 --- a/Package.swift +++ b/Package.swift @@ -39,7 +39,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/mattmassicotte/Queue", from: "0.2.0"), ], targets: [ .executableTarget( @@ -94,7 +93,6 @@ let package = Package( .product(name: "SwiftSyntaxBuilder", package: "swift-syntax"), .product(name: "SwiftyTextTable", package: "SwiftyTextTable"), .product(name: "Yams", package: "Yams"), - .product(name: "Queue", package: "Queue"), "SwiftLintCoreMacros", ], swiftSettings: swiftFeatures + strictConcurrency diff --git a/Source/SwiftLintCore/Models/Issue.swift b/Source/SwiftLintCore/Models/Issue.swift index aea20fbda9..b16417b139 100644 --- a/Source/SwiftLintCore/Models/Issue.swift +++ b/Source/SwiftLintCore/Models/Issue.swift @@ -1,5 +1,4 @@ import Foundation -import Queue /// All possible SwiftLint issues which are printed as warnings by default. public enum Issue: LocalizedError, Equatable { @@ -85,8 +84,7 @@ public enum Issue: LocalizedError, Equatable { /// Flag to enable warnings for deprecations being printed to the console. Printing is enabled by default. package nonisolated(unsafe) static var printDeprecationWarnings = true - @TaskLocal private static var messageConsumer: (@Sendable @MainActor (String) async -> Void)? - @TaskLocal private static var printQueue: AsyncQueue? + @TaskLocal private static var printQueueContinuation: AsyncStream.Continuation? /// Hook used to capture all messages normally printed to stdout and return them back to the caller. /// @@ -97,18 +95,10 @@ public enum Issue: LocalizedError, Equatable { /// - returns: The collected messages produced while running the code in the runner. @MainActor static func captureConsole(runner: @Sendable () throws -> Void) async rethrows -> String { - var content = [String]() - let queue = AsyncQueue() - try $printQueue.withValue( - queue, - operation: { - try $messageConsumer.withValue( - { content.append($0) }, - operation: runner - ) - } - ) - return await queue.addBarrierOperation { content.joined(separator: "\n") }.value + let (stream, continuation) = AsyncStream.makeStream(of: String.self) + try $printQueueContinuation.withValue(continuation, operation: runner) + continuation.finish() + return await stream.reduce(into: "") { @Sendable in $0 += $0.isEmpty ? $1 : "\n\($1)" } } /// Wraps any `Error` into a `SwiftLintError.genericWarning` if it is not already a `SwiftLintError`. @@ -142,7 +132,7 @@ public enum Issue: LocalizedError, Equatable { if case .ruleDeprecated = self, !Self.printDeprecationWarnings { return } - Self.printQueue?.addOperation { await Self.messageConsumer?(errorDescription) } + Self.printQueueContinuation?.yield(errorDescription) queuedPrintError(errorDescription) } diff --git a/bazel/Queue.BUILD b/bazel/Queue.BUILD deleted file mode 100644 index a7e9e849b3..0000000000 --- a/bazel/Queue.BUILD +++ /dev/null @@ -1,11 +0,0 @@ -load( - "@build_bazel_rules_swift//swift:swift.bzl", - "swift_library", -) - -swift_library( - name = "Queue", - srcs = glob(["Sources/Queue/*.swift"]), - module_name = "Queue", - visibility = ["//visibility:public"], -) diff --git a/bazel/repos.bzl b/bazel/repos.bzl index 920dc5930a..592acd82f2 100644 --- a/bazel/repos.bzl +++ b/bazel/repos.bzl @@ -56,14 +56,6 @@ def swiftlint_repos(bzlmod = False): url = "https://github.com/JohnSundell/CollectionConcurrencyKit/archive/refs/tags/0.2.0.tar.gz", ) - http_archive( - name = "com_github_mattmassicotte_queue", - sha256 = "2c00cde22406d868b2341432c119c73d0c1a8ce40a663469b1b8cc5c39007737", - build_file = "@SwiftLint//bazel:Queue.BUILD", - strip_prefix = "Queue-0.2.0", - url = "https://github.com/mattmassicotte/Queue/archive/refs/tags/0.2.0.tar.gz", - ) - http_archive( name = "com_github_krzyzanowskim_cryptoswift", sha256 = "69b23102ff453990d03aff4d3fabd172d0667b2b3ed95730021d60a0f8d50d14",