Skip to content

Infer a display name for tests and suites which have a raw identifier name consisting of a single token #1174

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
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
11 changes: 7 additions & 4 deletions Sources/TestingMacros/Support/AttributeDiscovery.swift
Original file line number Diff line number Diff line change
Expand Up @@ -138,14 +138,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'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 {
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)
}
}

Expand Down
57 changes: 49 additions & 8 deletions Tests/TestingMacrosTests/TestDeclarationMacroTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -263,19 +263,60 @@ struct TestDeclarationMacroTests {
}
}

@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 apiMisuseWarningsIncludingFixIts(input: String, expectedDiagnostic: (message: String, fixIts: [ExpectedFixIt])) throws {
let (_, diagnostics) = try parse(input)

#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")
func rawIdentifier() {
#expect(TokenSyntax.identifier("`hello`").rawIdentifier == nil)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you re-add these cases please? They're still valid.

#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")
Expand Down
10 changes: 10 additions & 0 deletions Tests/TestingTests/MiscellaneousTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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) {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add a test that uses an escaped keyword name too?

#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.
Expand Down Expand Up @@ -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")
Expand Down
3 changes: 1 addition & 2 deletions Tests/TestingTests/Support/GraphTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, Int>(value: 123, children: [
"C1": Graph(value: 456),
"C2": Graph(value: 789, children: [
Expand Down