nicklockwood / SwiftFormat

A command-line tool and Xcode Extension for formatting Swift code
MIT License
7.77k stars 629 forks source link

Can some or all of the rules be implemented using SwiftSyntax? #1519

Open gwynne opened 11 months ago

gwynne commented 11 months ago

In the present day, swiftformat is implemented using a hand-rolled Swift parser; this results in it having considerably more capabilities and flexibility than swift-format, enough so to still make it the formatter/linter of choice much of the time. Unfortunately, it also makes it extremely error-prone by comparison, making it a problematic choice for large projects.

This inverted usefulness/stability metric came to a head recently when we were looking into adding code formatting CI to Vapor; we had to reject swift-format for not being configurable enough to yield even a semblance of acceptable results, but we also had to reject swiftformat for outputting corrupted code - in the end we gave up completely for the time being.

IMO this is a torrid state of affairs - But, now that swift-syntax is no longer directly tied to the compiler (thanks to macros), it seems as though it would be at least possible to implement some (if not all?) of swiftformat's rules in terms of SwiftSyntax and its "I am the parser used by the actual Swift compiler" trustworthiness. Although Apple continues development on swift-format, it seems unlikely they'll actually implement all of the same customizations possible with swiftformat, so I doubt it would be wasted time.

To sum up my thought: Is there a chance of this happening, even just as an experiment?

nicklockwood commented 11 months ago

I'm not sure to what extent it's true that SwiftFormat's hand-rolled parser is the cause of corrupted code, or that SwiftSyntax would avoid such corruption.

From what I understand, SwiftSyntax is (by design) capable of representing invalid code, both semantically and syntactically.

Even if it were not capable of representing invalid syntax, it would still be possible for it to represent syntactically valid but semantically invalid code, since like SwiftFormat, it doesn't compile the code in the context of the project and doesn't know anything about types, etc.

In my experience, most of the errors introduced by SwiftFormat are of the semantic type rather than syntactic, and syntactic errors are generally easy to fix once identified.

The reason that SwiftFormat introduces more such errors than swift-format is simply that the rules are more ambitious. I try to do things beyond merely the syntactic level, and in some cases that involves taking a calculated risk about introducing errors.

What would probably make sense for your use-case would be to make it clearer which rules carry a risk of such corruption, and/or introduce a "safe" mode that only permits modifications that are 100% safe (barring bugs, which are of course unavoidable).

If I were to create SwiftFormat from scratch today I would probably use SwiftSyntax, but the benefits of switching at this point are far from clear.

One of the advantages of my custom parser is that it can support multiple (and even unreleased) versions of Swift, whereas SwiftSyntax is generally specific to whatever version of Swift it was built for.

An alternative I have considered is to do what SwiftLint does, and use SourceKit. SourceKit actually compiles the project and understands all the symbols. This makes it possible to guarantee that changes are safe, and also allows certain modifications that SwiftFormat can't currently do at all in any meaningful way, such as eliminating unused import statements or reversing trailing closures.

This comes at a considerable cost in terms of performance, as well as requiring the formatter to operate on an entire project instead of individual files, and also shares the problem of being tied to a particular Swift version, however it's possible the benefits would outweigh the disadvantages.

In any case, let me know if the "safe rules only mode" idea would be of use to you and I'll consider implementing it. I'd also be happy to just provide you with a list of rules that I'm confident won't break anything (or rather, where any such breakages are fixable bugs rather than known limitations).

gwynne commented 11 months ago

I'll write a more complete response when I get a chance, but I wanted to reply to this quickly:

I'm not sure to what extent it's true that SwiftFormat's hand-rolled parser is the cause of corrupted code, or that SwiftSyntax would avoid such corruption.

Yeah, I withdraw that statement; I shouldn't have phrased it in such absolute terms (and I apologize if I gave offense in doing so 😕). It'd be more accurate to say that a couple of bugs which I opened PRs to fix several months ago happened to be caused by issues in the parser logic, and that my general perception of that logic has been colored by the combination of that experience and several other similar issues I've run into in the time since (all of which were later fixed; I haven't seen any thus far in the last couple of releases).

nicklockwood commented 11 months ago

No offense taken, however I think your perception of the magnitude of this issue based on the particular bugs you encountered is not representative. The majority of bugs reports I encounter in SwiftFormat are not related to the actual parser, and the parser-related bugs are amongst the most easily fixed. I can't at this stage see a justification in replacing it with SwiftSyntax.

I would certainly like to help you get to a place where you feel comfortable using SwiftFormat with Vapor, but other than promptly fixing any actual bugs you encounter, I'm not sure what more I can offer.