swiftlang / swift-format

Formatting technology for Swift source code
Apache License 2.0
2.53k stars 229 forks source link

At least EndOfLineComment gets line numbers wrong if the lint run wants to change other things. #813

Open thomasvl opened 1 month ago

thomasvl commented 1 month ago

With a dummy test file containing exactly this:

let a = 1

var f = l.a  // This is a really long comment that goes on and on and on and on and on and on and on and on and on and on and on
swift format lint test.swift                 

Produces:

test.swift:3:14: warning: [EndOfLineComment] move end-of-line comment that exceeds the line length
test.swift:1:10: warning: [RemoveLine] remove line break
test.swift:2:1: warning: [RemoveLine] remove line break
test.swift:3:1: warning: [RemoveLine] remove line break

But that first lint warning is a wrong, line 3 is empty, it is only line 3 if you format the file first to remove the extra line breaks it is telling you to remove. The error should be reported for the original line, not a reformatted line number.

thomasvl commented 1 month ago

ps - I can also get this sorta problem in more complex cases where there doesn't appear to be things that need reformatting (i.e. - nothing else gets reporting and a format run on the file makes no changes). Makes me wonder if lint is formatting with something other than the passed in config to get the wrong lines (i.e. - it would format the file different if I wasn't providing an explicit config).

ahoppen commented 1 month ago

Synced to Apple’s issue tracker as rdar://136555256

thomasvl commented 1 month ago

I haven't been able to make a reduced test case of the second thing I mentioned, but you can see it on https://github.com/apple/swift-protobuf, check out at 551675bc196b14ffa6a882e750fd7d49a89747f7, and run:

git ls-files -z '*.swift' | xargs -0 xcrun swift-format lint --parallel

Towards the end you'll see:

.../swift-protobuf/Tests/SwiftProtobufTests/Test_AllTypes.swift:1178:73: warning: [EndOfLineComment] move end-of-line comment that exceeds the line length
.../swift-protobuf/Tests/SwiftProtobufTests/Test_AllTypes.swift:1181:57: warning: [EndOfLineComment] move end-of-line comment that exceeds the line length
.../swift-protobuf/Tests/SwiftProtobufTests/Test_AllTypes.swift:2570:75: warning: [EndOfLineComment] move end-of-line comment that exceeds the line length
.../swift-protobuf/Tests/SwiftProtobufTests/Test_AllTypes.swift:2760:84: warning: [EndOfLineComment] move end-of-line comment that exceeds the line length
.../swift-protobuf/Tests/SwiftProtobufTests/Test_AllTypes.swift:2832:44: warning: [EndOfLineComment] move end-of-line comment that exceeds the line length

None of those lines numbers are correct, but doing a format instead makes no changes, leading to my guess that it is reformatting for the lint reporting, but not fully honoring the .swift-format file that is checked in.