Ignore hidden values in modules

[panglesd]

It can happen that modules have hidden values.

For instance, in the following example:

module type M = sig
  val x : int
end

module type N = sig
  include M
  val x : float
end

the x value coming from include M is shadowed by the "redeclaration" of x.

The visibility of a signature item is given by the visibility value of the signature_item.

However, the API diffing currently ignores this visibility property.

This issue consists of two steps:

• Exhibit in a test a bug in the diffing that is due to this "visibility oversight"
• Fix it by taking the visibility into account when diffing two API.

[NchamJosephMuam]

@panglesd @NathanReb I have a PR open for module_type support #93. I think this issue can be a good continuation. Is it okay if I get assigned to this?

[panglesd]

I left you a review, let's finish your other PR before assigning you another issue! Besides, we are likely to assign you a custom medium-level issue that extends your work.

[NchamJosephMuam]

@panglesd @NathanReb I tried to write a test for this but I wasn't able to write a correct test to detect the bug.

module type M = sig 
  val x : int
end
module type N = sig 
  include M
   val x : float
end

and the second is

module type M = sig 
  val x : int
end
module type N = sig 
  include M
end

If I run the api-diff it outputs:

  -val x : float
  +val x : int

which I believe should the correct output. However, I don't know how to test if the value is correctly shadowed. Does the test need to be comprehensive with it's own dune environment like in https://github.com/ocaml-semver/ocaml-api-watch/blob/main/tests/api-diff/project_comparison.t

[NathanReb]

I assigned you the issue. From a quick look at the code, I can see why testing like this wouldn't work.

It seems that even though we don't explicitly ignore hidden items, the implementation will in practice not treat them. Do you have an idea why is that?

If you want to write a test you'll probably have to craft a signature by hand, or at least to modify one you built using Test_helpers. You probably won't be able to test this with a cram test and write an expect test instead.