Keep the shebang on its own line in OrderedImports#1207
Conversation
|
@allevato mind taking a look? looks like CI never got triggered on this one. |
| private func orderImports(in codeBlockItemList: CodeBlockItemListSyntax) -> CodeBlockItemListSyntax { | ||
| let lines = generateLines(codeBlockItemList: codeBlockItemList, context: context) | ||
|
|
||
| // If the file contains no imports the rule has nothing to reorder; return |
There was a problem hiding this comment.
I don't know if checking for no imports is what we want here, because I've also seen this recently on a file that does have an import. It's basically like this:
#!/usr/bin/swift
//
// some file comment
import Foundation
someCode()In that case, the comment line also gets brought up onto the shebang line.
Can you also add a test for that case and verify that it works with your change (or fix it if it doesn't)?
Also, can you make sure this is rebased on top of current main? For some reason I'm not seeing the "Run workflows" button on GitHub, but I'm not sure why.
The newline that ends a shebang line is stored as leading trivia on the first statement. Reordering imports treated it as a leading blank line of the file header and dropped it, which pulled the following comment or import up onto the shebang line. The earlier fix special-cased files with no imports, but the same thing happens when the file has imports. Restore the shebang's leading newlines after reordering instead, so it works with or without imports. Fixes swiftlang#1194.
631adc5 to
1560a38
Compare
|
the no-imports check only helped when there was nothing to reorder, so it missed your case. the real problem is that reordering drops the newline that ends the shebang line, which pulls the next line up onto it. so i dropped that check and instead restore the shebang's leading newline after reordering, which works with or without imports. added that case as a test, plus one where the imports actually get sorted. rebased on main. |
Problem
OrderedImportsrewrites every source file. When the file starts with a shebang, the rule pulls the line after the shebang up onto it. For example:formats to
which is no longer a valid shebang. This happens whether or not the file has imports:
collapses the same way.
The newline that ends a shebang line is stored as leading trivia on the first statement (the shebang token has no trailing newline of its own). When
orderImports(in:)rebuilds the statements it treats that newline as a leading blank line of the file header and drops it, so the following comment or import moves up onto the shebang line.Resolves #1194.
Fix
visit(_ node: SourceFileSyntax)can seenode.shebang, so after reordering it restores the shebang's leading newlines if any were dropped: it compares the number of leading newlines on the first statement before and after reordering and puts back the difference. This works with or without imports.An earlier version of this PR short-circuited the rule for files with no imports, which only avoided the bug when there was nothing to reorder. That special case is gone.
Tests
OrderedImportsTests.testShebangWithFileHeaderAndImport— a shebang, a file header comment, and an import (the case from the review).OrderedImportsTests.testShebangWithReorderedImports— the same, but with two imports that actually get sorted, to confirm reordering still works under a shebang.GarbageTextTests.testHashBangFollowedByLineCommentandtestHashBangFollowedByBlankLineAndComment— the no-imports reproducers, run through the fullSwiftFormatterpipeline.swift testlocally: 932 tests, 1 skipped, 0 failures.This contribution was developed with the help of Claude Code. The fix, regression tests, and the full local test suite were reviewed manually before submitting.