Skip to content
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
Original file line number Diff line number Diff line change
Expand Up @@ -174,12 +174,17 @@ extension FunctionParameterSyntax {
}

extension FunctionParameterSyntax {
/// The base type name of this parameter.
var baseTypeName: String {
/// The underlying type of this parameter with any attributed type wrappers
/// (such as `inout`) removed.
var baseType: TypeSyntax {
// Discard any specifiers such as `inout` or `borrowing`, since we're only
// trying to obtain the base type to reference it in an expression.
let baseType = type.as(AttributedTypeSyntax.self)?.baseType ?? type
return baseType.trimmedDescription
type.as(AttributedTypeSyntax.self)?.baseType ?? type
}

/// The base type name of this parameter.
var baseTypeName: String {
Comment thread
grynspan marked this conversation as resolved.
baseType.trimmedDescription
}
}

Expand Down
97 changes: 95 additions & 2 deletions Sources/TestingMacros/Support/AttributeDiscovery.swift
Original file line number Diff line number Diff line change
Expand Up @@ -169,9 +169,23 @@ struct AttributeInfo {
// 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
arguments += testFunctionArguments.enumerated().map { index, argument in
var copy = argument
copy.expression = .init(ClosureExprSyntax { argument.expression.trimmed })
var expr = copy.expression.trimmed
if let contextualType = _contextualTypeForLiteralArgument(
at: index,
for: expr,
among: testFunctionArguments
) {
expr = ExprSyntax(
AsExprSyntax(
expression: expr,
asKeyword: .keyword(.as, leadingTrivia: .space, trailingTrivia: .space),
type: contextualType.trimmed
)
)
}
copy.expression = .init(ClosureExprSyntax { expr })
return copy
}
}
Expand All @@ -180,4 +194,83 @@ struct AttributeInfo {

return LabeledExprListSyntax(arguments)
}

/// The contextual type to explicitly apply to a literal `arguments:`
/// expression after it is wrapped in a closure for lazy evaluation.
///
Comment thread
ojun9 marked this conversation as resolved.
/// Parameterized `@Test` declarations are modeled in terms of the collection
/// type supplied to the macro, but macro expansion only sees source syntax.
/// When the `arguments:` parameter is supplied as an array literal, derive
/// the corresponding array type from the test function's parameters so the
/// literal retains enough contextual type information after lazy wrapping.
///
/// This applies to both the single-collection form and the overloads where
/// each `arguments:` expression corresponds directly to one parameter.
///
/// - Parameters:
/// - index: The position of `expression` within `testFunctionArguments`.
/// - expression: The argument expression being wrapped for lazy evaluation.
/// - testFunctionArguments: The full list of argument expressions supplied
/// to the parameterized `@Test`.
///
/// - Returns: The array type to apply to `expression`, or `nil` if no
/// contextual type reconstruction is needed.
private func _contextualTypeForLiteralArgument(
at index: Int,
for expression: ExprSyntax,
among testFunctionArguments: [Argument]
) -> TypeSyntax? {
guard let functionDecl = declaration.as(FunctionDeclSyntax.self) else {
return nil
}

let parameters = Array(functionDecl.signature.parameterClause.parameters)
if parameters.isEmpty {
return nil
}

if expression.is(ArrayExprSyntax.self) {
if testFunctionArguments.count == parameters.count {
let parameter = parameters[index]
return TypeSyntax(
ArrayTypeSyntax(element: parameter.baseType.trimmed)
)
}

if testFunctionArguments.count == 1 {
if parameters.count == 1, let parameter = parameters.first {
// A single-parameter test expects collection elements of the parameter
// type itself, not tuple-shaped elements.
return TypeSyntax(
ArrayTypeSyntax(element: parameter.baseType.trimmed)
)
}
let elementType = TypeSyntax(
TupleTypeSyntax(elements: TupleTypeElementListSyntax {
for parameter in parameters {
TupleTypeElementSyntax(type: parameter.baseType.trimmed)
}
})
)
return TypeSyntax(ArrayTypeSyntax(element: elementType))
}
} else if expression.is(DictionaryExprSyntax.self) {
if testFunctionArguments.count == 1, parameters.count == 2 {
return TypeSyntax(
MemberTypeSyntax(
baseType: IdentifierTypeSyntax(name: .identifier("Swift")),
name: .identifier("KeyValuePairs"),
Comment thread
grynspan marked this conversation as resolved.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm surprised to see DictionaryExprSyntax being interpreted as KeyValuePairs. I see this comment which I'm guessing is what led to that:

We should make sure to handle dictionary literals correctly as well, and can improve test efficiency by inferring them as instances of KeyValuePairs rather than Dictionary.

That changes the semantics some, though: elements with equal keys will run the test multiple times, and the order we dispatch test cases in will be stable, whereas with a dictionary the order is nondeterministic due to hashing.

Is switching to KeyValuePairs necessary for this overall PR, or is it more of an opportunistic performance improvement? It feels like a potentially user-observable behavior change.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This was my suggestion, yes.

That changes the semantics some, though: elements with equal keys will run the test multiple times

Equal keys crash outright with a dictionary when it is initialized.

and the order we dispatch test cases in will be stable, whereas with a dictionary the order is nondeterministic due to hashing.

This change is more consistent with how we treat arrays, yes.

Is switching to KeyValuePairs necessary for this overall PR, or is it more of an opportunistic performance improvement? It feels like a potentially user-observable behavior change.

It isn't strictly necessary, but the only user-observable change is that the random order of elements is no longer random (which we never really intended it to be anyway).

genericArgumentClause: GenericArgumentClauseSyntax(
arguments: GenericArgumentListSyntax {
GenericArgumentSyntax(argument: .type(parameters[0].baseType.trimmed))
GenericArgumentSyntax(argument: .type(parameters[1].baseType.trimmed))
}
)
)
)
}
}

return nil
}
}
63 changes: 63 additions & 0 deletions Tests/TestingMacrosTests/TestDeclarationMacroTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,69 @@ struct TestDeclarationMacroTests {
}
}

