WIP: Performance/Feature Improvements to the Language Server

[tm1000]

This is something I've been working on for the past year with my fellow coworkers @bsandstrom @berrugo @GameGamer43 and @xrobau in v4 but just brought it to v5. Keeping it as a draft here against v5 while I work through tests and other issues though it currently works as is

Status: Updated to https://github.com/vimeo/psalm/releases/tag/5.7.7

TODO:

• Fixed errored test case
• Add back internal functions that were removed
• Fix dynamic configuration changes

Fixed:

• LSP: completion returns snippets when client does not support them #4139
• Incorrect issues classification when using baseline #9180

Added:

• ClientConfiguration class to store client configuration for the LSP
• Dynamically request configuration from client.
  • This means that clients can now set updates and not have to restart Psalm
• PHPMarkdownContent class that extends LanguageServerProtocol\MarkupContent.
  • This helps to standardize how hovers look in clients across the usage in Psalm
• Provide hover analysis results for any context in php
  • This actually ends up providing decent generic support (At least in my usage)
• Provide more granular control over what the LSP does:
  • --enable-code-actions Enables or disables code actions. Default is true.
  • --enable-provide-diagnostics Enables or disables providing diagnostics. Default is true.
  • --enable-provide-hover Enables or disables providing hover. Default is true.
  • --enable-provide-signature-help Enables or disables providing signature help. Default is true.
  • --enable-provide-definition Enables or disables providing definition. Default is true.
  • --show-diagnostic-warnings Enables or disables showing diagnostic warnings. Default is true. (continuation of Ability to hide warnings in Language Server #7379)
• Added --on-change-debounce-ms which helps to control CPU when working with large codebases . Default is 500ms
• Created Reference class to pass file reference information around instead of passing arrays (which are hard to type)
• AsyncTestCase base Test for testing anything in the LSP, this extends Amp\PHPUnit\AsyncTestCase and is now being used in a few of the test cases
• Added --use-baseline= to use a baseline file that was generated from Psalm CLI. This will turn any errors into warnings. The warnings can then be hidden with --show-diagnostic-warnings=false

Changed:

• Clear file cache for server on boot
• Created a Progress helper class that extends Psalm\Progress\Progress to help forward Psalm messages back to the LSP. This means Psalm's LSP is much more verbose than before
• Temp file array now contains the version of the file that was changed/opened as per the LSP spec, this value is then sent back to the client so that it knows when to ignore old updates.
• Because of the addition of --on-change-debounce-ms updates to the client are not always instant (comparatively to v4 which was 0ms) (default value for this is 500ms which is 1/2 second). This can be changed but it should be noted (I do not see a big difference in large code bases myself but it helps to prevent Psalm from backing up with change events and has actually helped the overall dev experience in my IDE)
• No longer run doAnalysis when event readMessageGroup is received. The reasoning for this is because when [Feat]: Initial Support for LSP Code Actions #7255 was done it caused a huge influx of readMessageGroup events

Testing:

composer.json:

    "require-dev": {
        "vimeo/psalm": "dev-feature/upgrade-lsp-v5 as 5.7.7"
    },
    "repositories": [
        {
            "type": "vcs",
            "url": "https://github.com/tm1000/psalm"
        }
    ]

vscode settings:

    "psalm.psalmVersion": "5.7.7",
    "psalm.psalmScriptArgs": [
        "--on-change-debounce-ms=500",
        "--show-diagnostic-warnings=false"
    ],
    "psalm.psalmScriptPath": "app/vendor/bin/psalm-language-server",

Note that psalmScriptPath is specific to my setup. The point is you want to use psalm-language-server not psalm (even though psalm does a pass-thru to psalm-language-server)

Meta

Fixes #4139
Fixes #9180

[tm1000]

The CI is strangely bugged on this PR. It refuses to run any tests except for two

I had to resolve the merge conflict with master

[tm1000]

@ging-dev in regards to the usage of pcntl_fork thats used when its initiated as a TCP server (which I don't use).

In vscode I can see how only one server services one connection but I dont know how this applies to people not using vscode, in those cases they technically could service n+1 connections even though I think thats wrong (one server should be used for one IDE and so forth)

Thinking about this more I think each fork of the language server should only accept 1 connection because of how it traces open/closed files internally. If two IDEs connect to the same server and both open the same file but one closes it then the second IDE will lose it's place because the server thinks the file is closed.

[weirdan]

@tm1000 I added the Meta section to the PR description so that issues this pull request fixes are properly linked.

[tm1000]

Technically this is still WIP and draft but I undrafted it to fix the non running checks.

[tm1000]

the language server uses pcntl_fork() to multiprocess, but the processes use the same cache. Not sure if it works or not but has the potential to cause race condition. Should consider deleting it and using In-memory cache?
In-memory cache in tests:
master/tests/Internal/Provider?

This was the magic to get the LSP to work without a cache directory.

Additionally I needed to then set $config->cache_directory = null; so that it wouldnt create any directories.

After the above changes everything is now in memory and as a result Psalm's LSP doesnt have to wait on disk I/O so it's faster as a result

[tm1000]

It seems the in memory cache still has issues vs the file base cache. I'll have to investigate further. Maybe add a switch as well

[tm1000]

added experimental --in-memory option. For some reason when using this option the reflected in file changes are very delayed or not reflected at all. I suspect this is happening because of a race condition where the disk IO normally blocks but because the in memory is so fast things are getting missed

[tm1000]

Technically this could be merged as-is right now and it would function without any breaking changes. There is nothing holding this up anymore. All checks now pass.

When I get time I will add additional tests. Either on this PR or a new one.

[orklah]

Thanks!