SanderRonde / phpstan-vscode

PHPStan plugin for VSCode
https://marketplace.visualstudio.com/items?itemName=SanderRonde.phpstan-vscode
MIT License
40 stars 9 forks source link

Cannot use phpstan in docker container #1

Closed Grldk closed 2 years ago

Grldk commented 2 years ago

I'd like to use this extension with phpstan running inside a container. Right now this is not possible because of this check:

https://github.com/SanderRonde/phpstan-vscode/blob/d4dbf24591e0a776b24af2b293195a9712a47ad2/src/lib/phpstan.ts#L283

I've configured "phpstan.binPath" to lando phpstan, which is analogue to running docker exec or docker-compose exec. Because this isn't a real file the _fileIfExists fails.

I'm not sure if there are other issues preventing running this extension in a container.

SanderRonde commented 2 years ago

I think I've fixed this issue. I don't really have any docker environments set up right now so would you be willing to test it for me? I've attached the VSIX file as a zip file (github doesn't allow VSIX file uploads). So just rename it to .vsix and install it in VSCode and it should work.

To make it work in your scenario, set the new phpstan.binCommand to lando phpstan and it should work.

phpstan-vscode-1.1.3.zip

Grldk commented 2 years ago

That was very quick, thanks!

Will try and report back tomorrow!

Grldk commented 2 years ago

Tried your update, still gives the same error unfortunately:

PHPStan: failed to find binary at "[path]/lando phpstan"

SanderRonde commented 2 years ago

Are you sure you set the phpstan.binCommand setting?

Grldk commented 2 years ago

Seems I got confused between phpstan.binPath and phpstan.binCommand...

My config right now looks like this:

    "phpstan.binPath": "${workspaceFolder}",
    "phpstan.binCommand": "lando phpstan"

Which seems to work, in the sense that I don't get any errors. Problem is I don't see any phpstan feedback in VS Code either. Is there a way to get more debug info in the developer console? I'm not getting anything there right now. I'd guess that there is a path mismatch between the container and the host (where vs code is running)

Thanks again for your quick replies btw.

SanderRonde commented 2 years ago

Yeah I'm gonna have to revisit the naming of those settings, it's a bit confusing now. There is the phpstan logging tab which might contain some useful info. Go to the output panel (next to the terminal panel) and select "phpstan" from the dropdown. Let me know if thst works.

On Thu, 17 Mar 2022, 09:33 Grldk, @.***> wrote:

Seems I got confused between phpstan.binPath and phpstan.binCommand...

My config right now looks like this:

"phpstan.binPath": "${workspaceFolder}",
"phpstan.binCommand": "lando phpstan"

Which seems to work, in the sense that I don't get any errors. Problem is I don't see any phpstan feedback in VS Code either. Is there a way to get more debug info in the developer console? I'm not getting anything there right now. I'd guess that there is a path mismatch between the container and the host (where vs code is running)

Thanks again for your quick replies btw.

— Reply to this email directly, view it on GitHub https://github.com/SanderRonde/phpstan-vscode/issues/1#issuecomment-1070535127, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABJCWNCSYEKAJ6WTNPXFOHTVALU5DANCNFSM5QZDLRGA . You are receiving this because you commented.Message ID: @.***>

ljubadr commented 2 years ago

@Grldk I would highly recommend looking into vscode remote containers

Before I was also using vscode on host and setting docker exec or docker-compose exec paths for this and other extensions. Some extensions even required creating .sh files to make them work

Since then I've switched to remote containers and most of the extensions work out of the box without having to setup anything

This also solved problem of not being able to save vscode settings in git that need absolute path. Now I can add setting and things will just work for everyone in the team

Added bonus is that if you are on win or mac you can have improved performance

Grldk commented 2 years ago

Had some time to work on this again. Used the version you previously posted here, noticed that newer versions don't have a phpstan.binCommand setting anymore, couldn't get that to work. With the version posted here I get the following output:

Checking file /[path]/app/Http/Kernel.php
Showing status bar
Spawning PHPStan with the following configuration:  {"binCmd":"lando","args":["analyse","-c","/[path]/phpstan.neon","--error-format=raw","--no-progress","--no-interaction","--memory-limit=1G","phpstan","/[path]/app/Http/Kernel.php"],"cwd":"/[path]"}
PHPStan process exited succesfully
Hiding status bar, last operation result = Success
File check done or file /[path]/app/Http/Kernel.php errors= 

I don't think phpstan is actually called.

@ljubadr I've looked into that before. Had problems getting one or two other extension to work within the container at the time unfortunately, while I got everything to work outside the container looking inwards. Will probably revisit that soon.

SanderRonde commented 2 years ago

noticed that newer versions don't have a phpstan.binCommand setting anymore

Yeah I didn't ship this feature yet, first wanted to see whether it works for you and whether I need to tune it still.

I don't think phpstan is actually called.

Ah I can see the problem already, lando is called with "analyse" being its first arg instead of phpstan. This might be an error in my script (or maybe in your binCommand configuration, not sure). Will investigate when I'm home some time tomorrow.