static var parameterizedArgumentTypePreservationInputs: [(String, String)] {
[
(
"""
@Test(arguments: [])
func f(i: Int) {}
""",
#"arguments:{[]as[Int]}"#
),
(
"""
@Test(arguments: [], [])
func f(i: Int, s: String) {}
""",
#"arguments:{[]as[Int]},{[]as[String]}"#
),
(
"""
@Test(arguments: [nil], [nil])
func f(i: Int?, s: String?) {}
""",
#"arguments:{[nil]as[Int?]},{[nil]as[String?]}"#
),
(
"""
@Test(arguments: [
(nil, 1),
("a", nil),
("b", nil)
])
func f(s: String?, i: Int?) {}
""",
#"arguments:{[(nil,1),("a",nil),("b",nil)]as[(String?,Int?)]}"#
),
(
"""
@Test(arguments: ["value": 123])
func f(s: String, i: Int) {}
""",
#"arguments:{["value":123]asSwift.KeyValuePairs<String,Int>}"#
),
]
}

@Test("Literal arguments preserve contextual types after lazy wrapping", arguments: parameterizedArgumentTypePreservationInputs)
func preservesParameterizedArgumentTypes(input: String, expectedOutput: String) throws {
let (output, _) = try parse(input, removeWhitespace: true)
#expect(output.contains(expectedOutput))
}

@Test("Non-literal parameterized arguments are left unchanged")
func nonLiteralParameterizedArgumentsRemainUncast() throws {
let input = """
let ints = [1, 2]
let strings = ["a", "b"]
@Test(arguments: ints, strings)
func f(i: Int, s: String) {}
"""
let (output, _) = try parse(input, removeWhitespace: true)
#expect(output.contains(#"arguments:{ints},{strings}"#))
#expect(!output.contains(#"arguments:{intsas[Int]},{stringsas[String]}"#))
}

@Test("Display name is preserved",
arguments: [
#"@Test("Display Name") func f() {}"#,
Expand Down
13 changes: 13 additions & 0 deletions Tests/TestingTests/Test.Case.ArgumentTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -165,11 +165,24 @@ struct ParameterizedTests {
@Test(.hidden, arguments: [("value", 123)])
func one2TupleParameter(x: (String, Int)) {}

@Test(.hidden, arguments: [
(nil, 123),
("value1", nil),
("value2", nil),
] as [(String?, Int?)])
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why does this test, and the other new one below, include an explicit as cast on the passed-in collection? I thought this PR was intended to remove the need to specify that at the point where @Test is used?

I just pulled the branch locally, and when I try this example mentioned in the PR description without an as cast and without <...> generic type parameters on @Test, it's still emitting an error:

@Test(arguments: [
  (nil, 2), // ❌ error: 'nil' cannot initialize specified type 'String'
  ("c", nil),
  ("d", nil)
])
func f(s: String?, i: Int?) {}

If the outcome of this PR is that a user still needs to either (a) use an explicit as cast or (b) include generic type parameters on @Test, then I'm not sure what the benefit is. Users can already do (a), and we recently began emitting a warning diagnostic for (b), and it's not meaningfully simpler than option (a) IMO anyway.

func contextualArrayLiteral(x: String?, y: Int?) {}

@Test(.hidden, arguments: ["value": 123])
func twoDictionaryElementParameters(x: String, y: Int) {}

@Test(.hidden, arguments: ["value": 123])
func oneDictionaryElementTupleParameter(x: (key: String, value: Int)) {}

@Test(.hidden, arguments: [
"value1": nil,
"value2": 123,
] as KeyValuePairs<String, Int?>)
func contextualDictionaryLiteral(key: String, value: Int?) {}

@Test(.disabled(), arguments: [1, 2, 3]) func disabled(x: Int) {}
}
Loading