Default behavior of "swift format"

Development Swift Format
1

Now that we're shipping swift format with toolchains in 6.0+ the following command became available "out of the box": swift format.

This is fantastic development and I'm very happy that we finally have a "just run swift format" for newcomers to Swift.

Within both my day to day on-the-job and my role in the server workgroup, I often work with people coming to Swift from other ecosystems and this will be a tremendous help in helping transitioning teams.

--

[1] Here's the catch though. Excited about the addition I recently attempted to use it, and the experience is as follows:

// cd MyCoolProject
swift format 
// "hangs" (waits for stdin)

This part can be pretty jarring. What actually happens is that swift-format waits on stdin input... and will then format that. I'd like to argue that this is a very confusing behavior for a swift format invocation, especially while in a directory with a swiftpm package.

Expected behavior, especially coming from other language ecosystems (go fmt, sbt format, cargo fmt, etc.), would be to format sources in the project.

[1.Q] While I understand changing default behavior can be difficult, but I do think we need to improve the default behavior here to actually format sources.

This will help fight the perception that Swift is this "weird thing that has weird stuff" that we often need to fight when bringing teams new to Swift over to the language.

And yes, I had developers (experienced ones even), at this point be shocked and "give up", moving ask "swift format seems broken, just hangs for me", which is a really bad first impression (even when we then explain what happened).

--

[2] The second part to this is, that given noticing this issue, some developers then try swift format . which one would expect to work on the directory.

[2.1] This also doesn't quite work, and we'll get:

-> % swift format .
Error: '.' is a path to a directory, not a Swift source file.
Use the '--recursive' option to handle directories.
Usage: swift-format format [--in-place] [--configuration <configuration>] [--offsets <offsets> ...] [--assume-filename <assume-filename>] [--recursive] [--ignore-unparsable-files] [--parallel] [--color-diagnostics] [--no-color-diagnostics] [--follow-symlinks] [<paths> ...]
  See 'swift-format format --help' for more information.

Which is all true, but still adds to the "why is Swift making things so hard for me?" experience for newcomers.

[2.2] Then we add the --recursive option:

swift format . --recursive

Which prints all files to stdout, which again isn't what I'd expect when already "work on directories" mode in my interactive command line session with my SwiftPM project (directory).

[2.3] So then one has to consult --help and realize we need another flag:

swift format . --recursive --in-place

which finally does what I wanted to do: format source files in my project, although with much more typing than in competing ecosystems. It makes Swift feel clunky as formatting source code is a common thing developers do, as often as git add . or similar.

[2.Q] I would like to argue these options should also be default when invoked as swift format. We could debate if perhaps a good default would be to look at Sources/, Tests/ and Package.swift files by default perhaps, rather than . which could do more than one would anticipate, but I do think all those flags do get in the way of making Swift a nice and simple language and ecosystem.

--

So, how can we figure out changing the default behavior of swift format to actually format sources? This will make the Swift adoption for people new to Swift a much nicer, and less of a jarring experience.

I understand the behavior of stdin->stdout may be useful for when swift-format is invoked by other tools, and we may have to keep it for compatibility reasons, but I really believe we have to improve the developer experience when invoked as swift format.

One workaround may be to force people to install a format plugin, and then use swift package format but I don't think this is a great solution. It adds to the "extra steps" and making things difficult for people who just opened up Swift and want to give it a shot, have a nice experience, and then keep learning about plugins, configuring formatters etc.

63 Likes
2

There's an experimental hidden swift package _format command.

% swift help package _format
USAGE: swift package _format [<swift-format-flags> ...]

ARGUMENTS:
  <swift-format-flags>    Pass flag through to the swift-format tool

OPTIONS:
  --version               Show the version.
  -h, -help, --help       Show help information.

However, it hasn't been updated to use the toolchain.

% swift package _format --version
error: Could not find swift-format in PATH or SWIFT_FORMAT
error: fatalError

Also, it passes the following arguments by default.

--mode format --in-place --parallel

(But --mode is an unknown option, and format is already the default subcommand.)

