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.

Sign up for GitHub

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

Jump to bottom

PHP support using bmewburn/intelephense #259

Closed
josecanciani opened this issue Jan 25, 2018 · 28 comments
Closed

PHP support using bmewburn/intelephense #259

josecanciani opened this issue Jan 25, 2018 · 28 comments

Comments

@josecanciani
Copy link

Current server (https://github.com/felixfbecker/php-language-server) explained in https://lsp.readthedocs.io/en/latest/ is very slow.
There are two projects for VS-Code that are much faster and complete:

  1. https://github.com/bmewburn/intelephense (this seems to best one today)
  2. https://github.com/HvyIndustries/crane

Thanks!
@tomv564
Copy link
Contributor

tomv564 commented Jan 25, 2018

Thanks for the pointers, perhaps this will inspire someone to try to get them working in LSP!

Both of these projects appear to only contain support for Visual Studio Code - intelephense is a library and Crane uses Node IPC (although easy to add tcp/stdio support). Do you know if there are stdio or tcp - based servers available around them? (eg. vim/emacs users may need standalone servers as well)

If not, unfortunately these projects cannot have any direct utility to LSP users in their current state, so it would be misleading to even advertise them in the documentation.

@josecanciani
Copy link
Author

I will ask in their issue trackers, I could not find how to run them standalone. I'll reply here once I find the answer.

@josecanciani
Copy link
Author

https://github.com/bmewburn/intelephense/issues/115
HvyIndustries/crane#375

@josecanciani josecanciani mentioned this issue Jan 26, 2018
Closed
@josecanciani
Copy link
Author

This was bmewburn's answer (from Intelephense):

Thanks for your interest, I'd like to see it used in other editors. The current limitation is that this is a lib that needs to be wrapped in a server layer that handles the language server protocol. I use this for vscode - https://github.com/Microsoft/vscode-languageserver-node/tree/master/server. I'm not sure how vscode specific it is, I suspect very little despite the name. See https://github.com/bmewburn/vscode-intelephense/tree/master/server for the vscode intelephense server that uses the above package. Theres some package.json compile and watch scripts specific to vscode extensions but apart from that this may even be able to be used as is if you have a client that can spawn the server and make lsp calls to it.

For what I saw, the server users a node's IPC stream, but it appears it accept any io stream, so we may be able to create a server using tcp I suppose.

@tomv564
Copy link
Contributor

tomv564 commented Jan 26, 2018

we may be able to create a server using tcp I suppose.

I encourage you (as a PHP developer) to give this a try, see https://github.com/tomv564/lsp-tsserver/blob/a8548e183a69c048799e560819a3b7b1f08661d7/server/src/server.ts#L17 for an example of how to configure vscode-languageserver-node for stdio.
Crane also uses vscode-languageserver-node.

I hope it's okay if I close this issue for now, but please (re) open when we can document with a working example!

@tomv564 tomv564 closed this as completed Jan 26, 2018
@josecanciani
Copy link
Author

Thanks! I'll give it a try

@josecanciani
Copy link
Author

josecanciani commented Jan 27, 2018 •
edited
Loading

Hello! I had some luck. Here's what I did:

  1. Ugraded to node 9 (not sure if really needed)
  2. cloned https://github.com/bmewburn/vscode-intelephense
  3. Modified the code as suggested, just had to import the right classes:
@@ -11,13 +11,14 @@ import {
        CompletionItem, CompletionItemKind, RequestType, TextDocumentItem,
        PublishDiagnosticsParams, SignatureHelp, DidChangeConfigurationParams,
        Position, TextEdit, Disposable, DocumentRangeFormattingRequest,
-       DocumentFormattingRequest, DocumentSelector, TextDocumentIdentifier
+       DocumentFormattingRequest, DocumentSelector, TextDocumentIdentifier,
+    StreamMessageReader, StreamMessageWriter
 } from 'vscode-languageserver';
 
 import { Intelephense, IntelephenseConfig, InitialisationOptions, LanguageRange } from 'intelephense';
 
 // Create a connection for the server. The connection uses Node's IPC as a transport
-let connection: IConnection = createConnection(new IPCMessageReader(process), new IPCMessageWriter(process));
+let connection: IConnection = createConnection(new StreamMessageReader(process.stdin), new StreamMessageWriter(process.stdout));
 let initialisedAt: [number, number];
 
 const languageId = 'php';
  1. run npm install to load dependencies
  2. compile the typescript file (tsc server/src/server.ts)
  3. added this configuration to sublime LSP settings:
{
    "clients": {
        "phpls": {
            "command": [
                "node",
                "~/www/vscode-intelephense/server/src/server.js"
            ],
            "enabled": true,
            "initializationOptions": {
                "nodePath": "/usr/bin/node"
            },
            "languageId": "php",
            "scopes": [
                "source.php",
                "embedding.php"
            ],
            "syntaxes": [
                "Packages/PHP/PHP.sublime-syntax"
            ]
        }
    },
    "log_debug": true,
    "log_payloads": true,
    "log_stderr": true,
    "only_show_lsp_completions": true
}

The last options are just for testing. I have mixed results. The server appears to be running fine, and there are moments when it works, but I have some errors. Here's the log:

Notification handler 'textDocument/didChange' failed with message: Cannot read property 'start' of undefined:
LSP:      {'isIncomplete': False, 'items': [{'kind': 6, 'label': '$this', 'documentation': 'Refers to the current object', 'detail': 'M_customreport_builder_ExpressionBase'}]}
LSP:  --> textDocument/didChange
LSP:  --> textDocument/completion
Notification handler 'textDocument/didChange' failed with message: Cannot read property 'start' of undefined:
LSP:      {'isIncomplete': False, 'items': [{'kind': 10, 'label': 'calculateSubTotal', 'detail': ''}, {'kind': 10, 'label': 'options', 'detail': ''}, {'command': {'command': 'editor.action.triggerParameterHints', 'title': 'Trigger Parameter Hints'}, 'sortText': 'zzz', 'insertText': '__construct($0)', 'kind': 2, 'label': '__construct', 'insertTextFormat': 2, 'detail': '__construct($name, $alias, $label, M_customreport_builder_ExpressionCollection $links, $calculateSubTotal, M_customreport_json_OptionList $options, $id)'}, {'kind': 2, 'sortText': 'isOrderable', 'label': 'isOrderable', 'insertText': 'isOrderable()', 'detail': 'isOrderable()'}, {'kind': 2, 'sortText': 'getClone', 'label': 'getClone', 'insertText': 'getClone()', 'detail': 'getClone()'}, {'kind': 2, 'sortText': 'toJsonValue', 'label': 'toJsonValue', 'insertText': 'toJsonValue()', 'detail': 'toJsonValue()'}, {'command': {'command': 'editor.action.triggerParameterHints', 'title': 'Trigger Parameter Hints'}, 'sortText': 'setCalculateSubtotal', 'insertText': 'setCalculateSubtotal($0)', 'kind': 2, 'label': 'setCalculateSubtotal', 'insertTextFormat': 2, 'detail': 'setCalculateSubtotal($calculateSubTotal)'}, {'kind': 2, 'sortText': 'getCalculateSubTotal', 'label': 'getCalculateSubTotal', 'insertText': 'getCalculateSubTotal()', 'detail': 'getCalculateSubTotal()'}, {'kind': 2, 'sortText': 'removeLinks', 'label': 'removeLinks', 'insertText': 'removeLinks()', 'detail': 'removeLinks()'}, {'command': {'command': 'editor.action.triggerParameterHints', 'title': 'Trigger Parameter Hints'}, 'sortText': 'setOptions', 'insertText': 'setOptions($0)', 'kind': 2, 'label': 'setOptions', 'insertTextFormat': 2, 'detail': 'setOptions(M_customreport_json_OptionList $options)'}, {'command': {'command': 'editor.action.triggerParameterHints', 'title': 'Trigger Parameter Hints'}, 'sortText': 'addOption', 'insertText': 'addOption($0)', 'kind': 2, 'label': 'addOption', 'insertTextFormat': 2, 'detail': 'addOption($name, $value)'}, {'command': {'command': 'editor.action.triggerParameterHints', 'title': 'Trigger Parameter Hints'}, 'sortText': 'getOption', 'insertText': 'getOption($0)', 'kind': 2, 'label': 'getOption', 'insertTextFormat': 2, 'detail': 'getOption($name)'}, {'kind': 2, 'sortText': 'getOptions', 'label': 'getOptions', 'insertText': 'getOptions()', 'detail': 'getOptions()'}, {'kind': 2, 'sortText': 'zzz', 'label': '__clone', 'insertText': '__clone()', 'detail': '__clone()'}]}
LSP:  --> textDocument/didChange
Notification handler 'textDocument/didChange' failed with message: Cannot read property 'start' of undefined:
LSP:  --> textDocument/documentHighlight
LSP:      [{'kind': 2, 'range': {'start': {'line': 8, 'character': 12}, 'end': {'line': 8, 'character': 20}}}, {'kind': 2, 'range': {'start': {'line': 17, 'character': 15}, 'end': {'line': 17, 'character': 22}}}, {'kind': 2, 'range': {'start': {'line': 114, 'character': 15}, 'end': {'line': 114, 'character': 22}}}, {'kind': 2, 'range': {'start': {'line': 118, 'character': 20}, 'end': {'line': 118, 'character': 27}}}, {'kind': 2, 'range': {'start': {'line': 121, 'character': 15}, 'end': {'line': 121, 'character': 22}}}, {'kind': 2, 'range': {'start': {'line': 125, 'character': 22}, 'end': {'line': 125, 'character': 29}}}, {'kind': 2, 'range': {'start': {'line': 129, 'character': 22}, 'end': {'line': 129, 'character': 29}}}]
LSP:  --> textDocument/didChange
Notification handler 'textDocument/didChange' failed with message: Cannot read property 'start' of undefined:
LSP:  --> textDocument/didChange
Notification handler 'textDocument/didChange' failed with message: Cannot read property 'start' of undefined:
LSP:  --> textDocument/didClose
LSP:  --> textDocument/didChange
LSP:  --> textDocument/completion
Notification handler 'textDocument/didChange' failed with message: Cannot read property 'start' of undefined:
LSP:      {'isIncomplete': False, 'items': []}
LSP:  --> textDocument/didChange
Notification handler 'textDocument/didChange' failed with message: Cannot read property 'start' of undefined:
LSP:  --> textDocument/didChange
LSP:  --> textDocument/completion
Notification handler 'textDocument/didChange' failed with message: Cannot read property 'start' of undefined:
LSP:      {'isIncomplete': False, 'items': []}
LSP:  --> textDocument/didChange
Notification handler 'textDocument/didChange' failed with message: Cannot read property 'start' of undefined:
LSP:  --> textDocument/documentHighlight
LSP:      []
LSP:  --> textDocument/didChange

I constantly get the "cannot read property 'start' of undefined, not sure whose error is it. I'm just trying to get an autocomplete with a public method from my parent class, but it's not getting it.

Any ideas?

@tomv564
Copy link
Contributor

tomv564 commented Jan 27, 2018 via email

@josecanciani
Copy link
Author

Great, thanks! Let me know if I can do anything to help.

@bmewburn
Copy link

Hello, looks like this is an intelephense issue. I wasn't following the spec here which allows undefined range. Fixed in master.

@josecanciani
Copy link
Author

wow, that was quick too. I'll keep playing with it tomorrow and let you know.

@josecanciani
Copy link
Author

josecanciani commented Jan 28, 2018 •
edited
Loading

@bmewburn Great, now there's no errors I can see. Question: I opened a file and triggered an autocomplete on $this->, expecting to see the parent classes's public and protected methods. I could not see them, but after I opened and close the parent class, they started appearing.
Is this because the indexing was not complete? If that's the case, some questions/suggestions:

  1. It would be nice that if the client asks for something in a file, the server could find class symbols in that file and try to index them too before replying (for what I read, LSP is supposed to work even during indexing). It seems it is only parsing the current file. I suppose it could be problematic if there are a lot of classes and the tree is big. Maybe there could be a recursion limit or something like that so it doesn't hang forever. Anyway, treat this as a suggestion only :)

  2. Is there a way we can inform the client the indexing status? It would be nice to know at what point we are (not sure if that's already in the SPEC). I've noticed that in vscode there's a status message telling you the progress.

  3. For easy configuration, we would need to create a new project that can be installed with npm. Something like "php-intellephense-server-io" (io for the stdin/stdout version, but I guess the listen method could be sent via a parameter). Today the server is in a folder inside your project but the rest of the code is not really needed, as it's added by npm install automatically on build. Is that something you'll be interested on doing? To avoid code duplication, you can add the server as a submodule in your main project.

This is looking great guys!!

Jose

@bmewburn
Copy link

bmewburn commented Jan 28, 2018 •
edited
Loading

Nice work, @josecanciani !

  1. There is some client side code that handles indexing in vscode. It uses the vscode api to get a list of workspace files -- workspace.findFiles -- and then sends the documents to the server. So an implementation of this function and the indexing logic would need to be written into the server to work with sublime LSP.
  2. This notification is from the client but could be moved to the server with above.
  3. I'm interested in forking https://github.com/bmewburn/vscode-intelephense/tree/master/server so the server can be used with any client and creating an npm package. I think the connection class params are optional and it will default to whatever cmd line flags are passed. There's a couple of things blocking me from working on the issues in 1 & 2 for now -- time and some of that indexing logic above will be changing soon with the next release of intelephense.

@josecanciani
Copy link
Author

That's great! When you have the server as a standalone package, can you reply back here so that @tomv564 can update the documentation?

I tried it a bit more and I notice that static methods (ClassName::method) are not being suggested, and also that new ClassName or directly start typing a class name is not suggesting anything. I don't know if that's expected (haven't used vscode enough to compare), or if there's something the client needs to inform when triggering the auto complete.

Jose

@tomv564 tomv564 reopened this Jan 28, 2018
@bmewburn
Copy link

bmewburn commented Feb 3, 2018

I forked to https://github.com/bmewburn/intelephense-server and npm published intelephense-server@0.8.5-alpha.1 . I've removed the IPC params and will continue working on it when I have the time.

This was referenced Feb 3, 2018
Closed
Closed
@tomv564
Copy link
Contributor

tomv564 commented Feb 11, 2018

For documentation's sake, here is a configuration that allows a successful startup on OS-X after npm install -g intelephense-server

    "intelephense-ls":
    {
      "command": ["node", "/usr/local/lib/node_modules/intelephense-server/lib/server.js", "--stdio"],
      "scopes": ["source.php", "embedding.php"],
      "syntaxes": ["Packages/PHP/PHP.sublime-syntax"],
      "languageId": "php",
      "initializationOptions": {
        "storagePath": "/tmp/intelephense"
      }
    },

I'll close this issue for now but link it in the documentation for PHP support.

@tomv564 tomv564 closed this as completed Feb 11, 2018
@josecanciani
Copy link
Author

Thank you both! I'll switch to the new server.
@bmewburn, great work, I hope you can keep moving features to the server as possible :) Will the server always use the last version automatically, right?
Thanks you guys again.
Jose

@bmewburn
Copy link

@josecanciani the vscode intelephense client now depends on intelephense-server so there will be new versions published periodically. You'll have to npm upgrade -g intelephense-server to get new versions.

@joehoyle
Copy link

joehoyle commented Apr 1, 2018

Does anyone have a way to index the full project in Sublime with intelephense-server? If I understand it correctly, the intelephense-server doesn't index a whole directory - it expects to be passed each file post-initiation, which is what the VS Code plugin does?

@joehoyle
Copy link

joehoyle commented Apr 2, 2018 •
edited
Loading

FYI I totally hacked in index-project support in master...joehoyle:index-project

@tomv564
Copy link
Contributor

tomv564 commented Apr 2, 2018

Nice, is that really what the VS Code plugin does?
Indexing to get all workspace symbols etc. feels like a job for the server.

If you are expecting the client to open and return syntax errors for all files when opening a workspace, this was never in the VS Code / LSP model (as it will choke servers on large projects)

@jfcherng
Copy link
Contributor

jfcherng commented Apr 2, 2018 •
edited
Loading

I would like to know that is there a way to make LSP's autocompletion (function parameters hinting) work without openning the definition file every time when I start Sublime Text. Linting only on the current opened file is okay and reasonable for me. But it is really weird that I have to open the definition file manually before the autocompletion works.

But I am not sure should this be done by LSP or the server.

@joehoyle
Copy link

joehoyle commented Apr 2, 2018

Nice, is that really what the VS Code plugin does?

Yeah, it's in https://github.com/bmewburn/vscode-intelephense/blob/master/src/extension.ts#L205. Also LSP doesn't dictate the server should do it, didOpen is (though maybe arguable) just to tell the LSP-server about the existence of a file, not literally that the file is open for the user, see https://microsoft.github.io/language-server-protocol/specification#textDocument_didOpen.

If you hadn't seen it, there;s usuful discussion in microsoft/language-server-protocol#54 on this, which appears to lean in the direction of the client passing all files with existing APIs

@bmewburn
Copy link

bmewburn commented Apr 3, 2018

The plan is to move indexing to the server (bmewburn/intelephense-server#1) in an effort to make it simpler to integrate with any editor. It ended up client side to leverage the vscode find files API and I was of the opinion that the server should be workspace ignorant and work on what it receives from the client.

@tomv564
Copy link
Contributor

tomv564 commented Apr 3, 2018

Thanks for chiming in!

@joehoyle: Following the code in https://github.com/bmewburn/vscode-intelephense/blob/cdd5f66f09e8ee26a2d69a0799ba761c90bc8d43/src/workspaceDiscovery.ts#L10-L11 , it looks like custom discoverSymbols and discoverReferences requests are used (didn't see any textDocument/didOpen notifications ).

@joehoyle
Copy link

joehoyle commented Apr 3, 2018

@tomv564 ah yes you are right - I don't know if this repo makes use of discoverSymbols or discoverReferences, but the same applies, currently we'd need to tell the LSP server about each file, whether that's via didOpen or discoverSymbols. That would all be moot of course if bmewburn/intelephense-server#1 is completed, @bmewburn any clues on how high a priority that one is?

@bmewburn
Copy link

bmewburn commented Apr 3, 2018

@joehoyle this one is high on the to do list and will be part of next release.

@EmilMoe
Copy link

EmilMoe commented Apr 25, 2018

Just a quick question here as I am trying to make it work, do I need to do something in order to make intelephense-ls do an indexing?

@rwols rwols mentioned this issue Sep 8, 2018
Closed
@khanhicetea khanhicetea mentioned this issue Dec 2, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@joehoyle @EmilMoe @josecanciani @tomv564 @bmewburn @jfcherng