From 4bdbb28d538794b8212b811ae91ef101a5f4bede Mon Sep 17 00:00:00 2001 From: Tony Allevato Date: Mon, 20 Nov 2023 08:53:46 -0500 Subject: [PATCH] [UseShorthandTypeNames] Fix shorthand for optional attributed types. 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. --- .../Rules/UseShorthandTypeNames.swift | 21 +++++----- .../Rules/UseShorthandTypeNamesTests.swift | 40 +++++++++++++++++++ 2 files changed, 52 insertions(+), 9 deletions(-) diff --git a/Sources/SwiftFormat/Rules/UseShorthandTypeNames.swift b/Sources/SwiftFormat/Rules/UseShorthandTypeNames.swift index a66aafb1..c93cb76c 100644 --- a/Sources/SwiftFormat/Rules/UseShorthandTypeNames.swift +++ b/Sources/SwiftFormat/Rules/UseShorthandTypeNames.swift @@ -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): @@ -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 @@ -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 } diff --git a/Tests/SwiftFormatTests/Rules/UseShorthandTypeNamesTests.swift b/Tests/SwiftFormatTests/Rules/UseShorthandTypeNamesTests.swift index a54b98a9..ce1f3d79 100644 --- a/Tests/SwiftFormatTests/Rules/UseShorthandTypeNamesTests.swift +++ b/Tests/SwiftFormatTests/Rules/UseShorthandTypeNamesTests.swift @@ -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 = S() + var y: 2️⃣Optional<@Sendable (Int) -> Void> = S() + var z = [3️⃣Optional]([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"), + ] + ) + } }