Skip to content

Commit 75fde1d

Browse files
authored
Emit a diagnostic if a display name string is empty (#1256)
1 parent a68a681 commit 75fde1d

File tree

4 files changed

+83
-2
lines changed

4 files changed

+83
-2
lines changed

Sources/TestingMacros/Support/Additions/EditorPlaceholderExprSyntaxAdditions.swift

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
//
1010

1111
import SwiftSyntax
12+
import SwiftSyntaxBuilder
1213

1314
extension EditorPlaceholderExprSyntax {
1415
/// Initialize an instance of this type with the given placeholder string and
@@ -39,7 +40,7 @@ extension EditorPlaceholderExprSyntax {
3940

4041
// Manually concatenate the string to avoid it being interpreted as a
4142
// placeholder when editing this file.
42-
self.init(placeholder: .identifier("<#\(placeholderContent)#" + ">"))
43+
self.init(placeholder: .identifier(_editorPlaceholder(containing: placeholderContent)))
4344
}
4445

4546
/// Initialize an instance of this type with the given type, using that as the
@@ -62,6 +63,32 @@ extension TypeSyntax {
6263
///
6364
/// - Returns: A new `TypeSyntax` instance representing a placeholder.
6465
static func placeholder(_ placeholder: String) -> Self {
65-
return Self(IdentifierTypeSyntax(name: .identifier("<#\(placeholder)#" + ">")))
66+
Self(IdentifierTypeSyntax(name: .identifier(_editorPlaceholder(containing: placeholder))))
6667
}
6768
}
69+
70+
extension StringLiteralExprSyntax {
71+
/// Construct a string literal expression syntax node containing an editor
72+
/// placeholder string.
73+
///
74+
/// - Parameters
75+
/// - placeholder: The placeholder string, not including surrounding angle
76+
/// brackets or pound characters.
77+
init(placeholder: String) {
78+
self.init(content: _editorPlaceholder(containing: placeholder))
79+
}
80+
}
81+
82+
/// Format a source editor placeholder string with the specified content.
83+
///
84+
/// - Parameters:
85+
/// - content: The placeholder string, not including surrounding angle
86+
/// brackets or pound characters
87+
///
88+
/// - Returns: A fully-formatted formatted editor placeholder string, including
89+
/// necessary surrounding punctuation.
90+
private func _editorPlaceholder(containing content: String) -> String {
91+
// Manually concatenate the string to avoid it being interpreted as a
92+
// placeholder when editing this file.
93+
"<#\(content)#" + ">"
94+
}

Sources/TestingMacros/Support/AttributeDiscovery.swift

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import SwiftSyntax
1212
import SwiftSyntaxBuilder
1313
import SwiftSyntaxMacros
14+
import SwiftParser
1415

1516
/// A syntax rewriter that removes leading `Self.` tokens from member access
1617
/// expressions in a syntax tree.
@@ -149,6 +150,16 @@ struct AttributeInfo {
149150
}
150151
}
151152

153+
// If there was a display name but it's completely empty, emit a diagnostic
154+
// since this can cause confusion isn't generally recommended. Note that
155+
// this is only possible for string literal display names; the compiler
156+
// enforces that raw identifiers must be non-empty.
157+
if let namedDecl = declaration.asProtocol((any NamedDeclSyntax).self),
158+
let displayName, let displayNameArgument,
159+
displayName.representedLiteralValue?.isEmpty == true {
160+
context.diagnose(.declaration(namedDecl, hasEmptyDisplayName: displayName, fromArgument: displayNameArgument, using: attribute))
161+
}
162+
152163
// Remove leading "Self." expressions from the arguments of the attribute.
153164
// See _SelfRemover for more information. Rewriting a syntax tree discards
154165
// location information from the copy, so only invoke the rewriter if the

Sources/TestingMacros/Support/DiagnosticMessage.swift

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -643,6 +643,41 @@ struct DiagnosticMessage: SwiftDiagnostics.DiagnosticMessage {
643643
)
644644
}
645645

646+
/// Create a diagnostic message stating that a string literal expression
647+
/// passed as the display name to a `@Test` or `@Suite` attribute is empty
648+
/// but should not be.
649+
///
650+
/// - Parameters:
651+
/// - decl: The declaration that has an empty display name.
652+
/// - displayNameExpr: The display name string literal expression.
653+
/// - argumentContainingDisplayName: The argument node containing the node
654+
/// `displayNameExpr`.
655+
/// - attribute: The `@Test` or `@Suite` attribute.
656+
///
657+
/// - Returns: A diagnostic message.
658+
static func declaration(
659+
_ decl: some NamedDeclSyntax,
660+
hasEmptyDisplayName displayNameExpr: StringLiteralExprSyntax,
661+
fromArgument argumentContainingDisplayName: LabeledExprListSyntax.Element,
662+
using attribute: AttributeSyntax
663+
) -> Self {
664+
Self(
665+
syntax: Syntax(displayNameExpr),
666+
message: "Attribute \(_macroName(attribute)) specifies an empty display name for this \(_kindString(for: decl))",
667+
severity: .error,
668+
fixIts: [
669+
FixIt(
670+
message: MacroExpansionFixItMessage("Remove display name argument"),
671+
changes: [.replace(oldNode: Syntax(argumentContainingDisplayName), newNode: Syntax("" as ExprSyntax))]
672+
),
673+
FixIt(
674+
message: MacroExpansionFixItMessage("Add display name"),
675+
changes: [.replace(oldNode: Syntax(argumentContainingDisplayName), newNode: Syntax(StringLiteralExprSyntax(placeholder: "display name")))]
676+
),
677+
]
678+
)
679+
}
680+
646681
/// Create a diagnostic message stating that a declaration has two display
647682
/// names.
648683
///

Tests/TestingMacrosTests/TestDeclarationMacroTests.swift

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,14 @@ struct TestDeclarationMacroTests {
149149
"Attribute 'Test' cannot be applied to a function within structure 'S' because its conformance to 'Escapable' has been suppressed",
150150
"struct S: ~(Escapable) { @Test func f() {} }":
151151
"Attribute 'Test' cannot be applied to a function within structure 'S' because its conformance to 'Escapable' has been suppressed",
152+
153+
// empty display name string literal
154+
#"@Test("") func f() {}"#:
155+
"Attribute 'Test' specifies an empty display name for this function",
156+
##"@Test(#""#) func f() {}"##:
157+
"Attribute 'Test' specifies an empty display name for this function",
158+
#"@Suite("") struct S {}"#:
159+
"Attribute 'Suite' specifies an empty display name for this structure",
152160
]
153161
)
154162
func apiMisuseErrors(input: String, expectedMessage: String) throws {

0 commit comments

Comments
 (0)