Enhancement request: A warning to flag misleading indentation

I think people naturally pay more attention to the indentation than the exact syntax in front of them. It’s too easy to write OCaml code where indentation is inconsistent with how a construct is interpreted. This leads to mistakes. For example, the following indentation (inside another let) suggests that the line with the let should have ended with in, which likely leads to an error message that may be many, many lines below. These errors can be tedious to track down. It would be helpful to have a warning in this case.

let x = 10;
my_func ();

There are similar possibilities for confusion with if, match and try that could beneficially generate warnings, e.g.:

if condition then
  my_func (); another_func ();

or

if condition then
  my_func ();
  another_func ();

This would not require a syntax change. The warning could easily be disabled, which could be the default setting. The check should be forgiving, e.g. only consider whether certain pairs of lines have the same, more or less indentation or that there’s a missing newline after a semicolon.

Opinions?

1 Like

I think the number of people who would want this and don’t use ocamlformat (which achieves the same thing) is quite small, but I could be wrong.

2 Likes

I couldn’t tell from the ocamlformat documentation whether it reformats entire files or just the changed lines. If it’s the former, that could hide significant chunks of the git history behind trivial reformatting. That’s not so good for larger projects.

What exactly would ocamlformat do with the let example? Give you a message somehow?

Ocamlformat will reformat the entire file. Ocp-indent might be a better fit, since it can correct the indentation of a subset of lines.

The idea is to use ocamlformat from the beginning and ensure all commits/PRs are well formatted. That way you avoid any big formatting changes.
Of course, that doesn’t solve the problem of how to start on an existing large project. But as roddy indicated, it’s probably a problem for a minority of users/projects.

It’s not a question of “how to start on an existing large project”. It’s that because a project has become large (lines of code, people) that reformatting is not helpful. I expect the Coq developers would oppose reformatting lots of the Coq code, for example.

If you use git you could commit a big reformatting change and then use ’—ignore-rev’ to ignore changes from said commit.

The commit could be recorded in a file and then referenced by a .gitconfig both of which you also commit.

See this blog post for a more detailed explanation: Ignoring mass reformatting commits with git blame – Rob Allen's DevNotes

2 Likes

These errors can be tedious to track down. It would be helpful to have a warning in this case.

This is something I’ve wanted in OCaml for a very long time. It would eliminate a whole load of beginner mistakes in one go. In particular, it eliminates the most difficult ones to track down, where the error location reported is completely wrong. e.g.

let greet name =
  Printf.printf "Hi %s\n" name;

let () =
  greet "Alice"

This reports Error: Syntax error at “line 6” (which doesn’t even exist!).

Running ocp-indent to detect these problems does work, but:

  1. It doesn’t help beginners, who don’t start by installing and configuring all the tools. It needs to be a compiler warning that’s on by default.

  2. Doing it in CI can only check that the indentation exactly matches how ocp-indent does it, which might be annoying if another tool does it slightly differently, or if the submitter isn’t using an auto-indenter but their code is still fine.

3 Likes