Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Experimental project structure #773

Merged
merged 21 commits into from
Jan 20, 2023

Conversation

scohen
Copy link
Contributor

@scohen scohen commented Nov 17, 2022

Experimental project structure

There is only one commit in this pr, d8b9694

This commit represents a new structure for the experimental project, and a path forward. With these changes the project now has:

  • A build option enabling the experimental server
  • Per-message routing for the experimental server. If enabled, it can
    either share messages with the existing server or take them over.
    Presently, the find references and formatting providers are
    implemented and "exclusive", meaning that they're handled solely by
    the experimental server if it's enabled.
  • A consistent interface for building providers.
  • A consistent way to convert LSP messages into data structures and
    back again. This conversion is handled automatically for providers.
  • A genserver-like interface for providers to implement
  • Data structures representing LSP messages that are simple to define
    and build.
  • Fast and efficient conversion between utf8 and utf16.
  • A separation between what a provider does and how it responds to
    messages. This allows the work that underpins providers to be tested
    independently from the language server.

This branch is designed to be able to be merged without affecting the rest of the project, but if this path looks good, I recommend the following course of action.

  1. We should pause feature development and focus on porting existing features to the experimental server. There aren't that many existing providers, and that should happen fairly quickly. New feature development should take place in the experimental module from then on.

  2. We should also standardize on some type of source code modification, likely https://github.com/doorgan/sourceror. It's an excellent little library that has features that are missing from the elixir macro modules and will allow us to transform source code more readily than before.

  3. We should also focus on feature quality over quantity. The goals for this stage of the project would be completely eliminating crash bugs, increasing performance and correctness and usability of the existing features. If i may pick on two recently added features, Code actions are completely broken and automatic imports have severe usability issues that make them fairly worthless. We should put together and publish a product roadmap for this.

  4. Once the experimental server has gained feature parity with the classic server, we should do a cutover release, at which point the project can start adding features again, with more rigor and confidence than was possible in the past.

FYI, I can work on this pretty much full time until March.

@scohen scohen mentioned this pull request Nov 17, 2022
@scohen scohen force-pushed the scohen/references-request branch 2 times, most recently from 9c7e44e to 5322d1d Compare November 21, 2022 23:38
@scohen scohen marked this pull request as ready for review November 30, 2022 03:45
@scohen scohen force-pushed the scohen/references-request branch 2 times, most recently from a5f79da to 1a3692f Compare December 2, 2022 06:17
@scohen scohen force-pushed the scohen/references-request branch 2 times, most recently from 48fd12f to 443b959 Compare December 19, 2022 23:57
This commit represents a new structure for the experimental project, and
a path forward. With these changes the project now has:

  * A build option enabling the experimental server
  * Per-message routing for the experimental server. If enabled, it can
    either share messages with the existing server or take them over.
    Presently, the find references and formatting providers are
    implemented and "exclusive", meaning that they're handled solely by
    the experimental server
  * A consistent interface for building providers.
  * A consistent way to convert lsp messages into data structures and
    back again. This conversion is handled automatically for providers.
  * A genserver-like interface for providers to implement
  * Data structures representing LSP messages that are simple to define
    and build.
  * Fast and efficient conversion between utf8 and utf16.
  * A separation between what a provider does and how it responds to
    messages. This allows the work that underpins providers to be tested
    independently from the language server.
Created a code action that prepends an underscore to unused variable
names.
First attempt at a standard interface for code modification. Code mod
modules take the original text, the ast of the original text and
arguments that they specify. They return a list of code edits or an
error.
The problem was that the character positions are _before_ the reported
unit, so the 0th code unit is before the start of the line and the 1st
code unit is the first character. The prior code added one to
character counts to smooth this out, but you can't do that, because
you could end up indexing into the middle of a multibyte character.
Code mods deal with snippets of code that need to have their line
numbers fixed up by the code actions.
The AST type is very complicated, and dialyzer was telling us I got it wrong.
While working on the automatic protocol generators, it became clear
that type aliases needed to be their own thing, as they operate quite
differently from the other defined things in the jsonrpc
protocol. Since they're just aliases, it makes sense to keep their
definitions on hand and then spit them out when other things make use
of them during encode and decode.

This did require going back to encoding and ensuring all the encode
functions return OK tuples.
When patches are unapplied, getting the beam file returned an empty
path charlist, which dialyzer assumed was a real file name due to a
weak assumption, which caused unit tests to fail. This was remedied by
checking for a non-empty charlist, which allows tests to succeed.

Also made patch a test only dependency for .formatter.exs, as this was
causing formatters to fail.
Under 1.12, Macro.to_string proudces wonky output, making `def` calls
look like function calls by adding needless parenthesis. These
parenthesis throw off the diff algorithm, and caused an off-by-one
error in the code mod.
Sourceror has backported the newer code generation so that it's
compatible all the way back to 1.10, and produces the correct output.
Patch's assertions will fail in CI due to `mix format
--check-formatted` running in dev. Importing patch's deps in test will
fix this.
@scohen
Copy link
Contributor Author

scohen commented Jan 19, 2023

@lukaszsamson This is ready to go.
One possible controversial decision: I added sourceror as a dependency in order to get AST -> source code consistency across elixir versions.

@lukaszsamson lukaszsamson merged commit 29b91a6 into elixir-lsp:master Jan 20, 2023
@scohen scohen deleted the scohen/references-request branch January 23, 2023 18:31
@robmckinnon
Copy link

robmckinnon commented Mar 9, 2023

@lukaszsamson This is ready to go. One possible controversial decision: I added sourceror as a dependency in order to get AST -> source code consistency across elixir versions.

Shall we bump sourceror to v0.12? It has some non-backwards compatible changes, particularly the removal of "ended" zippers.

I've started to develop a refactor.extract.function code action that utilizes sourceror. I've already ported my extract function code to work with sourceror v0.12.1.

Shall I develop the refactor.extract.function code action in the Experimental project structure, and PR when ready?

@scohen
Copy link
Contributor Author

scohen commented Mar 9, 2023

Shall we bump sourceror to v0.12?

Yes, feel free to bump it

Shall I develop the refactor.extract.function code action in the Experimental project structure

Go for it.

@robmckinnon
Copy link

Shall we bump sourceror to v0.12?

Yes, feel free to bump it

I've opened a PR to bump sourceror version: #841

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants