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 1b725a94..074d2a4b 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 3832d687..e7d77342 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 074d2a4b..48fccc5e 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)