VHDL-LS / rust_hdl

Other
328 stars 65 forks source link

[Bug] Format of `textDocument/documentSymbol` LSP response doesn't allegedly match the LSP specification #158

Closed pidgeon777 closed 3 months ago

pidgeon777 commented 1 year ago

Neovim is a text editor tool which makes use of the Language Server Protocol (LSP) to provide code completion, diagnostics, and other features. The LSP specification defines a response format for the textDocument/documentSymbol request, and the one generated by rust_hdl seems not be be 100% compliant to the specification.

For example, let consider the following code:

library ieee;
use ieee.std_logic_1164.all;

entity ENTITY_TOP is
  generic (
    GEN : integer := 0
  );
  port (
    INP : in std_logic
  );
end entity;

architecture arch of ENTITY_TOP is
  signal sig : std_logic := '0';

  component ENTITY_1
    generic (
      GEN : integer := 0
    );
    port (
      INP : in std_logic
    );
  end component;

  component ENTITY_2
    generic (
      GEN : integer := 0
    );
    port (
      INP : in std_logic
    );
  end component;

begin

  ENTITY_1_1 : entity work.ENTITY_1(arch1)
  generic map(
    GEN => GEN
  )
  port map(
    INP => INP
  );

  ENTITY_1_2 : entity work.ENTITY_1(arch2)
  generic map(
    GEN => GEN
  )
  port map(
    INP => INP
  );

  ENTITY_2_1 : entity work.ENTITY_2(arch1)
  generic map(
    GEN => GEN
  )
  port map(
    INP => INP
  );

  ENTITY_2_2 : entity work.ENTITY_2(arch2)
  generic map(
    GEN => GEN
  )
  port map(
    INP => INP
  );

  PROC_p: process(INP)
  -----------------------------
  variable var_v : integer := 0;
  -----------------------------
  begin
      -----------------------------
      var_v := 0;
      -----------------------------
      if (INP = '1') then
        sig <= '1';
      else
        sig <= '0';
      end if;
      -----------------------------
  end process;
  -----------------------------

  process
  -----------------------------
  variable var_v : integer := 0;
  -----------------------------
  begin
      -----------------------------
      var_v := 0;
      -----------------------------
      if (INP = '1') then
        sig <= '1';
      else
        sig <= '0';
      end if;
      -----------------------------
  end process;
  -----------------------------
end architecture;

In my test, I compared the textDocument/documentSymbol response of the following LSP servers:

Here is the JSON response to the textDocument/documentSymbol request of rust_hdl:

