Skip to content
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

Parenthesize some/any types when converting Optional to ?. #589

Merged
merged 1 commit into from
Aug 11, 2023
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
81 changes: 50 additions & 31 deletions Sources/SwiftFormatRules/UseShorthandTypeNames.swift
Original file line number Diff line number Diff line change
Expand Up @@ -236,20 +236,16 @@ public final class UseShorthandTypeNames: SyntaxFormatRule {
) -> TypeSyntax {
var wrappedType = wrappedType

if let functionType = wrappedType.as(FunctionTypeSyntax.self) {
// Function types must be wrapped as a tuple before using shorthand optional syntax,
// otherwise the "?" applies to the return type instead of the function type. Attach the
// leading trivia to the left-paren that we're adding in this case.
let tupleTypeElement = TupleTypeElementSyntax(
inoutKeyword: nil, firstName: nil, secondName: nil, colon: nil, type: TypeSyntax(functionType),
ellipsis: nil, trailingComma: nil)
let tupleTypeElementList = TupleTypeElementListSyntax([tupleTypeElement])
let tupleType = TupleTypeSyntax(
leftParen: TokenSyntax.leftParenToken(leadingTrivia: leadingTrivia),
elements: tupleTypeElementList,
rightParen: TokenSyntax.rightParenToken())
wrappedType = TypeSyntax(tupleType)
} else {
// 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.
switch Syntax(wrappedType).as(SyntaxEnum.self) {
case .functionType(let functionType):
wrappedType = parenthesizedType(functionType, leadingTrivia: leadingTrivia)
case .someOrAnyType(let someOrAnyType):
wrappedType = parenthesizedType(someOrAnyType, leadingTrivia: leadingTrivia)
default:
// Otherwise, the argument type can safely become an optional by simply appending a "?", but
// we need to transfer the leading trivia from the original `Optional` token over to it.
// By doing so, something like `/* comment */ Optional<Foo>` will become `/* comment */ Foo?`
Expand Down Expand Up @@ -316,38 +312,58 @@ public final class UseShorthandTypeNames: SyntaxFormatRule {
/// key type or value type does not have a valid expression representation.
private func makeOptionalTypeExpression(
wrapping wrappedType: TypeSyntax,
leadingTrivia: Trivia? = nil,
leadingTrivia: Trivia = [],
questionMark: TokenSyntax
) -> OptionalChainingExprSyntax? {
guard var wrappedTypeExpr = expressionRepresentation(of: wrappedType) else { return nil }

if wrappedType.is(FunctionTypeSyntax.self) {
// Function types must be wrapped as a tuple before using shorthand optional syntax,
// otherwise the "?" applies to the return type instead of the function type. Attach the
// leading trivia to the left-paren that we're adding in this case.
let tupleExprElement =
LabeledExprSyntax(
label: nil, colon: nil, expression: wrappedTypeExpr, trailingComma: nil)
let tupleExprElementList = LabeledExprListSyntax([tupleExprElement])
let tupleExpr = TupleExprSyntax(
leftParen: TokenSyntax.leftParenToken(leadingTrivia: leadingTrivia ?? []),
elements: tupleExprElementList,
rightParen: TokenSyntax.rightParenToken())
wrappedTypeExpr = ExprSyntax(tupleExpr)
} else {
// 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.
switch Syntax(wrappedType).as(SyntaxEnum.self) {
case .functionType, .someOrAnyType:
wrappedTypeExpr = parenthesizedExpr(wrappedTypeExpr, leadingTrivia: leadingTrivia)
default:
// Otherwise, the argument type can safely become an optional by simply appending a "?". If
// we were given leading trivia from another node (for example, from `Optional` when
// converting a long-form to short-form), we need to transfer it over. By doing so, something
// like `/* comment */ Optional<Foo>` will become `/* comment */ Foo?` instead of discarding
// the comment.
wrappedTypeExpr.leadingTrivia = leadingTrivia ?? []
wrappedTypeExpr.leadingTrivia = leadingTrivia
}

return OptionalChainingExprSyntax(
expression: wrappedTypeExpr,
questionMark: questionMark)
}

/// Returns the given type wrapped in parentheses.
private func parenthesizedType<TypeNode: TypeSyntaxProtocol>(
_ typeToWrap: TypeNode,
leadingTrivia: Trivia
) -> TypeSyntax {
let tupleTypeElement = TupleTypeElementSyntax(type: TypeSyntax(typeToWrap))
let tupleType = TupleTypeSyntax(
leftParen: .leftParenToken(leadingTrivia: leadingTrivia),
elements: TupleTypeElementListSyntax([tupleTypeElement]),
rightParen: .rightParenToken())
return TypeSyntax(tupleType)
}

/// Returns the given expression wrapped in parentheses.
private func parenthesizedExpr<ExprNode: ExprSyntaxProtocol>(
_ exprToWrap: ExprNode,
leadingTrivia: Trivia
) -> ExprSyntax {
let tupleExprElement = LabeledExprSyntax(expression: exprToWrap)
let tupleExpr = TupleExprSyntax(
leftParen: .leftParenToken(leadingTrivia: leadingTrivia),
elements: LabeledExprListSyntax([tupleExprElement]),
rightParen: .rightParenToken())
return ExprSyntax(tupleExpr)
}

/// Returns an `ExprSyntax` that is syntactically equivalent to the given `TypeSyntax`, or nil if
/// it wouldn't be valid.
///
Expand Down Expand Up @@ -404,7 +420,7 @@ public final class UseShorthandTypeNames: SyntaxFormatRule {
case .optionalType(let optionalType):
let result = makeOptionalTypeExpression(
wrapping: optionalType.wrappedType,
leadingTrivia: optionalType.firstToken(viewMode: .sourceAccurate)?.leadingTrivia,
leadingTrivia: optionalType.firstToken(viewMode: .sourceAccurate)?.leadingTrivia ?? [],
questionMark: optionalType.questionMark)
return ExprSyntax(result)

Expand All @@ -427,6 +443,9 @@ public final class UseShorthandTypeNames: SyntaxFormatRule {
rightParen: tupleType.rightParen)
return ExprSyntax(result)

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

default:
return nil
}
Expand Down
35 changes: 35 additions & 0 deletions Tests/SwiftFormatRulesTests/UseShorthandTypeNamesTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -413,4 +413,39 @@ final class UseShorthandTypeNamesTests: LintOrFormatRuleTestCase {
var e: [String: Int?]
""")
}

func testSomeAnyTypesInOptionalsAreParenthesized() {
// If we need to insert parentheses, verify that we do, but also verify that we don't insert
// them unnecessarily.
XCTAssertFormatting(
UseShorthandTypeNames.self,
input:
"""
func f(_: Optional<some P>) {}
func g(_: Optional<any P>) {}
var x: Optional<some P> = S()
var y: Optional<any P> = S()
var z = [Optional<any P>]([S()])

func f(_: Optional<(some P)>) {}
func g(_: Optional<(any P)>) {}
var x: Optional<(some P)> = S()
var y: Optional<(any P)> = S()
var z = [Optional<(any P)>]([S()])
""",
expected:
"""
func f(_: (some P)?) {}
func g(_: (any P)?) {}
var x: (some P)? = S()
var y: (any P)? = S()
var z = [(any P)?]([S()])

func f(_: (some P)?) {}
func g(_: (any P)?) {}
var x: (some P)? = S()
var y: (any P)? = S()
var z = [(any P)?]([S()])
""")
}
}