Skip to content

Commit

Permalink
[UseShorthandTypeNames] Fix shorthand for optional attributed types.
Browse files Browse the repository at this point in the history
Like with `some/any` type sugar, when we simplify an `Optional` whose
type parameter is an attributed type, we need to wrap it in parentheses
so that the `?` doesn't bind with the contained type.

Fixes #657.
  • Loading branch information
allevato authored and ahoppen committed Mar 16, 2024
1 parent 949b0d0 commit 4bdbb28
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 9 deletions.
21 changes: 12 additions & 9 deletions Sources/SwiftFormat/Rules/UseShorthandTypeNames.swift
Original file line number Diff line number Diff line change
Expand Up @@ -238,11 +238,12 @@ public final class UseShorthandTypeNames: SyntaxFormatRule {
) -> TypeSyntax {
var wrappedType = wrappedType

// Function types and some-or-any types must be wrapped in parentheses before using shorthand
// optional syntax, otherwise the "?" will bind incorrectly (in the function case it binds to
// only the result, and in the some-or-any case it only binds to the child protocol). Attach the
// leading trivia to the left-paren that we're adding in these cases.
// Certain types must be wrapped in parentheses before using shorthand optional syntax to avoid
// the "?" from binding incorrectly when re-parsed. Attach the leading trivia to the left-paren
// that we're adding in these cases.
switch Syntax(wrappedType).as(SyntaxEnum.self) {
case .attributedType(let attributedType):
wrappedType = parenthesizedType(attributedType, leadingTrivia: leadingTrivia)
case .functionType(let functionType):
wrappedType = parenthesizedType(functionType, leadingTrivia: leadingTrivia)
case .someOrAnyType(let someOrAnyType):
Expand Down Expand Up @@ -319,12 +320,11 @@ public final class UseShorthandTypeNames: SyntaxFormatRule {
) -> OptionalChainingExprSyntax? {
guard var wrappedTypeExpr = expressionRepresentation(of: wrappedType) else { return nil }

// Function types and some-or-any types must be wrapped in parentheses before using shorthand
// optional syntax, otherwise the "?" will bind incorrectly (in the function case it binds to
// only the result, and in the some-or-any case it only binds to the child protocol). Attach the
// leading trivia to the left-paren that we're adding in these cases.
// Certain types must be wrapped in parentheses before using shorthand optional syntax to avoid
// the "?" from binding incorrectly when re-parsed. Attach the leading trivia to the left-paren
// that we're adding in these cases.
switch Syntax(wrappedType).as(SyntaxEnum.self) {
case .functionType, .someOrAnyType:
case .attributedType, .functionType, .someOrAnyType:
wrappedTypeExpr = parenthesizedExpr(wrappedTypeExpr, leadingTrivia: leadingTrivia)
default:
// Otherwise, the argument type can safely become an optional by simply appending a "?". If
Expand Down Expand Up @@ -448,6 +448,9 @@ public final class UseShorthandTypeNames: SyntaxFormatRule {
case .someOrAnyType(let someOrAnyType):
return ExprSyntax(TypeExprSyntax(type: someOrAnyType))

case .attributedType(let attributedType):
return ExprSyntax(TypeExprSyntax(type: attributedType))

default:
return nil
}
Expand Down
40 changes: 40 additions & 0 deletions Tests/SwiftFormatTests/Rules/UseShorthandTypeNamesTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -692,4 +692,44 @@ final class UseShorthandTypeNamesTests: LintOrFormatRuleTestCase {
]
)
}

func testAttributedTypesInOptionalsAreParenthesized() {
// If we need to insert parentheses, verify that we do, but also verify that we don't insert
// them unnecessarily.
assertFormatting(
UseShorthandTypeNames.self,
input: """
var x: 1️⃣Optional<consuming P> = S()
var y: 2️⃣Optional<@Sendable (Int) -> Void> = S()
var z = [3️⃣Optional<consuming P>]([S()])
var a = [4️⃣Optional<@Sendable (Int) -> Void>]([S()])
var x: 5️⃣Optional<(consuming P)> = S()
var y: 6️⃣Optional<(@Sendable (Int) -> Void)> = S()
var z = [7️⃣Optional<(consuming P)>]([S()])
var a = [8️⃣Optional<(@Sendable (Int) -> Void)>]([S()])
""",
expected: """
var x: (consuming P)? = S()
var y: (@Sendable (Int) -> Void)? = S()
var z = [(consuming P)?]([S()])
var a = [(@Sendable (Int) -> Void)?]([S()])
var x: (consuming P)? = S()
var y: (@Sendable (Int) -> Void)? = S()
var z = [(consuming P)?]([S()])
var a = [(@Sendable (Int) -> Void)?]([S()])
""",
findings: [
FindingSpec("1️⃣", message: "use shorthand syntax for this 'Optional' type"),
FindingSpec("2️⃣", message: "use shorthand syntax for this 'Optional' type"),
FindingSpec("3️⃣", message: "use shorthand syntax for this 'Optional' type"),
FindingSpec("4️⃣", message: "use shorthand syntax for this 'Optional' type"),
FindingSpec("5️⃣", message: "use shorthand syntax for this 'Optional' type"),
FindingSpec("6️⃣", message: "use shorthand syntax for this 'Optional' type"),
FindingSpec("7️⃣", message: "use shorthand syntax for this 'Optional' type"),
FindingSpec("8️⃣", message: "use shorthand syntax for this 'Optional' type"),
]
)
}
}

0 comments on commit 4bdbb28

Please sign in to comment.