{
  [7] = {
    result = { {
        children = { {
            kind = 11,
            name = "generic 'GEN'",
            range = {
              ["end"] = {
                character = 7,
                line = 5
              },
              start = {
                character = 4,
                line = 5
              }
            },
            selectionRange = {
              ["end"] = {
                character = 7,
                line = 5
              },
              start = {
                character = 4,
                line = 5
              }
            }
          }, {
            kind = 11,
            name = "port 'INP' : in",
            range = {
              ["end"] = {
                character = 7,
                line = 8
              },
              start = {
                character = 4,
                line = 8
              }
            },
            selectionRange = {
              ["end"] = {
                character = 7,
                line = 8
              },
              start = {
                character = 4,
                line = 8
              }
            }
          }, {
            children = { {
                kind = 24,
                name = "signal 'sig'",
                range = {
                  ["end"] = {
                    character = 12,
                    line = 13
                  },
                  start = {
                    character = 9,
                    line = 13
                  }
                },
                selectionRange = {
                  ["end"] = {
                    character = 12,
                    line = 13
                  },
                  start = {
                    character = 9,
                    line = 13
                  }
                }
              }, {
                children = { {
                    kind = 11,
                    name = "generic 'GEN'",
                    range = {
                      ["end"] = {
                        character = 9,
                        line = 17
                      },
                      start = {
                        character = 6,
                        line = 17
                      }
                    },
                    selectionRange = {
                      ["end"] = {
                        character = 9,
                        line = 17
                      },
                      start = {
                        character = 6,
                        line = 17
                      }
                    }
                  }, {
                    kind = 11,
                    name = "port 'INP' : in",
                    range = {
                      ["end"] = {
                        character = 9,
                        line = 20
                      },
                      start = {
                        character = 6,
                        line = 20
                      }
                    },
                    selectionRange = {
                      ["end"] = {
                        character = 9,
                        line = 20
                      },
                      start = {
                        character = 6,
                        line = 20
                      }
                    }
                  } },
                kind = 5,
                name = "component 'ENTITY_1'",
                range = {
                  ["end"] = {
                    character = 20,
                    line = 15
                  },
                  start = {
                    character = 12,
                    line = 15
                  }
                },
                selectionRange = {
                  ["end"] = {
                    character = 20,
                    line = 15
                  },
                  start = {
                    character = 12,
                    line = 15
                  }
                }
              }, {
                children = { {
                    kind = 11,
                    name = "generic 'GEN'",
                    range = {
                      ["end"] = {
                        character = 9,
                        line = 26
                      },
                      start = {
                        character = 6,
                        line = 26
                      }
                    },
                    selectionRange = {
                      ["end"] = {
                        character = 9,
                        line = 26
                      },
                      start = {
                        character = 6,
                        line = 26
                      }
                    }
                  }, {
                    kind = 11,
                    name = "port 'INP' : in",
                    range = {
                      ["end"] = {
                        character = 9,
                        line = 29
                      },
                      start = {
                        character = 6,
                        line = 29
                      }
                    },
                    selectionRange = {
                      ["end"] = {
                        character = 9,
                        line = 29
                      },
                      start = {
                        character = 6,
                        line = 29
                      }
                    }
                  } },
                kind = 5,
                name = "component 'ENTITY_2'",
                range = {
                  ["end"] = {
                    character = 20,
                    line = 24
                  },
                  start = {
                    character = 12,
                    line = 24
                  }
                },
                selectionRange = {
                  ["end"] = {
                    character = 20,
                    line = 24
                  },
                  start = {
                    character = 12,
                    line = 24
                  }
                }
              }, {
                kind = 2,
                name = "instance 'ENTITY_1_1'",
                range = {
                  ["end"] = {
                    character = 12,
                    line = 35
                  },
                  start = {
                    character = 2,
                    line = 35
                  }
                },
                selectionRange = {
                  ["end"] = {
                    character = 12,
                    line = 35
                  },
                  start = {
                    character = 2,
                    line = 35
                  }
                }
              }, {
                kind = 2,
                name = "instance 'ENTITY_1_2'",
                range = {
                  ["end"] = {
                    character = 12,
                    line = 43
                  },
                  start = {
                    character = 2,
                    line = 43
                  }
                },
                selectionRange = {
                  ["end"] = {
                    character = 12,
                    line = 43
                  },
                  start = {
                    character = 2,
                    line = 43
                  }
                }
              }, {
                kind = 2,
                name = "instance 'ENTITY_2_1'",
                range = {
                  ["end"] = {
                    character = 12,
                    line = 51
                  },
                  start = {
                    character = 2,
                    line = 51
                  }
                },
                selectionRange = {
                  ["end"] = {
                    character = 12,
                    line = 51
                  },
                  start = {
                    character = 2,
                    line = 51
                  }
                }
              }, {
                kind = 2,
                name = "instance 'ENTITY_2_2'",
                range = {
                  ["end"] = {
                    character = 12,
                    line = 59
                  },
                  start = {
                    character = 2,
                    line = 59
                  }
                },
                selectionRange = {
                  ["end"] = {
                    character = 12,
                    line = 59
                  },
                  start = {
                    character = 2,
                    line = 59
                  }
                }
              }, {
                children = { {
                    kind = 13,
                    name = "variable 'var_v'",
                    range = {
                      ["end"] = {
                        character = 16,
                        line = 69
                      },
                      start = {
                        character = 11,
                        line = 69
                      }
                    },
                    selectionRange = {
                      ["end"] = {
                        character = 16,
                        line = 69
                      },
                      start = {
                        character = 11,
                        line = 69
                      }
                    }
                  } },
                kind = 3,
                name = "process 'PROC_p'",
                range = {
                  ["end"] = {
                    character = 8,
                    line = 67
                  },
                  start = {
                    character = 2,
                    line = 67
                  }
                },
                selectionRange = {
                  ["end"] = {
                    character = 8,
                    line = 67
                  },
                  start = {
                    character = 2,
                    line = 67
                  }
                }
              }, {
                kind = 13,
                name = "variable 'var_v'",
                range = {
                  ["end"] = {
                    character = 16,
                    line = 86
                  },
                  start = {
                    character = 11,
                    line = 86
                  }
                },
                selectionRange = {
                  ["end"] = {
                    character = 16,
                    line = 86
                  },
                  start = {
                    character = 11,
                    line = 86
                  }
                }
              } },
            kind = 2,
            name = "architecture 'arch'",
            range = {
              ["end"] = {
                character = 17,
                line = 12
              },
              start = {
                character = 13,
                line = 12
              }
            },
            selectionRange = {
              ["end"] = {
                character = 17,
                line = 12
              },
              start = {
                character = 13,
                line = 12
              }
            }
          } },
        kind = 2,
        name = "entity 'ENTITY_TOP'",
        range = {
          ["end"] = {
            character = 17,
            line = 3
          },
          start = {
            character = 7,
            line = 3
          }
        },
        selectionRange = {
          ["end"] = {
            character = 17,
            line = 3
          },
          start = {
            character = 7,
            line = 3
          }
        }
      } }
  }
}

