rescript-lang / rescript-vscode

Official VSCode plugin for ReScript
MIT License
329 stars 56 forks source link

docgen id is ambiguous #1008

Open woeps opened 4 months ago

woeps commented 4 months ago

The way docgen currently calculates the id is ambiguous if

This defeats the purpose of having a (supposedly unique) id.

Value / Type Example

type x = int
let x = 42

generates

{
  "name": "Example",
  "docstrings": [],
  "source": {
    "filepath": "src/Example.res",
    "line": 1,
    "col": 1
  },
  "items": [
  {
    "id": "Example.x",   <-- here
    "kind": "type",
    "name": "x",
    "signature": "type x = int",
    "docstrings": [],
    "source": {
      "filepath": "src/Example.res",
      "line": 1,
      "col": 1
    }
  }, 
  {
    "id": "Example.x",    <-- here
    "kind": "value",
    "name": "x",
    "signature": "let x: int",
    "docstrings": [],
    "source": {
      "filepath": "src/Example.res",
      "line": 2,
      "col": 5
    }
  }]
}

Module (Type) Example

// Example.res
module type M = { type x = int}
module M = {
  let x = 42
}

generates

{
  "name": "Example",
  "docstrings": [],
  "source": {
    "filepath": "src/Example.res",
    "line": 1,
    "col": 1
  },
  "items": [
  {
    "id": "Example.M",  <-- here #1
    "name": "M",
    "kind": "moduleType",
    "docstrings": [],
    "source": {
      "filepath": "src/Example.res",
      "line": 6,
      "col": 13
    },
    "items": [
    {
      "id": "Example.M.x",  <-- here #2
      "kind": "type",
      "name": "x",
      "signature": "type x = int",
      "docstrings": [],
      "source": {
        "filepath": "src/Example.res",
        "line": 6,
        "col": 19
      }
    }]
  }, 
  {
    "id": "Example.M",  <-- here #1
    "name": "M",
    "kind": "module",
    "docstrings": [],
    "source": {
      "filepath": "src/Example.res",
      "line": 7,
      "col": 8
    },
    "items": [
    {
      "id": "Example.M.x",  <-- here #2
      "kind": "value",
      "name": "x",
      "signature": "let x: int",
      "docstrings": [],
      "source": {
        "filepath": "src/Example.res",
        "line": 8,
        "col": 7
      }
    }]
  }]
}

Proposal

Either we state an id is only unique per kind which will probably confuse people, or we extend the id. The simplest thing would probably be to additionaly include kind in the id: <kind>.<modulePath>.<itemName>.

For the above examples:


P.S: If above seems appropriate, I think I'd be able to send a PR to implement this: I'd adapt the makeId function: https://github.com/rescript-lang/rescript-vscode/blob/a6108b369cf7bb6c69675dc090e96b64a5222ccf/tools/src/tools.ml#L301-L302 to take an additional optional labeled argument prefix and call it accordingly.

woeps commented 4 months ago

@zth do you have some thoughts on my proposal?

zth commented 4 months ago

@woeps that looks good to me and should cover all use cases.