Closed awilliambauer closed 5 months ago
Changes Missing Coverage | Covered Lines | Changed/Added Lines | % | ||
---|---|---|---|---|---|
ocaml-lsp-server/src/code_actions/action_update_signature.ml | 14 | 15 | 93.33% | ||
ocaml-lsp-server/src/inference.ml | 91 | 109 | 83.49% | ||
<!-- | Total: | 112 | 131 | 85.5% | --> |
Files with Coverage Reduction | New Missed Lines | % | ||
---|---|---|---|---|
ocaml-lsp-server/src/inference.ml | 10 | 65.31% | ||
<!-- | Total: | 10 | --> |
Totals | |
---|---|
Change from base Build 4265: | 0.3% |
Covered Lines: | 5345 |
Relevant Lines: | 24529 |
LGTM after a couple of changes:
removed ppx_let
in favor of the syntax that comes with OCaml. It's not as good as ppx_let
, but for the simple use cases here it's enough and it's nice to not require users to install ppx for lsp
remove all the base references to Import
. Eventually, we should just write include Base
in Import
and replace Stdune
completely, but that's a bit too much for this PR.
This PR improves the
insert-inferred-interface
code-action by restricting the set of signatures inserted to only the names not already present in the interface file. Previously, this code action was only really useful on a completely empty .mli file, since all signaures from the corresponding .ml file were inserted, including duplicates of signatures already present in the .mli.Repeated applications of this code-action will now more appropriately be no-ops.
This PR also adds an
update-signatures
code action that updates the signatures for selected elements of the interface based on types inferred from the corresponding implementation.The code action works by checking for an implementation file corresponding to the current interface and querying Merlin for type analysis on both. It then extracts items from the interface that overlap with the range the user has selected and attempts to identify matching items from the inferred interface of the implementation. For any matching items, it asks Merlin to print signatures for the old and new types, and if those strings differ, it produces a text edit for the interface to make it match the implementation.
Testing
I added a new expect-test that confirms the inserted signatures exclude ones already present in the interface. The existing expect-test that assumed an empty .mli is unaffected.
New expect-tests verify the intended behavior of
update-signatures
in the following situations:This can be slow (~2s) on very large .mli files (~5k lines), but I've confirmed that the pre-existing version of
insert-inferred-interface
took a similarly long time.