Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use @Test function parameters to explicitly type array literal argument expressions #808

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 84 additions & 1 deletion Sources/Testing/Test+Macro.swift
Original file line number Diff line number Diff line change
Expand Up @@ -199,14 +199,34 @@ extension [Test.__Parameter] {
///
/// ## See Also
///
/// - ``Test(_:arguments:)-35dat``
/// - ``Test(_:_:arguments:)-8kn7a``
@attached(peer)
@_documentation(visibility: private)
public macro Test<C>(
_ traits: any TestTrait...,
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<C>(
_ traits: any TestTrait...,
arguments collection: C
) = #externalMacro(module: "TestingMacros", type: "TestDeclarationMacro") where C: Collection

/// Declare a test parameterized over a collection of values.
///
/// - Parameters:
Expand Down Expand Up @@ -234,6 +254,27 @@ public macro Test<C>(
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<C>(
_ 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.
///
Expand Down Expand Up @@ -280,6 +321,7 @@ extension Test {
///
/// ## See Also
///
/// - ``Test(_:_:arguments:_:)-40xp7``
/// - <doc:DefiningTests>
@attached(peer)
@_documentation(visibility: private)
Expand All @@ -288,6 +330,26 @@ public macro Test<C1, C2>(
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<C1, C2>(
_ 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:
Expand Down Expand Up @@ -315,6 +377,27 @@ public macro Test<C1, C2>(
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<C1, C2>(
_ 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.
Expand Down
106 changes: 78 additions & 28 deletions Sources/TestingMacros/Support/AttributeDiscovery.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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)
}
}
}
Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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<Argument> {
/// 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will fail to compile if any arguments have specifiers like borrowing. (I'm not sure if that's correct in this position.) You should check the exact type of type node.

.map(\.type)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.trimmedDescription?

.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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You probably need to strip parentheses here.

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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You probably need to strip parentheses here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, does this branch even need to do anything interesting at all?

return argument
}

var argument = argument
argument.expression = .init(AsExprSyntax(expression: argument.expression, type: ArrayTypeSyntax(element: parameter.type)))
return argument
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand Down
3 changes: 1 addition & 2 deletions Sources/TestingMacros/TestDeclarationMacro.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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.")
Expand Down
30 changes: 29 additions & 1 deletion Tests/TestingMacrosTests/TestDeclarationMacroTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand Down Expand Up @@ -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]"))
}
}
}
20 changes: 20 additions & 0 deletions Tests/TestingTests/MiscellaneousTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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]) {}
2 changes: 1 addition & 1 deletion Tests/TestingTests/TypeInfoTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down