Skip to content

Remove the new diagnostic added in #302. #352

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

Merged
merged 1 commit into from
Apr 15, 2024
Merged
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
69 changes: 0 additions & 69 deletions Sources/TestingMacros/Support/ConditionArgumentParsing.swift
Original file line number Diff line number Diff line change
Expand Up @@ -128,64 +128,6 @@ func removeParentheses(from expr: ExprSyntax) -> ExprSyntax? {
return nil
}

/// A class that walks a syntax tree looking for `try` and `await` expressions.
///
/// - Bug: This class does not use `lexicalContext` to check for the presence of
/// `try` or `await` _outside_ the current macro expansion.
private final class _EffectFinder: SyntaxVisitor {
/// The effectful expressions discovered so far.
var effectfulExprs = [ExprSyntax]()

/// Common implementation for `visit(_: TryExprSyntax)` and
/// `visit(_: AwaitExprSyntax)`.
///
/// - Parameters:
/// - node: The `try` or `await` expression.
/// - expression: The `.expression` property of `node`.
///
/// - Returns: Whether or not to recurse into `node`.
private func _visitEffectful(_ node: some ExprSyntaxProtocol, expression: ExprSyntax) -> SyntaxVisitorContinueKind {
if let parentNode = node.parent, parentNode.is(TryExprSyntax.self) {
// Suppress this expression as its immediate parent is also an effectful
// expression (e.g. it's a `try await` expression overall.) The diagnostic
// reported for the parent expression should include both as needed.
return .visitChildren
} else if expression.is(AsExprSyntax.self) {
// Do not walk into explicit `as T` casts. This provides an escape hatch
// for expressions that should not diagnose.
return .skipChildren
} else if let awaitExpr = expression.as(AwaitExprSyntax.self), awaitExpr.expression.is(AsExprSyntax.self) {
// As above but for `try await _ as T`.
return .skipChildren
}
effectfulExprs.append(ExprSyntax(node))
return .visitChildren
}

override func visit(_ node: TryExprSyntax) -> SyntaxVisitorContinueKind {
_visitEffectful(node, expression: node.expression)
}

override func visit(_ node: AwaitExprSyntax) -> SyntaxVisitorContinueKind {
_visitEffectful(node, expression: node.expression)
}

override func visit(_ node: AsExprSyntax) -> SyntaxVisitorContinueKind {
// Do not walk into explicit `as T` casts. This provides an escape hatch for
// expressions that should not diagnose.
return .skipChildren
}

override func visit(_ node: ClosureExprSyntax) -> SyntaxVisitorContinueKind {
// Do not walk into closures. Although they are not meant to be an escape
// hatch like `as` casts, it is very difficult (often impossible) to reason
// about effectful expressions inside the scope of a closure. If the closure
// is invoked locally, its caller will also need to say `try`/`await` and we
// can still diagnose those outer expressions.
return .skipChildren
}
}

// MARK: -

