Skip to content

Simplify the thunk names we generate for test functions. #750

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

Merged
merged 2 commits into from
Oct 7, 2024
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -62,44 +62,18 @@ extension MacroExpansionContext {
.tokens(viewMode: .fixedUp)
.map(\.textWithoutBackticks)
.joined()
let crcValue = crc32(identifierCharacters.utf8)
let suffix = String(crcValue, radix: 16, uppercase: false)

// Strip out any characters in the function's signature that won't play well
// in a generated symbol name.
let identifier = String(
identifierCharacters.map { character in
if character.isLetter || character.isWholeNumber {
return character
}
return "_"
}
)

// If there is a non-ASCII character in the identifier, we might be
// stripping it out above because we are only looking for letters and
// digits. If so, add in a hash of the identifier to improve entropy and
// reduce the risk of a collision.
//
// For example, the following function names will produce identical unique
// names without this mutation:
//
// @Test(arguments: [0]) func A(🙃: Int) {}
// @Test(arguments: [0]) func A(🙂: Int) {}
//
// Note the check here is not the same as the one above: punctuation like
// "(" should be replaced, but should not cause a hash to be emitted since
// it does not contribute any entropy to the makeUniqueName() algorithm.
//
// The intent here is not to produce a cryptographically strong hash, but to
// disambiguate between superficially similar function names. A collision
// may still occur, but we only need it to be _unlikely_. CRC-32 is good
// enough for our purposes.
if !identifierCharacters.allSatisfy(\.isASCII) {
let crcValue = crc32(identifierCharacters.utf8)
let suffix = String(crcValue, radix: 16, uppercase: false)
return makeUniqueName("\(prefix)\(identifier)_\(suffix)")
// If the caller did not specify a prefix and the CRC32 value starts with a
// digit, include a single-character prefix to ensure that Swift's name
// demangling still works correctly.
var prefix = prefix
if prefix.isEmpty, let firstSuffixCharacter = suffix.first, firstSuffixCharacter.isWholeNumber {
prefix = "Z"
}

return makeUniqueName("\(prefix)\(identifier)")
return makeUniqueName("\(prefix)\(suffix)")
}
}

Expand Down
22 changes: 7 additions & 15 deletions Tests/TestingMacrosTests/UniqueIdentifierTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,50 +27,35 @@ struct UniqueIdentifierTests {
return BasicMacroExpansionContext().makeUniqueName(thunking: functionDecl).text
}

@Test("Thunk identifiers contain a function's name")
func thunkNameContainsFunctionName() throws {
let uniqueName = try makeUniqueName("func someDistinctFunctionName() async throws")
#expect(uniqueName.contains("someDistinctFunctionName"))
}

@Test("Thunk identifiers do not contain backticks")
func noBackticks() throws {
let uniqueName = try makeUniqueName("func `someDistinctFunctionName`() async throws")
#expect(uniqueName.contains("someDistinctFunctionName"))

#expect(!uniqueName.contains("`"))
}

@Test("Thunk identifiers do not contain arbitrary Unicode")
func noArbitraryUnicode() throws {
let uniqueName = try makeUniqueName("func someDistinctFunction🌮Name🐔() async throws")
#expect(uniqueName.contains("someDistinctFunction"))

#expect(!uniqueName.contains("🌮"))
#expect(!uniqueName.contains("🐔"))
#expect(uniqueName.contains("Name"))
}

@Test("Argument types influence generated identifiers")
func argumentTypes() throws {
let uniqueNameWithInt = try makeUniqueName("func someDistinctFunctionName(i: Int) async throws")
#expect(uniqueNameWithInt.contains("someDistinctFunctionName"))
let uniqueNameWithUInt = try makeUniqueName("func someDistinctFunctionName(i: UInt) async throws")
#expect(uniqueNameWithUInt.contains("someDistinctFunctionName"))

#expect(uniqueNameWithInt != uniqueNameWithUInt)
}

@Test("Effects influence generated identifiers")
func effects() throws {
let uniqueName = try makeUniqueName("func someDistinctFunctionName()")
#expect(uniqueName.contains("someDistinctFunctionName"))
let uniqueNameAsync = try makeUniqueName("func someDistinctFunctionName() async")
#expect(uniqueNameAsync.contains("someDistinctFunctionName"))
let uniqueNameThrows = try makeUniqueName("func someDistinctFunctionName() throws")
#expect(uniqueNameThrows.contains("someDistinctFunctionName"))
let uniqueNameAsyncThrows = try makeUniqueName("func someDistinctFunctionName() async throws")
#expect(uniqueNameAsyncThrows.contains("someDistinctFunctionName"))

#expect(uniqueName != uniqueNameAsync)
#expect(uniqueName != uniqueNameThrows)
Expand All @@ -89,4 +74,11 @@ struct UniqueIdentifierTests {
#expect(uniqueName1 != uniqueName3)
#expect(uniqueName2 != uniqueName3)
}

@Test("Body does not influence generated identifiers")
func body() throws {
let uniqueName1 = try makeUniqueName("func f() { abc() }")
let uniqueName2 = try makeUniqueName("func f() { def() }")
#expect(uniqueName1 == uniqueName2)
}
}