Linter errors are not reported by Dune

[bbc2]

I'm not sure if this problem is most related to ppxlib or Dune but here it is. I'm trying to make a linter to be used with dune build @lint. I made a minimal example that I'm going to show here.

Here's the dune file for the linter:

(library
  (name linter)
  (kind ppx_rewriter)
  (libraries ppxlib)
)

Its implementation:

let () =
  Ppxlib.Driver.register_transformation
    ~lint_impl:(fun _structure ->
        [Ppxlib.Driver.Lint_error.of_string Location.none "Error"])
    "linter"

The dune file of the library I'm trying to apply the linter to:

(library
  (name foo)
  (lint (pps linter))
)

The content of foo.ml:

()

When I run dune build --verbose @lint, I can see that the linter is indeed used:

Running[2]: (cd _build/default && .ppx/215007d224b0286d986f29b8c129090a/ppx.exe --cookie 'library-name="foo"' --impl foo/foo.ml -corrected-suffix .lint-corrected -diff-cmd - -null)

But no error is reported by Dune (exit code 0 and no output).

I noticed that Dune passes the -null argument to the linter, which means that only errors are shown. However, if I run the same command without the -null argument, I get the error I expected output as a warning:

> cd ./_build/default && .ppx/215007d224b0286d986f29b8c129090a/ppx.exe --cookie 'library-name="foo"' --impl foo/foo.ml -corrected-suffix .lint-corrected -diff-cmd -
[@@@ocaml.ppwarning "Error"]
;;()

Am I using the feature the way it was meant to be used? Perhaps linters produced by ppxlib are supposed to be used differently with Dune? Perhaps linters should output errors, not warnings? Perhaps Dune shouldn't add the -null argument?

While searching for similar issues, I realized that this feature might not be quite stable yet, so maybe what I'm experiencing is currently to be expected and that there's already a plan to fix it.

Versions used:

• OCaml 4.12.0
• Dune 2.9.1
• ppxlib 0.23.0 (I also tried on master)

[pitag-ha]

Thanks for reporting this!

Am I using the feature the way it was meant to be used? Perhaps linters produced by ppxlib are supposed to be used differently with Dune?

That's a very good question. One other way to use PPX linters would probably be to consider them normal preprocessors (using the (preprocess (pps linter)) stanza) and to see the possible warnings captured by the linter every time you build your project. Have you already tried that? Or would that break with your workflow?

Anyways, it's also a very good question why dune passes the -null flag to the linter when using the lint stanza. I will try to find out.

[bbc2]

One other way to use PPX linters would probably be to consider them normal preprocessors (using the (preprocess (pps linter)) stanza) and to see the possible warnings captured by the linter every time you build your project. Have you already tried that? Or would that break with your workflow?

That's a good point. I have tried it and it works. Perhaps it's a little weird to have a linter listed along PPXs which are actually needed for compilation but that shouldn't be a big deal. In any case, I'm still interested in how the lint stanza is supposed to be used.

[pitag-ha]

We now have some more information about your question here. So I think the lint_impl and lint_intf arguments of the ppxlib driver are unrelated to dune's lint stanza. My understanding now is the following:

lint_impl and lint_intf allow you to write a PPX that lints your code during compilation. Via them, you can generate warnings that aren't offered by the ocaml compiler. Such lint PPXs are added to the build configuration just like any other PPX: with the preprocess stanza.

The dune lint stanza allows you to lint a given PPX (or possibly also something else) on your code. When used, the @lint alias will check if the PPX can be run on your code; it exits successfully returning nothing if yes and raises the first error otherwise.

I hope I've understood that correctly! If not, @jeremiedimino, please let us know.

[bbc2]

Thank you, it makes a lot more sense to me now.

If I understand correctly, Dune lets the user choose between applying a PPX as part of the compilation (with a preprocess stanza) or as a separate task (with the lint stanza).

ppxlib, on the other hand, notably enables the creation of PPXs which don't transform the code but just emit warnings, with the lint_impl and lint_intf parameters.

I suppose that, in theory, those linters should be usable with either Dune use case (compilation or lint task). While the first use case definitely works, the second currently doesn't because ppxlib linters emit warnings and ppxlib, as pointed out in the other issue, instructs Dune to use the -null argument for the lint task, which silences those warnings.

While my problem is solved (I'll use Dune's preprocess stanza), I have to wonder why ppxlib would have to tell Dune to use that -null argument. I understand the fact that a ppxlib linter doesn't have to be used with dune build @lint and that this @lint target can be used with regular PPXs (not just linters), but it does seem like combining @lint and lint_impl/lint_intf would be a convenient use case.

[pitag-ha]

to use the -null argument for the lint task, which silences those warnings.

In fact the -null argument silences everything, not just warnings. When passing in -null, the ppxlib driver won't do anything; as in, it won't return anything, but it will still choke on an exception.

I have to wonder why ppxlib would have to tell Dune to use that -null argument

If not passing in -null, ppxlib would return the whole AST, also containing the created warnings. But when running the @lint task, dune doesn't want anyone to process such an AST further (whereas in the @build task it asks the compiler to do so); it just wants to know if the driver would succeed or not.

Does that resolve the doubt?

[ghost]

Hi there,

Just chiming in to give a bit of context. When I added the (lint ...) field in Dune, I had two uses cases in mind:

• support for generated and committed code, for instance with type t [@@deriving_inline sexp] ... [@@@end]. I did that for the Base library, so that we could benefit from ppx rewriters without having a hard dependency of ppx
• support for "code checkers", to enforce various properties on a code base, such as some coding style rules, again without introducing a hard dependency on ppx

If you are already using ppx rewriters, then you might as well put your linters in the preprocess field. This way the properties are checked all the time, not just when building @lint. But if you want to avoid a hard dependency on ppx, then the lint field comes in handy.

In this world, a linter is a pass over the AST that should just emit errors, warnings and corrections. There is no output AST: just errors, warnings and corrections. That's why we pass -null in the lint flags.

In fact the -null argument silences everything, not just warnings. When passing in -null, the ppxlib driver won't do anything; as in, it won't return anything, but it will still choke on an exception.

Ah, so warnings being silenced by linters is not what was initially intended. Looking back at it, I should probably have used a more specific flag for linters instead of -null.

[pitag-ha]

Thanks for the context! It's very good to understand that the idea for the dune (lint ...) field is to avoid PPX as a hard dependency. By the way, how do you reflect that in the opam file? Do you simply use with-test? I've just had a look and there doesn't seem to be any with-lint variable or similar.

a linter is a pass over the AST that should just emit errors, warnings and corrections.

You're right about corrections! It also emits corrections (which I didn't think about when writing my comment above). The reason why warnings get lost is that ppxlib doesn't print them itself, but it embedds them as warning attributes in the AST (which with -null is thrown away). It shouldn't be much work though to implement a distinguishing logic that depending on the use case either puts the warnings on stderr (which seems like should be done when using dune's (lint ...)) or embeds the warnings into the AST (as is done currently always).