Skip to content

Commit bde5d35

Browse files
authored
Use lexicalContext to improve diagnostics for tag declarations. (#334)
This PR uses the swift-syntax-600 `lexicalContext` property to enforce the compile-time requirement that tags declared with `@Tag` must be declared in an extension to `Tag` or in a type declared inside `Tag`. Fix-its are provided for a few scenarios. The new diagnostics are: ```swift @tag var x: Tag // 🛑 Attribute 'Tag' cannot be applied to a global variable // Declare in an extension to 'Tag' [Fix] // Remove attribute 'Tag' [Fix] extension String { @tag var x: Tag // 🛑 Attribute 'Tag' cannot be applied to a property except in an extension to 'Tag' } extension Tag { @tag var x: Tag // 🛑 Attribute 'Tag' cannot be applied to an instance property // Add 'static' [Fix] } extension Tag { @tag var x: String // 🛑 Attribute 'Tag' cannot be applied to a property of type 'String' // Change type to 'Tag' [Fix] // Remove attribute 'Tag' [Fix] } ``` As with #332 and #333, these problems are only diagnosed when using swift-syntax-600. ### Checklist: - [x] Code and documentation should follow the style of the [Style Guide](https://github.com/apple/swift-testing/blob/main/Documentation/StyleGuide.md). - [x] If public symbols are renamed or modified, DocC references should be updated.
1 parent 52a4565 commit bde5d35

File tree

11 files changed

+504
-191
lines changed

11 files changed

+504
-191
lines changed

Sources/Testing/Traits/Tags/Tag+Macro.swift

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,18 @@ extension Tag {
2727
// described static member.
2828
var fullyQualifiedMemberNameComponents = TypeInfo(describing: type).fullyQualifiedNameComponents
2929

30+
#if canImport(SwiftSyntax600)
31+
// Strip off the "Testing" and "Tag" components of the fully-qualified name
32+
// since they're redundant. Macro expansion will have already checked that
33+
// the type is nested inside `Tag`.
34+
#else
3035
// Ensure that the tag is nested somewhere inside Testing.Tag, then strip
3136
// off those elements of the fully-qualified type name. These preconditions
3237
// are necessary because we do not currently have access, during macro
3338
// expansion, to the lexical context in which a tag is declared.
3439
precondition(fullyQualifiedMemberNameComponents.count >= 2, "Tags must be specified as members of the Tag type or a nested type in Tag.")
3540
precondition(fullyQualifiedMemberNameComponents[0 ..< 2] == ["Testing", "Tag"], "Tags must be specified as members of the Tag type or a nested type in Tag.")
41+
#endif
3642
fullyQualifiedMemberNameComponents = Array(fullyQualifiedMemberNameComponents.dropFirst(2))
3743

3844
// Add the specified tag name to the fully-qualified name and reconstruct
@@ -49,4 +55,4 @@ extension Tag {
4955
/// Use this tag with members of the ``Tag`` type declared in an extension to
5056
/// mark them as usable with tests. For more information on declaring tags, see
5157
/// <doc:AddingTags>.
52-
@attached(accessor) public macro Tag() = #externalMacro(module: "TestingMacros", type: "TagMacro")
58+
@attached(accessor) @attached(peer) public macro Tag() = #externalMacro(module: "TestingMacros", type: "TagMacro")

Sources/Testing/Traits/Tags/Tag.swift

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,16 @@ public struct Tag: Sendable {
2121
public enum Kind: Sendable, Hashable {
2222
/// The tag is a static member of ``Tag`` such as ``Tag/red``, declared
2323
/// using the ``Tag()`` macro.
24+
///
25+
/// - Parameters:
26+
/// - name: The (almost) fully-qualified name of the static member. The
27+
/// leading `"Testing.Tag."` is not included as it is redundant.
2428
case staticMember(_ name: String)
2529

2630
/// The tag is a string literal declared directly in source.
31+
///
32+
/// - Parameters:
33+
/// - stringLiteral: The string literal specified by the test author.
2734
case stringLiteral(_ stringLiteral: String)
2835
}
2936

Sources/TestingMacros/SuiteDeclarationMacro.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ public struct SuiteDeclarationMacro: MemberMacro, PeerMacro, Sendable {
3434
) throws -> [DeclSyntax] {
3535
// The peer macro expansion of this macro is only used to diagnose misuses
3636
// on symbols that are not decl groups.
37-
if declaration.asProtocol((any DeclGroupSyntax).self) == nil {
37+
if !declaration.isProtocol((any DeclGroupSyntax).self) {
3838
_ = _diagnoseIssues(with: declaration, suiteAttribute: node, in: context)
3939
}
4040
return []

Sources/TestingMacros/Support/Additions/FunctionDeclSyntaxAdditions.swift

Lines changed: 0 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -164,67 +164,3 @@ extension FunctionParameterSyntax {
164164
return MemberAccessExprSyntax(base: metatypeMemberAccessBase, name: .identifier("self"))
165165
}
166166
}
167-
168-
// MARK: -
169-
170-
extension MacroExpansionContext {
171-
/// Create a unique name for a function that thunks another function.
172-
///
173-
/// - Parameters:
174-
/// - functionDecl: The function to thunk.
175-
/// - prefix: A prefix to apply to the thunked name before returning.
176-
///
177-
/// - Returns: A unique name to use for a thunk function that thunks
178-
/// `functionDecl`.
179-
func makeUniqueName(thunking functionDecl: FunctionDeclSyntax, withPrefix prefix: String = "") -> TokenSyntax {
180-
// Find all the tokens of the function declaration including argument
181-
// types, specifiers, etc. (but not any attributes nor the body of the
182-
// function.) Use them as the base name we pass to makeUniqueName(). This
183-
// ensures that we will end up with a unique identifier even if two
184-
// functions in the same scope have the exact same identifier.
185-
let identifierCharacters = functionDecl
186-
.with(\.attributes, [])
187-
.with(\.body, nil)
188-
.tokens(viewMode: .fixedUp)
189-
.map(\.textWithoutBackticks)
190-
.joined()
191-
192-
// Strip out any characters in the function's signature that won't play well
193-
// in a generated symbol name.
194-
let identifier = String(
195-
identifierCharacters.map { character in
196-
if character.isLetter || character.isWholeNumber {
197-
return character
198-
}
199-
return "_"
200-
}
201-
)
202-
203-
// If there is a non-ASCII character in the identifier, we might be
204-
// stripping it out above because we are only looking for letters and
205-
// digits. If so, add in a hash of the identifier to improve entropy and
206-
// reduce the risk of a collision.
207-
//
208-
// For example, the following function names will produce identical unique
209-
// names without this mutation:
210-
//
211-
// @Test(arguments: [0]) func A(🙃: Int) {}
212-
// @Test(arguments: [0]) func A(🙂: Int) {}
213-
//
214-
// Note the check here is not the same as the one above: punctuation like
215-
// "(" should be replaced, but should not cause a hash to be emitted since
216-
// it does not contribute any entropy to the makeUniqueName() algorithm.
217-
//
218-
// The intent here is not to produce a cryptographically strong hash, but to
219-
// disambiguate between superficially similar function names. A collision
220-
// may still occur, but we only need it to be _unlikely_. CRC-32 is good
221-
// enough for our purposes.
222-
if !identifierCharacters.allSatisfy(\.isASCII) {
223-
let crcValue = crc32(identifierCharacters.utf8)
224-
let suffix = String(crcValue, radix: 16, uppercase: false)
225-
return makeUniqueName("\(prefix)\(identifier)_\(suffix)")
226-
}
227-
228-
return makeUniqueName("\(prefix)\(identifier)")
229-
}
230-
}
Lines changed: 190 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,190 @@
1+
//
2+
// This source file is part of the Swift.org open source project
3+
//
4+
// Copyright (c) 2024 Apple Inc. and the Swift project authors
5+
// Licensed under Apache License v2.0 with Runtime Library Exception
6+
//
7+
// See https://swift.org/LICENSE.txt for license information
8+
// See https://swift.org/CONTRIBUTORS.txt for Swift project authors
9+
//
10+
11+
#if compiler(>=5.11)
12+
import SwiftSyntax
13+
import SwiftSyntaxMacros
14+
#else
15+
public import SwiftSyntax
16+
public import SwiftSyntaxMacros
17+
#endif
18+
import SwiftDiagnostics
19+
20+
extension MacroExpansionContext {
21+
/// Get the type of the lexical context enclosing the given node.
22+
///
23+
/// - Parameters:
24+
/// - node: The node whose lexical context should be examined.
25+
///
26+
/// - Returns: The type of the lexical context enclosing `node`, or `nil` if
27+
/// the lexical context cannot be represented as a type.
28+
///
29+
/// If the lexical context includes functions, closures, or some other
30+
/// non-type scope, the value of this property is `nil`.
31+
///
32+
/// If swift-syntax-600 or newer is available, `node` is ignored. The argument
33+
/// will be removed once the testing library's swift-syntax dependency is
34+
/// updated to swift-syntax-600 or later.
35+
func typeOfLexicalContext(containing node: some WithAttributesSyntax) -> TypeSyntax? {
36+
#if canImport(SwiftSyntax600)
37+
var typeNames = [String]()
38+
for lexicalContext in lexicalContext.reversed() {
39+
guard let decl = lexicalContext.asProtocol((any DeclGroupSyntax).self) else {
40+
return nil
41+
}
42+
typeNames.append(decl.type.trimmedDescription)
43+
}
44+
if typeNames.isEmpty {
45+
return nil
46+
}
47+
48+
return "\(raw: typeNames.joined(separator: "."))"
49+
#else
50+
// Find the beginning of the first attribute on the declaration, including
51+
// those embedded in #if statements, to account for patterns like
52+
// `@MainActor @Test func` where there's a space ahead of @Test, but the
53+
// whole function is still at the top level.
54+
func firstAttribute(in attributes: AttributeListSyntax) -> AttributeSyntax? {
55+
attributes.lazy
56+
.compactMap { attribute in
57+
switch (attribute as AttributeListSyntax.Element?) {
58+
case let .ifConfigDecl(ifConfigDecl):
59+
ifConfigDecl.clauses.lazy
60+
.compactMap { clause in
61+
if case let .attributes(attributes) = clause.elements {
62+
return firstAttribute(in: attributes)
63+
}
64+
return nil
65+
}.first
66+
case let .attribute(attribute):
67+
attribute
68+
default:
69+
nil
70+
}
71+
}.first
72+
}
73+
let firstAttribute = firstAttribute(in: node.attributes)!
74+
75+
// HACK: If the test function appears to be indented, assume it is nested in
76+
// a type. Use `Self` as the presumptive name of the type.
77+
//
78+
// This hack works around rdar://105470382.
79+
if let lastLeadingTrivia = firstAttribute.leadingTrivia.pieces.last,
80+
lastLeadingTrivia.isWhitespace && !lastLeadingTrivia.isNewline {
81+
return TypeSyntax(IdentifierTypeSyntax(name: .keyword(.Self)))
82+
}
83+
return nil
84+
#endif
85+
}
86+
}
87+
88+
// MARK: -
89+
90+
extension MacroExpansionContext {
91+
/// Create a unique name for a function that thunks another function.
92+
///
93+
/// - Parameters:
94+
/// - functionDecl: The function to thunk.
95+
/// - prefix: A prefix to apply to the thunked name before returning.
96+
///
97+
/// - Returns: A unique name to use for a thunk function that thunks
98+
/// `functionDecl`.
99+
func makeUniqueName(thunking functionDecl: FunctionDeclSyntax, withPrefix prefix: String = "") -> TokenSyntax {
100+
// Find all the tokens of the function declaration including argument
101+
// types, specifiers, etc. (but not any attributes nor the body of the
102+
// function.) Use them as the base name we pass to makeUniqueName(). This
103+
// ensures that we will end up with a unique identifier even if two
104+
// functions in the same scope have the exact same identifier.
105+
let identifierCharacters = functionDecl
106+
.with(\.attributes, [])
107+
.with(\.body, nil)
108+
.tokens(viewMode: .fixedUp)
109+
.map(\.textWithoutBackticks)
110+
.joined()
111+
112+
// Strip out any characters in the function's signature that won't play well
113+
// in a generated symbol name.
114+
let identifier = String(
115+
identifierCharacters.map { character in
116+
if character.isLetter || character.isWholeNumber {
117+
return character
118+
}
119+
return "_"
120+
}
121+
)
122+
123+
// If there is a non-ASCII character in the identifier, we might be
124+
// stripping it out above because we are only looking for letters and
125+
// digits. If so, add in a hash of the identifier to improve entropy and
126+
// reduce the risk of a collision.
127+
//
128+
// For example, the following function names will produce identical unique
129+
// names without this mutation:
130+
//
131+
// @Test(arguments: [0]) func A(🙃: Int) {}
132+
// @Test(arguments: [0]) func A(🙂: Int) {}
133+
//
134+
// Note the check here is not the same as the one above: punctuation like
135+
// "(" should be replaced, but should not cause a hash to be emitted since
136+
// it does not contribute any entropy to the makeUniqueName() algorithm.
137+
//
138+
// The intent here is not to produce a cryptographically strong hash, but to
139+
// disambiguate between superficially similar function names. A collision
140+
// may still occur, but we only need it to be _unlikely_. CRC-32 is good
141+
// enough for our purposes.
142+
if !identifierCharacters.allSatisfy(\.isASCII) {
143+
let crcValue = crc32(identifierCharacters.utf8)
144+
let suffix = String(crcValue, radix: 16, uppercase: false)
145+
return makeUniqueName("\(prefix)\(identifier)_\(suffix)")
146+
}
147+
148+
return makeUniqueName("\(prefix)\(identifier)")
149+
}
150+
}
151+
152+
// MARK: -
153+
154+
extension MacroExpansionContext {
155+
/// Emit a diagnostic message.
156+
///
157+
/// - Parameters:
158+
/// - message: The diagnostic message to emit. The `node` and `position`
159+
/// arguments to `Diagnostic.init()` are derived from the message's
160+
/// `syntax` property.
161+
func diagnose(_ message: DiagnosticMessage) {
162+
diagnose(
163+
Diagnostic(
164+
node: message.syntax,
165+
position: message.syntax.positionAfterSkippingLeadingTrivia,
166+
message: message,
167+
fixIts: message.fixIts
168+
)
169+
)
170+
}
171+
172+
/// Emit a sequence of diagnostic messages.
173+
///
174+
/// - Parameters:
175+
/// - messages: The diagnostic messages to emit.
176+
func diagnose(_ messages: some Sequence<DiagnosticMessage>) {
177+
for message in messages {
178+
diagnose(message)
179+
}
180+
}
181+
182+
/// Emit a diagnostic message for debugging purposes during development of the
183+
/// testing library.
184+
///
185+
/// - Parameters:
186+
/// - message: The message to emit into the build log.
187+
func debug(_ message: some Any, node: some SyntaxProtocol) {
188+
diagnose(DiagnosticMessage(syntax: Syntax(node), message: String(describing: message), severity: .warning))
189+
}
190+
}

0 commit comments

Comments
 (0)