Make a `ppx_odoc` that does the comment checking that `ppx_js_style` does

[lpw25]

ppx_js_style is the Jane Street style checking ppx. One of the things it checks is that documentation comments have valid syntax using the documentation comment parser from the old version of odoc (octavius).

Now that the parser is no longer a separate library ppx_js_style can't use an up-to-date documentation parser. Now that odoc is getting more widely used there are probably people who want to check their documentation comments without using the other parts of ppx_js_style. So my suggestion is to implement a stand-alone ppx that just checks the syntax of documentation comments.

At the moment I think it would have to live inside the odoc repository, since that is where the parser is available, but it could also be implemented separately if odoc added a parser library. Either approach seems fine to me.

[aantron]

Right now, it should be possible to make the parser sublibrary of odoc, in this repo, installable by just adding a (public_name odoc.parser) stanza to its jbuild file. ppx_odoc could then live in a separate repo, but depend on odoc.parser. I think model will also have to be installed, perhaps as odoc.model. For reference, right now, package odoc installs only the odoc binary.

[pmetzger]

This seems like a very good idea.

[rizo]

@aantron Dune currently does not permit having private dependencies for public libraries. This means that adding (public_name odoc.parser) would require making other odoc libraries public too.

[rizo]

@dbuenzli mentioned in https://github.com/ocaml/odoc/pull/226#issuecomment-432581384 that parsing docstrings directly from mli files – which is what syntax rewriters does, of course – might not be reliable because other syntax rewriters might change the AST.

Is there a way to enforce the order of application of rewriters?

Is it possible to load a cmti file from a syntax rewriter for a given source file and only add warning attributes to the AST for merlin?

[lpw25]

Is there a way to enforce the order of application of rewriters?

You can give a position parameter to Migrate_parsetree.Driver.register to control the order the ppx passes are applied. However, using a non-zero position means that the AST must be processed an additional time which can slow down preprocessing. Personally, I think it would be fine to not worry about the order for this use case. It will still give warnings for any problems with user written comments, which is the main thing you want.

[avsm]

Just cross-referencing the Dune issue on having a public library depend on private modules: https://github.com/ocaml/dune/issues/1017 -- this restriction can be lifted in the future if it's a blocker for this ppx odoc feature.

[aantron]

As a further note, we ended up exposing the sublibraries as described in https://github.com/ocaml/odoc/issues/236#issuecomment-440248694. Of course, for a supported tool, we would want to fix the names, if the tool is developed separately.

[lpw25]

I'm not sure where this bot has come from, but it's clearly set to use a too short time span. The first time it ran I carefully unmarked the issues that are still important to fix, but I can't waste time doing that every month. I can't see how this won't leave the repository with 0 open issues, which makes the issue tracker just as useless as when there are 200 open stale issues.

The aim should be to distinguish issues that people still care about and are likely to get solved from issues that people don't care about or will not likely get solved. Marking nothing as stale and marking everything as stale are equivalently bad approaches to addressing this.

[dbuenzli]

@lpw25 I noticed there's a not stale label. I suppose you have to apply that.

But yes except on open ended/transient issue tracker like the opam-respository the way these new bots behave are user hostile and a poor substitute for a carefully curated issue tracker. Your typical programmer solution to a human problem, instead people should just learn to say "no"...

[Et7f3]

@lpw25 the new bot is a "user" associated with GitHub Action: https://github.com/ocaml/odoc/blob/master/.github/workflows/stale.yml. GitHub Action allow to do automatically bunch of task that maintainer have to do manually. It is a super-set from CI.

[lpw25]

@dbuenzli My opinion is less strong than that: I don't mind these bots generally -- I find the one on ocaml/ocaml useful for example -- but I think that this one is misconfigured. They generate an amount of work for people that is related to the time span for which things are considered stale. I don't mind that amount of work being non-zero -- having to do something once a year for each issue I care about is fine for me -- but once a month is far too high.

[dbuenzli]

Is there still interest in this issue ? Isn't the solution to the problem to simply generate your docs ?