treefmt icon indicating copy to clipboard operation
treefmt copied to clipboard

A `--check` flag that won't actually apply the formatting but only tell us if some files need formatting and if possible, the list of those

Open jraygauthier opened this issue 1 year ago • 5 comments

Same behavior as any formatter's --check flag. If the formatter does not provide a --check flag, the formatting could be done to a temp location and a diff helper could be used to check if there are differences.

Same reason why --check mode is needed for formatter. I don't want my CI to mutate the files (which is the current behavior of the --ci flag).

jraygauthier avatar Dec 19 '24 23:12 jraygauthier

There isn't really a uniform approach for --check across formatters, so the current --ci approach is just to run the formatters and see if anything has changed.

I'd like to understand better why mutating the files in CI is an issue for you.

brianmcgee avatar Dec 20 '24 08:12 brianmcgee

There isn't really a uniform approach for --check across formatters

I wasn't aware of this fact. All formatters I used to date always provided some option (usually --check) to dry run the formatting and report if something would change were I to apply the actual format operation. The bare minimum (common point of all) was to return a non 0 exit status to indicate changes are required.

I'd like to understand better why mutating the files in CI is an issue for you.

Our approach is that the CI reuse the same logic as the local developer through a justfile run within a nix develop reproducible environment. Basically, both the developer and CI run the exact same code to validate the build, which is the default just task: $ just. The default just task usually perform some non versioned file initialization (as/if needed), the build (when applicable), run some static checks and then run the tests. We run our check-format task (where the --check behavior would be required) alongside other static checkers such as linter and type checkers. None of the sub tasks run by the default just task touch any the version controlled code. We provide a separate format task (this is where the current treefmt behavior is needed) if the developer needs to format the code base (in addition to optional IDE integrations). We believe that any change to source code must come from an explicit developer action and want to avoid unintended side effect. Our CI never mutate the source code and we intend to keep it that way. It will only tell if the current changeset meet the minimal standards (as defined by the default justfile task).

In the meantime, we need to manually call individual formatters in our check-format task which kind of defeat the purpose of treefmt (avoid repeated boilerplate). We were a bit disappointed when we understood treefmt did not expose any way to use the underlying formatter' --check mode when all of the required formatter provide one. Note that even if the formatter would provide this option, it would still be possible for treefmt to provide a nice fallback that make use of a diff based (or go specific diff tool) approach.

jraygauthier avatar Dec 20 '24 13:12 jraygauthier

I'll dig into this a bit more over the holidays. @zimbatm @jfly I'd like your take as well.

A simple pass-through is possible if -check is well supported. Where it gets trickier, is having to support formatters which don't.

On a related note though, this sounds like what treefmt-nix already has some plumbing for. It adds a flake check which fails and produces a diff if there were changes (within a nix build, so it doesn't touch the local filesystem).

Could that be of use in this instance @jraygauthier ?

brianmcgee avatar Dec 20 '24 16:12 brianmcgee

Could that be of use in this instance @jraygauthier ?

Thanks for the ref, I wasn't aware it was available. Will have a look.

jraygauthier avatar Dec 20 '24 18:12 jraygauthier

This seems like a reasonable thing for core treefmt to be able to do.

It's a little unclear to me how we'd do this, though. As mentioned above, we need to support formatters that don't have a --check flag. Could we copy files to a temp directory before formatting them? Does that make sense in general? Or do some formatters behave differently based on the "surrounding files"? I certainly hope there aren't any that act that way. (Perhaps this is something the Formatter Specification should call out explicitly?)

edit: I just realized that most formatters do behave differently based on the presence of other files, such as configuration files. For example, ruff format does different things based on the contents of a nearby pyproject.toml file (see https://github.com/numtide/treefmt/discussions/283#discussioncomment-11850852)

jfly avatar Jan 16 '25 05:01 jfly