SE-0250: Swift Code Style Guidelines and Formatter

(Matthew Johnson) #152

I don't disagree with you and I support having an official tool eventually. But I am skeptical about the timing and method in which a specific tool is being proposed.

The point of my post is that swift-format can play a nearly identical role in evaluating style guideline proposals no matter where it is hosted. The only difference is that users wishing to try out the proposed guidelines on their code may need to do an additional tool installation. I don't think that is an unreasonable bar for those who wish to participate in the SE review of proposed style guidelines.

2 Likes
(Karim Nassar) #153

Especially since anyone wishing to try out the proposed guidelines on their code who are already using another tool (e.g: Swiftlint) will need to undergo the extra steps of disabling formatting functions of those tools. We shouldn’t minimize the disruption that the introduction of a new tool will have on code bases and projects that already have overlapping tools in place.

(Goffredo Marocchi) #154

What is the point of putting this through Swift Evolution instead of continuing to work on the open source project as it stands?

Also, one moment it is useful and a real big problem when moving from project to project (which having an official style and formatter would fix) and on the other you do not expect and call toxic when existing projects that would cause the context switching are pushed by some of their members to conform to the “official” style... we expect this to change things significantly or not?

(Goffredo Marocchi) #155

Uhm... yeah, I think you reinforced the point there :).

(Goffredo Marocchi) #156

SwiftFormat is really easy to use and has very very sensible defaults, I would actually propose that one over swift-format in terms of speed and default formatting option.

3 Likes
(Argyrios Kyrtzidis) #157

Hi Goffredo,

A formatter tool that is part of the swift.org projects should be using SwiftSyntax for its functionality. SwiftSyntax is directly integrated with the Swift parser and we are focused to keep improving in terms of performance and convenience for building Swift tools that need to understand the syntactic structure of Swift code. It has performance features like incremental re-parsing so that a live editor will do only the minimum work necessary to get an updated syntax tree after small user edits. Any change in the Swift grammar is going to be reflected in the SwiftSyntax APIs, either from the direct implementation of a SE proposal or at least soon after a proposal is accepted.

To minimize maintenance overhead it is desirable to have a clear separation of concerns for the swift.org projects. That means that the functionality of understanding Swift syntax is going to be provided by SwiftSyntax and other projects like SourceKit-LSP and a formatter tool are going to build their functionality on top of SwiftSyntax, instead of trying to duplicate it.
I'm mentioning this because SwiftFormat has developed its own lexer and parser and it would need work to rewrite significant parts of it.

I'd like to clarify that the NSHipster article compared swift-format using swift-4.2 which only provides the slowest available way to get a SwiftSyntax tree (invoke the swiftc binary and parse a large JSON file) and SwiftSyntax itself did not have good visitation performance. SwiftSyntax in master/swift-5.1 has been re-designed and has orders of magnitude better performance in almost all aspects of using it, like parsing, visitation, incremental re-parsing, etc. Some details on how we improved parsing performance are in this forum post.

Another thing I'd like to clarify about a formatter project that is part of swift.org, is that it will aim to provide the building blocks for doing formatting rule transformations. Regardless of how configurable the resulting binary itself will be, the project will be a SwiftPM project and designed in a way that other projects can depend on its libraries to either use as is or modify its formatting pipeline or even plug in their own transformations. That will allow, for example, SwiftLint (which is doing a lot more than just whitespace transformations), to use the same building blocks and just focus on adding additional rules and functionality on top of the underlying infrastructure. They can be free from worrying about the particulars of handling a Swift grammar construct unless it is relevant for some particular rule.

16 Likes
(Tino) #158

That sounds like a sugarcoated way to say "we wanted to avoid the terrible backlash from those who disagree with our preferences" ;-). Also, I don't buy the rationale:
What kind of detraction could happen when you ask the community simple questions like "how should indention look like?", accompanied with a small tool that can transform code according to the choices given in the proposal?

Imho SE-0250 tries way too hard to please everyone, and @hybridcattt's plan is more honest.
Of course, this alternative could pose a serious danger to the idea of incorporating an official (complete) style guide, because of all the unavoidable bikeshedding.

The proposal says that

This meta-proposal does not attempt to define any specific style guidelines

