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

PHP 5 runtime support #23

Closed
felixfbecker opened this issue Sep 10, 2016 · 14 comments
Closed

PHP 5 runtime support #23

felixfbecker opened this issue Sep 10, 2016 · 14 comments

Comments

@felixfbecker
Copy link
Owner

felixfbecker commented Sep 10, 2016

We would lose the following features:

  • scalar type hinting
  • return type annotations
  • group use statements
  • anonymous classes
  • ?? operator

Is there anything like BabelJS for PHP that transpiles it?

@felixfbecker
Copy link
Owner Author

felixfbecker commented Sep 18, 2016

Since a lot of people ask for this: I like the new features of PHP 7 and use PHP 7 in all my projects. I understand though that many do not. So if you care about this, and want to do the refactoring work (low hanging fruit really compared to other features), I will happily merge a PR, but I myself will rather concentrate on features. I would also prefer a transpilation solution if that is possible.

@batopa
Copy link

batopa commented Sep 23, 2016

I found this PHP transpiler https://github.com/jaytaph/Transphpile but I don't know if it works well.
Anyway if it works well how should work the transpiling action? I mean do you intend to release two version of the plugin or what?

@felixfbecker
Copy link
Owner Author

@batopa I would write the source in PHP 7, but before releasing transpile to PHP 5.

Actually, I don't know atm if Composer supports this. I come from a Node background and on NPM it is pretty common to transpile code before releasing...

@batopa
Copy link

batopa commented Sep 23, 2016

Composer permits something similar https://getcomposer.org/doc/articles/scripts.md.
How do you have to release the package for work in vscode?

@felixfbecker
Copy link
Owner Author

felixfbecker commented Sep 23, 2016

The VS Code extension has a composer.json and the vendor directory is bundled with each release.

@felixfbecker
Copy link
Owner Author

felixfbecker commented Sep 23, 2016

So we would add the transpiler as devdep and call it in a post-package-install script?

@batopa
Copy link

batopa commented Sep 23, 2016

Yes that is the idea. But frankly for me it would be preferable to have two versions of the plugin. The transpiler seems immature and use it for production seems risky to me.

Having something like a stable version for PHP >= 7 and a preview version transpiled for PHP 5.6.

@felixfbecker
Copy link
Owner Author

I am against two versions. We should rather find a way to only transpile src, not tests, and then run the tests against the transpiled version (in CI). Only need to modify the autoloading to point to the dist folder. That way we can 100% verify everything works (well, 90% with current coverage :P).

@batopa
Copy link

batopa commented Sep 23, 2016

Meanwhile I'll have a look at transpiler to see how it works

@batopa
Copy link

batopa commented Sep 27, 2016

The issue is more complicated than I expected.
The thing is that we need to transpile src and test and also the vendor package AdvancedJsonRpc (src and test) that require PHP 7. Then we should run tests against the transpiled version both in php-language-server and in php-advanced-json-rpc

The transpiler works quite well but I have some issues with anonymous classes (I found it in \LanguageServer\Tests\Server\TextDocumentTest) and other little things.

@felixfbecker
Copy link
Owner Author

Since Im the author of that lib as well it's no problem to transpile it as well :)
The anonymous class could get replaced by some mocking framework.

@Ciantic
Copy link

Ciantic commented Oct 6, 2016

Does this mean the Language Server should run in PHP 5? Or does this mean it should be able to parse PHP 5?

When people ask for PHP 5 support, they probably mean their code is in PHP 5 and think since this runs on PHP 7, their code can't be parsed / validated. Even though it seems to work just fine on my PHP 5 source files.

I think having language server written as PHP 5 is useless waste of time, since this project works just fine with PHP 5 source files. Developers can easily install PHP 7 just for running the language server IMHO.

@felixfbecker
Copy link
Owner Author

@Ciantic this is just about running the language server in PHP so people don't have to install two versions side-by-side. Parsing PHP 5 is totally fine, and depends only on the PHPParser. As I stated, I will not put any work into this, but if someone wants to set up a transpilation solution I would merge a PR.

@felixfbecker felixfbecker changed the title PHP 5 support PHP 5 runtime support Oct 6, 2016
@felixfbecker
Copy link
Owner Author

felixfbecker commented Oct 26, 2016

Declining this because some dependencies require PHP 7 now.
Please note that parsing PHP5 files is and has always been supported.

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

No branches or pull requests

3 participants