From fca02c2d0e5cdf6ca12f0deee7255f01b9442c5d Mon Sep 17 00:00:00 2001 From: Shawn Hyam Date: Mon, 21 Oct 2024 10:41:02 -0400 Subject: [PATCH 1/2] Add failing test case for IfConfigDecl/ImportDecl interaction. --- .../PrettyPrint/IfConfigTests.swift | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/Tests/SwiftFormatTests/PrettyPrint/IfConfigTests.swift b/Tests/SwiftFormatTests/PrettyPrint/IfConfigTests.swift index 1b725a94b..074d2a4b7 100644 --- a/Tests/SwiftFormatTests/PrettyPrint/IfConfigTests.swift +++ b/Tests/SwiftFormatTests/PrettyPrint/IfConfigTests.swift @@ -557,4 +557,19 @@ final class IfConfigTests: PrettyPrintTestCase { configuration.indentConditionalCompilationBlocks = false assertPrettyPrintEqual(input: input, expected: input, linelength: 45, configuration: configuration) } + + func testIfConfigDeclPartOfImport() { + let input = + """ + #if os(Foo) + @_spiOnly + #endif + @_spi(Foo) import Foundation + + """ + var configuration = Configuration.forTesting + configuration.indentConditionalCompilationBlocks = false + assertPrettyPrintEqual(input: input, expected: input, linelength: 80, configuration: configuration) + + } } From 4f4be5d8702aa75111d22adc3843d963e5daa67d Mon Sep 17 00:00:00 2001 From: Shawn Hyam Date: Tue, 22 Oct 2024 11:24:51 -0400 Subject: [PATCH 2/2] Only disable wrapping in import decls when it is safe to do so. The ImportDeclSyntax attributes might include IfConfigDecls, which absolutely require line breaks in certain positions. The disable breaking logic of ImportDecl was preventing these from being emitted. --- .../PrettyPrint/TokenStreamCreator.swift | 25 +++++++++++++------ .../PrettyPrint/IfConfigTests.swift | 19 ++++++++------ 2 files changed, 30 insertions(+), 14 deletions(-) diff --git a/Sources/SwiftFormat/PrettyPrint/TokenStreamCreator.swift b/Sources/SwiftFormat/PrettyPrint/TokenStreamCreator.swift index 3832d6874..e7d773427 100644 --- a/Sources/SwiftFormat/PrettyPrint/TokenStreamCreator.swift +++ b/Sources/SwiftFormat/PrettyPrint/TokenStreamCreator.swift @@ -1492,7 +1492,7 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor { override func visit(_ node: IfConfigDeclSyntax) -> SyntaxVisitorContinueKind { // there has to be a break after an #endif - after(node.poundEndif, tokens: .break(.same, size: 0)) + after(node.poundEndif, tokens: .break(.same, size: 0, newlines: .soft)) return .visitChildren } @@ -1951,17 +1951,28 @@ fileprivate final class TokenStreamCreator: SyntaxVisitor { } override func visit(_ node: ImportDeclSyntax) -> SyntaxVisitorContinueKind { - // Import declarations should never be wrapped. - before( - node.firstToken(viewMode: .sourceAccurate), - tokens: .printerControl(kind: .disableBreaking(allowDiscretionary: false)) - ) + // Import declarations ignore wrapping when it is safe to do so (no #if constructs) + let isSafeToIgnoreBreaking = !node.attributes.contains(where: { element in + return element.is(IfConfigDeclSyntax.self) + }) + + if isSafeToIgnoreBreaking { + before( + node.firstToken(viewMode: .sourceAccurate), + tokens: .printerControl(kind: .disableBreaking(allowDiscretionary: false)) + ) + } arrangeAttributeList(node.attributes) after(node.importKeyword, tokens: .space) after(node.importKindSpecifier, tokens: .space) - after(node.lastToken(viewMode: .sourceAccurate), tokens: .printerControl(kind: .enableBreaking)) + if isSafeToIgnoreBreaking { + after( + node.lastToken(viewMode: .sourceAccurate), + tokens: .printerControl(kind: .enableBreaking) + ) + } return .visitChildren } diff --git a/Tests/SwiftFormatTests/PrettyPrint/IfConfigTests.swift b/Tests/SwiftFormatTests/PrettyPrint/IfConfigTests.swift index 074d2a4b7..48fccc5e0 100644 --- a/Tests/SwiftFormatTests/PrettyPrint/IfConfigTests.swift +++ b/Tests/SwiftFormatTests/PrettyPrint/IfConfigTests.swift @@ -383,10 +383,10 @@ final class IfConfigTests: PrettyPrintTestCase { .toggleStyle( SwitchToggleStyle(tint: Color.blue)) #endif - .accessibilityValue( - binding.wrappedValue == true - ? "On" : "Off" - ) + .accessibilityValue( + binding.wrappedValue == true + ? "On" : "Off" + ) } """ @@ -482,7 +482,7 @@ final class IfConfigTests: PrettyPrintTestCase { #if os(iOS) .iOSSpecificModifier() #endif - .commonModifier() + .commonModifier() """ @@ -510,7 +510,7 @@ final class IfConfigTests: PrettyPrintTestCase { #if os(iOS) .iOSSpecificModifier() #endif - .commonModifier() + .commonModifier() """ @@ -563,11 +563,16 @@ final class IfConfigTests: PrettyPrintTestCase { """ #if os(Foo) @_spiOnly + #elseif os(Bar) + @_spiOnly + #else + @_spiOnly #endif @_spi(Foo) import Foundation - + """ var configuration = Configuration.forTesting + configuration.respectsExistingLineBreaks = false configuration.indentConditionalCompilationBlocks = false assertPrettyPrintEqual(input: input, expected: input, linelength: 80, configuration: configuration)