This is the response of the VHDL-Tool LSP:

{
  [5] = {
    result = { {
        kind = 2,
        location = {
          range = {
            ["end"] = {
              character = 0,
              line = 3
            },
            start = {
              character = 0,
              line = 0
            }
          },
          uri = "file:///c:/src_test/ENTITY_TOP.vhd"
        },
        name = "imports"
      }, {
        kind = 2,
        location = {
          range = {
            ["end"] = {
              character = 0,
              line = 1
            },
            start = {
              character = 0,
              line = 0
            }
          },
          uri = "file:///c:/src_test/ENTITY_TOP.vhd"
        },
        name = "library ieee"
      }, {
        kind = 2,
        location = {
          range = {
            ["end"] = {
              character = 0,
              line = 3
            },
            start = {
              character = 0,
              line = 1
            }
          },
          uri = "file:///c:/src_test/ENTITY_TOP.vhd"
        },
        name = "use ieee.std_logic_1164.all"
      }, {
        kind = 2,
        location = {
          range = {
            ["end"] = {
              character = 0,
              line = 12
            },
            start = {
              character = 0,
              line = 3
            }
          },
          uri = "file:///c:/src_test/ENTITY_TOP.vhd"
        },
        name = "ENTITY_TOP"
      }, {
        kind = 13,
        location = {
          range = {
            ["end"] = {
              character = 2,
              line = 9
            },
            start = {
              character = 4,
              line = 8
            }
          },
          uri = "file:///c:/src_test/ENTITY_TOP.vhd"
        },
        name = "INP"
      }, {
        kind = 14,
        location = {
          range = {
            ["end"] = {
              character = 2,
              line = 6
            },
            start = {
              character = 4,
              line = 5
            }
          },
          uri = "file:///c:/src_test/ENTITY_TOP.vhd"
        },
        name = "GEN"
      }, {
        kind = 2,
        location = {
          range = {
            ["end"] = {
              character = 0,
              line = 101
            },
            start = {
              character = 0,
              line = 12
            }
          },
          uri = "file:///c:/src_test/ENTITY_TOP.vhd"
        },
        name = "arch"
      }, {
        kind = 13,
        location = {
          range = {
            ["end"] = {
              character = 2,
              line = 15
            },
            start = {
              character = 2,
              line = 13
            }
          },
          uri = "file:///c:/src_test/ENTITY_TOP.vhd"
        },
        name = "sig"
      }, {
        kind = 2,
        location = {
          range = {
            ["end"] = {
              character = 2,
              line = 24
            },
            start = {
              character = 2,
              line = 15
            }
          },
          uri = "file:///c:/src_test/ENTITY_TOP.vhd"
        },
        name = "ENTITY_1"
      }, {
        kind = 2,
        location = {
          range = {
            ["end"] = {
              character = 0,
              line = 33
            },
            start = {
              character = 2,
              line = 24
            }
          },
          uri = "file:///c:/src_test/ENTITY_TOP.vhd"
        },
        name = "ENTITY_2"
      }, {
        kind = 2,
        location = {
          range = {
            ["end"] = {
              character = 2,
              line = 43
            },
            start = {
              character = 2,
              line = 35
            }
          },
          uri = "file:///c:/src_test/ENTITY_TOP.vhd"
        },
        name = "ENTITY_1_1"
      }, {
        kind = 2,
        location = {
          range = {
            ["end"] = {
              character = 2,
              line = 51
            },
            start = {
              character = 2,
              line = 43
            }
          },
          uri = "file:///c:/src_test/ENTITY_TOP.vhd"
        },
        name = "ENTITY_1_2"
      }, {
        kind = 2,
        location = {
          range = {
            ["end"] = {
              character = 2,
              line = 59
            },
            start = {
              character = 2,
              line = 51
            }
          },
          uri = "file:///c:/src_test/ENTITY_TOP.vhd"
        },
        name = "ENTITY_2_1"
      }, {
        kind = 2,
        location = {
          range = {
            ["end"] = {
              character = 2,
              line = 67
            },
            start = {
              character = 2,
              line = 59
            }
          },
          uri = "file:///c:/src_test/ENTITY_TOP.vhd"
        },
        name = "ENTITY_2_2"
      }, {
        kind = 2,
        location = {
          range = {
            ["end"] = {
              character = 2,
              line = 84
            },
            start = {
              character = 2,
              line = 67
            }
          },
          uri = "file:///c:/src_test/ENTITY_TOP.vhd"
        },
        name = "PROC_p"
      }, {
        kind = 13,
        location = {
          range = {
            ["end"] = {
              character = 2,
              line = 71
            },
            start = {
              character = 2,
              line = 69
            }
          },
          uri = "file:///c:/src_test/ENTITY_TOP.vhd"
        },
        name = "var_v"
      }, {
        kind = 2,
        location = {
          range = {
            ["end"] = {
              character = 0,
              line = 100
            },
            start = {
              character = 2,
              line = 84
            }
          },
          uri = "file:///c:/src_test/ENTITY_TOP.vhd"
        },
        name = "Unlabelled process"
      }, {
        kind = 13,
        location = {
          range = {
            ["end"] = {
              character = 2,
              line = 88
            },
            start = {
              character = 2,
              line = 86
            }
          },
          uri = "file:///c:/src_test/ENTITY_TOP.vhd"
        },
        name = "var_v"
      }, {
        kind = 2,
        location = {
          range = {
            ["end"] = {
              character = 0,
              line = 33
            },
            start = {
              character = 2,
              line = 13
            }
          },
          uri = "file:///c:/src_test/ENTITY_TOP.vhd"
        },
        name = "head"
      }, {
        kind = 2,
        location = {
          range = {
            ["end"] = {
              character = 0,
              line = 100
            },
            start = {
              character = 2,
              line = 35
            }
          },
          uri = "file:///c:/src_test/ENTITY_TOP.vhd"
        },
        name = "body"
      } }
  }
}