/// Parse a condition argument from a binary operation expression.
Expand Down Expand Up @@ -554,17 +496,6 @@ private func _parseCondition(from expr: ExprSyntax, for macro: some Freestanding
///
/// - Returns: An instance of ``Condition`` describing `expr`.
func parseCondition(from expr: ExprSyntax, for macro: some FreestandingMacroExpansionSyntax, in context: some MacroExpansionContext) -> Condition {
// Handle `await` first. If present in the expression or a subexpression,
// diagnose and don't expand further.
let effectFinder = _EffectFinder(viewMode: .sourceAccurate)
effectFinder.walk(expr)
guard effectFinder.effectfulExprs.isEmpty else {
for effectfulExpr in effectFinder.effectfulExprs {
context.diagnose(.effectfulExpressionNotParsed(effectfulExpr, in: macro))
}
return Condition(expression: expr)
}

let result = _parseCondition(from: expr, for: macro, in: context)
if result.arguments.count == 1, let onlyArgument = result.arguments.first {
_diagnoseTrivialBooleanValue(from: onlyArgument.expression, for: macro, in: context)
Expand Down
25 changes: 0 additions & 25 deletions Sources/TestingMacros/Support/DiagnosticMessage.swift
Original file line number Diff line number Diff line change
Expand Up @@ -56,31 +56,6 @@ struct DiagnosticMessage: SwiftDiagnostics.DiagnosticMessage {
)
}

/// Create a diagnostic message stating that an effectful (`try` or `await`)
/// expression cannot be parsed and should be broken apart.
///
/// - Parameters:
/// - expr: The expression being diagnosed.
/// - macro: The macro expression.
///
/// - Returns: A diagnostic message.
static func effectfulExpressionNotParsed(_ expr: ExprSyntax, in macro: some FreestandingMacroExpansionSyntax) -> Self {
let effectful = if let tryExpr = expr.as(TryExprSyntax.self) {
if tryExpr.expression.is(AwaitExprSyntax.self) {
"throwing/asynchronous"
} else {
"throwing"
}
} else {
"asynchronous"
}
return Self(
syntax: Syntax(expr),
message: "Expression '\(expr.trimmed)' will not be expanded on failure; move the \(effectful) part out of the call to \(_macroName(macro))",
severity: .warning
)
}

/// Get the human-readable name of the given freestanding macro.
///
/// - Parameters:
Expand Down
43 changes: 0 additions & 43 deletions Tests/TestingMacrosTests/ConditionMacroTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -331,49 +331,6 @@ struct ConditionMacroTests {
#expect(diagnostics.isEmpty)
}

@Test("#expect(try/await) produces a diagnostic",
arguments: [
"#expect(try foo())": ["Expression 'try foo()' will not be expanded on failure; move the throwing part out of the call to '#expect(_:_:)'"],
"#expect(await foo())": ["Expression 'await foo()' will not be expanded on failure; move the asynchronous part out of the call to '#expect(_:_:)'"],
"#expect(try await foo())": ["Expression 'try await foo()' will not be expanded on failure; move the throwing/asynchronous part out of the call to '#expect(_:_:)'"],
"#expect(try await foo(try bar(await quux())))": [
"Expression 'try await foo(try bar(await quux()))' will not be expanded on failure; move the throwing/asynchronous part out of the call to '#expect(_:_:)'",
"Expression 'try bar(await quux())' will not be expanded on failure; move the throwing part out of the call to '#expect(_:_:)'",
"Expression 'await quux()' will not be expanded on failure; move the asynchronous part out of the call to '#expect(_:_:)'",
],

// Diagnoses because the diagnostic for `await` is suppressed due to the
// `as T` cast, but the parentheses limit the effect of the suppression.
"#expect(try (await foo() as T))": ["Expression 'try (await foo() as T)' will not be expanded on failure; move the throwing part out of the call to '#expect(_:_:)'"],
]
)
func effectfulExpectationDiagnoses(input: String, diagnosticMessages: [String]) throws {
let (_, diagnostics) = try parse(input)
#expect(diagnostics.count == diagnosticMessages.count)
for message in diagnosticMessages {
#expect(diagnostics.contains { $0.diagMessage.message == message }, "Missing \(message): \(diagnostics.map(\.diagMessage.message))")
}
}

@Test("#expect(try/await as Bool) suppresses its diagnostic",
arguments: [
"#expect(try foo() as Bool)",
"#expect(await foo() as Bool)",
"#expect(try await foo(try await bar()) as Bool)",
"#expect(try foo() as T?)",
"#expect(await foo() as? T)",
"#expect(try await foo(try await bar()) as! T)",
"#expect((try foo()) as T)",
"#expect((await foo()) as T)",
"#expect((try await foo(try await bar())) as T)",
"#expect(try (await foo()) as T)",
]
)
func effectfulExpectationDiagnosticSuppressWithExplicitBool(input: String) throws {
let (_, diagnostics) = try parse(input)
#expect(diagnostics.isEmpty)
}

@Test("Expectation inside an exit test diagnoses",
arguments: [
"#expectExitTest(exitsWith: .failure) { #expect(1 > 2) }",
Expand Down
11 changes: 6 additions & 5 deletions Tests/TestingTests/EventRecorderTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -150,11 +150,12 @@ struct EventRecorderTests {
One(.anyGraphemeCluster)
" \(isSuite ? "Suite" : "Test") \(testName) started."
}
let match = try buffer
.split(whereSeparator: \.isNewline)
.compactMap(testFailureRegex.wholeMatch(in:))
.first
#expect(match != nil)
#expect(
try buffer
.split(whereSeparator: \.isNewline)
.compactMap(testFailureRegex.wholeMatch(in:))
.first != nil
)
}

@available(_regexAPI, *)
Expand Down
6 changes: 3 additions & 3 deletions Tests/TestingTests/IssueTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ final class IssueTests: XCTestCase {
}

await Test { () throws in
#expect(try { throw MyError() }() as Bool)
#expect(try { throw MyError() }())
}.run(configuration: configuration)

await Test { () throws in
Expand Down Expand Up @@ -283,8 +283,8 @@ final class IssueTests: XCTestCase {
}

await Test { () throws in
#expect(try TypeWithMemberFunctions.n(0) as Bool)
#expect(TypeWithMemberFunctions.f(try { () throws in 0 }()) as Bool)
#expect(try TypeWithMemberFunctions.n(0))
#expect(TypeWithMemberFunctions.f(try { () throws in 0 }()))
}.run(configuration: configuration)

await fulfillment(of: [expectationFailed], timeout: 0.0)
Expand Down
18 changes: 8 additions & 10 deletions Tests/TestingTests/MiscellaneousTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -194,26 +194,24 @@ struct TestsWithAsyncArguments {
struct MiscellaneousTests {
@Test("Free function's name")
func unnamedFreeFunctionTest() async throws {
let tests = await Test.all
let testFunction = try #require(tests.first(where: { $0.name.contains("freeSyncFunction") }))
let testFunction = try #require(await Test.all.first(where: { $0.name.contains("freeSyncFunction") }))
#expect(testFunction.name == "freeSyncFunction()")
}

@Test("Test suite type's name")
func unnamedMemberFunctionTest() async throws {
let testType = try #require(await test(for: SendableTests.self) as Test?)
let testType = try #require(await test(for: SendableTests.self))
#expect(testType.name == "SendableTests")
}

@Test("Free function has custom display name")
func namedFreeFunctionTest() async throws {
let tests = await Test.all
#expect(tests.first { $0.displayName == "Named Free Sync Function" && !$0.isSuite && $0.containingTypeInfo == nil } != nil)
#expect(await Test.all.first { $0.displayName == "Named Free Sync Function" && !$0.isSuite && $0.containingTypeInfo == nil } != nil)
}

@Test("Member function has custom display name")
func namedMemberFunctionTest() async throws {
let testType = try #require(await test(for: NamedSendableTests.self) as Test?)
let testType = try #require(await test(for: NamedSendableTests.self))
#expect(testType.displayName == "Named Sendable test type")
}

Expand Down Expand Up @@ -321,18 +319,18 @@ struct MiscellaneousTests {
@Test("Test.parameters property")
func parametersProperty() async throws {
do {
let theTest = try #require(await test(for: SendableTests.self) as Test?)
let theTest = try #require(await test(for: SendableTests.self))
#expect(theTest.parameters == nil)
}

do {
let test = try #require(await testFunction(named: "succeeds()", in: SendableTests.self) as Test?)
let test = try #require(await testFunction(named: "succeeds()", in: SendableTests.self))
let parameters = try #require(test.parameters)
#expect(parameters.isEmpty)
} catch {}

do {
let test = try #require(await testFunction(named: "parameterized(i:)", in: NonSendableTests.self) as Test?)
let test = try #require(await testFunction(named: "parameterized(i:)", in: NonSendableTests.self))
let parameters = try #require(test.parameters)
#expect(parameters.count == 1)
let firstParameter = try #require(parameters.first)
Expand All @@ -345,7 +343,7 @@ struct MiscellaneousTests {
} catch {}

do {
let test = try #require(await testFunction(named: "parameterized2(i:j:)", in: NonSendableTests.self) as Test?)
let test = try #require(await testFunction(named: "parameterized2(i:j:)", in: NonSendableTests.self))
let parameters = try #require(test.parameters)
#expect(parameters.count == 2)
let firstParameter = try #require(parameters.first)
Expand Down
Loading