OCamlformat changes force code reformats

The default and configureable style of OCamlformat changes over time. This forces reformatting code - which in turn is a risk in large projects because they are difficult to review and could be used to hide malicious changes. How are projects dealing with this?

3 Likes

Not sure if this is the answer you are looking for, but git blame can ignore some commits

This of course, assumes that a) git is used b) all purely format changes are in one commit.

About this git blame feaure: Ignoring bulk change commits with git blame - Moxio

I think we should reinvent some of the features of the Nix. I.e. we need to install a few versions of ocamlformat as different executables and use a specific one.

As for me, I’m using ocamlformat to preprocess PPX-generated code in cram tests for my PPX rewriters. Changes in the output produced by ocamlformat will be very annoying for me.

In general, IMHO, their idea to force specific code style for all OCaml projects is too opinionated.

1 Like

One solution is to only accept the reformats of people you trust, e.g. you do the reformatting and commit it, instead of taking the PR of a random stranger.

But while I see the issue, lately the changes between ocamlformat releases (at least in the conventional profile) haven’t been that big so the formatting was pretty easy albeit tedious to review.

My plan for now is to sit on pre-1.0.0 versions of ocamlformat as long as possible. The planned removal of the swath of currently-deprecated options post-1.0.0 will probably keep me from making that upgrade. What to do at that point (fork, un-deprecate, and stumble along?; downshift to ocp-indent?; or something else?) is very much up in the air.

Agreed. The urge toward unitary authority made popular by gofmt has unfortunately spread widely. There is also the argument re: “each option increases implementation complexity”, which I appreciate on some level, but the result contrasts poorly with wildly configurable formatting tools common in other ecosystems, which allow people to set formatting rules at whatever granularity they decide is right (per project, per team, etc). Surely there is room for both approaches for OCaml.

1 Like

Another is to use automated CI/scripts do to the reformat or see the diffs.
If the project build is reproducible, one could also check that the build output is identical (unless I’m mistaken, the reformat doesn’t change the AST). (EDIT: As pointed out below, reformat can change the AST).

The reformatting can change line numbers, which will change the AST in observable ways (backtraces, assertion payloads, …)

1 Like

The idea is not to force specific code style, more, to observe what are the most common styles (this number cannot be too high for obvious reasons) and make these styles available for people.

The issue with options is that the combinatorics was too big so we couldn’t properly test every combination, and obviously some options don’t play well together, or they include each other, or negate each other. More than we expected when we said yes to every option at the beginning of the project. The result is that there are a lot of unforeseen weird interactions, bugs, and not tested combinations, and the result is not always consistent.

My personal take on this is that it would be better for the tool to have a smaller number of configurations (we call them profiles) but that are consistent and less buggy than the current options.

About being opinionated, well the whole tool ism it’s in the README iirc, ocamlformat does not edit a source file to add/remove spaces. The source file is parsed and we operate on the AST and do not rely on the original shape of the file too much (a rare exception to that is to decide where to attach comments that’s a hard problem that is not tackled when parsing).

3 Likes

What would be interesting to know is which options being deprecated are a deal breaker for you. A lot of them do not add any value and only pollute ocamlformat’s code.

I’m all for hearing more from users, as long as everyone keeps in mind that ocamlformat cannot preserve every particularity from the original source (because by its nature, it is operating on the AST, see my other response in this thread)

3 Likes

FWIW, I too find some of ocamlformat’s choice unpleasant (specifically parens around tuples option being removed). But I find it more valuable to just use it rather than not so I take a deep breath, tell myself “it’s just syntax” and move on. On each new ocamlformat release I do tend to have to do a big “reformat all my code” commit.

4 Likes

From what I remember, it was not made popular by gofmt it was made popular by prettier. And this was by far one of the best things in the Reason ecosystem, everything just used refmt.

I don’t want to have options.

3 Likes

align-cases, match-indent, and nested-match are a couple of the deprecated options that I use, but are going away.

But really, the biggest gaps for me are in the options/behaviour that simply doesn’t exist. Examples include function-indent being a numeric constant, rather than driving the alignment of the body of a function with it’s “head” (likewise carried over to function-indent-nested); and the often-surprising break behaviour around various operators, which almost always yields code that is easy to misread IMO. (https://github.com/ocaml-ppx/ocamlformat/issues/993 talks about this some, but gets into the weeds of correspondence between a style involving infix operators and a style involving let operators.)

I should start cataloguing these more systematically, so as to inform later decisions, but those are some issues that I can think of off the top of my head.

That’s actually backwards: prettier was inspired by refmt[1]. And gofmt certainly predated refmt, the naming of which I presume was an homage.

[1] A Prettier JavaScript Formatter

We have a similar issue. When we finally got a formatting that seemed fine I found that many options were deprecated. We use a style that is derived from the sparse because I’m dyslexic and I feel that some of the profiles are just too crammed and hard to read.

Every time that there is big code style change, I try to plan ocamlformat update during vacations that way there are not so many developers working. The code changes can mess up with the merging and we have had instances where it has been hard to fix conflicts.

4 Likes

I use patdiff for viewing diffs, both at home and at work:

It does a much better job of ignoring whitespace-only changes, which makes reviewing ocamlformat changes much easier. At work, when we release a new version of ocamlformat, we have one big-bang change that reformats the whole tree, and people get a chance to review their part of it.

I do think that opam could really use a way to support multiple concurrent versions of ocamlformat, so that projects could simply pick which version they want, and only have to do one of these upgrade-reformats when they choose to absorb the new version.

y

5 Likes