Converting warning-50-clean OCaml code results in misplaced doc comments

[jberdine]

It seems that reasonfmt is not using the documentation comment attributes, and its own heuristics for comment association are incompatibly different. Is there a way to use the ocaml ast attributes? If so, then checking that ocaml code has no instances of warning 50 would be enough to ensure that converting it to reason would accurately preserve the documentation comments.

[jordwalke]

and its own heuristics for comment association are incompatibly different.

Only currently. Hopefully not for very long. Maybe you could help us sequence the features. Can you point me to the specification for the new ocaml document attribution policy? Is the following accurate? (I'm sure there's an error with it - can you help me correct it?)

(* on x *)
let x = 10  (* on x *)
(* on x *)

(* on t *)
type t =
   (* on A *)
   | A of int   (* on A *)
   (* on A*)

  (* on B *)
   | B of int
  (* on C *)
   | C of int
(* on t *)

• We need to make Reason parse comments into the AST even if it never uses them to print - this is so that we can build awesome automated documentation for Reason on top of this (probably using codoc?)
• Currently Reason doesn't use the AST stored comments for reprinting the AST even though when parsing from OCaml, they are correctly present in the AST attributes. When printing as Reason, we filter them out and ignore them when we see them. Even when we eventually do parse comments into the AST, I suspect we still may only use them for generating documentation, and not for printing of the AST:
  • The primary reason is that the AST comments aren't sufficient sadly. Not every comment shows up in the AST, so we have to do a from scratch interleaving of some comments anyways - may as well do it for all of them.
  • The comments stored in the AST seem broken, sometimes including the wrong substring (at least on 4.02.3).
  • Even if we have the correct attribution of these AST attribute comments, we still want more information, namely whether or not it was an "end of line" comment vs. a "before the line" comment. Is this correct?

[jordwalke]

checking that ocaml code has no instances of warning 50 would be enough to ensure that converting it to reason would accurately preserve the documentation comments.

Yes, once we match the new ocaml documentation convention, warning 50 would be a good way to ensure that we the project is "reason clean" w.r.t. comments. We would then be able to encourage people to toggle that warning in preparation of conversion.

reason would accurately preserve the documentation comments.

It would accurately preserve the attribution - but I think there's some information lost that we're going to need to do some work to recover. I mentioned above the distinction between end of line and "before the line" comments. That isn't encoded in the AST attributes IIRC so we have to do our custom trickery to preserve it.

[jberdine]

and its own heuristics for comment association are incompatibly different.

Only currently. Hopefully not for very long. Maybe you could help us sequence the features. Can you point
me to the specification for the new ocaml document attribution policy? Is the following accurate? (I'm sure
there's an error with it - can you help me correct it?)

The closest thing to documentation I know of is ocaml/ocaml#149 (comment) .

(* on x )
let x = 10 ( on x )
( on x *)

None of these are doc comments (** ... *), but assuming they were, only one doc comment per item is allowed. But otherwise the association seems correct.

(* on t )
type t =
( on A )
| A of int ( on A )
( on A*)

'Sub-items' (sum type alternates and record fields and maybe some other things?) only allow doc comments after the item. And also only 1 comment is allowed.

(* on B )
| B of int
( on C )
| C of int
( on t *)

For the final comment to be associated with t, an empty (** *) comment would be needed on C of int, so:

(** on x )
let x = 10 (* would be on x, but there already is one )
(* would be on x, but there already is one *)

(** on t )
type t =
(* NOT on A )
| A of int (* on A )
(* would be on A, but there already is one *)

(* NOT on B )
| B of int
(* NOT on C )
| C of int (* )
(* on t, because of the (* *) above *)

• We need to make Reason parse comments into the AST even if it never uses them to print - this is so
  that we can build awesome automated documentation for Reason on top of this (probably using codoc?)

Absolutely! And we want this info to be accurate so that merlin-document can work well.

• Currently Reason doesn't use the AST stored comments for reprinting the AST. It filters them out and
  ignore them when it sees them.
  • The primary reason is that those aren't sufficient sadly. Not every comment shows up in the AST, so
    we have to do a from scratch interleaving of some comments anyways - may as well do it for all of
    them.

Yes, only the documentation comments are intended to show up in the ocaml ast

• The comments stored in the AST seem broken, sometimes including the wrong substring (at least on
  4.02.3).

Argh, is it known if the situation is better with 4.03?

• Even if we have the correct attribution of these AST attribute comments, we still want more
  information, namely whether or not it was an "end of line" comment vs. a "before the line" comment. Is
  this correct?

That sounds right. Although it may be that the "end of line" and "before the line" info is not needed for documentation comments, only the other free-form comments. The formatter could put doc comments on signature items (and top-level items of an .rei file) after the item, doc comments on structure items (and top-level item of an .re file) before the item, and doc comments on sum type alternates and records fields, etc. after the item. The end of line versus next line question could be based on whether it fits without breaking the line (later, when such sensitivity is possible since easy_format doesn't support it currently).

At a high level, it would be nice if reason could have its own heuristics for associating comments in reason code, but use the attributes when converting, so that reason doesn't have to get into the business of replicating the ocaml heuristics. But I guess that isn't fully possible given the additional info and non-doc comments issues you mention.

As an intermediate, would it make sense to add a warning to reasonfmt that compares the ocaml doc comment attributes and the doc comments that reason has found, and complain if they differ? That way for warning-50-clean ocaml code, converting to reason could check that its heuristics make the same choices for the doc comments.