And finally the response of the vhdl-linter LSP:

{
  [5] = {
    result = { {
        detail = "entity",
        kind = 11,
        name = "ENTITY_TOP",
        range = {
          ["end"] = {
            character = 10,
            line = 10
          },
          start = {
            character = 0,
            line = 3
          }
        },
        selectionRange = {
          ["end"] = {
            character = 17,
            line = 3
          },
          start = {
            character = 7,
            line = 3
          }
        }
      }, {
        children = { {
            detail = "instantiation",
            kind = 2,
            name = "ENTITY_1_1: ENTITY_1",
            range = {
              ["end"] = {
                character = 3,
                line = 41
              },
              start = {
                character = 2,
                line = 35
              }
            },
            selectionRange = {
              ["end"] = {
                character = 3,
                line = 41
              },
              start = {
                character = 2,
                line = 35
              }
            }
          }, {
            detail = "instantiation",
            kind = 2,
            name = "ENTITY_1_2: ENTITY_1",
            range = {
              ["end"] = {
                character = 3,
                line = 49
              },
              start = {
                character = 2,
                line = 43
              }
            },
            selectionRange = {
              ["end"] = {
                character = 3,
                line = 49
              },
              start = {
                character = 2,
                line = 43
              }
            }
          }, {
            detail = "instantiation",
            kind = 2,
            name = "ENTITY_2_1: ENTITY_2",
            range = {
              ["end"] = {
                character = 3,
                line = 57
              },
              start = {
                character = 2,
                line = 51
              }
            },
            selectionRange = {
              ["end"] = {
                character = 3,
                line = 57
              },
              start = {
                character = 2,
                line = 51
              }
            }
          }, {
            detail = "instantiation",
            kind = 2,
            name = "ENTITY_2_2: ENTITY_2",
            range = {
              ["end"] = {
                character = 3,
                line = 65
              },
              start = {
                character = 2,
                line = 59
              }
            },
            selectionRange = {
              ["end"] = {
                character = 3,
                line = 65
              },
              start = {
                character = 2,
                line = 59
              }
            }
          }, {
            children = {},
            detail = "process",
            kind = 12,
            name = "PROC_p",
            range = {
              ["end"] = {
                character = 13,
                line = 81
              },
              start = {
                character = 2,
                line = 67
              }
            },
            selectionRange = {
              ["end"] = {
                character = 8,
                line = 67
              },
              start = {
                character = 2,
                line = 67
              }
            }
          }, {
            children = {},
            detail = "process",
            kind = 12,
            name = "no label",
            range = {
              ["end"] = {
                character = 13,
                line = 98
              },
              start = {
                character = 2,
                line = 84
              }
            },
            selectionRange = {
              ["end"] = {
                character = 13,
                line = 98
              },
              start = {
                character = 2,
                line = 84
              }
            }
          } },
        detail = "architecture",
        kind = 5,
        name = "ENTITY_TOP",
        range = {
          ["end"] = {
            character = 17,
            line = 100
          },
          start = {
            character = 0,
            line = 12
          }
        },
        selectionRange = {
          ["end"] = {
            character = 31,
            line = 12
          },
          start = {
            character = 21,
            line = 12
          }
        }
      } }
  }
}

