[feature request] add a flag to specify tool location

Question

[feature request] add a flag to specify tool location

ju1ius opened this issue 2 years ago · 14 comments

Hi,

For IDEs to be able to provide autocompletion for a library compiled as a PHAR, they must be able to include it's content, which is only possible if the PHAR file has the .phar extension.

Currently phive install allows to change the target directory but not the target location (the location attribute in phars.xml).

For example when running phive install -c phpunit, it creates a phpunit file inside the tools directory of the project. But if you then write a test case like this:

class MyTest extends \PHPUnit\Framework\TestCase {}

Your IDE will complain that the \PHPUnit\Framework\TestCase doesn't exist.

So now you have to:

manually rename tools/phpunit to tools/phpunit.phar
manually edit the location attribute in .phive/phars.xml to add the .phar extension

A minor nuisance, but which gets repeated for every project.

So it would be nice if we could either:

have a flag to specify the full path of the installed phar, i.e.: phive install -c -l tools/phpunit.phar phpunit
have a flag and/or config option to tell phive to always add/keep the .phar extension.

Thanks!

Answer 1 · 2023-07-24T13:29:36.000Z

Duplicate? #116

Answer 2 · 2023-07-24T13:45:36.000Z

The correct answer to fix the auto complete issue would be to have phpunit stubs installed rather than (ab)using the phar runtime. I'm working on that, and hopefully i'll be able to build this soon.

Answer 3 · 2023-07-24T18:48:34.000Z

Hi,

Duplicate? #116

Indeed seems like a dupe, sorry for missing that.

The correct answer to fix the auto complete issue would be to have phpunit stubs installed rather than (ab)using the phar runtime. I'm working on that, and hopefully i'll be able to build this soon.

Woule you care to elaborate?

I fail to see how this is abusing the phar runtime since:

include-ing phar archives is an explicit feature (actually the second example of the documentation).
putting them in your include path works out-of-the-box for autocompletion (in PHPStorm at least), provided the phar archive has the correct extension.

Taking a step back, the deeper issue is that the phpunit phar bundles the test-runner and the library together. Whereas the test-runner shouldn't be a direct dependency of your test suite, the library is a de-facto hard dependency.

But even if there were two separate archives, you would still need to tell your IDE to include the library, which wouldn't work without the .phar extension.

Providing separate stubs just adds the need to configure the IDE to load them properly without causing conflicts, adds the risk of desynchronization between versions, and makes the go to definition feature bring you to a stub file instead of the real implementation. All in all it's just a waste of bandwith 🌱 since it provides degraded information on something that's already there.

