Skip to content

Commit a71b141

Browse files
committed
Respond to review feedback
1 parent 8f9e023 commit a71b141

File tree

3 files changed

+39
-20
lines changed

3 files changed

+39
-20
lines changed

Sources/SwiftSyntaxMacroExpansion/MacroArgument.swift

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,15 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// This source file is part of the Swift.org open source project
4+
//
5+
// Copyright (c) 2014 - 2024 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See https://swift.org/LICENSE.txt for license information
9+
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
//===----------------------------------------------------------------------===//
12+
113
import SwiftDiagnostics
214
import SwiftSyntax
315

@@ -20,7 +32,9 @@ enum DeclReferenceError: DiagnosticMessage {
2032
}
2133
}
2234

23-
class CheckDeclReferenceVisitor: SyntaxAnyVisitor {
35+
/// Check sub-expressions to ensure all expressions are literals, and call
36+
/// `diagnoseNonLiteral` for all other expressions.
37+
class OnlyLiteralExprChecker: SyntaxAnyVisitor {
2438
var diagnostics: [Diagnostic] = []
2539

2640
init() {
@@ -127,8 +141,14 @@ class CheckDeclReferenceVisitor: SyntaxAnyVisitor {
127141
}
128142

129143
extension MacroExpansionExprSyntax {
144+
/// For compiler to check a macro expression used as default argument.
145+
///
146+
/// Only literals are permitted as arguments to these expressions.
147+
///
148+
/// If there are diagnostics, they will be wrapped into an error and thrown.
149+
@_spi(Compiler)
130150
public func checkDefaultArgumentMacroExpression() throws {
131-
let visitor = CheckDeclReferenceVisitor()
151+
let visitor = OnlyLiteralExprChecker()
132152
visitor.walk(arguments)
133153

134154
if !visitor.diagnostics.isEmpty {

Sources/SwiftSyntaxMacroExpansion/MacroReplacement.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ extension MacroDefinition {
9393
}
9494
}
9595

96-
fileprivate class ParameterReplacementVisitor: CheckDeclReferenceVisitor {
96+
fileprivate class ParameterReplacementVisitor: OnlyLiteralExprChecker {
9797
let macro: MacroDeclSyntax
9898
var replacements: [MacroDefinition.Replacement] = []
9999
var genericReplacements: [MacroDefinition.GenericArgumentReplacement] = []

Tests/SwiftSyntaxMacroExpansionTest/MacroArgumentTests.swift

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
import SwiftDiagnostics
1414
import SwiftSyntax
1515
import SwiftSyntaxBuilder
16-
import SwiftSyntaxMacroExpansion
16+
@_spi(Compiler) import SwiftSyntaxMacroExpansion
1717
import XCTest
1818
import _SwiftSyntaxTestSupport
1919

@@ -34,22 +34,21 @@ final class MacroArgumentTests: XCTestCase {
3434
#otherMacro(first: b, second: "\(false)", third: 1 + 2)
3535
"""#
3636

37-
let diags: [Diagnostic]
38-
do {
39-
try macro.as(MacroExpansionExprSyntax.self)!
40-
.checkDefaultArgumentMacroExpression()
41-
XCTFail("should have failed with an error")
42-
fatalError()
43-
} catch let diagError as DiagnosticsError {
44-
diags = diagError.diagnostics
45-
}
37+
XCTAssertThrowsError(try macro.as(MacroExpansionExprSyntax.self)!
38+
.checkDefaultArgumentMacroExpression()) { error in
39+
guard let diagError = error as? DiagnosticsError else {
40+
XCTFail("should have failed with a diagnostics error")
41+
return
42+
}
43+
let diags = diagError.diagnostics
4644

47-
XCTAssertEqual(diags.count, 3)
48-
for diag in diags {
49-
XCTAssertEqual(
50-
diag.diagMessage.message,
51-
"only literals are permitted"
52-
)
53-
}
45+
XCTAssertEqual(diags.count, 3)
46+
for diag in diags {
47+
XCTAssertEqual(
48+
diag.diagMessage.message,
49+
"only literals are permitted"
50+
)
51+
}
52+
}
5453
}
5554
}

0 commit comments

Comments
 (0)