Emit a diagnostic if a display name string is empty (swiftlang#1256)

Author: stmontgomery

Sources/TestingMacros/Support/Additions/EditorPlaceholderExprSyntaxAdditions.swift

@@ -9,6 +9,7 @@
 //
 
 import SwiftSyntax
+import SwiftSyntaxBuilder
 
 extension EditorPlaceholderExprSyntax {
   /// Initialize an instance of this type with the given placeholder string and
@@ -39,7 +40,7 @@ extension EditorPlaceholderExprSyntax {
 
     // Manually concatenate the string to avoid it being interpreted as a
     // placeholder when editing this file.
-    self.init(placeholder: .identifier("<#\(placeholderContent)#" + ">"))
+    self.init(placeholder: .identifier(_editorPlaceholder(containing: placeholderContent)))
   }
 
   /// Initialize an instance of this type with the given type, using that as the
@@ -62,6 +63,32 @@ extension TypeSyntax {
   ///
   /// - Returns: A new `TypeSyntax` instance representing a placeholder.
   static func placeholder(_ placeholder: String) -> Self {
-    return Self(IdentifierTypeSyntax(name: .identifier("<#\(placeholder)#" + ">")))
+    Self(IdentifierTypeSyntax(name: .identifier(_editorPlaceholder(containing: placeholder))))
   }
 }
+
+extension StringLiteralExprSyntax {
+  /// Construct a string literal expression syntax node containing an editor
+  /// placeholder string.
+  ///
+  /// - Parameters
+  ///   - placeholder: The placeholder string, not including surrounding angle
+  ///     brackets or pound characters.
+  init(placeholder: String) {
+    self.init(content: _editorPlaceholder(containing: placeholder))
+  }
+}
+
+/// Format a source editor placeholder string with the specified content.
+///
+/// - Parameters:
+///   - content: The placeholder string, not including surrounding angle
+///     brackets or pound characters
+///
+/// - Returns: A fully-formatted formatted editor placeholder string, including
+///   necessary surrounding punctuation.
+private func _editorPlaceholder(containing content: String) -> String {
+  // Manually concatenate the string to avoid it being interpreted as a
+  // placeholder when editing this file.
+  "<#\(content)#" + ">"
+}

Sources/TestingMacros/Support/AttributeDiscovery.swift

@@ -11,6 +11,7 @@
 import SwiftSyntax
 import SwiftSyntaxBuilder
 import SwiftSyntaxMacros
+import SwiftParser
 
 /// A syntax rewriter that removes leading `Self.` tokens from member access
 /// expressions in a syntax tree.
@@ -149,6 +150,16 @@ struct AttributeInfo {
       }
     }
 
+    // If there was a display name but it's completely empty, emit a diagnostic
+    // since this can cause confusion isn't generally recommended. Note that
+    // this is only possible for string literal display names; the compiler
+    // enforces that raw identifiers must be non-empty.
+    if let namedDecl = declaration.asProtocol((any NamedDeclSyntax).self),
+       let displayName, let displayNameArgument,
+        displayName.representedLiteralValue?.isEmpty == true {
+      context.diagnose(.declaration(namedDecl, hasEmptyDisplayName: displayName, fromArgument: displayNameArgument, using: attribute))
+    }
+
     // Remove leading "Self." expressions from the arguments of the attribute.
     // See _SelfRemover for more information. Rewriting a syntax tree discards
     // location information from the copy, so only invoke the rewriter if the

Sources/TestingMacros/Support/DiagnosticMessage.swift

@@ -643,6 +643,41 @@ struct DiagnosticMessage: SwiftDiagnostics.DiagnosticMessage {
     )
   }
 
+  /// Create a diagnostic message stating that a string literal expression
+  /// passed as the display name to a `@Test` or `@Suite` attribute is empty
+  /// but should not be.
+  ///
+  /// - Parameters:
+  ///   - decl: The declaration that has an empty display name.
+  ///   - displayNameExpr: The display name string literal expression.
+  ///   - argumentContainingDisplayName: The argument node containing the node
+  ///     `displayNameExpr`.
+  ///   - attribute: The `@Test` or `@Suite` attribute.
+  ///
+  /// - Returns: A diagnostic message.
+  static func declaration(
+    _ decl: some NamedDeclSyntax,
+    hasEmptyDisplayName displayNameExpr: StringLiteralExprSyntax,
+    fromArgument argumentContainingDisplayName: LabeledExprListSyntax.Element,
+    using attribute: AttributeSyntax
+  ) -> Self {
+    Self(
+      syntax: Syntax(displayNameExpr),
+      message: "Attribute \(_macroName(attribute)) specifies an empty display name for this \(_kindString(for: decl))",
+      severity: .error,
+      fixIts: [
+        FixIt(
+          message: MacroExpansionFixItMessage("Remove display name argument"),
+          changes: [.replace(oldNode: Syntax(argumentContainingDisplayName), newNode: Syntax("" as ExprSyntax))]
+        ),
+        FixIt(
+          message: MacroExpansionFixItMessage("Add display name"),
+          changes: [.replace(oldNode: Syntax(argumentContainingDisplayName), newNode: Syntax(StringLiteralExprSyntax(placeholder: "display name")))]
+        ),
+      ]
+    )
+  }
+
   /// Create a diagnostic message stating that a declaration has two display
   /// names.
   ///

Tests/TestingMacrosTests/TestDeclarationMacroTests.swift

@@ -149,6 +149,14 @@ struct TestDeclarationMacroTests {
         "Attribute 'Test' cannot be applied to a function within structure 'S' because its conformance to 'Escapable' has been suppressed",
       "struct S: ~(Escapable) { @Test func f() {} }":
         "Attribute 'Test' cannot be applied to a function within structure 'S' because its conformance to 'Escapable' has been suppressed",
+
+      // empty display name string literal
+      #"@Test("") func f() {}"#:
+        "Attribute 'Test' specifies an empty display name for this function",
+      ##"@Test(#""#) func f() {}"##:
+        "Attribute 'Test' specifies an empty display name for this function",
+      #"@Suite("") struct S {}"#:
+        "Attribute 'Suite' specifies an empty display name for this structure",
     ]
   )
   func apiMisuseErrors(input: String, expectedMessage: String) throws {