SE-0250: Swift Code Style Guidelines and Formatter

(Goffredo Marocchi) #21

If the discussion is over because the tool admits no configuration or very limited configuration options perhaps, although people will be frustrated anyways (they will just not be able to do much about it...).

If the tools is configurable, see the great swiftformat by Nick Lockwood (it is a different tool than the one presented here), then people will still argue about how to configure it.

image1.png

6 Likes
(Andreas Jönsson) #22

From Twitter probably - that’s where I came from. Also a tiny hint of a glass ceiling there? :blush:

This is a topic I as a professional Swift developer feel very strongly about and where I really want to make my voice heard.

Personally I wish I’d have the time to engage more in other evolution proposals too, but the time just isn’t there.

18 Likes
(Ling Wang) #23
  • What is your evaluation of the proposal?
    +1
  • Is the problem being addressed significant enough to warrant a change to Swift?
    Yes
  • Does this proposal fit well with the feel and direction of Swift?
    Yes
  • If you have used other languages with a similar feature, how do you feel that this proposal compares to those?
    Good
  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
    Careful reading
#24
  • What is your evaluation of the proposal?

    +1.

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

    Yes, trying to enforce a coding style to be used consistently throughout a project is tough, and having a blessed way to do this (assuming it will be integrated into Xcode) would be a big help.

    Before somebody asks, "Why not just use a community tool?" besides them not being comprehensive enough in my experience, it's much harder to get people to bother installing and configuring a community tool, compared to a blessed formatter which could be as simple as ticking a checkbox, or would be enabled by default.

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

    Yes, part of Swift's charm is it's clean syntax and readability. Working on projects that have multiple people working on them who sometimes fail to follow the agreed coding style harms that. Having a blessed formatter will be helpful, even if it's opt-in/opt-out.

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

    It will bring parity to them. I have used the formatter in Android Studio and Xamarin and it is a great relief not having to worry about manually following a coding style. I can write it however I feel at the time, and when I press cmd+s to save, everything is formatted for me, I don't even have to think about it.

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

    Read the draft, the proposal and discussion and noted my experience using formatters for other languages.

#25

-0.5, for very similar reasons to @Ponyboy47.

If there were specific configuration options that are for accessibility or purely practical reasons, I would understand that being there.

But if this tool allows configuration just for preference I see no reason why it should be an "official" tool in any way. It's just like other tools like SwiftFormat, except it has less functionality. It also won't achieve what was previously a goal of having consistent styling between projects.

I think this has some validity do it, which is why I went -0.5 instead of -1.

We have chosen not to do so given that Swift is a well-established language.

If it can be controlled to just reformat code on a file you've touched, I don't see the problem (or just convert all of it in one go using the command line tool).

Yeah, I don't think it's a big issue but it's worth addressing.

I don't think this has anything to do with Swift itself.

I've played with gofmt and talked to other people that do it more regularly than I. They all seemed to like the consistency.

participated in discussion and read the proposal

2 Likes
(Daniel Duan) #26

What is your evaluation of the proposal?

+1. Especially for the official style guidelines (elaborated at the end).

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

Oh yeah. If we think time is a limited resource, an official style guide will save time, and the saving scales with popularity of Swift -- then adoption of a this proposal will be priceless.

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

This fits right in with Swift's API Design Guidelines, which in a lot ways are unique and outstanding among programming languages.

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

gofmt is the target for comparison. In Go communities, coding style is rarely discussed or even thought about. Programmers type in valid code however they want and formatting is done automatically. The result is they spend more time building what they care about.

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

Read original pitch, discussion of original pitch, this proposal. Have worked professionally with Swift from 1-person team to projects that involves 70+ and growing number of Swift programmers, where an style guide exists since the beginning the project, enforced by linters and code reviews rigrously, with only limited success.

Notes on official style guidelines

Among the two things proposed, I believe a set of official guidelines with no ambiguities in their recommended coding styles, will be more important than the tools that help enforce them.

