diff --git a/Sources/SwiftFormat/Rules/OrderedImports.swift b/Sources/SwiftFormat/Rules/OrderedImports.swift index 1a1463197..292bbd472 100644 --- a/Sources/SwiftFormat/Rules/OrderedImports.swift +++ b/Sources/SwiftFormat/Rules/OrderedImports.swift @@ -315,17 +315,17 @@ fileprivate func generateLines(codeBlockItemList: CodeBlockItemListSyntax, conte blockWithoutTrailingTrivia.trailingTrivia = [] currentLine.syntaxNode = .importCodeBlock(blockWithoutTrailingTrivia, sortable: sortable) } else { - guard let syntaxNode = currentLine.syntaxNode else { - currentLine.syntaxNode = .nonImportCodeBlocks([block]) - continue - } - // Multiple code blocks can be merged, as long as there isn't an import statement. - switch syntaxNode { - case .importCodeBlock: - appendNewLine() + if let syntaxNode = currentLine.syntaxNode { + // Multiple code blocks can be merged, as long as there isn't an import statement. + switch syntaxNode { + case .importCodeBlock: + appendNewLine() + currentLine.syntaxNode = .nonImportCodeBlocks([block]) + case .nonImportCodeBlocks(let existingCodeBlocks): + currentLine.syntaxNode = .nonImportCodeBlocks(existingCodeBlocks + [block]) + } + } else { currentLine.syntaxNode = .nonImportCodeBlocks([block]) - case .nonImportCodeBlocks(let existingCodeBlocks): - currentLine.syntaxNode = .nonImportCodeBlocks(existingCodeBlocks + [block]) } } diff --git a/Tests/SwiftFormatTests/Rules/LintOrFormatRuleTestCase.swift b/Tests/SwiftFormatTests/Rules/LintOrFormatRuleTestCase.swift index 16ddda951..e989bd804 100644 --- a/Tests/SwiftFormatTests/Rules/LintOrFormatRuleTestCase.swift +++ b/Tests/SwiftFormatTests/Rules/LintOrFormatRuleTestCase.swift @@ -112,7 +112,7 @@ class LintOrFormatRuleTestCase: DiagnosingTestCase { let formatter = formatType.init(context: context) let actual = formatter.visit(sourceFileSyntax) - assertStringsEqualWithDiff(actual.description, expected, file: file, line: line) + assertStringsEqualWithDiff("\(actual)", expected, file: file, line: line) assertFindings( expected: findings, @@ -123,15 +123,22 @@ class LintOrFormatRuleTestCase: DiagnosingTestCase { line: line) // Verify that the pretty printer can consume the transformed tree (e.g., it does not contain - // any unfolded `SequenceExpr`s). We don't need to check the actual output here (we don't want - // the rule tests to be pretty-printer dependent), but this will catch invariants that aren't - // satisfied. - _ = PrettyPrinter( + // any unfolded `SequenceExpr`s). Then do a whitespace-insensitive comparison of the two trees + // to verify that the format rule didn't transform the tree in such a way that it caused the + // pretty-printer to drop important information (the most likely case is a format rule + // misplacing trivia in a way that the pretty-printer isn't able to handle). + let prettyPrintedSource = PrettyPrinter( context: context, node: Syntax(actual), printTokenStream: false, whitespaceOnly: false ).prettyPrint() + let prettyPrintedTree = Parser.parse(source: prettyPrintedSource) + XCTAssertEqual( + whitespaceInsensitiveText(of: actual), + whitespaceInsensitiveText(of: prettyPrintedTree), + "After pretty-printing and removing fluid whitespace, the files did not match", + file: file, line: line) var emittedPipelineFindings = [Finding]() // Disable default rules, so only select rule runs in pipeline @@ -149,3 +156,34 @@ class LintOrFormatRuleTestCase: DiagnosingTestCase { emittedFindings: emittedPipelineFindings, context: context, file: file, line: line) } } + +/// Returns a string containing a whitespace-insensitive representation of the given source file. +private func whitespaceInsensitiveText(of file: SourceFileSyntax) -> String { + var result = "" + for token in file.tokens(viewMode: .sourceAccurate) { + appendNonspaceTrivia(token.leadingTrivia, to: &result) + result.append(token.text) + appendNonspaceTrivia(token.trailingTrivia, to: &result) + } + return result +} + +/// Appends any non-whitespace trivia pieces from the given trivia collection to the output string. +private func appendNonspaceTrivia(_ trivia: Trivia, to string: inout String) { + for piece in trivia { + switch piece { + case .carriageReturnLineFeeds, .carriageReturns, .formfeeds, .newlines, .spaces, .tabs: + break + case .lineComment(let comment), .docLineComment(let comment): + // A tree transforming rule might leave whitespace at the end of a line comment, which the + // pretty printer will remove, so we should ignore that. + if let lastNonWhitespaceIndex = comment.lastIndex(where: { !$0.isWhitespace }) { + string.append(contentsOf: comment[...lastNonWhitespaceIndex]) + } else { + string.append(comment) + } + default: + piece.write(to: &string) + } + } +} diff --git a/Tests/SwiftFormatTests/Rules/OrderedImportsTests.swift b/Tests/SwiftFormatTests/Rules/OrderedImportsTests.swift index bae1b3e77..a61a07539 100644 --- a/Tests/SwiftFormatTests/Rules/OrderedImportsTests.swift +++ b/Tests/SwiftFormatTests/Rules/OrderedImportsTests.swift @@ -618,4 +618,38 @@ final class OrderedImportsTests: LintOrFormatRuleTestCase { ] ) } + + func testTrailingCommentsOnTopLevelCodeItems() { + assertFormatting( + OrderedImports.self, + input: """ + import Zebras + 1️⃣import Apples + #if canImport(Darwin) + import Darwin + #elseif canImport(Glibc) + import Glibc + #endif // canImport(Darwin) + + foo() // calls the foo + bar() // calls the bar + """, + expected: """ + import Apples + import Zebras + + #if canImport(Darwin) + import Darwin + #elseif canImport(Glibc) + import Glibc + #endif // canImport(Darwin) + + foo() // calls the foo + bar() // calls the bar + """, + findings: [ + FindingSpec("1️⃣", message: "sort import statements lexicographically"), + ] + ) + } }