JakeBecker / elixir-ls

A frontend-independent IDE "smartness" server for Elixir. Implements the JSON-based "Language Server Protocol" standard and provides debugger support via VS Code's debugger protocol.
Apache License 2.0
846 stars 52 forks source link

Prevent @spec codelens for implemented callbacks #162

Closed jeroenvisser101 closed 4 years ago

jeroenvisser101 commented 5 years ago

When callbacks are marked with @impl (true | mod) their typespec/contract is inferred from the specified implementation.

In this forum post @fishcakez mentions this:

In general it does not make sense to infer the callback as the spec for explicit calls because the callback implementation will likely have a subset of the callback definition. For example with GenServer the state can be term but it is likely required to be a struct.

Normally for callback implementations the code is never called directly and so dialyzer can’t infer it is called. Often it will be called using a variable: mod.callback_fun(arg)

Given a very generic behaviours such as GenServer where the type of the state is unknown, it might be better to introduce a custom spec, but in the case of less generic behaviours, inferring seems the right thing to do.

Anyway, very much debatable if this is the right thing to do, and after having implemented this change I'm less certain this is a good change. Can we discuss the pros/cons of this?

JakeBecker commented 4 years ago

This project has moved!

It's now being maintained by proactive volunteers from the Elixir community over at elixir-lsp/elixir-ls. Updates will continue to be published from that repo to the original VS Code extension, so no need to switch plugins if you're using VS Code.

If you're still interested in merging this PR, please do the following:

  1. Check that the issue you're addressing still occurs with the latest version of the VS Code plugin (which is published from the new repo)
  2. If your PR references a Github issue, check if there's already an issue filed on the new repo and create one if not
  3. Rebase your PR from the new repo (which has diverged significantly) and re-submit it there.

Thank you!