but at the same time, it aims to integrate an existing formatter that inherently defines a style via its defaults.
The text speaks about subsequent proposals to decide about the guidelines, but imho it would be completely nuts to adopt swift-format with its default style and amend it afterwards:
Do we really want to have an official style that changes every few weeks, until all options have been reviewed?

So in the worst interpretation, acceptance of SE-0250 will silently establish a set of rules with no ratification by the community at all - and if that is not the case, then building a new formatter and a official ruleset side by side would be much faster delivering usable results, because you could apply each rule as soon as inclusion is decided and added to the toolchain.

4 Likes
(Jeremy David Giesbrecht) #159

That is not the case. The proposal author has said this:

The proposed tool is a bare‐bones prototype—implemented enough to prove that it works. It has no rules beyond a few stand‐ins for preliminary tests. It is literally an attempt to, in your words, “[build] a new formatter and [an] official ruleset side by side”. @allevato and his team got the underlying framework ready before the pitch so that if it is accepted, the tool is ready to immediately start implementing individual rules as they are reviewed and accepted.

It may not have been explained that way clearly enough the proposal, but from talking to him and looking through the swift-format project, that is the understanding I come away with.

6 Likes
(David Hart) #160
  • What is your evaluation of the proposal?

I've always been a big proponent of style guides, formatters and linters to help developers work together. I've also been a big fan of the Swift Style Guide that proposals authors have written. I therefore wholeheartedly hope this proposal will be accepted and we can move to the second stage of getting the Style Guide under review.

  • Is the problem being addressed significant enough to warrant a change to Swift?

Yes. While discussing this topic takes a lot of energy from many people, I believe that the benefits from having an official Style Guide and formatter are worth it. While the guide will not satisfy everybody, I hope it will please enough people to cause a majority adoption and therefore harmonise the style of Swift code in the whole community. People can then move project to project with little syntactic friction.

  • Does this proposal fit well with the feel and direction of Swift?

Not sure how to answer this question for this proposal.

  • If you have used other languages with a similar feature, how do you feel that this proposal compares to those?

I have a fair amount of experience with PHP, where many developers follow the PSR2 formatting standard. And this has tremendously helped me to stop bothering about syntax and just accept the standard most projects use. But its not an official standard, so it still requires developers to install third-party tools to enforce. I believe there is a real advantage to having those tools come bundled with every toolchain.

  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

In-depth study and read of the comments on this thread.

4 Likes
(Ian Partridge) #161

+1.

I believe for Swift adoption to grow, focus needs to shift to improvements in tooling, the package manager, stdlib richness etc. rather than adding more language features. Adding a code formatter to the project is an important part of fleshing out Swift's tooling.

From an architectural POV, building on SwiftSyntax is the right direction so I don't object to choosing swift-format at this stage, although I think before it was actually shipped as part of a release the tool would need to prove its functionality, stability and performance in comparison to other formatting tools in the ecosystem.