Those are the tree different Neovim GUIs when using the three LSP servers:

rust_hdl:

image

VHDL-Tool:

image

vhdl-linter:

image

As you can see the breadcrumb, which is displayed right at the top of buffer, is not correctly generated with rust_hdl.

Also, it seems that symbol scope information is missing in rust_hdl. You can see how the symbols panel report a yellow highlight for a node (symbol) and all of its parents (scopes). This means that when the cursor is placed somewhere inside the buffer, then the curren scope is detected up to the highest-level parent node.

It is important to observe that when using other LSP servers for different languages, for example LUA or CPP, the breadcrumb and symbol scope information are always correctly detected, but that's not the case with rust_hdl.

In conclusion, it seems that in its current implementation, rust_hdl produces an output in a different format than the other LSPs, not allowing to make use of the breadcrumb and symbol scope features.

kraigher commented 1 year ago

The LSP protocol defines two ways to answer the textDocument/symbol request: https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_documentSymbol

I am using the new way, maybe your client only supports the old way. There is a client capability for this which I do not look at, I can disable this feature for clients that do not support the new format.

pidgeon777 commented 1 year ago

If possible, it would be great to test the version of rust_hdl which implements the old way, to verify if our hypotheses are right.

pidgeon777 commented 1 year ago

I can try to find out which LSP standard version is implemented in the Neovim client.

