ocaml-semver / ocaml-api-watch

Libraries and tools to keep watch on you OCaml lib's API changes
ISC License
21 stars 15 forks source link

Add support for diffing submodules #65

Closed NathanReb closed 3 months ago

NathanReb commented 4 months ago

At the moment the only items we diff ourselves (i.e. not through Includemod.signatures) are value_descriptions.

We'd like to add support for diffing module_declarations as it's one of the simplest ones to add since it should roughly amount to a recursive call to our current diff_interface. That means that we will treat those modules the same way we treat the main one, that is, we diff value_descriptions (and modules) in them, and use Includemod.signatures if we couldn't detect a change that way.

Diffing modules

We'll need to make our diff type recursive as well to be able to represent at least module modifications, not sure whether we want to represent the addition or removal of a module as the addition or removal of it's whole content.

It should look something along those lines:

type 'a change =
  | Added of 'a
  | Removed of 'a
  | Modified of {ref_: 'a; current: 'a}

type module_diff =
  { module_name : Ident.t
  ; changes : module_change
  }
and item_change =
  | Value of { name : Ident.t; change : value_description change }
  | Module of module_diff
and module_change =
  | Unsupported
  | Supported of item_change list

We could then change our entry point diff_interface to return a module_diff option, where None means no change.

Starting from this point, even changes in the main module would be "namespaced" as changes to the main module.

The end goal would be to ultimately drop the intermediate module_change type that only serves as a way to report changes that we do not detect ourselves yet.

Converting to text diff

We'll now also need a good way to turn this into a text diff as it won't be as easy to represent as it is now.

One approach would be to have a map from fully qualified module paths (e.g. M, M.M2 or M.M2.M3) to Diffutils.Diff.t. This not necessarily the perfect approach but has the advantage to be fairly simple to build and non recursive while still allowing to use Diffutils printing utilities.

We'd then need to prefix the printing of diffs with the module they belong to:

diff module M
- val f : int -> int
+ val f : int -> (int, string) result
diff module M.M2
- val x : int
...
NathanReb commented 4 months ago

CC @Siddhi-agg

Siddhi-agg commented 4 months ago

@NathanReb Commenting to be assigned.