include ModuleAlias target id in doc extraction

[woeps]

Currently ModuleAlias is defined in DocExtraction.docItem as:

 | ModuleAlias of {
      id: string;
      docstring: string list;
      name: string;
      items: docItem list;
    }

While this differentiates between a module implementation and an alias, there is no way of knowing if 2 aliases reference the same target.

E.g:

// Root.res
module A = Core.Array // {id: "Root.A", name: "A",...}
module B = Core.Array // {id: "Root.B", name: "B",...}

While the above is obviously bad interface design, it becomes more important if we were to extract docs from several different modules, which alias the same module. When generating documentation for a complete project we most likely want to be able to avoid duplication.

If we were to run doc extraction on every module (file) in the example below, it would be nice to have an id stating the aliased module is the same as the one from another extracted json.

src/
    Pkg__A.res
    Pkg__B.res
    Pkg.res // aliases Pkg__A & Pkg__B

Therefore we need a way to uniquely identify a target module.

P.S: I guess we could compare identities by content of the json structures, but a simple id string is easier to handle.

[zth]

This sounds reasonable. @aspeddro something you could look into?

[aspeddro]

I was thinking about adding a field for the file path.

e.g:

{
  filepath: "file://src/Pkg__A.res"
}

This could be an id?

[woeps]

I guess it's good enough? The only issue I could think of is: If people were to use symlinks in their codebases, it could get tricky.

Edit: Not knowing about the implications regarding implementing it.. Currently the id is the "access path" starting with the current file/module. (e.g. Root.Child - the same string I'd use to access this module in the codebase) Therefore, I was thinking a targetId could be introduced of the same shema, but the string representing, how I'd access the targeted module directly. (e.g. Child)
Since aliases can also reference submodules, I think just the path is not enough? (e.g. What should be the targetId of this alias:

// Root.res
module X = Helper.Render.Calc2

// Helper.res
module Calc = {
  // ...
}

module Render = {
  module Calc2 = Calc
  // ...
}

• If I were to access this alias in the codebase, it would be Root.X, which is exactly the extracted id.
• If I were to access the module directly in the codebase, it would be Helper.Calc, which I suggest to be the newly introduced targetId.

edit2: I think it's not enough to just use the righthand-side as the targetId since (theoretically) the alias could reference another alias. The righthand-side currently gets resolved to the actual file (somehow?), if the target is a submodule it should be checked weither itself is just an alias: yes -> recurse | no -> use current filename and append it with the path to the submodule. I updated above example.

[aspeddro]

Since aliases can also reference submodules, I think just the path is not enough? (e.g. What should be the targetId of this alias:

The path where the module definition was made. Like go-to definition, LSP feature.

You example:

module X = Helper.Render.Calc2
//                        ^ go to definition here

{
  [2] = {
    result = {
      range = {
        ["end"] = {
          character = 11,
          line = 0
        },
        start = {
          character = 7,
          line = 0
        }
      },
      uri = "file:///tmp/test/src/Helper.res"
    }
  }
}

// Helper.res
module Calc = {
  let a = 1
}

module Render = {
  module Calc2 = Calc
  // ...
}

[woeps]

@aspeddro after thinking a bit about it, here's why I'm hesitant towards the idea of using the filename as an id:

1. ReScript has no notion of namespaces for modules. The most important question docs should answer is "how may I use this". Which in turn, means I want to see, how I can access certain modules. I need the module name for that and the submodule my desired value lives in.
2. As of now, docs don't include any paths in general.

On the other hand, it may be interesting to add the "metadata" of the path and location(like your goto definition example) to every doc item for downstream tooling consuming the docs. But I think of this as additional information, besides a targetId.

[aspeddro]

The typed parsetree has a id by module

Example:

module Calc = {
  let a = 1
}

module Render = {
  module Calc2 = Calc
  // ...
}


module A = Render.Calc2
module B = Render.Calc2

./node_modules/.bin/bsc -dtypedtree src/Helper.res

[
  structure_item 
    Tstr_module
    Calc/1002
      module_expr 
        Tmod_structure
        [
          structure_item 
            Tstr_value Nonrec
            [
              <def>
                pattern 
                  Tpat_var "a/1003"
                expression 
                  Texp_constant Const_int 1
            ]
        ]
  structure_item 
    Tstr_module
    Render/1004
      module_expr 
        Tmod_structure
        [
          structure_item 
            Tstr_module
            Calc2/1005
              module_expr 
                Tmod_ident "Calc/1002"
        ]
  structure_item 
    Tstr_module
    A/1006
      module_expr 
        Tmod_ident "Render/1004.Calc2"
  structure_item 
    Tstr_module
    B/1007
      module_expr 
        Tmod_ident "Render/1004.Calc2"
]

Here module_binding record https://github.com/ocaml/ocaml/blob/5348fa9e7c040174f895ee10b5b57d4d22bf7490/typing/typedtree.ml#L312-L320

So now we know that, module A and B is alias for same module "Render/1004.Calc2"

[aspeddro]

module A = Render.Calc2
module B = Render.Calc2
module C = Calc

In this case, A, B, and C should have the same targerId, right?

[woeps]

module A = Render.Calc2
module B = Render.Calc2
module C = Calc

In this case, A, B, and C should have the same targerId, right?

Yes, I think so.