Skip to content

Property accesses in failed expectations sometimes include their value twice in expanded description #601

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
@@ -547,7 +547,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
@@ -577,7 +577,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
2 changes: 1 addition & 1 deletion Sources/Testing/SourceAttribution/Expression+Macro.swift
Original file line number Diff line number Diff line change
@@ -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))
}

18 changes: 9 additions & 9 deletions Sources/Testing/SourceAttribution/Expression.swift
Original file line number Diff line number Diff line change
@@ -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.
///
@@ -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 {
@@ -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:
@@ -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(
@@ -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)
@@ -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]
}
2 changes: 1 addition & 1 deletion Sources/TestingMacros/Support/SourceCodeCapturing.swift
Original file line number Diff line number Diff line change
@@ -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))"
18 changes: 9 additions & 9 deletions Tests/TestingMacrosTests/ConditionMacroTests.swift
Original file line number Diff line number Diff line change
@@ -83,13 +83,13 @@ 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()"##,
##"#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()"##,
##"#expect(isolation: somewhere) {}"##:
##"Testing.__checkClosureCall(performing: {}, expression: .__fromSyntaxNode("{}"), comments: [], isRequired: false, isolation: somewhere, sourceLocation: Testing.SourceLocation.__here()).__expected()"##,
]
@@ -163,13 +163,13 @@ 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()"##,
##"#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()"##,
##"#require(isolation: somewhere) {}"##:
##"Testing.__checkClosureCall(performing: {}, expression: .__fromSyntaxNode("{}"), comments: [], isRequired: true, isolation: somewhere, sourceLocation: Testing.SourceLocation.__here()).__required()"##,
]
@@ -183,7 +183,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)"##:
57 changes: 56 additions & 1 deletion Tests/TestingTests/IssueTests.swift
Original file line number Diff line number Diff line change
@@ -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")
@@ -1206,7 +1260,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 {