The LSP protocol defines two ways to answer the textDocument/symbol request: https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_documentSymbol

I am using the new way, maybe your client only supports the old way.

So, this would mean that a critical difference exists between the JSON response of your LSP, compared to the others? Because that's the one that gets parsed for the symbols panel and breadcrumb creation.

kraigher commented 1 year ago

SymbolInformation[] which is a flat list of all symbols found in a given text document. Then neither the symbol’s location range nor the symbol’s container name should be used to infer a hierarchy. DocumentSymbol[] which is a hierarchy of symbols found in a given text document.

The above two ways are supported. I use the DocumentSymbol which is never. There is a clientCapability that says the client supports this which I ignore. I could choose to either disable this feature for a client which does not support the new format or choose to implement the old SymbolInformation[] format. I can see how much work it would be to implement the old format as well.

pidgeon777 commented 1 year ago

I could choose to either disable this feature for a client which does not support the new format or choose to implement the old SymbolInformation[] format.

It would be great to dynamically switch to the old SymbolInformation[] format according to the client's capabilities because by doing so it would be possible to use your LSP tool with other IDEs as well which implement an older LSP protocol on their client side.

kraigher commented 1 year ago

I pushed a commit to master now that uses the old SymbolInformation structure if the client does not support the hierarchical document symbols. Please try and give report back.

pidgeon777 commented 1 year ago

Extremely quick! Just tried, but unfortunately, the same behaviour as before.

It is very strange because I also tried with LUS, C (clangd) LSPs, and all of them don't present this issue (breadcrumb always available and also symbol scope highlight).

kraigher commented 1 year ago

Ok when I read your initial text carefully I see what the true problem is. The DocumentSymbol has two ranges and I fill in the same value for both, because I do not save have the other location information yet from the parser.

/**
     * The range enclosing this symbol not including leading/trailing whitespace
     * but everything else like comments. This information is typically used to
     * determine if the clients cursor is inside the symbol to reveal in the
     * symbol in the UI.
     */
    range: Range

    /**
     * The range that should be selected and revealed when this symbol is being
     * picked, e.g. the name of a function. Must be contained by the `range`.
     */
    selectionRange: Range

This is not really a bug but an expected limitation. I do not compute the full source range of symbols today. This is actually something I am working on right now so hopefully it will become available soon. I want to reword how I store the source ranges a bit before I add more of it into the AST-tree in the current way I do it.

pidgeon777 commented 1 year ago

You're right, that would explain why the current LSP implementation of rust_hdl acts differently from the other LSPs.

We know that your current implementation supports hierarchy, defining parent and children symbols.

Situation is as follows, for selectionRange:

Parent1 [ start = 0, end = 7 ]
  Child1 [ start = 10, end = 16 ]
  Child2 [ start = 19, end = 25 ]
Parent2 [ start = 26, end = 33 ]
  Child3 [ start = 36, end = 42 ]

Actually, start and end delimit the starting and ending positions of the symbol.

Maybe as a temporary workaround, knowing the start and end of each symbol at the different hierarchy levels, we could make use of that information for example to determine the end of the scope for a given symbol?

For example, the values for range would become:

Parent1 [ start = 0, end = Parent2.start ]
  Child1 [ start = 10, end = Parent1.Child2.start ]
  Child2 [ start = 19, end = Parent1.end ]
Parent2 [ start = 26, end = <EOB> ]
  Child3 [ start = 36, end = Parent2.end ]

where <EOB> is the position of the latest character in the buffer.

Maybe this could be a quick and easy temporary workaround to integrate?

kraigher commented 1 year ago

Maybe this could be a quick and easy temporary workaround to integrate?

I can wait until it is done properly, it is just a waste to implement it with a work-around. If some clients renders the selection range on the editor it will also look strange.

Schottkyc137 commented 4 months ago

Some (but not all) selection ranges should now have been included as part of #262