SanderRonde commented 2 years ago

Can you give this new version a try? Now phpstan.binCommand is an array (for example ['lando', 'phpstan']). Be sure to put the entire binary array into binCommand and let me know what happens. Same thing goes here: it's a zip file. Rename it to .vsix and install it.

phpstan-vscode-1.1.5.zip

Grldk commented 2 years ago

Just had some time to continue with this. Thanks for the next try!

This lets me actually run phpstan inside the container. Next problem is folder mapping. The command that is run now:

Spawning PHPStan with the following configuration:  {"binCmd":"lando phpstan","args":["analyse","-c","[host_folder]/phpstan.neon","--error-format=raw","--no-progress","--no-interaction","--memory-limit=1G","[host_folder]/app/Models/User.php"],"cwd":"[host_folder]"}

Problem is that [host_folder] does not exist within the container, and should be mapped to /app (in my container at least).

SanderRonde commented 2 years ago

I'm not really sure whether I'll be adding a path mapping feature as well. It kind of depends on how many people are going to be using it vs the complexity of implementing it. This sounds like a pretty niche use case to me, now that remote development in docker containers is supported. What exactly is your setup? So I can get an idea of whether it's something a lot of people use. To me it sounds like the following, but please correct me if I'm wrong:

If so, wouldn't it be pretty simple to just run PHPStan outside of the container instead?

Grldk commented 2 years ago

You're right about my setup. I'd understand if you don't want to support it. There are some extensions that do support this though (calebporzio.better-phpunit for example).

I'd like to run this inside the container so phpstan is always using the correct php version, which is not necessarily the case when running outside of the container.

If you don't want to support this feel free to close this issue.

SanderRonde commented 2 years ago

Ah that makes sense. I tried giving it a go but ran into a pretty big issue. When the current file is dirty (unsaved), this extension creates a temporary file and checks that instead. Of course this file is created in the filesystem where the extension runs. In that case it couldn't be checked with the "remote" binary, which would mean that it would break your use case right?

I could just disable checking entirely for a dirty file when it is being pathmapped. That would mean that you can only perform checks on already saved files. Do you think that would be worth having this feature? If so I'd be fine with implementing it.

If yes, can you test it out for me? I've added the file again.

phpstan-vscode-1.1.6.zip

Grldk commented 2 years ago

Thanks for sticking with me!

I've had some time this week to try to move my dev environment inside the container as suggested earlier, but couldn't get that to work unfortunately. Which means I'm still interested in this..

I see the problems with the temp file, didn't think of that use case. I'd still be interested in this without the temp file creation though. So I've tried you're latest file. The path mapping seems to work, but it's not complete. Right now it's running the following:

Spawning PHPStan with the following configuration:  {"binCmd":"lando phpstan","args":["analyse","-c","[host_folder]/phpstan.neon","--error-format=raw","--no-progress","--no-interaction","--memory-limit=1G","[container_folder]/app/Models/User.php"],"cwd":"[host_folder]"}

As you can see, the mapping works for the php file being checked, but the path to the phpstan config file and cwd are not mapped, which means it's still erroring out..

SanderRonde commented 2 years ago

Ah yes, that should be fixed in this version: phpstan-vscode-1.1.6.zip

Grldk commented 2 years ago

That's better but still not there. Path to the config file is mapped now, but I'm getting:

"cwd":"[host_folder] -> /app" in the phpstan configuration. It should be just "cwd":"/app", where /app is the container path I configured.

SanderRonde commented 2 years ago

Yeah that's just for logging purposes, the cwd actually being should be /app. The -> is there to show that it's being mapped (I might have made that a bit unclear). Does it still work though?

Grldk commented 2 years ago

Ah that makes sense.

It doesn't work though. Getting a popup with: PHPStan: process exited with error, see log for details

Output gives:

Spawning PHPStan with the following configuration:  {"binCmd":"lando phpstan","args":["analyse","-c","/app/phpstan.neon.dist","--error-format=raw","--no-progress","--no-interaction","--memory-limit=1G","/app/app/Models/User.php"],"cwd":"[host_folder] -> /app"}
PHPStan process exited with error, data= 
Hiding status bar, last operation result = Error
File check failed for file [host_folder]/app/Models/User.php

If i run phpstan from the terminal with what I assume is the same command (apart from the cwd I guess, not sure where I should put that) it works:

lando phpstan analyse -c /app/phpstan.neon.dist --error-format=raw --no-progress --no-interaction --memory-limit=1G /app/app/Models/User.php

SanderRonde commented 2 years ago

What value do you have for phpstan.binCommand? It should be ['lando', 'phpstan'] but it looks like it's ['lando phpstan'].

Grldk commented 2 years ago

Ah you're right. Changing that doesn't help though, unfortunately. Output's the same, same error.

SanderRonde commented 2 years ago

What happens to the "Spawning PHPStan with..." log? Does it turn into binCmd: 'lando', args: ['phpstan', 'analyse']?

Grldk commented 2 years ago

It kinda does. The order's not right though. I'm guessing that phpstan should be the first argument, not sure how the arguments are presented to the command..