On the other hand adding a CLI flag that puts the user in total control of the final phar path (a feature that's already there but only through manually editing the location attribute in phars.xml) solves the issue in a much simpler way IMO.

From a UX point of view, having a --target flags that sets the target directory instead of the expected target path is rather counter-intuitive. Having something like:

install
  -t, --target        Sets the target path for the PHAR
  -d, --target-dir    Sets the target directory for the PHAR

...looks like better UX to me.

Just my 2¢.

Answer 4 · 2023-07-24T19:18:17.000Z

After reading through #116, I still think adding a CLI flag to set the full path is the right thing to do.

First because it's use case is not limited to this particular issue, and secondly because it would provide us mere mortals a nice workaround while we wait another 5 years for the ecosystem to catch-up.

Meanwhile, feel free to close this as a duplicate. 😉

Answer 5 · 2023-07-24T19:59:35.000Z

I understand that being in more control of the final filename might be handy, though so far every time someone asked for that option the reason was PHPUnit and to have autocomplete work for it. Nobody really cared about the actual filename.

Side note: I also do not understand why PHPStorm requires the extension. It's not like we're restricted to a 90ties windows, where extensions actually were required to determine the file type. But that's a different story.

Apart from that, indeed having a hybrid of runtime and library is the edge case. I do not believe your example of "jumping to the implementation" is actually a real requirement. You do not do that to PHP's code either, yet you use the autocomplete stubs just fine. PHPUnit should be no exception here, imho. Additionally, using the hybrid phar, you get autocomplete for things you do not want nor need, independent of your acutal use case. When writing tests, you do not need anything except the Testframe-Work and associated classes, interfaces or traits. Yet, you get autocomplete for a lot more.

Again, I do understand the issue with autocomplete - i'm just not convinced the control of the filename is the answer to that. I might add an option to change the filename nevertheless..

Answer 6 · 2023-07-24T20:39:28.000Z

I do not believe your example of "jumping to the implementation" is actually a real requirement.

I do. That's how I learn(t) A LOT. I know, there is documentation, examples, etc. But I like the cmd+click "go to definition" and just read the code to really know what's going on. I've done that with Symfony, I've done that with PHPUnit.

Not being able to do that, would be a loss in my opinion…

the deeper issue is that the phpunit phar bundles the test-runner and the library together. Whereas the test-runner shouldn't be a direct dependency of your test suite, the library is a de-facto hard dependency.

I guess this is the real source of the issue here. It would be great if PHPUnit was split into 2 "libraries" – one for the "client side" (to "require-dev" in Composer) and another as the "tool side" (to be downloaded using Phive: BUT, it can require the "client side" internally anyway).

But that's for PHPUnit to solve, not Phive, of course.

As for Phive, I was a little surprised it removes the .phar extension from the tool names. Why? To get rid of a few characters to type? Why is the extension a thing then anyway? I don't really mind it, but if the "original" is with the extension, then I would keep it, and allow to get rid of it optionally.

So, I question the defaults, but agree that giving the option would be a solution. Probably for years to come xd

Answer 7 · 2023-07-24T21:08:17.000Z

I also do not understand why PHPStorm requires the extension.

I don't know either.

However I do remember having had issues when trying to include things using a phar stream URL when there's no .phar extension, i.e:

// Both /src/php/example.phar and /src/php/example exist and are valid phar archives
// Works
include 'phar:///src/php/example.phar/path/to/file.php';
// Fails
include 'phar:///src/php/example/path/to/file.php';
// WARNING  include(phar:///src/php/example/path/to/file.php): Failed to open stream: phar error: invalid url or non-existent phar "phar:///src/php/example/path/to/file.php" in phar://eval()'d code.

Haven't tried recently but this might very well be the reason why...

Answer 8 · 2023-07-24T21:32:34.000Z

I do not believe your example of "jumping to the implementation" is actually a real requirement. You do not do that to PHP's code either, yet you use the autocomplete stubs just fine.

I do this all the time in any language, PHP included. This, combined with step-debugging sessions is basically the only sane way you can really deeply learn the ins and outs of any tool/framework.

The only things stubs are really useful for in PHP is for core ZendEngine or C extensions APIs.

I also happen to write PHP extensions from time to time, so any IDE were to implement jump-to-source and step-debugging through the engine barriers, let me tell you I would be more than happy to use that! 😆 Unfortunately I do not expect this to happen any time soon...

Answer 9 · 2023-07-24T21:47:30.000Z

Additionally, using the hybrid phar, you get autocomplete for things you do not want nor need, independent of your acutal use case. When writing tests, you do not need anything except the Testframe-Work and associated classes, interfaces or traits.

Until you have to write a CI job that requires custom reporting and you have to implement something that hooks into the runner lifecycle (like a test listener or any other poorly documented feature). Then having the source available along with IDE tooling becomes invaluable.

Answer 10 · 2023-07-24T22:06:57.000Z

The only things stubs are really useful for in PHP is for core ZendEngine or C extensions APIs.

Kinda useful. PhpStorm has PHP–built–in functions stubs, but I need to look for the official docs at php.net all the time. It's mostly the quirks, like something changed from version to version, or the details, how the thingy behaves on edge cases.

any IDE were to implement jump-to-source and step-debugging through the engine barriers, let me tell you I would be more than happy to use that! 😆

Hah, yeah, I've also thought about it. Would be awesome!

Just like a month ago, I was searching PHP source code for what does strcoll really do under the hood*. If only I could jump–click to definition… <3

*BTW, turns out that PHP's strcoll is a pass–through to C's strcoll – which is messed up on MacOS -> giving wrong results for UTF8 strings in PHP; PostgreSQL has the same issue (while MySQL replaces the MacOS's implementation with a different one, working OK). Yeah… quirks… :P

Answer 11 · 2023-07-24T22:32:48.000Z

Again, I do understand the issue with autocomplete - i'm just not convinced the control of the filename is the answer to that.

I think everybody here can agree that for this precise issue this is not the true, clean answer.

However the underlying isssue is non-trivial and no true and clean solution has emerged in the 5+ years since the opening of the original ticket. Having control over the full path would be useful in and of itself, and as a side-effect also a workaround for this issue, all the while being far easier to implement.

By the way, last time I checked, the only way to add custom stubs to PHPStorm was to fork their stubs repository, add your custom stubs there and point the IDE to your local checkout in order to use these instead of the builtin ones. And of course rebase your custom stubs repo regularly so that the builtin ones stay in sync... 😓

So nope, you can't (yet) just drop a stub file somewhere in your project and having it recognized automatically. What you can do is use the stub file as a regular php file and tell the IDE to ignore the hundreds of warnings and errors that arise because the function/method implementations are empty.

All this makes me think that a stub based solution is not going to help anyone with this issue. But of course I might very well be overlooking something.

Answer 12 · 2023-07-27T14:06:01.000Z

So now you have to:

manually rename tools/phpunit to tools/phpunit.phar

manually edit the location attribute in .phive/phars.xml to add the .phar extension

I can confirm, as I just checked: PhpStorm 2023.1.2 Build #PS-231.9011.38, built on May 16, 2023 does not recognize classes like PhpCsFixer\Config by default -> only after I manually added the .phar extension, then it works.

Answer 13 · 2023-07-27T14:53:18.000Z

I got the message ;)

But, simply for the sake of discussing:

That's yet another edge case to me: Having autocomplete when editing the config file is probably required, I certainly do not want any autocomplete for anything PHP-CS-Fixer in my actual application.

The same holds true for PHPUnit. To me, it's rather annoying to constantly get classes shown from PHPUnit when writing application code.

We'd need a context-aware autocomplete...

Answer 14 · 2023-07-27T15:20:06.000Z

Having autocomplete when editing the config file is probably required, I certainly do not want any autocomplete for anything PHP-CS-Fixer in my actual application.

Yep, I have no idea how to achieve that.

But from 2 wrongs: not having autocomplete when I need it, or having too much autocomplete when I don't need it -> I prefer having the second one ;-)

BTW, for C++ with CLion + PlatformIO it is solved like this:

PlatformIO defines build targets, like debug, test, upload.
In PIO config I define environments, like native, microcontroller, native_test, microcontroller_test
The combination of build target and environment specifies which files get built and for what "platform".
In CLion's Build/Run Configurations I choose which one is "current" for CLion

As the result, CLion autocompletes only for classes that are available for "current" build/run configuration (target+env).

How can this be helpful here? Well, I can guess it could also somehow be configurated in PhpStorm. I don't know how. I don't mind the extra autocomplete that much :P