3

I totally agree that the default should change, it currently feels like an old unix tool, not a tool in a modern programming language toolchain. I'd also love to not have to make a format.sh script for every project just cause Swift Format doesn't have knowledge of SwiftPM (and therefore ends up formatting the code of all dependencies checked out in .build if you do swift format -r -i .). The format command often becomes too long to efficiently type out every time .

In my ideal world, swift format would find the locations of your package's source files and format them in place. And if you wanted to format stdin you could do swift format - or swift format --stdin.

One workaround may be to force people to install a format plugin, and then use swift package format but I don't think this is a great solution. It adds to the "extra steps" and making things difficult for people who just opened up Swift and want to give it a shot, have a nice experience, and then keep learning about plugins, configuring formatters etc.

I also agree that that wouldn't be great. It's pretty annoying having to add a package plugin to every new package just to format your code with the standard formatter. Additionally this often creates annoying dependency clashes with your package even though it's just a tool that your code never interacts with. (I have similar opinions about the docc plugin but those should go in another thread).

Re how to actually change the default

We could possibly leave swift format as is for now but have it print out Formatting stdin... (silence with SWIFT_FORMAT_NO_STDIN_WARN=1) to stderr, and then have a new subcommand that people can use to format their package's source code; swift format package. It's not ideal but it would at least avoid breaking tools that rely on the existing behaviour if that's an issue for people. If we did that we could also possibly change the default behaviour to be swift format package if the package has // swift-tools-version: 6.0, but that could be pretty unintuitive.

Another idea; we could make running swift format with no arguments just print out the usage, requiring you to explicitly run swift format --stdin if you want to format stdin, with an additional swift format package subcommand for formatting swiftpm packages. This would improve discoverability and would also make it really obvious why your tool that relies on swift format has suddenly stopped working (cause if you check the output it explicitly tells you what the new usage is instead of just silently doing a different thing and then possibly failing in another part of your script). If we decide on this approach we could have something really close to the top of the usage that tells users to run swift format package if they want to format a SwiftPM package.

10 Likes
4

Just placing my 2 cents here.

As a swift developer, the most intuitive way for me to format code by default, apart from press the option in Xcode, is to type swift format command within the desired directory, in the terminal.
Of course you can add in different parameters or flag within the command, but that's another story for formatters customization.

I personally think we should keep it as simple as possible, just formatting the files in the directory as the default behavior. And I agree with @stackotter, we can use subcommands to provide different formatting behaviours.
(Is swift format help also an option, if devs need guidelines?)

In case if there are devs who would like to try some bleeding edge features of swift format, or maybe they are using nighty builds, enabling --experimental flag might become handy.

Side note: Some formatters in other programming languages will actually showed a list of files that have been formatted, highlighted in different colors.

Free free to correct me if I mixed up sth, thanks for reading!

1 Like
5

I concur, this has to change. It’s daunting enough as is to get into a new ecosystem. This certainly won’t be helpful for newcomers that think of adopting Swift

8 Likes
6

I fully agree with this. Having recently setup our swift-format CI pipelines in NIO this was more complicated than I expected swift-nio/scripts/check-swift-format.sh at main · apple/swift-nio · GitHub.

I understand that the binary itself takes a list of file paths or uses stdin/stdout; however, I agree with @benrimmington here. Having swift package format do the right thing for a package seems logical. FWIW, this is similar to how rustfmt works. Where rustfmt itself takes file paths or stdin/stdout and cargo fmt works on the whole crate.

6 Likes
7

IIRC, swift package _format cannot format Package.swift. In the case of Rust, the package manifest is written in TOML, but in the case of Swift, the package manifest is also written in Swift, so we may want to format it as well.

2 Likes
8

It's good to see general consensus on the direction here, that's one problem solved. Now just™ figuring out the how and when :slight_smile:

I wasn't aware of swift package _format but it sounds like this should just fade away. The swift package <thing> spelling is nowadays reserved for command plugins. And since there is no format plugin here -- unless someone installs one (and then this would invoke THEIR format plugin) -- there's no need to muddy the waters of that one.

