Skip to content

Commit

Permalink
Merge pull request swiftlang#554 from allevato/poundif-after-paren
Browse files Browse the repository at this point in the history 
Fix postfix-`#if` formatting when they come after a closing parenthesis.
  • Loading branch information
@allevato
allevato committed Jun 29, 2023
1 parent 6a8d51a commit 23068d4
Show file tree
Hide file tree
Showing 2 changed files with 116 additions and 58 deletions.
72 changes: 34 additions & 38 deletions Sources/SwiftFormatPrettyPrint/TokenStreamCreator.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -988,14 +988,22 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {

override func visit(_ node: MemberAccessExprSyntax) -> SyntaxVisitorContinueKind {
preVisitInsertingContextualBreaks(node)

return .visitChildren
}

override func visitPost(_ node: MemberAccessExprSyntax) {
clearContextualBreakState(node)
}

override func visit(_ node: PostfixIfConfigExprSyntax) -> SyntaxVisitorContinueKind {
preVisitInsertingContextualBreaks(node)
return .visitChildren
}

override func visitPost(_ node: PostfixIfConfigExprSyntax) {
clearContextualBreakState(node)
}

override func visit(_ node: FunctionCallExprSyntax) -> SyntaxVisitorContinueKind {
preVisitInsertingContextualBreaks(node)

Expand Down Expand Up @@ -1456,29 +1464,10 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
before(tokenToOpenWith.nextToken(viewMode: .all), tokens: .break(breakKindClose, newlines: .soft), .close)
}

if isNestedInPostfixIfConfig(node: Syntax(node)) {
let breakToken: Token
let currentIfConfigDecl = node.parent?.parent?.as(IfConfigDeclSyntax.self)

if let currentIfConfigDecl = currentIfConfigDecl,
let tokenBeforeCurrentIfConfigDecl = currentIfConfigDecl.previousToken(viewMode: .sourceAccurate),
isNestedInIfConfig(node: Syntax(tokenBeforeCurrentIfConfigDecl)) ||
tokenBeforeCurrentIfConfigDecl.text == "}" {
breakToken = .break(.reset)
} else {
breakToken = .break
before(currentIfConfigDecl?.poundEndif, tokens: [.break])
}

if !isNestedInPostfixIfConfig(node: Syntax(node)), let condition = node.condition {
before(
node.firstToken(viewMode: .sourceAccurate),
tokens: [
.printerControl(kind: .enableBreaking),
breakToken,
]
)
} else if let condition = node.condition {
before(condition.firstToken(viewMode: .sourceAccurate), tokens: .printerControl(kind: .disableBreaking(allowDiscretionary: true)))
condition.firstToken(viewMode: .sourceAccurate),
tokens: .printerControl(kind: .disableBreaking(allowDiscretionary: true)))
after(
condition.lastToken(viewMode: .sourceAccurate),
tokens: .printerControl(kind: .enableBreaking), .break(.reset, size: 0))
Expand Down Expand Up @@ -3287,7 +3276,11 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
private func mustBreakBeforeClosingDelimiter<T: ExprSyntaxProtocol>(
of expr: T, argumentListPath: KeyPath<T, TupleExprElementListSyntax>
) -> Bool {
guard let parent = expr.parent, parent.is(MemberAccessExprSyntax.self) else { return false }
guard
let parent = expr.parent,
parent.is(MemberAccessExprSyntax.self) || parent.is(PostfixIfConfigExprSyntax.self)
else { return false }

let argumentList = expr[keyPath: argumentListPath]

// When there's a single compact argument, there is no extra indentation for the argument and
Expand Down Expand Up @@ -3722,6 +3715,23 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor {
after(expr.lastToken(viewMode: .sourceAccurate), tokens: .contextualBreakingEnd)
}
return (hasCompoundExpression, true)
} else if let postfixIfExpr = expr.as(PostfixIfConfigExprSyntax.self),
let base = postfixIfExpr.base
{
// For postfix-if expressions with bases (i.e., they aren't the first `#if` nested inside
// another `#if`), add contextual breaks before the top-level clauses (and the terminating
// `#endif`) so that they nest or line-up properly based on the preceding node. We don't do
// this for initial nested `#if`s because they will already get open/close breaks to control
// their indentation from their parent clause.
before(postfixIfExpr.firstToken(viewMode: .sourceAccurate), tokens: .contextualBreakingStart)
after(postfixIfExpr.lastToken(viewMode: .sourceAccurate), tokens: .contextualBreakingEnd)

for clause in postfixIfExpr.config.clauses {
before(clause.poundKeyword, tokens: .break(.contextual, size: 0))
}
before(postfixIfExpr.config.poundEndif, tokens: .break(.contextual, size: 0))

return insertContextualBreaks(base, isTopLevel: false)
} else if let callingExpr = expr.asProtocol(CallingExprSyntaxProtocol.self) {
let calledExpression = callingExpr.calledExpression
let (hasCompoundExpression, hasMemberAccess) =
Expand Down Expand Up @@ -3788,20 +3798,6 @@ private func isNestedInPostfixIfConfig(node: Syntax) -> Bool {
return false
}

private func isNestedInIfConfig(node: Syntax) -> Bool {
var this: Syntax? = node

while this?.parent != nil {
if this?.is(IfConfigClauseSyntax.self) == true {
return true
}

this = this?.parent
}

return false
}

extension Syntax {
/// Creates a pretty-printable token stream for the provided Syntax node.
func makeTokenStream(configuration: Configuration, operatorTable: OperatorTable) -> [Token] {
Expand Down
102 changes: 82 additions & 20 deletions Tests/SwiftFormatPrettyPrintTests/IfConfigTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,25 +163,25 @@ final class IfConfigTests: PrettyPrintTestCase {

func testInvalidDiscretionaryLineBreaksRemoved() {
let input =
"""
#if (canImport(SwiftUI) &&
!(os(iOS) &&
arch(arm)) &&
((canImport(AppKit) ||
canImport(UIKit)) && !os(watchOS)))
conditionalFunc(foo, bar, baz)
#endif
"""

let expected =
"""
#if (canImport(SwiftUI) && !(os(iOS) && arch(arm)) && ((canImport(AppKit) || canImport(UIKit)) && !os(watchOS)))
conditionalFunc(foo, bar, baz)
#endif
"""

assertPrettyPrintEqual(input: input, expected: expected, linelength: 40)
"""
#if (canImport(SwiftUI) &&
!(os(iOS) &&
arch(arm)) &&
((canImport(AppKit) ||
canImport(UIKit)) && !os(watchOS)))
conditionalFunc(foo, bar, baz)
#endif
"""

let expected =
"""
#if (canImport(SwiftUI) && !(os(iOS) && arch(arm)) && ((canImport(AppKit) || canImport(UIKit)) && !os(watchOS)))
conditionalFunc(foo, bar, baz)
#endif
"""

assertPrettyPrintEqual(input: input, expected: expected, linelength: 40)
}

func testValidDiscretionaryLineBreaksRetained() {
Expand Down Expand Up @@ -299,6 +299,8 @@ final class IfConfigTests: PrettyPrintTestCase {
#if os(iOS) || os(watchOS)
#if os(iOS)
.iOSModifier()
#elseif os(tvOS)
.tvOSModifier()
#else
.watchOSModifier()
#endif
Expand All @@ -314,6 +316,8 @@ final class IfConfigTests: PrettyPrintTestCase {
#if os(iOS) || os(watchOS)
#if os(iOS)
.iOSModifier()
#elseif os(tvOS)
.tvOSModifier()
#else
.watchOSModifier()
#endif
Expand All @@ -326,7 +330,6 @@ final class IfConfigTests: PrettyPrintTestCase {
assertPrettyPrintEqual(input: input, expected: expected, linelength: 45)
}


func testPostfixPoundIfAfterVariables() {
let input =
"""
Expand Down Expand Up @@ -398,6 +401,7 @@ final class IfConfigTests: PrettyPrintTestCase {
.padding([.vertical])
#if os(iOS)
.iOSSpecificModifier()
.anotherIOSSpecificModifier()
#endif
.commonModifier()
"""
Expand All @@ -408,6 +412,7 @@ final class IfConfigTests: PrettyPrintTestCase {
.padding([.vertical])
#if os(iOS)
.iOSSpecificModifier()
.anotherIOSSpecificModifier()
#endif
.commonModifier()
Expand Down Expand Up @@ -454,4 +459,61 @@ final class IfConfigTests: PrettyPrintTestCase {

assertPrettyPrintEqual(input: input, expected: expected, linelength: 45)
}

func testPostfixPoundIfNotIndentedIfClosingParenOnOwnLine() {
let input =
"""
SomeFunction(
foo,
bar
)
#if os(iOS)
.iOSSpecificModifier()
#endif
.commonModifier()
"""

let expected =
"""
SomeFunction(
foo,
bar
)
#if os(iOS)
.iOSSpecificModifier()
#endif
.commonModifier()
"""

assertPrettyPrintEqual(input: input, expected: expected, linelength: 45)
}

func testPostfixPoundIfForcesPrecedingClosingParenOntoNewLine() {
let input =
"""
SomeFunction(
foo,
bar)
#if os(iOS)
.iOSSpecificModifier()
#endif
.commonModifier()
"""

let expected =
"""
SomeFunction(
foo,
bar
)
#if os(iOS)
.iOSSpecificModifier()
#endif
.commonModifier()
"""

assertPrettyPrintEqual(input: input, expected: expected, linelength: 45)
}
}

0 comments on commit 23068d4

Please sign in to comment.