Spawning PHPStan with the following configuration:  {"binCmd":"lando","args":["analyse","-c","/app/phpstan.neon.dist","--error-format=raw","--no-progress","--no-interaction","--memory-limit=1G","phpstan","/app/app/Models/User.php"],"cwd":"[host_folder] -> /app"}
SanderRonde commented 2 years ago

Ah yeah that's not the order it's supposed to be in. Should be fixed in this version. phpstan-vscode-1.1.6.zip

Grldk commented 2 years ago

That one's broken:

Activating extension 'SanderRonde.phpstan-vscode' failed: let is not defined.

  | $onExtensionActivationError | @ | mainThreadExtensionService.ts:107
-- | -- | -- | --
  | _doInvokeHandler | @ | rpcProtocol.ts:473
  | _invokeHandler | @ | rpcProtocol.ts:458
  | _receiveRequest | @ | rpcProtocol.ts:374
  | _receiveOneMessage | @ | rpcProtocol.ts:296
  | (anonymous) | @ | rpcProtocol.ts:161
  | invoke | @ | event.ts:569
  | fire | @ | event.ts:736
  | fire | @ | ipc.net.ts:638
  | _receiveMessage | @ | ipc.net.ts:958
  | (anonymous) | @ | ipc.net.ts:831
  | invoke | @ | event.ts:569
  | fire | @ | event.ts:736
  | acceptChunk | @ | ipc.net.ts:382
  | (anonymous) | @ | ipc.net.ts:338
  | (anonymous) | @ | browserSocketFactory.ts:230
  | invoke | @ | event.ts:569
  | fire | @ | event.ts:736
  | _fileReader.onload | @ | browserSocketFactory.ts:91
SanderRonde commented 2 years ago

Ah the builder just sometimes dies. Does this one work? phpstan-vscode-1.1.6.zip

Grldk commented 2 years ago

Nope, same problem.

SanderRonde commented 2 years ago

Alright this version should work then. phpstan-vscode-1.1.6.zip

Grldk commented 2 years ago

Yeah that one works. But still errors out:

Showing status bar
Spawning PHPStan with the following configuration:  {"binCmd":"lando","args":["phpstan","analyse","-c","/app/phpstan.neon.dist","--error-format=raw","--no-progress","--no-interaction","--memory-limit=1G","/app/app/Models/User.php"],"cwd":"[host_folder] -> /app"}
PHPStan process exited with error, data= 
Hiding status bar, last operation result = Error
File check failed for file [host_folder]/app/Models/User.php

Argument order seems fine now. Not sure what's going on tbh. Again, if I run that command from the terminal it works.

SanderRonde commented 2 years ago

Alright I'm trying to debug this myself locally (to make it a bit easier to debug) but I'm not really familiar with lando. Could you help me out here? Is there any project I can clone that is similar to the one you're working on right now? And what do I need to do to get my dev setup as close as is needed to yours?

Grldk commented 2 years ago

Thanks! I'll try to make a debug repo with a simple laravel install for you later this week!

Grldk commented 2 years ago

Haven't found any time so far, hope to get to this tomorrow!

Grldk commented 2 years ago

Hey! Sorry this took a while, have been very busy lately.

Whipped up a laravel repo with added lando config for you: https://github.com/Grldk/LaravelLando

Copy-pasted the readme together from some internal repo's. If you've got any questions I'll try to answer them as quickly as possible.

SanderRonde commented 2 years ago

Thanks that helped a lot. I'm pretty sure I got it working now. Can you try out this version?

phpstan-vscode-1.2.0.zip

Grldk commented 2 years ago

Hey! Sorry for the late reply (again.. :S ) (was away on holiday).

But this version works for me now! Thanks very much for all your time and patience!

SanderRonde commented 2 years ago

Good to hear! The fixed version should be published now (version 1.3.0)

ultimike commented 1 year ago

I believe I'm trying to do something similar to what @Grldk did - running PhpStan inside a Docker container. I'm not using Lando, but something very similar (DDEV). I tried using a similar config as https://github.com/SanderRonde/phpstan-vscode/issues/1#issuecomment-1070535127, but I get the PHPStan: failed to find config file in "phpstan.neon" error.

@Grldk Would you mind sharing your complete phpstan configuration here? So far, mine is:

    /* PhpStan */
    "phpstan.binPath": "${workspaceFolder}",
    "phpstan.binCommand": [
        "ddev",
        "exec",
        "phpstan"
    ],

I'm using phpstan/extension-installer and when running phpstan from the command line I never have to specify a neon file, so I'm unsure what to use in the extension settings for phpstan.configFile, if anything.

Thanks in advance, -mike

Grldk commented 1 year ago

Hey @ultimike,

We are not using phpstan in this way anymore, have moved to using vs code inside the container as suggested by https://github.com/SanderRonde/phpstan-vscode/issues/1#issuecomment-1075533864. Have looked around a bit but cannot seem to find our old config anymore.. In the end it never made it to production.. So I'm sorry but I cannot help you..

ultimike commented 1 year ago

@Grldk I appreciate the response.