From 64418b69f7d31782f8d346c4e11f5791a7234d10 Mon Sep 17 00:00:00 2001 From: Stuart Montgomery Date: Mon, 4 Nov 2024 14:37:19 -0600 Subject: [PATCH] Use @Test function parameters to explicitly type array literal argument expressions --- Sources/Testing/Test+Macro.swift | 85 +++++++++++++- .../Support/AttributeDiscovery.swift | 106 +++++++++++++----- .../DiagnosticMessage+Diagnosing.swift | 5 +- .../TestingMacros/TestDeclarationMacro.swift | 3 +- .../TestDeclarationMacroTests.swift | 30 ++++- Tests/TestingTests/MiscellaneousTests.swift | 20 ++++ Tests/TestingTests/TypeInfoTests.swift | 2 +- 7 files changed, 214 insertions(+), 37 deletions(-) diff --git a/Sources/Testing/Test+Macro.swift b/Sources/Testing/Test+Macro.swift index 891b37fe..edc07511 100644 --- a/Sources/Testing/Test+Macro.swift +++ b/Sources/Testing/Test+Macro.swift @@ -199,7 +199,7 @@ extension [Test.__Parameter] { /// /// ## See Also /// -/// - ``Test(_:arguments:)-35dat`` +/// - ``Test(_:_:arguments:)-8kn7a`` @attached(peer) @_documentation(visibility: private) public macro Test( @@ -207,6 +207,26 @@ public macro Test( arguments collection: C ) = #externalMacro(module: "TestingMacros", type: "TestDeclarationMacro") where C: Collection & Sendable, C.Element: Sendable +/// This macro declaration is necessary to allow non-`Sendable` collections to +/// be passed as test arguments. +/// +/// This macro declaration is similar to the above overload, but with all +/// `Sendable` constraints removed, so it does not need to be documented +/// separately. This macro's expansion code still calls APIs from the testing +/// library which require `Sendable`, however, so despite its relaxed generic +/// constraints, callers must still pass sendable collections to avoid compiler +/// diagnostics. +/// +/// ## See Also +/// +/// - ``Test(_:_:arguments:)-8kn7a`` +@attached(peer) +@_documentation(visibility: private) +public macro Test( + _ traits: any TestTrait..., + arguments collection: C +) = #externalMacro(module: "TestingMacros", type: "TestDeclarationMacro") where C: Collection + /// Declare a test parameterized over a collection of values. /// /// - Parameters: @@ -234,6 +254,27 @@ public macro Test( arguments collection: C ) = #externalMacro(module: "TestingMacros", type: "TestDeclarationMacro") where C: Collection & Sendable, C.Element: Sendable +/// This macro declaration is necessary to allow non-`Sendable` collections to +/// be passed as test arguments. +/// +/// This macro declaration is similar to the above overload, but with all +/// `Sendable` constraints removed, so it does not need to be documented +/// separately. This macro's expansion code still calls APIs from the testing +/// library which require `Sendable`, however, so despite its relaxed generic +/// constraints, callers must still pass sendable collections to avoid compiler +/// diagnostics. +/// +/// ## See Also +/// +/// - ``Test(_:_:arguments:)-8kn7a`` +@attached(peer) +@_documentation(visibility: private) +public macro Test( + _ displayName: _const String? = nil, + _ traits: any TestTrait..., + arguments collection: C +) = #externalMacro(module: "TestingMacros", type: "TestDeclarationMacro") where C: Collection + extension Test { /// Create an instance of ``Test`` for a parameterized function. /// @@ -280,6 +321,7 @@ extension Test { /// /// ## See Also /// +/// - ``Test(_:_:arguments:_:)-40xp7`` /// - @attached(peer) @_documentation(visibility: private) @@ -288,6 +330,26 @@ public macro Test( arguments collection1: C1, _ collection2: C2 ) = #externalMacro(module: "TestingMacros", type: "TestDeclarationMacro") where C1: Collection & Sendable, C1.Element: Sendable, C2: Collection & Sendable, C2.Element: Sendable +/// This macro declaration is necessary to allow non-`Sendable` collections to +/// be passed as test arguments. +/// +/// This macro declaration is similar to the above overload, but with all +/// `Sendable` constraints removed, so it does not need to be documented +/// separately. This macro's expansion code still calls APIs from the testing +/// library which require `Sendable`, however, so despite its relaxed generic +/// constraints, callers must still pass sendable collections to avoid compiler +/// diagnostics. +/// +/// ## See Also +/// +/// - ``Test(_:_:arguments:_:)-40xp7`` +@attached(peer) +@_documentation(visibility: private) +public macro Test( + _ traits: any TestTrait..., + arguments collection1: C1, _ collection2: C2 +) = #externalMacro(module: "TestingMacros", type: "TestDeclarationMacro") where C1: Collection, C2: Collection + /// Declare a test parameterized over two collections of values. /// /// - Parameters: @@ -315,6 +377,27 @@ public macro Test( arguments collection1: C1, _ collection2: C2 ) = #externalMacro(module: "TestingMacros", type: "TestDeclarationMacro") where C1: Collection & Sendable, C1.Element: Sendable, C2: Collection & Sendable, C2.Element: Sendable +/// This macro declaration is necessary to allow non-`Sendable` collections to +/// be passed as test arguments. +/// +/// This macro declaration is similar to the above overload, but with all +/// `Sendable` constraints removed, so it does not need to be documented +/// separately. This macro's expansion code still calls APIs from the testing +/// library which require `Sendable`, however, so despite its relaxed generic +/// constraints, callers must still pass sendable collections to avoid compiler +/// diagnostics. +/// +/// ## See Also +/// +/// - ``Test(_:_:arguments:_:)-40xp7`` +@attached(peer) +@_documentation(visibility: private) +public macro Test( + _ displayName: _const String? = nil, + _ traits: any TestTrait..., + arguments collection1: C1, _ collection2: C2 +) = #externalMacro(module: "TestingMacros", type: "TestDeclarationMacro") where C1: Collection, C2: Collection + // MARK: - @Test(arguments: zip(...)) /// Declare a test parameterized over two zipped collections of values. diff --git a/Sources/TestingMacros/Support/AttributeDiscovery.swift b/Sources/TestingMacros/Support/AttributeDiscovery.swift index f1cfd665..bc083bc4 100644 --- a/Sources/TestingMacros/Support/AttributeDiscovery.swift +++ b/Sources/TestingMacros/Support/AttributeDiscovery.swift @@ -66,17 +66,19 @@ struct AttributeInfo { /// The traits applied to the attribute, if any. var traits = [ExprSyntax]() + /// Test arguments passed to a parameterized test function, if any. + /// + /// When non-`nil`, the value of this property is an array beginning with the + /// argument passed to this attribute for the parameter labeled `arguments:` + /// followed by all of the remaining, unlabeled arguments. + var testFunctionArguments: [Argument]? + /// Whether or not this attribute specifies arguments to the associated test /// function. var hasFunctionArguments: Bool { - otherArguments.lazy - .compactMap(\.label?.tokenKind) - .contains(.identifier("arguments")) + testFunctionArguments != nil } - /// Additional arguments passed to the attribute, if any. - var otherArguments = [Argument]() - /// The source location of the attribute. /// /// When parsing, the testing library uses the start of the attribute's name @@ -98,6 +100,7 @@ struct AttributeInfo { init(byParsing attribute: AttributeSyntax, on declaration: some SyntaxProtocol, in context: some MacroExpansionContext) { self.attribute = attribute + var nonDisplayNameArguments: [Argument] = [] if let arguments = attribute.arguments, case let .argumentList(argumentList) = arguments { // If the first argument is an unlabelled string literal, it's the display // name of the test or suite. If it's anything else, including a nil @@ -106,11 +109,11 @@ struct AttributeInfo { let firstArgumentHasLabel = (firstArgument.label != nil) if !firstArgumentHasLabel, let stringLiteral = firstArgument.expression.as(StringLiteralExprSyntax.self) { displayName = stringLiteral - otherArguments = argumentList.dropFirst().map(Argument.init) + nonDisplayNameArguments = argumentList.dropFirst().map(Argument.init) } else if !firstArgumentHasLabel, firstArgument.expression.is(NilLiteralExprSyntax.self) { - otherArguments = argumentList.dropFirst().map(Argument.init) + nonDisplayNameArguments = argumentList.dropFirst().map(Argument.init) } else { - otherArguments = argumentList.map(Argument.init) + nonDisplayNameArguments = argumentList.map(Argument.init) } } } @@ -119,7 +122,7 @@ struct AttributeInfo { // See _SelfRemover for more information. Rewriting a syntax tree discards // location information from the copy, so only invoke the rewriter if the // `Self` keyword is present somewhere. - otherArguments = otherArguments.map { argument in + nonDisplayNameArguments = nonDisplayNameArguments.map { argument in var expr = argument.expression if argument.expression.tokens(viewMode: .sourceAccurate).map(\.tokenKind).contains(.keyword(.Self)) { let selfRemover = _SelfRemover(in: context) @@ -131,18 +134,25 @@ struct AttributeInfo { // Look for any traits in the remaining arguments and slice them off. Traits // are the remaining unlabelled arguments. The first labelled argument (if // present) is the start of subsequent context-specific arguments. - if !otherArguments.isEmpty { - if let labelledArgumentIndex = otherArguments.firstIndex(where: { $0.label != nil }) { + if !nonDisplayNameArguments.isEmpty { + if let labelledArgumentIndex = nonDisplayNameArguments.firstIndex(where: { $0.label != nil }) { // There is an argument with a label, so splice there. - traits = otherArguments[otherArguments.startIndex ..< labelledArgumentIndex].map(\.expression) - otherArguments = Array(otherArguments[labelledArgumentIndex...]) + traits = nonDisplayNameArguments[nonDisplayNameArguments.startIndex ..< labelledArgumentIndex].map(\.expression) + testFunctionArguments = Array(nonDisplayNameArguments[labelledArgumentIndex...]) } else { // No argument has a label, so all the remaining arguments are traits. - traits = otherArguments.map(\.expression) - otherArguments.removeAll(keepingCapacity: false) + traits = nonDisplayNameArguments.map(\.expression) } } + // If this attribute is attached to a parameterized test function, augment + // the argument expressions with explicit type information based on the + // parameters of the function signature to help the type checker infer the + // types of passed-in collections correctly. + if let testFunctionArguments, let functionDecl = declaration.as(FunctionDeclSyntax.self) { + self.testFunctionArguments = testFunctionArguments.testArguments(typedUsingParameters: functionDecl.signature.parameterClause.parameters) + } + // Combine traits from other sources (leading comments and availability // attributes) if applicable. traits += createCommentTraitExprs(for: declaration) @@ -178,23 +188,63 @@ struct AttributeInfo { } })) - // Any arguments of the test declaration macro which specify test arguments - // need to be wrapped a closure so they may be evaluated lazily by the - // testing library at runtime. If any such arguments are present, they will - // begin with a labeled argument named `arguments:` and include all - // subsequent unlabeled arguments. - var otherArguments = self.otherArguments - if let argumentsIndex = otherArguments.firstIndex(where: { $0.label?.tokenKind == .identifier("arguments") }) { - for index in argumentsIndex ..< otherArguments.endIndex { - var argument = otherArguments[index] - argument.expression = .init(ClosureExprSyntax { argument.expression.trimmed }) - otherArguments[index] = argument + // If there are any parameterized test function arguments, wrap each in a + // closure so they may be evaluated lazily at runtime. + if let testFunctionArguments { + arguments += testFunctionArguments.map { argument in + var copy = argument + copy.expression = .init(ClosureExprSyntax { argument.expression.trimmed }) + return copy } } - arguments += otherArguments arguments.append(Argument(label: "sourceLocation", expression: sourceLocation)) return LabeledExprListSyntax(arguments) } } + +extension Collection { + /// This collection of test function arguments augmented with explicit type + /// information based on the specified function parameters, if appropriate. + /// + /// - Parameters: + /// - parameters: The parameters of the function to which this collection of + /// test arguments was passed. + /// + /// - Returns: An array containing this collection of test arguments with + /// any array literal expressions given explicit type information via an + /// `as ...` cast. + fileprivate func testArguments(typedUsingParameters parameters: FunctionParameterListSyntax) -> [Argument] { + if count == 1 { + let tupleTypes = parameters.lazy + .map(\.type) + .map(String.init(describing:)) + .joined(separator: ",") + + return map { argument in + // Only add explicit types below if this is an Array literal expression. + guard argument.expression.is(ArrayExprSyntax.self) else { + return argument + } + + var argument = argument + argument.expression = "\(argument.expression) as [(\(raw: tupleTypes))]" + return argument + } + } else { + return zip(self, parameters) + .map { argument, parameter in + // Only add explicit types below if this is an Array literal + // expression. + guard argument.expression.is(ArrayExprSyntax.self) else { + return argument + } + + var argument = argument + argument.expression = .init(AsExprSyntax(expression: argument.expression, type: ArrayTypeSyntax(element: parameter.type))) + return argument + } + } + } +} diff --git a/Sources/TestingMacros/Support/DiagnosticMessage+Diagnosing.swift b/Sources/TestingMacros/Support/DiagnosticMessage+Diagnosing.swift index e0bd906c..e6682dc8 100644 --- a/Sources/TestingMacros/Support/DiagnosticMessage+Diagnosing.swift +++ b/Sources/TestingMacros/Support/DiagnosticMessage+Diagnosing.swift @@ -142,10 +142,7 @@ private func _diagnoseIssuesWithParallelizationTrait(_ traitExpr: MemberAccessEx return } - let hasArguments = attributeInfo.otherArguments.lazy - .compactMap(\.label?.textWithoutBackticks) - .contains("arguments") - if !hasArguments { + if !attributeInfo.hasFunctionArguments { // Serializing a non-parameterized test function has no effect. context.diagnose(.traitHasNoEffect(traitExpr, in: attributeInfo.attribute)) } diff --git a/Sources/TestingMacros/TestDeclarationMacro.swift b/Sources/TestingMacros/TestDeclarationMacro.swift index c9837346..b10bc602 100644 --- a/Sources/TestingMacros/TestDeclarationMacro.swift +++ b/Sources/TestingMacros/TestDeclarationMacro.swift @@ -423,9 +423,8 @@ public struct TestDeclarationMacro: PeerMacro, Sendable { // case the availability checks fail below. let unavailableTestName = context.makeUniqueName(thunking: functionDecl) - // TODO: don't assume otherArguments is only parameterized function arguments var attributeInfo = attributeInfo - attributeInfo.otherArguments = [] + attributeInfo.testFunctionArguments = nil result.append( """ @available(*, deprecated, message: "This property is an implementation detail of the testing library. Do not use it directly.") diff --git a/Tests/TestingMacrosTests/TestDeclarationMacroTests.swift b/Tests/TestingMacrosTests/TestDeclarationMacroTests.swift index fef2da83..4458e4b6 100644 --- a/Tests/TestingMacrosTests/TestDeclarationMacroTests.swift +++ b/Tests/TestingMacrosTests/TestDeclarationMacroTests.swift @@ -369,7 +369,8 @@ struct TestDeclarationMacroTests { @Test("Self. in @Test attribute is removed") func removeSelfKeyword() throws { - let (output, _) = try parse("@Test(arguments: Self.nested.uniqueArgsName, NoTouching.thisOne) func f() {}") + let (output, diagnostics) = try parse("@Test(arguments: Self.nested.uniqueArgsName, NoTouching.thisOne) func f(a: A, b: B) {}") + try #require(diagnostics.isEmpty) #expect(output.contains("nested.uniqueArgsName")) #expect(!output.contains("Self.nested.uniqueArgsName")) #expect(output.contains("NoTouching.thisOne")) @@ -470,4 +471,31 @@ struct TestDeclarationMacroTests { #expect(diagnostic.message == #"URL "\#(id)" is invalid and cannot be used with trait 'bug' in attribute 'Test'"#) } } + + @Suite("Test function arguments") + struct Arguments { + @Test("A heterogeneous array literal ([...]) without an explicit type") + func heterogeneousArrayLiteral() throws { + let input = """ + @Test(arguments: [ + (String.self, "1"), + (Int.self, "2"), + ]) + func example(type: Any.Type, label: String) {} + """ + let (output, _) = try parse(input) + #expect(output.contains("as [(Any.Type, String)]")) + } + + @Test("A heterogeneous array literal ([...]) with explicit 'as ...' type") + func heterogeneousArrayLiteralWithExplicitAs() throws { + let input = """ + @Test(arguments: [Child.self, Child.self] as [Parent]) + func example(type: Grandparent.Type) {} + """ + let (output, _) = try parse(input) + #expect(output.contains("as [Parent]")) + #expect(!output.contains("as [Grandparent.Type]")) + } + } } diff --git a/Tests/TestingTests/MiscellaneousTests.swift b/Tests/TestingTests/MiscellaneousTests.swift index eebdf857..64a794b8 100644 --- a/Tests/TestingTests/MiscellaneousTests.swift +++ b/Tests/TestingTests/MiscellaneousTests.swift @@ -572,3 +572,23 @@ struct MiscellaneousTests { #expect(duration < .seconds(1)) } } + +@Test(.hidden, arguments: [String.self, Int.self]) +func heterogeneousArray(type: Any.Type) {} + +@Test(.hidden, arguments: [ + (String.self, "1"), + (Int.self, "2"), +]) +func heterogeneousTupleArray(type: Any.Type, label: String) {} + +@Test(.hidden, arguments: + [(String.self, 1), (Int.self, 2)], + ["A", "B", "C"]) +func heterogeneousCombinatoric(tuple: (Any.Type, Int), letter: Character) {} + +@Test(.hidden, arguments: [ + "1": ["one", 1], + "2": ["two", 2], +]) +func heterogeneousDictionary(label: String, type: [any Sendable]) {} diff --git a/Tests/TestingTests/TypeInfoTests.swift b/Tests/TestingTests/TypeInfoTests.swift index a8d8327b..2c226852 100644 --- a/Tests/TestingTests/TypeInfoTests.swift +++ b/Tests/TestingTests/TypeInfoTests.swift @@ -34,7 +34,7 @@ struct TypeInfoTests { (() -> String).self, TypeInfo(fullyQualifiedName: "() -> Swift.String", unqualifiedName: "() -> String", mangledName: "") ), - ] as [(Any.Type, TypeInfo)]) + ]) func initWithType(type: Any.Type, expectedTypeInfo: TypeInfo) { let typeInfo = TypeInfo(describing: type) #expect(typeInfo == expectedTypeInfo)