From 531f993a21d2497dd4684f37cc0fd268e3686187 Mon Sep 17 00:00:00 2001 From: Stuart Montgomery Date: Wed, 25 Jun 2025 11:54:58 -0500 Subject: [PATCH 1/2] Infer a display name for tests and suites which have a raw identifier name consisting of a single token --- .../FunctionDeclSyntaxAdditions.swift | 3 ++- .../Additions/TokenSyntaxAdditions.swift | 2 +- .../Support/AttributeDiscovery.swift | 13 ++++++++--- .../TestDeclarationMacroTests.swift | 23 ++++++++++++------- Tests/TestingTests/MiscellaneousTests.swift | 10 ++++++++ 5 files changed, 38 insertions(+), 13 deletions(-) diff --git a/Sources/TestingMacros/Support/Additions/FunctionDeclSyntaxAdditions.swift b/Sources/TestingMacros/Support/Additions/FunctionDeclSyntaxAdditions.swift index 9b9378283..c4dd580cf 100644 --- a/Sources/TestingMacros/Support/Additions/FunctionDeclSyntaxAdditions.swift +++ b/Sources/TestingMacros/Support/Additions/FunctionDeclSyntaxAdditions.swift @@ -11,6 +11,7 @@ import SwiftSyntax import SwiftSyntaxBuilder import SwiftSyntaxMacros +import SwiftParser extension FunctionDeclSyntax { /// Whether or not this function a `static` or `class` function. @@ -38,7 +39,7 @@ extension FunctionDeclSyntax { /// colons. var completeName: DeclReferenceExprSyntax { func possiblyRaw(_ token: TokenSyntax) -> TokenSyntax { - if let rawIdentifier = token.rawIdentifier { + if let rawIdentifier = token.rawIdentifier, !rawIdentifier.isValidSwiftIdentifier(for: .memberAccess) { return .identifier("`\(rawIdentifier)`") } return .identifier(token.textWithoutBackticks) diff --git a/Sources/TestingMacros/Support/Additions/TokenSyntaxAdditions.swift b/Sources/TestingMacros/Support/Additions/TokenSyntaxAdditions.swift index 26b9d1923..a148993f0 100644 --- a/Sources/TestingMacros/Support/Additions/TokenSyntaxAdditions.swift +++ b/Sources/TestingMacros/Support/Additions/TokenSyntaxAdditions.swift @@ -35,7 +35,7 @@ extension TokenSyntax { /// token, or `nil` if it does not represent one. var rawIdentifier: String? { let (textWithoutBackticks, backticksRemoved) = _textWithoutBackticks - if backticksRemoved, !textWithoutBackticks.isValidSwiftIdentifier(for: .memberAccess) { + if backticksRemoved { return textWithoutBackticks } diff --git a/Sources/TestingMacros/Support/AttributeDiscovery.swift b/Sources/TestingMacros/Support/AttributeDiscovery.swift index 3d95df294..05bfed770 100644 --- a/Sources/TestingMacros/Support/AttributeDiscovery.swift +++ b/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. @@ -138,11 +139,17 @@ struct AttributeInfo { } } - // Disallow an explicit display name for tests and suites with raw - // identifier names as it's redundant and potentially confusing. + // If this declaration has a raw identifier name, implicitly use the content + // of that raw identifier as its display name. if let namedDecl = declaration.asProtocol((any NamedDeclSyntax).self), let rawIdentifier = namedDecl.name.rawIdentifier { - if let displayName, let displayNameArgument { + // Disallow having an explicit display name for tests and suites with raw + // identifier names as it's redundant and potentially confusing. An + // exception to this rule is if the content of the raw identifier is not a + // valid identifier on its own. For example: + // + // @Test("...") func `subscript`() { ... } + if let displayName, let displayNameArgument, !rawIdentifier.isValidSwiftIdentifier(for: .memberAccess) { context.diagnose(.declaration(namedDecl, hasExtraneousDisplayName: displayName, fromArgument: displayNameArgument, using: attribute)) } else { displayName = StringLiteralExprSyntax(content: rawIdentifier) diff --git a/Tests/TestingMacrosTests/TestDeclarationMacroTests.swift b/Tests/TestingMacrosTests/TestDeclarationMacroTests.swift index 6c04eb9eb..2f1af82b0 100644 --- a/Tests/TestingMacrosTests/TestDeclarationMacroTests.swift +++ b/Tests/TestingMacrosTests/TestDeclarationMacroTests.swift @@ -263,19 +263,26 @@ struct TestDeclarationMacroTests { } } + @Test("No diagnostics are emitted for tests with both a raw identifier name and explicit display name when deemed non-redundant", arguments: [ + #"@Test("Class") func `class`()"#, + #"@Test("Struct") func `struct`()"#, + #"@Test("Subscript") func `subscript`()"#, + ]) + func nonRedundantRawIdentifierDisplayName(input: String) throws { + let (_, diagnostics) = try parse(input) + #expect(diagnostics.isEmpty) + } + @Test("Raw identifier is detected") func rawIdentifier() { - #expect(TokenSyntax.identifier("`hello`").rawIdentifier == nil) - #expect(TokenSyntax.identifier("`helloworld`").rawIdentifier == nil) - #expect(TokenSyntax.identifier("`hélloworld`").rawIdentifier == nil) - #expect(TokenSyntax.identifier("`hello_world`").rawIdentifier == nil) + #expect(TokenSyntax.identifier("hello").rawIdentifier == nil) + #expect(TokenSyntax.identifier("`hello").rawIdentifier == nil) + #expect(TokenSyntax.identifier("hello`").rawIdentifier == nil) + #expect(TokenSyntax.identifier("hélloworld").rawIdentifier == nil) + #expect(TokenSyntax.identifier("hello_world").rawIdentifier == nil) #expect(TokenSyntax.identifier("`hello world`").rawIdentifier != nil) #expect(TokenSyntax.identifier("`hello/world`").rawIdentifier != nil) #expect(TokenSyntax.identifier("`hello\tworld`").rawIdentifier != nil) - - #expect(TokenSyntax.identifier("`class`").rawIdentifier == nil) - #expect(TokenSyntax.identifier("`struct`").rawIdentifier == nil) - #expect(TokenSyntax.identifier("`class struct`").rawIdentifier != nil) } @Test("Raw function name components") diff --git a/Tests/TestingTests/MiscellaneousTests.swift b/Tests/TestingTests/MiscellaneousTests.swift index 6a65fb658..259ef4766 100644 --- a/Tests/TestingTests/MiscellaneousTests.swift +++ b/Tests/TestingTests/MiscellaneousTests.swift @@ -28,6 +28,10 @@ private import Foundation @Sendable func freeSyncFunctionParameterized2(_ i: Int, _ j: String) {} +#if compiler(>=6.2) && hasFeature(RawIdentifiers) +@Test(.hidden, arguments: [0]) func `ValidSingleCapitalizedToken`(someLengthyParameterName: Int) {} +#endif + // This type ensures the parser can correctly infer that f() is a member // function even though @Test is preceded by another attribute or is embedded in // a #if statement. @@ -320,6 +324,12 @@ struct MiscellaneousTests { func `Test with raw identifier and raw identifier parameter labels can compile`(`argument name` i: Int) { #expect(i == 0) } + + @Test func singleValidTokenRawIdentifiers() async throws { + let test = try #require(await Test.all.first { $0.name.contains("ValidSingleCapitalizedToken") }) + #expect(test.name == "ValidSingleCapitalizedToken(someLengthyParameterName:)") + #expect(test.displayName == "ValidSingleCapitalizedToken") + } #endif @Test("Free functions are runnable") From 2bc3ee5a96d94ffa13bf93e89aa64425179d7846 Mon Sep 17 00:00:00 2001 From: Stuart Montgomery Date: Wed, 25 Jun 2025 15:50:42 -0500 Subject: [PATCH 2/2] Apply this policy more broadly to all backtick-enclosed identifiers --- .../FunctionDeclSyntaxAdditions.swift | 3 +- .../Additions/TokenSyntaxAdditions.swift | 2 +- .../Support/AttributeDiscovery.swift | 20 ++++---- .../TestDeclarationMacroTests.swift | 46 ++++++++++++++++--- Tests/TestingTests/Support/GraphTests.swift | 3 +- 5 files changed, 51 insertions(+), 23 deletions(-) diff --git a/Sources/TestingMacros/Support/Additions/FunctionDeclSyntaxAdditions.swift b/Sources/TestingMacros/Support/Additions/FunctionDeclSyntaxAdditions.swift index c4dd580cf..9b9378283 100644 --- a/Sources/TestingMacros/Support/Additions/FunctionDeclSyntaxAdditions.swift +++ b/Sources/TestingMacros/Support/Additions/FunctionDeclSyntaxAdditions.swift @@ -11,7 +11,6 @@ import SwiftSyntax import SwiftSyntaxBuilder import SwiftSyntaxMacros -import SwiftParser extension FunctionDeclSyntax { /// Whether or not this function a `static` or `class` function. @@ -39,7 +38,7 @@ extension FunctionDeclSyntax { /// colons. var completeName: DeclReferenceExprSyntax { func possiblyRaw(_ token: TokenSyntax) -> TokenSyntax { - if let rawIdentifier = token.rawIdentifier, !rawIdentifier.isValidSwiftIdentifier(for: .memberAccess) { + if let rawIdentifier = token.rawIdentifier { return .identifier("`\(rawIdentifier)`") } return .identifier(token.textWithoutBackticks) diff --git a/Sources/TestingMacros/Support/Additions/TokenSyntaxAdditions.swift b/Sources/TestingMacros/Support/Additions/TokenSyntaxAdditions.swift index a148993f0..26b9d1923 100644 --- a/Sources/TestingMacros/Support/Additions/TokenSyntaxAdditions.swift +++ b/Sources/TestingMacros/Support/Additions/TokenSyntaxAdditions.swift @@ -35,7 +35,7 @@ extension TokenSyntax { /// token, or `nil` if it does not represent one. var rawIdentifier: String? { let (textWithoutBackticks, backticksRemoved) = _textWithoutBackticks - if backticksRemoved { + if backticksRemoved, !textWithoutBackticks.isValidSwiftIdentifier(for: .memberAccess) { return textWithoutBackticks } diff --git a/Sources/TestingMacros/Support/AttributeDiscovery.swift b/Sources/TestingMacros/Support/AttributeDiscovery.swift index 05bfed770..29de508b2 100644 --- a/Sources/TestingMacros/Support/AttributeDiscovery.swift +++ b/Sources/TestingMacros/Support/AttributeDiscovery.swift @@ -11,7 +11,6 @@ import SwiftSyntax import SwiftSyntaxBuilder import SwiftSyntaxMacros -import SwiftParser /// A syntax rewriter that removes leading `Self.` tokens from member access /// expressions in a syntax tree. @@ -139,20 +138,17 @@ struct AttributeInfo { } } - // If this declaration has a raw identifier name, implicitly use the content - // of that raw identifier as its display name. + // If this declaration's name is surrounded by backticks, it may be an + // escaped or raw identifier. If it has an explicitly-specified display name + // string, honor that, otherwise use the content of the backtick-enclosed + // name as the display name. if let namedDecl = declaration.asProtocol((any NamedDeclSyntax).self), - let rawIdentifier = namedDecl.name.rawIdentifier { - // Disallow having an explicit display name for tests and suites with raw - // identifier names as it's redundant and potentially confusing. An - // exception to this rule is if the content of the raw identifier is not a - // valid identifier on its own. For example: - // - // @Test("...") func `subscript`() { ... } - if let displayName, let displayNameArgument, !rawIdentifier.isValidSwiftIdentifier(for: .memberAccess) { + case let nameWithoutBackticks = namedDecl.name.textWithoutBackticks, + nameWithoutBackticks != namedDecl.name.text { + if let displayName, let displayNameArgument { context.diagnose(.declaration(namedDecl, hasExtraneousDisplayName: displayName, fromArgument: displayNameArgument, using: attribute)) } else { - displayName = StringLiteralExprSyntax(content: rawIdentifier) + displayName = StringLiteralExprSyntax(content: nameWithoutBackticks) } } diff --git a/Tests/TestingMacrosTests/TestDeclarationMacroTests.swift b/Tests/TestingMacrosTests/TestDeclarationMacroTests.swift index 2f1af82b0..bd9034254 100644 --- a/Tests/TestingMacrosTests/TestDeclarationMacroTests.swift +++ b/Tests/TestingMacrosTests/TestDeclarationMacroTests.swift @@ -263,14 +263,48 @@ struct TestDeclarationMacroTests { } } - @Test("No diagnostics are emitted for tests with both a raw identifier name and explicit display name when deemed non-redundant", arguments: [ - #"@Test("Class") func `class`()"#, - #"@Test("Struct") func `struct`()"#, - #"@Test("Subscript") func `subscript`()"#, + @Test("Warning diagnostics which include fix-its emitted on API misuse", arguments: [ + #"@Test("Subscript") func `subscript`()"#: + ( + message: "Attribute 'Test' specifies display name 'Subscript' for function with implicit display name 'subscript'", + fixIts: [ + ExpectedFixIt( + message: "Remove 'Subscript'", + changes: [.replace(oldSourceCode: #""Subscript""#, newSourceCode: "")] + ), + ExpectedFixIt( + message: "Rename 'subscript'", + changes: [.replace(oldSourceCode: "`subscript`", newSourceCode: "\(EditorPlaceholderExprSyntax("name"))")] + ), + ] + ), ]) - func nonRedundantRawIdentifierDisplayName(input: String) throws { + func apiMisuseWarningsIncludingFixIts(input: String, expectedDiagnostic: (message: String, fixIts: [ExpectedFixIt])) throws { let (_, diagnostics) = try parse(input) - #expect(diagnostics.isEmpty) + + #expect(diagnostics.count == 1) + let diagnostic = try #require(diagnostics.first) + #expect(diagnostic.diagMessage.severity == .warning) + #expect(diagnostic.message == expectedDiagnostic.message) + + try #require(diagnostic.fixIts.count == expectedDiagnostic.fixIts.count) + for (fixIt, expectedFixIt) in zip(diagnostic.fixIts, expectedDiagnostic.fixIts) { + #expect(fixIt.message.message == expectedFixIt.message) + + try #require(fixIt.changes.count == expectedFixIt.changes.count) + for (change, expectedChange) in zip(fixIt.changes, expectedFixIt.changes) { + switch (change, expectedChange) { + case let (.replace(oldNode, newNode), .replace(expectedOldSourceCode, expectedNewSourceCode)): + let oldSourceCode = String(describing: oldNode.formatted()) + #expect(oldSourceCode == expectedOldSourceCode) + + let newSourceCode = String(describing: newNode.formatted()) + #expect(newSourceCode == expectedNewSourceCode) + default: + Issue.record("Change \(change) differs from expected change \(expectedChange)") + } + } + } } @Test("Raw identifier is detected") diff --git a/Tests/TestingTests/Support/GraphTests.swift b/Tests/TestingTests/Support/GraphTests.swift index 5f956dd60..9de7e28f3 100644 --- a/Tests/TestingTests/Support/GraphTests.swift +++ b/Tests/TestingTests/Support/GraphTests.swift @@ -53,8 +53,7 @@ struct GraphTests { #expect(graph.subgraph(at: "C2", "C3")?.value == 2468) } - @Test("subscript([K]) operator") - func `subscript`() { + @Test func `subscript([K]) operator`() { let graph = Graph(value: 123, children: [ "C1": Graph(value: 456), "C2": Graph(value: 789, children: [