I am happy for the tool to have as many configurable options as people want, provided the defaults match my personal preferences ;-) . More seriously though, I see that swift-format accepts a --configuration option which can be passed a JSON file. I would like the tool to automatically detect a swift-format.json file at the root of an SPM package, and use it automatically when swift format is invoked from that directory. If swift-format.json is not found it should look for ~/.swift-format (or something) and use that instead if found (forgive me if it does that already, I haven't played with swift-format myself yet).

Thank you to Google for bringing this forward.

4 Likes
(Tino) #162

Imho that is an important information that should be more visible - but where is it actually coming from?
At a first sight, https://github.com/google/swift/tree/format/Sources/SwiftFormatRules does look like anything but rudimentary, and https://google.github.io/swift/ has a very long list of decisions that have already been made (including controversial ones, like the rule that you have to use two spaces for indentation).

The link says about the styleguide

It is a living document and the basis upon which the formatter is implemented.

I wonder why I haven't seen this link before… is this actually missing in the proposal?

4 Likes
(Jon Hull) #163

I am a strong -1 on this proposal.

I would be happy if Xcode adds some configurable tools which help with formatting, but I have zero interest in having large style debates on Swift Evolution.

If there is something which lets my team setup and maintain a particular format for our particular project, that would be helpful... but defining a default for all of Swift would make me just not want to use Swift (unless the default was very close to my own preferences... which is unlikely).

Furthermore, I really don't want to spend our time and energy on this forum debating style guidelines, as opposed to actual features. I anticipate it making these forums more hostile than they already are.

9 Likes
(Jeremy David Giesbrecht) #164

That is a very good question. My understanding was based on @allevato’s comments and posted links, the read‐me file, the repository’s Documentation subfolder, etc. The directory you found makes it look like there is far more implemented than the documentation implies, so maybe I was wrong. @allevato, do you have clarification?

That is Google’s internal style guide for its former Swift for TensorFlow project and not directly connected with swift-format or this proposal. (Though due to stemming from the same company, it does probably describe the style the authors of swift-format are most accustomed to.)

Since that style guide has been brought up, I will share my thoughts about it—but only in an abstract sense that informs on what the general scope and philosophy of the proposed style guide should be. Largely I agree with Google’s style guide. The parts I dislike fall under two classes:

  1. Rules that do not make sense for a code base not written in English (such as the ASCII recommendation for identifiers). Such a rule makes perfect sense for a company whose working language is English, but not for a company whose working language is Chinese. It would get annoying fast if the formatter flagged warnings for this. However, a more abstract rule that lets the project configure its preferred set of characters would be welcome, since neither team likely wants Russian letters sneaking in and vice versa. Since participation in these forums is largely dominated by the English language, I am a little worried that the developer communities who code Swift in Portuguese, Japanese, etc. and have less voice here will end up with a tool working against them in these sorts of ways. I hope that outcome can be consciously avoided by review managers.

  2. Rules that are at odds with Xcode right now. For these, I really don’t care which style it is, as long as it is consistent. But it becomes annoying when the various tools are fighting against each other. This point is actually in favour of the proposal at hand though, since it would help resolve the disparity in one direction or the other: Xcode would adopt the standard style, or the standard style would adopt Xcode’s way. Either way, problem solved!

Bonus points for a standard configuration file that Xcode and other tools can all read and obey, like how any tool can read a package manifest using the SwiftPM package.

3 Likes
(Tony Allevato) #165

swift-format splits its implementation into two phases. Phase 1 (the SwiftFormatRules module) is syntax-tree-based transforms, like sorting imports, stripping unnecessary parens around Boolean conditions (e.g., if (x) -> if x); basically things that for the most part don't involve whitespace or wrapping. Phase 2 (the SwiftFormatPrettyPrint module) is where we handle line-wrapping, indentation, and so forth, by describing how the parts of specific syntax nodes should be line-broken. It's not 100% complete in terms of syntax nodes, but it's fairly close.

It's also used by the various iOS apps released by Google that use Swift. We made it public at the time that we finalized it internally just as we make public our style guides for other languages (http://google.github.io/styleguide/), as something that people can adopt, base their own off of, or ignore as they wish.

So to @Tino's comment about "decisions that have already been made", they've only been made for projects within Google. Furthermore, if any "default style guidelines" that come out of this process end up differing from what is in Google's current style guide, then Google's style guide would be updated to adopt those changes (I can say that, because I'm its owner), because we would want our style to be the same as what the community decides is a good default.

Google's style guide served as the basis for the implementation of swift-format because it makes sense that we would implement rules that we're familiar with, but even the current implementation of swift-format differs from Google's style guide in a handful of ways (mainly because we wanted to make sure that our implementation handled other possibilities and that we didn't hardcode any assumptions based on our own style). The pretty printer implementation is nicely parameterized and generic such that we could change its behavior or make it configurable with relative ease.

And I think this is a prime example of why we clearly have no interest in foisting Google's style guide upon everyone. There are rules that make perfect sense for us internally that don't hold for broader audiences.

I mentioned this earlier up-thread, but I think some of the concern that people have stems from how they're interpreting the word "adopt", which means I should have been clearer in the proposal. If this proposal is accepted, in no way is the current version of swift-format going to start being included instantaneously in Swift release toolchains. The source would be pulled into the project where it would continue development and eventually serve that purpose, after suitable default style guidelines were decided upon.

4 Likes
(Tino) #166

I'm getting more and more confused:

It is a living document and the basis upon which the formatter is implemented.

Which formatter is referenced in the guide??

Update: As I understand Tonys answer, swift-format is directly connected to the guide — so at least it looks like I'm not the only one who got confused ;-)

Given the fact that the proposal has a somewhat hidden link to a detailed styleguide, I'd like to know what exactly should happen if it is accepted:

  • Will there be a proposal for each rule?
  • How can we ensure those proposals are neutral, and not biased by the choices that have already been made?
  • What will be the schedule, and when will the process be finished?
1 Like
(Tony Allevato) #167

I think this is implying an intent that isn't there; we're not trying to hide anything. The formatter's current implementation started from Google's style guide (but the current version differs even from that), but that's purely an artifact of where it was developed (and because we wanted a couple teams internally to be able to test it effectively). The specific rules it applies might serve as inspiration for rules proposed in the future, but they would presumably go through some process involving community discussion.

If you're concerned about bias, keep in mind that:

  • I'm not a member of the Swift core team or employed at Apple, so my personal opinions matter very little here
  • I trust the core team to come up with a process for proposing style guidelines that is effective and fair
  • I certainly don't want an entire community of developers cursing my name for eternity :wink:
4 Likes
(Ben Cohen) #168

I am a strong supporter of there being an official tool, that ships as part of Swift, that formats code in a particular style. That tool should be without configuration options to tinker with, other than column width. It should be an easy and natural decision to adopt it (though of course optional to do so).

I review and consult on a lot of Swift code, from all sorts of teams and from all sorts of places. Many of those teams are new to Swift, and are coming from Objective-C, C++, Java, Python and other languages. These new Swift developers are clearly struggling to find the best way to format their code. Often a single project will show 3 different styles of formatting: the way code comes out from using Xcode; the standard library/llvm-ish format; and the formatting style of the language they are coming from.

Personally, I find it very distracting, when trying to quickly understand a medium-sized code base, to find it chopping and changing like this. But more importantly, it must be distracting for these developers, who are trying to learn a new language with new idioms and vocabularies, to have to also deal with trivial formatting decisions. We should be doing everything we can to help them become comfortable with the language as quickly as possible, and one great way to do that is to give them a smooth quick path to writing code that "looks and feels" right; that looks like the good sample code they see, like the code in evolution proposals, and like code in the standard library* and other core libraries. This is the win that a simple consistent way to format your code, performed by a tool that ships as part of Swift, will provide.

* this does not presuppose the standard library's current format is the format that will be adopted. The standard library should be reformatted to whichever style is adopted.

31 Likes
(Xiaodi Wu) #169

This goes to the heart of the issue; it also demonstrates why, in my view, substantially what is proposed in this proposal is inevitable.

As @Ben_Cohen points out, the standard library/LLVM-inspired format differs from the format used by default in Xcode, which itself is distinct from that of The Swift Programming Language and other Apple documentation. In each case, some style has already been adopted, since the mere fact of there being code forces a choice as to where to put braces and colons, how to indent, and so on.

I would posit that the principal reason why Swift does not already have an official style guide and formatter is that there is no single first-party style. Consider: if Apple were to adopt a single style across all of its documentation, cause Xcode's default formatting to match that style and the compiler-generated interfaces to use that style, and align Swift code in the standard library and core libraries to that style--which, of course, would necessitate the creation of a tool to automate that process--then essentially everything proposed here would already exist, all without Swift community input!

The style choices made for the Swift project itself and Apple's Swift-related documentation, APIs, and developer tools are not strictly internal to the project but necessarily become the default for all developers who approach the language, both in terms of the documentation they see and in terms of the default formatting applied to their code. As Swift expands its support for more and more tooling, it would be healthy for the community to have some input into that style. But if the community rejects that approach, it does not change the fact that these tools will have to default to some style, and once standardized internally, it will be for all intents and purposes the "official" style.

11 Likes
(Boris Triebel) #170

If Apple and or the Swift Core Team wants to do that it surely would be a valuable reference and a proof of concept. Until that is done we should put this here on hold.

3 Likes
(Xiaodi Wu) #171

So you support "that" but not "this here"? What is the distinction? I can see none other than the degree of community involvement.

2 Likes