Skip to content

Commit

Permalink
Property accesses in failed expectations sometimes include their valu…
Browse files Browse the repository at this point in the history
…e twice in expanded description
  • Loading branch information
stmontgomery committed Aug 5, 2024
1 parent ace555d commit 44662e1
Show file tree
Hide file tree
Showing 6 changed files with 76 additions and 21 deletions.
4 changes: 2 additions & 2 deletions Sources/Testing/Expectations/ExpectationChecking+Macro.swift
Original file line number Diff line number Diff line change
Expand Up @@ -545,7 +545,7 @@ public func __checkPropertyAccess<T>(
return __checkValue(
condition,
expression: expression,
expressionWithCapturedRuntimeValues: expression.capturingRuntimeValues(lhs, condition),
expressionWithCapturedRuntimeValues: expression.capturingRuntimeValues(lhs),
comments: comments(),
isRequired: isRequired,
sourceLocation: sourceLocation
Expand Down Expand Up @@ -575,7 +575,7 @@ public func __checkPropertyAccess<T, U>(
return __checkValue(
optionalValue,
expression: expression,
expressionWithCapturedRuntimeValues: expression.capturingRuntimeValues(lhs, optionalValue as U??),
expressionWithCapturedRuntimeValues: expression.capturingRuntimeValues(lhs),
comments: comments(),
isRequired: isRequired,
sourceLocation: sourceLocation
Expand Down
2 changes: 1 addition & 1 deletion Sources/Testing/SourceAttribution/Expression+Macro.swift
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ extension __Expression {
///
/// - Warning: This function is used to implement the `@Test`, `@Suite`,
/// `#expect()` and `#require()` macros. Do not call it directly.
public static func __fromPropertyAccess(_ value: Self, _ keyPath: Self) -> Self {
public static func __fromPropertyAccess(_ value: Self, _ keyPath: String) -> Self {
return Self(kind: .propertyAccess(value: value, keyPath: keyPath))
}

Expand Down
18 changes: 9 additions & 9 deletions Sources/Testing/SourceAttribution/Expression.swift
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public struct __Expression: Sendable {
/// - value: The value whose property was accessed.
/// - keyPath: The key path, relative to `value`, that was accessed, not
/// including a leading backslash or period.
indirect case propertyAccess(value: __Expression, keyPath: __Expression)
indirect case propertyAccess(value: __Expression, keyPath: String)

/// The expression negates another expression.
///
Expand Down Expand Up @@ -124,7 +124,7 @@ public struct __Expression: Sendable {
}
return "\(functionName)(\(argumentList))"
case let .propertyAccess(value, keyPath):
return "\(value.sourceCode).\(keyPath.sourceCode)"
return "\(value.sourceCode).\(keyPath)"
case let .negation(expression, isParenthetical):
var sourceCode = expression.sourceCode
if isParenthetical {
Expand Down Expand Up @@ -298,7 +298,9 @@ public struct __Expression: Sendable {

// Convert the variadic generic argument list to an array.
var additionalValuesArray = [Any?]()
repeat additionalValuesArray.append(each additionalValues)
for additionalValue in repeat each additionalValues {
additionalValuesArray.append(additionalValue)
}

switch kind {
case .generic, .stringLiteral:
Expand All @@ -320,7 +322,7 @@ public struct __Expression: Sendable {
case let .propertyAccess(value, keyPath):
result.kind = .propertyAccess(
value: value.capturingRuntimeValues(firstValue),
keyPath: keyPath.capturingRuntimeValues(additionalValuesArray.first ?? nil)
keyPath: keyPath
)
case let .negation(expression, isParenthetical):
result.kind = .negation(
Expand Down Expand Up @@ -421,9 +423,7 @@ public struct __Expression: Sendable {
"\(functionName)(\(argumentList))"
}
case let .propertyAccess(value, keyPath):
var keyPathContext = childContext
keyPathContext.includeParenthesesIfNeeded = false
result = "\(value._expandedDescription(in: childContext)).\(keyPath._expandedDescription(in: keyPathContext))"
result = "\(value._expandedDescription(in: childContext)).\(keyPath)"
case let .negation(expression, isParenthetical):
childContext.includeParenthesesIfNeeded = !isParenthetical
var expandedDescription = expression._expandedDescription(in: childContext)
Expand Down Expand Up @@ -475,8 +475,8 @@ public struct __Expression: Sendable {
} else {
arguments.lazy.map(\.value)
}
case let .propertyAccess(value: value, keyPath: keyPath):
[value, keyPath]
case let .propertyAccess(value, _):
[value]
case let .negation(expression, _):
[expression]
}
Expand Down
2 changes: 1 addition & 1 deletion Sources/TestingMacros/Support/SourceCodeCapturing.swift
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func createExpressionExprForFunctionCall(_ value: (any SyntaxProtocol)?, _ funct
func createExpressionExprForPropertyAccess(_ value: ExprSyntax, _ keyPath: DeclReferenceExprSyntax) -> ExprSyntax {
let arguments = LabeledExprListSyntax {
LabeledExprSyntax(expression: createExpressionExpr(from: value))
LabeledExprSyntax(expression: createExpressionExpr(from: keyPath.baseName))
LabeledExprSyntax(expression: StringLiteralExprSyntax(content: keyPath.baseName.text))
}

return ".__fromPropertyAccess(\(arguments))"
Expand Down
14 changes: 7 additions & 7 deletions Tests/TestingMacrosTests/ConditionMacroTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,11 @@ struct ConditionMacroTests {
##"#expect(a, sourceLocation: someValue)"##:
##"Testing.__checkValue(a, expression: .__fromSyntaxNode("a"), comments: [], isRequired: false, sourceLocation: someValue).__expected()"##,
##"#expect(a.isB)"##:
##"Testing.__checkPropertyAccess(a.self, getting: { $0.isB }, expression: .__fromPropertyAccess(.__fromSyntaxNode("a"), .__fromSyntaxNode("isB")), comments: [], isRequired: false, sourceLocation: Testing.SourceLocation.__here()).__expected()"##,
##"Testing.__checkPropertyAccess(a.self, getting: { $0.isB }, expression: .__fromPropertyAccess(.__fromSyntaxNode("a"), "isB"), comments: [], isRequired: false, sourceLocation: Testing.SourceLocation.__here()).__expected()"##,
##"#expect(a???.isB)"##:
##"Testing.__checkPropertyAccess(a.self, getting: { $0???.isB }, expression: .__fromPropertyAccess(.__fromSyntaxNode("a"), .__fromSyntaxNode("isB")), comments: [], isRequired: false, sourceLocation: Testing.SourceLocation.__here()).__expected()"##,
##"Testing.__checkPropertyAccess(a.self, getting: { $0???.isB }, expression: .__fromPropertyAccess(.__fromSyntaxNode("a"), "isB"), comments: [], isRequired: false, sourceLocation: Testing.SourceLocation.__here()).__expected()"##,
##"#expect(a?.b.isB)"##:
##"Testing.__checkPropertyAccess(a?.b.self, getting: { $0?.isB }, expression: .__fromPropertyAccess(.__fromSyntaxNode("a?.b"), .__fromSyntaxNode("isB")), comments: [], isRequired: false, sourceLocation: Testing.SourceLocation.__here()).__expected()"##,
##"Testing.__checkPropertyAccess(a?.b.self, getting: { $0?.isB }, expression: .__fromPropertyAccess(.__fromSyntaxNode("a?.b"), "isB"), comments: [], isRequired: false, sourceLocation: Testing.SourceLocation.__here()).__expected()"##,
]
)
func expectMacro(input: String, expectedOutput: String) throws {
Expand Down Expand Up @@ -159,11 +159,11 @@ struct ConditionMacroTests {
##"#require(a, sourceLocation: someValue)"##:
##"Testing.__checkValue(a, expression: .__fromSyntaxNode("a"), comments: [], isRequired: true, sourceLocation: someValue).__required()"##,
##"#require(a.isB)"##:
##"Testing.__checkPropertyAccess(a.self, getting: { $0.isB }, expression: .__fromPropertyAccess(.__fromSyntaxNode("a"), .__fromSyntaxNode("isB")), comments: [], isRequired: true, sourceLocation: Testing.SourceLocation.__here()).__required()"##,
##"Testing.__checkPropertyAccess(a.self, getting: { $0.isB }, expression: .__fromPropertyAccess(.__fromSyntaxNode("a"), "isB"), comments: [], isRequired: true, sourceLocation: Testing.SourceLocation.__here()).__required()"##,
##"#require(a???.isB)"##:
##"Testing.__checkPropertyAccess(a.self, getting: { $0???.isB }, expression: .__fromPropertyAccess(.__fromSyntaxNode("a"), .__fromSyntaxNode("isB")), comments: [], isRequired: true, sourceLocation: Testing.SourceLocation.__here()).__required()"##,
##"Testing.__checkPropertyAccess(a.self, getting: { $0???.isB }, expression: .__fromPropertyAccess(.__fromSyntaxNode("a"), "isB"), comments: [], isRequired: true, sourceLocation: Testing.SourceLocation.__here()).__required()"##,
##"#require(a?.b.isB)"##:
##"Testing.__checkPropertyAccess(a?.b.self, getting: { $0?.isB }, expression: .__fromPropertyAccess(.__fromSyntaxNode("a?.b"), .__fromSyntaxNode("isB")), comments: [], isRequired: true, sourceLocation: Testing.SourceLocation.__here()).__required()"##,
##"Testing.__checkPropertyAccess(a?.b.self, getting: { $0?.isB }, expression: .__fromPropertyAccess(.__fromSyntaxNode("a?.b"), "isB"), comments: [], isRequired: true, sourceLocation: Testing.SourceLocation.__here()).__required()"##,
]
)
func requireMacro(input: String, expectedOutput: String) throws {
Expand All @@ -175,7 +175,7 @@ struct ConditionMacroTests {
@Test("Unwrapping #require() macro",
arguments: [
##"#require(Optional<Int>.none)"##:
##"Testing.__checkPropertyAccess(Optional<Int>.self, getting: { $0.none }, expression: .__fromPropertyAccess(.__fromSyntaxNode("Optional<Int>"), .__fromSyntaxNode("none")), comments: [], isRequired: true, sourceLocation: Testing.SourceLocation.__here()).__required()"##,
##"Testing.__checkPropertyAccess(Optional<Int>.self, getting: { $0.none }, expression: .__fromPropertyAccess(.__fromSyntaxNode("Optional<Int>"), "none"), comments: [], isRequired: true, sourceLocation: Testing.SourceLocation.__here()).__required()"##,
##"#require(nil ?? 123)"##:
##"Testing.__checkBinaryOperation(nil, { $0 ?? $1() }, 123, expression: .__fromBinaryOperation(.__fromSyntaxNode("nil"), "??", .__fromSyntaxNode("123")), comments: [], isRequired: true, sourceLocation: Testing.SourceLocation.__here()).__required()"##,
##"#require(123 ?? nil)"##:
Expand Down
57 changes: 56 additions & 1 deletion Tests/TestingTests/IssueTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,60 @@ final class IssueTests: XCTestCase {
await fulfillment(of: [expectationChecked], timeout: 0.0)
}

func testPropertyAccessExpressionExpansion() async {
let expectationFailed = expectation(description: "Expectation failed")

var configuration = Configuration()
configuration.eventHandler = { event, _ in
guard case let .issueRecorded(issue) = event.kind,
case let .expectationFailed(expectation) = issue.kind
else {
return
}

let desc = expectation.evaluatedExpression.expandedDescription()
XCTAssertEqual(desc, "!([].isEmpty → true)")
expectationFailed.fulfill()
}

await Test {
#expect(![].isEmpty)
}.run(configuration: configuration)
await fulfillment(of: [expectationFailed], timeout: 0.0)
}

func testChainedOptionalPropertyAccessExpressionExpansion() async {
let expectationFailed = expectation(description: "Expectation failed")

var configuration = Configuration()
configuration.eventHandler = { event, _ in
guard case let .issueRecorded(issue) = event.kind,
case let .expectationFailed(expectation) = issue.kind
else {
return
}

let desc = expectation.evaluatedExpression.expandedDescription()
XCTAssertEqual(desc, "(outer.middle.inner → Inner(value: nil)).value → nil")
expectationFailed.fulfill()
}

await Test {
struct Outer {
struct Middle {
struct Inner {
var value: Int? = nil
}
var inner = Inner()
}
var middle = Middle()
}
let outer = Outer()
_ = try #require(outer.middle.inner.value)
}.run(configuration: configuration)
await fulfillment(of: [expectationFailed], timeout: 0.0)
}

func testExpressionLiterals() async {
func expectIssue(containing content: String, in testFunction: @escaping @Sendable () async throws -> Void) async {
let issueRecorded = expectation(description: "Issue recorded")
Expand Down Expand Up @@ -1174,7 +1228,8 @@ final class IssueTests: XCTestCase {
return
}
let expression = expectation.evaluatedExpression
XCTAssertTrue(expression.expandedDescription().contains("<not evaluated>"))
let expandedDescription = expression.expandedDescription()
XCTAssertTrue(expandedDescription.contains("<not evaluated>"), "expandedDescription: \(expandedDescription)")
}

@Sendable func rhs() -> Bool {
Expand Down

0 comments on commit 44662e1

Please sign in to comment.