I doubt anyone really questions the value of code style consistency in a collaborative environment. So having a guideline that everyone should follow for a project is good (lots of discussion in the past were centered around this point, let's get that out of the way).

There is an unique property of an official style guide that I want to point out.

Today, enforcing a guideline in any kind of scale is a doomed effort, assuming the goal is 100% conformance. The fundamental issue is this: people have different backgrounds. They learn Swift in different projects, they grow their syntatical preferences from their personal experiences with a diverse set of languages through a number of years. When you ask people to follow guidelines of the project, they'll at best spend time to learn, discuss and adapt to it. And whatever things your guidelines don't cover, they'll apply their own interpretation. As programmers, our instinct then, would be to enhance said guidelines -- good luck bringing that up to your team/community and have fun doing that!

So, if you value consistency in any particular project, the only way to achieve it in a future-proof way, is to have consistency community-wide: ask everyone who comes through the Swift door to learn the official reccommendations once and only once. Point to the official text if someone makes a mistake (assuming that's possible with tool enforcement).

From a individual perspective, not having to learn or worry about a new set of guidelines when contributing to a new project is nice. It means we get to focus on things that matter. I hold the belief that "readability" is an over-weighted concept. Beyond a threhold of consistency, anything can become readable very quickly (even Lisp, Haskell, ASM!), this is an ability that we all possess.

14 Likes
(Roy Hsu) #28
  • What is your evaluation of the proposal?
    Huge + 1.

  • Is the problem being addressed significant enough to warrant a change to Swift?
    It’s so important to have a consistent code syle when cooperating with others on the same project. An official guidelines can solve lots of problems we have to deal with everyday. Besides, I think it will also help beginners to catch up much quicker based on my teahching experience in Swift.

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

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

  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
    Carefully follow from the beginning of the pitch thread.

1 Like
(Jeremy David Giesbrecht) #29

I have a request for clarification, @allevato.

The proposal involves adopting swift-format into the Swift project, but says that the proposal defers any decision of what the specific guidelines should be.

So what is the proposed process for adopting swift‐format? Is it something like one of these:

  • Zero swift-format’s default configuration so that it does nothing by default and matches the initial state of the null guidelines. Starting from there, its defaults would then take on new rules as they are pitched, reviewed and accepted into the guidelines.

  • Adopt swift-format now with its current default configuration. Those defaults would then require going through the evolution process to have them removed or adjusted along with the corresponding guidelines.

If the process will be something like the second, then it would be helpful to know what guidelines we are implicitly reviewing. While I see a few notes about configuring swift-format, I cannot find a list of what it does or what guidelines it follows at present.

7 Likes
(Tony Allevato) #30

Right now we're closer to the second option, but I disagree that this:

follows from this:

While there is currently an "implicit style" applied based on the choices made in the implementation so far, we don't want those to be seen as under "implicitly review" here, and users shouldn't be concerned that any of the current choices will be imposed without further review. Some of the choices were made arbitrarily, some were to explore what we needed to implement in the core formatting algorithm to handle a particular choice, and some were to test edge cases. (I'll also point out that what it applies today differs in a number of ways even from Google's style guide, despite that being where it's been developed, so we're not trying to steer the community toward our style by any means.)

"Zeroing out" the default configuration doesn't make much sense based on the current implementation because it would essentially mean "delete the bodies of all the syntax node handlers", and I personally don't think it would be a productive use of engineering time/resources. Today, the available configuration options are described in Configuration.md but those aren't meant to be the final set. Instead, as we begin discussing specific style guidelines, I foresee us using PRs to show how the implementation changes to handle different style variations that are under review (either by making a particular feature configurable, or just modifying the implementation to do something else), and then people can easily download those variants to experiment with them and so forth.

2 Likes
(Gwendal Roué) #31

-1

Should the Swift language adopt a set of code style guidelines and a formatting tool?

Yes, please.

Meanwhile, the sentences between "The proposal authors (among others) have collaborated on the swift-format tool currently hosted at ..." and "... continuing support commitment from active maintainers" should be removed from the proposal.

The sentence "If the answer to this is "yes", then subsequent proposals will be pitched to discuss and ratify those guidelines" should list the tool in the list of deferred decisions.

This means the unlike the pitch, the proposal sounds like a hijacking.

Until a +1 for the proposal is not implicitly translated into a +1 for a unique tool that the community has not evaluated, I can't vote for this proposal.

That is to say I am -1, and that is a pity.

What is your evaluation of the proposal?

Negative.

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

Yes.

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

No. The community must be able to evaluate the proposed tool. This is Swift Evolution, not a recording chamber.

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

n/d

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

Read thoroughly, participated in the pitch phase, able to spot a hijack.

4 Likes
(Gwendal Roué) #32

Our friend @mattt has posted https://nshipster.com/swift-format/ :slight_smile:

It will help readers see the wide variety of the formatting space. And I hope it will help more people here also ask that the choice of the tool is deferred in another proposal.

7 Likes
(Tony Allevato) #33

Can you please elaborate a bit more on your concerns here? If you're characterizing the proposal as strongly as a "hijacking", I'd like to understand better why you feel that way, rather than just asking for parts of the text to be removed. Updates to the text between the pitch thread and now were based entirely on feedback from the pitch thread and meant to clarify questions that arose during those discussions, which happens with nearly every proposal between the initial pitch and the proposal that goes up for review.

Similarly, the swift-format tool hosted at https://github.com/google/swift/tree/format has been a visible part of the pitch/proposal from the initial draft and pitch thread, as it has been intended to be the specific implementation that is required for a proposal to reach the review stage. If your concern is specifically about the changing of the wording from "intend to propose" to "propose", the sole factor there was that we felt "intend to propose" was appropriate for the pitch phase (because it was a pitch, not a proposal), and that "propose" was appropriate for the actual proposal. That was deliberate, but I apologize if that particular phrasing obfuscated or misconstrued our intent.

1 Like
(Jeremy David Giesbrecht) #34

I do not mean removing functionality. I only meant converting any default validation or formatting to being opt‐in until they individually pass review. Otherwise we are effectively reviewing them now. (A reasonable alternative would be a guarantee they will go through review before they see release; then work must only be done to adjust rejected guidelines.)

For example, in a Unicode world I don’t even know what “line length” is supposed to mean. (Measured in clusters or scalars? What about scripts where the very concept of “monospace” is nonsensical?) If the community hashes out what it is supposed to mean, then I would be okay with having to opt out of it if the resulting decision seemed weird to me, but I do not want deal with that type of warning if it has not even gone through review.

I am fine with guidelines, and I am fine with swift-format being the starting point for the tool implementation, but I do not like the idea of swift-format being the starting point for the guidelines unless they are reviewed first.

2 Likes
(Matthew Johnson) #35

I was very happy to see that the core team intends to consider feedback provided in the pitch thread. I won’t repeat what I said there other than to say I am happy to see a pragmatic perspective on configuration has been made clear.

There is some irony in that many of the strongest advocates of the style guidelines are those who claim to most strongly wish to avoid style debates. Yet acceptance of this proposal will inevitably lead to the most epic and detailed style debates the Swift community has ever seen. It’s a bit like starting the war to end all wars (this is especially true of those advocating an unconfigurable mandatory tool).

With that in mind, I definitely share the concern some others have expressed that this may not be the best use of time and energy for the Swift evolution community. If the reviews produce a set of default style guidelines that feels reasonable to the majority of Swift users the community would benefit enormously. But I don’t think that outcome is inevitable and I don’t think it will be easy to achieve, especially after people have spent years forming varying preferences.

On a personal note, I really don’t have much desire to spend time and energy debating style on Swift evolution. At the same time, as a passionate Swift programmer with well-formed and well-reasoned style preferences I know I will feel compelled to do so on style issues where I have strong opinions as it is unlikely style decisions will ever be reconsidered. I am probably not alone in feeling this way. The toll these debates could take on the community should be considered carefully before we decide to engage in them.

This is unfortunate (if you intend to merge the tool immediately upon acceptance of this proposal).

With all due respect, I don’t think this is a very realistic perspective. This tool will start to receive a lot more users once it is included in official Swift releases. Those users will often have a status quo bias for style decisions they are not explicitly dissatisfied with.

So the timing and method in which this tool is introduced into official Swift releases does matter and introducing the tool before the style guidelines have been reviewed will influence those future discussions and reviews. This will happen even if the proposal authors and core team do everything they can to prevent that from happening. The one action the proposal authors and core team can take to minimize the influence of the current defaults the tool has is to simply be patient and wait to introduce the tool until all of its defaults have been reviewed and accepted (and modified when necessary).

If the goal is to introduce a tool and see widespread acceptance of both the tool and its defaults I think it would best to review the guidelines it implements before the tool is officially introduced. Prior to that the tool can at best be considered a “beta” tool as it by definition cannot operate as intended (i.e. used to apply the recommended style guidelines). Swift has gotten by thus far without official style guidelines or an official formatter. What’s the rush to adopt an official tool before it is actually ready?

19 Likes
(Gwendal Roué) #36

@allevato

Sure, I can.

The proposal contains the sentence "We propose this specific tool because it satisfies all of the following goals". That tool being https://github.com/google/swift/tree/format.

Such a sentence makes anyone able to turn votes for the proposal into votes for this tool. This is not quite correct. Most +1 here are not for this formatting tool in particular. They are for a formatting tool.

What if this tool is not a good fit for the future set of guidelines and rules the community agrees on?

This is why the proposal would be enhanced in this sentence and immediate context were removed.

Similarly, the swift-format tool hosted at https://github.com/google/swift/tree/format has been a visible part of the pitch/proposal from the initial draft and pitch thread, as it has been intended to be the specific implementation that is required for a proposal to reach the review stage.

You are totally right. I was blinded by the goal, and did not notice this specific during the pitch phase. I have nothing against your tool, but we have a logical twist when the tool is chosen before we know what the tool has to do.

If your concern is specifically about the changing of the wording from "intend to propose" to "propose", the sole factor there was that we felt "intend to propose" was appropriate for the pitch phase (because it was a pitch, not a proposal), and that "propose" was appropriate for the actual proposal.

Thanks for writing this. I do think that the proposal intent will be better expressed in the tool choice is explicitly listed in the deferred decisions.

2 Likes
(Gwendal Roué) #37

@allevato

I'm more and more confused, actually. Is this actually a proposal for Google's formatting tool? If yes, why isn't it written front and foremost? Why aren't other formatters listed as alternatives?

1 Like
(Jon Shier) #39

I'm +1 on a default Swift style but -1 on swift-format as the tool to implement it.

Overall I would like to see some set of styling integrated into Swift's tooling, both in the hope that Xcode might finally have configurable formatting, and in the more general desire that we can develop Swift on all platforms with consistent formatting. I would hope that such a formatter could integrate with the upcoming LSP implementation so that formatting can be separated from the editors used to write Swift.

I also think some common styling defaults where not everything is configurable could be fine, if the community can approximate some sort of consensus about them. It makes sense to me that there would be some formatting rules which are configurable (line length, function wrapping behavior) and some that aren't (brace placement, colon placement). We'll see how that discussion goes. Like @anandabits, I anticipate quite the discussion.

However, as it stands right now, swift-format is the inferior formatting tool and I'd rather see a community tool integrated instead of a lesser but Apple blessed tool. Most notably, the use of a JSON configuration file is both a poor choice from a user perspective (JSON is a pain to write by hand and is really only better than XML for config files) as well as the long term flexibility of the project (JSON types are inherently limited). I also find it funny that, at least for now, it's not the best performer, despite the proposal's claims. If adopting a community tool is impossible for some reason, at least adopting YAML or TOML for the config file would make it more acceptable to me. Then we just need to hash out the actual styling.

3 Likes
#40

Continuing the discussion from SE-0250: Swift Code Style Guidelines and Formatter:

-1

  1. We propose formally adopting a formatting tool into the Swift project that will allow users to update their code in accordance with those guidelines.

I do not support an official tool that only enforces a single recommended style and has no room for local/team based customization. The reasons provided in the pitch comment here Pitch: an Official Style Guide and Formatter for Swift still stand.

In particular,

One thing to consider is that some individuals have visual impairments and need some extra spaces in the code they write to be able to visually parse it. If you make a strictly enforced style guide agreed upon by the majority then you will almost certainly leave those with accessibility challenges out in the cold suffering.

An official tool that only supports one format will become the de facto standard and Xcode and other tools are unlikely to support using alternative formatting tools as easily. This will make it more difficult for individuals who need a format different from the single, majority-defined, one supported by the default tool to advocate for their needs. The Swift project should not be making it harder for those with disabilities or wither needs outside the average/majority to participate in the community and profession.

Make it configurable or don’t do it. There are enough third party formatters that already exist to prove it’s not impossible - nor even particularly difficult - to do so.

4 Likes
(Tony Allevato) #41

I'll preface the following question with a disclaimer that even though this has been collaborative work, since I'm not on the Core Team, it's not my place to make any assertions about how toolchains will be packaged or distributed.

Can you define what "official Swift releases" means, in your mind? For example, would you be opposed to it being distributed with any snapshots of the toolchain? Would you be comfortable with it being distributed with trunk snapshots, but not release snapshots? What if it were in a separate repository and users built it from source until the style guidelines have been ratified?

Based on feedback so far, I think there's a concern from some users that if this proposal is accepted, that swift-format as it is implemented today will be dropped into everyone's toolchains as a precompiled binary and that it will be unable to be changed. That's not what would happen here; my understanding is that the code would be contributed to the Swift project but that it would not be distributed in release toolchains until the community were able to discuss and ratify guidelines.

Thinking about this more, I wonder if our disagreements are purely around different interpretations of what the word "adopt" means, and that we should attempt to clarify that. It's certainly possible that, as proposal author, I'm assuming that other readers would interpret it the same way when it's actually open to more possible interpretations than the one I intended.

3 Likes
(Tony Allevato) #42

The current version of the proposal was updated based on feedback around this in the pitch thread (for example, these changes). Do those modifications not address your concerns fully?