I would lean into changing the default behavior and figuring out who depends on the behavior of stdin (if anyone). One would assume that existing tools would depend on swift-format and that swift format invocations are "new", although we'll have to check with IDE plugins :thinking:

This sounds ideal indeed. I hope we can get to that? The only thing to check is if any "important" user exists for the swift format handling stdin and migrating them through the new way AFAICS... :thinking:

I realize I missed to ping @allevato in the initial post (and it's a long holiday in the US), but let's make sure he has a moment to think through and chime in as well.

8 Likes
9

I think this all makes sense but there's one thing I wanted to point out to make sure it's considered: Switching swift format to run on all sources in the current directory in place is potentially a destructive operation.

Even if you have a git repository (very likely) there still might be other changes sitting there which likely wouldn't be overwritten but drowned out with so much other formatting noise as to be effectively clobbered.

So maybe there should at least be a confirmation prompt that can be overridden with --force?

(I still remember when a colleague ran a Java formatter over my changes way back, making them unreviewable :sweat_smile:)

6 Likes
10

I'd recommend:

  1. Require - for stdin: swift format -. I.e., have no default, because there are no perfect ones, and bad ones lose (lots of) data.
  2. Build out SPM to manage formatting using its knowledge of project sources and any required package/project configuration. Policy is specified once, and travels with project as needed for good branches/PR's.

Here's why...

I agree, so I disagree with making the swift format command by default try to do some right thing.

Data loss is much, much more important than confusion. I'd rather confuse a thousand users than mess up the code for one user.

And that effect is amplified for anyone not using the default configuration, which does a lot more than changing whitespace. And amplified again if the new default changes to recursing through directories.

I also don't think that any default would always do the right thing - which means we're always back to trading confusion for data loss.

So the conservative and correct thing is to balk when not given complete instructions, i.e., to have no default.

Then for standard input the change would be to require -, as it is for swift command: swift format -. When nothing is specified, swift format would issue the help text.

Yes, for current clients of swift format in standard-input mode, it would be a breaking change, but with no data loss, clear notice, and an easy, understandable fix.

With that said about the swift-format command, I think SPM should drive formatting.

We instead should build it up.

SPM knows all package source locations, and it's likely the configuration would be common to all such sources. And then the configuration would travel with the project declarations for all modifying clients, making it trivial for branches and pull requests to do the right thing.

Implementation:

Ideally the swift-format - change would be made before the in-toolchain version of swift format is officially released, to avoid reliance the default behavior of consuming standard input.

Then the SPM formatting capability can be built as time/demand/ permits.

Then IDE's and CMake-based builds can follow suit by implementing the same logic as SPM.

edit: also then the swift format command could have a --package option which would do everything specified in the SPM configuration, but from the command line (and assuming discovery of the correct Package{...}.swift declarations).

1 Like
11

Could the default behavior be changed only when isTTY(.standardInput) returns true? (Along with using _isatty for Windows.)

1 Like
12

I agree mostly but I think the user experience should be as if it was a plugin without requiring it to be added as an explicit dependency. Therefore swift format should require --allow-writing-to-package-directory to modify anything on disk.

As an alternative we could also check if there are any uncommitted swift files and at least warn before formatting in place.

13

Sorry, but how can formatting result in a loss of data? I can see how it could make diffs messier by increasing the number of differences, but I've never encountered a formatter which can actually lose anything.

IMO swift format should only default to formatting when it detects the presence of a config file in the current directory. Otherwise it should offer help, especially something like swift format init to create such a config file. swift format with a config file present should run the configured formatting, as the user's intention is clear from the presence of that file. IME there's no need to worry about inadvertent formatting as it's rare, easily reversible, and usually what the user's going to do anyway.

3 Likes
14

Say you have a bunch of unstaged changes in your repo across a good number of files and then you run swift format that touches every file. You're not technically losing any data but in some sense you've clobbered it.

Depending on what project you're working on, policy might not allow you to just merge with the formatting changes so that's not a route and you have to manually undo the formatting changes instead.

It's just safer to at least ask for confirmation and require --force to override it.

2 Likes
15

How do other language ecosystems (go fmt, sbt format, cargo fmt, etc.) handle this concern? That would help inform what user expectations are in this scenario.

1 Like
16

Dart has a nice format guide:

1 Like
17

Fully in favor of the change to make this more ergonomic, @ktoso.

Ironically, Dart is itself a little inconsistent on whether to apply changes or not. It also has a dart fix command, which does not write changes by default (since its changes are more intrusive than whitespace). dart fix offers --dry-run and --apply flags, which test and apply the change.

I do like the idea of a --dry-run flag, perhaps we can adopt that as part of this change?

1 Like
18

Rust & Go just format straight away. No warnings on un-committed or un-staged changes. The same way behave SwiftFormat and SwiftLint tools.

6 Likes
19

Thanks for the detailed experience report, @ktoso! I definitely agree that there's room for improvement now that we've reached a point where swift-format is included in the toolchain.

The current command line interface has grown somewhat organically from the beginning, but there's still a bit of a theme woven through it. Since formatting is a potentially "destructive[1]" action, we wanted users to opt-in to the "more destructive" levels of those actions. Hence why the default behavior is to output to stdout (though I'll agree that this doesn't make sense when formatting multiple files, that behavior is an accident more than anything) and to modify the files you have to use an explicit flag, and likewise why you have to use an explicit flag to recurse into subdirectories. I'd be pretty hesitant to change the defaults to something that's "more destructive" by default.

I also pretty strongly feel that it's good and meaningful to have separation of concerns and proper layering around the tools in the ecosystem. swiftc compiles source files, but doesn't automatically try to detect the layout of a whole package and build it. swift-format is analogous in that it lints or corrects source files, which may or may not be part of a Swift package. A tool like swift-format can try to guess where the sources are located in a Swift package but packages allow users to customize things like the source root, so if we were to offer a way to have swift-format by default format all the sources in a package, we'd only be able to do packages with standard layouts unless we actually processed the manifest, which is a clear non-starter. Having SwiftPM communicate the layout of the package to swift-format instead of privileging SwiftPM in the swift-format implementation feels like a much cleaner, more future-proof approach. Seeing that Rust does this with cargo fmt makes me comfortable that Swift wouldn't be an outlier here.

I think there are some concrete improvements we can make to smooth out the user experience and guide them toward the right path, without sacrificing some of these themes:

  1. Formalize the swift package format workflow to have SwiftPM invoke swift-format with the correct paths to sources/tests/manifest(s). Make that the well-lit path for formatting/linting an entire package at once. It would be ideal if a CI workflow, for example, could just run swift package lint and see any violations.
  2. To process stdin, swift-format should require an explicit - argument.
  3. Given (2), we update swift-format so that if it is invoked with no flags, instead of hanging waiting for input, it displays a more friendly usage message. This could recommend that they run swift package format/lint instead, and tell them to invoke it with --help for more detailed standalone usage.
  4. Disallow silly mode combinations as errors, like formatting multiple files to stdout.
  5. Smooth out some of the remaining rough edges in non-SwiftPM usage. For example, if someone explicitly writes swift-format foo.swift bar.swift, maybe that's a clear enough signal that they want to explicitly format those files in-place (because after (4), there is no other possible interpretation) and we don't need the extra flag. But we might still want to keep some of the protection around things like recursively drilling into directories without a special opt-in.

  1. "Destructive" here is relative; bugs do happen that could break working code, but more commonly, you might have some special situation that needs to be tagged with // swift-format-ignore and if you run the formatter without it, it might undo some intentional formatting you had in place. ↩︎

12 Likes
20

Perhaps a basic question from me, but do we need a 1:1 coupling between swift format and the underlying swift-format tooling? swift build doesn't just invoke swiftc: it has an understanding of the package format in the same way that we're looking for here.

The ideal scenario is that the core swift commands have great ergonomics and lead people automatically to the golden path, even if there are advanced options or commands that it abstracts.

7 Likes