OCaml/ReasonML best practice: warnings and errors

IN MY previous post I briefly touched on turning a specific warning into a fatal error to help enforce that private modules can't be used by external projects. In general though, OCaml provides a lot of warnings that can help guide you in writing better code. In this post I will recommend a configuration for warnings and errors that provides more safety than the out-of-the-box defaults that come with BuckleScript and dune.

Without further ado, here are the settings:

BuckleScript's bsconfig.json

{
  ...
  "warnings": {
    "number": "+A-42-48",
    "error": "+A-3-44-102-103"
  }
  ...
}

Dune's dune file

(library
  ...
  (flags (:standard -w +A-42-48 -warn-error +A-3-44))
  ...)

Now, I'll explain what these mean and why I chose them.

The more (warnings) the merrier

First, an explanation of OCaml/Reason warning configuration: the compiler can warn about a variety of different code smells. Each warning is assigned an identifying number, and some groups of warnings are also assigned identifying letters. You can see all the details in this reference: https://caml.inria.fr/pub/docs/manual-ocaml/comp.html#s:comp-options (scroll down to the -w flag).

In the warnings configuration, you can turn on a warning by listing the number or letter with a + in front of it, and turn it off by listing it with a - in front. Furthermore, you can set an 'errors' configuration to turn some warnings into errors–but only if they are actually triggered by being in the existing warnings configuration. So warnings have to jump through two hoops to become fatal errors that break the build. (There's also a way to turn warnings on and fatal directly by placing the @ symbol in front of their numbers, but I feel that's a duplication we don't need to get into.)

My assertion in this post is that the more warnings the better–to an extent. It's one that's shared by the dune build system, which by default compiles with almost all warnings turned on, and moreover turned into errors. In contrast, BuckleScript sets up project templates with no warnings turned on by default. I believe the former is the safer approach, because not only does it force you to deal with more issues in the code earlier on, it also guides you towards OCaml/ReasonML best practices. Think of these warnings as super-fast built-in lints.

The warnings and their meanings

Specifically, here is the configuration I recommend:

Warn

• +A: turn on all warnings. This will warn about all the issues I linked to earlier. But see the following point.
• -42: turn off 'type-directed disambiguation' warning, it's only relevant for OCaml versions below 4.00, which we will never use in practice
• -48: turn off 'implicit elimination of optional arguments' warning. This warning is triggered when you (1) have a function f with optional parameters and a positional (non-optional) parameter, and (2) when you pass f into another function which calls it with only the positional parameter. Here's an example:

let f(~x=0, y) = x + y;
let warning1 = 0 |> f;
let warning2 = List.map(f, [1]);

Both warning1 and warning2 would trigger the warning because they implicitly (automatically) 'eliminate' or forget about the optional x parameter to the f function to make it conform to the expected type 'a => 'b at the callsites. In other words, a function of type (~x: int=?, int) => int is automatically simplified to a function of type int => int.

This warning doesn't really tell us anything new. Optional parameters are a great convenience feature in OCaml/ReasonML and, used with a final positional parameter, rarely cause any issues. Hence, this is a warning we can safely disable.

Error

• +A: turn all triggered warnings into fatal errors. This means that any warnings not triggered, i.e. in this configuration 48, will not be an error either.
• -3: turn off 'deprecated' error; people deprecate things from time to time and we want to have some grace period to move to the new things. Will still be a warning so we know things have been deprecated.
• -44: turn off 'Open statement shadows an already defined identifier' error. This will still be a warning (it's useful to know what identifiers are being shadowed) but at the same time, shadowing is a useful technique and is used quite a lot in the OCaml/ReasonML community.
• -102: turn off polymorphic comparison error. This one is specific to BuckleScript and, while it's useful to see a warning on using potentially-unsafe polymorphic comparisons (i.e. =, <, etc.), we don't quite want to quit cold-turkey without having a better default comparison system in place. For complex data types, though, you should provide at least the type t and value let compare: (t, t) => int which allows using the type in maps, sets, and other abstractions.
• -103: prevent BuckleScript 8.2's [@bs.string] FFI warning from being an error, since many projects on earlier versions of BuckleScript still need to use [@bs.string] but it may be a good idea to encourage those projects to upgrade

Summary

I'd actually thought this post would have been smaller, but it turned out to be quite substantial. But at the end of the day, it's worth it: these warnings and errors will save you a lot of debugging effort throughout your OCaml/ReasonML career.

Comments

[JC]

Yawar, I justr tried your recommended warnings and errors. I got dozens of messages like this:

Warning 42: this use of quantityAccessor relies on type-directed disambiguation,
it will not compile with OCaml 4.00 or earlier.

where the code is

let totalPriceAccessor = (inputs: quantityInputType) =>
  inputs.quantityAccessor(inputs) *. inputs.unitPriceAccessor(inputs);

I'm totally lost about what is wrong in this one

[Yawar Amin]

Hi! As suggested in Discord I think it's safe to turn it off. Adjust warnings to taste :-) See also discuss.ocaml.org/t/can-warning-42...