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

Support for editorUrl on Docker runtime #7043

Closed
Wirone opened this issue Apr 15, 2022 · 7 comments · Fixed by phpstan/phpstan-src#1414
Closed

Support for editorUrl on Docker runtime #7043

Wirone opened this issue Apr 15, 2022 · 7 comments · Fixed by phpstan/phpstan-src#1414

Comments

@Wirone
Copy link
Contributor

Wirone commented Apr 15, 2022

Feature request

I would like to ask for support for Docker runtime in terms of editorUrl. I really like this feature but it requires running PHPStan with locally installed PHP while we work on Docker runtime. Paths are not the same, so editorUrl does not work properly because it prints links with container’s paths, not local paths. Clicking such link ends with empty PHPStorm window.

Maybe some kind of %rel_file%, so phpstorm://open?file=/local/path/%%rel_file%%&line=%%line%% could be used 🤔 ?

ℹ️ IMPORTANT: relative path should be calculated based on project's root (where PHPStan's config resides)

@ondrejmirtes
Copy link
Member

Can you post examples of a few paths of how they look inside your Docker container and in your actual host filesystem? How does the TableErrorFormatter looks like vs. how would you like it to look like?

@Wirone
Copy link
Contributor Author

Wirone commented Apr 16, 2022

No problem 🙂

  • local path: /Users/gkorba/Dev/GetResponse/app
  • container path: /app

In Docker Composer file we mount local path to container's /app.

Docker usage

Let's illustrate how our structure looks like:

app/
├─ utils/
│  ├─ phpstan/
│  │  ├─ analysis/
│  │  │  ├─ src.neon
│  │  │  ├─ packages.neon
│  │  │  ├─ api.neon
│  │  │  ├─ legacy.neon
├─ composer.json
├─ phpstan.neon
├─ phpstan.local.neon

PHPStan is executed from /app through Composer script, where:

  • shared config is located in /app/phpstan.neon (tmpDir, excludePaths, bootstrapFiles, baselines etc)
  • we have analysis in 4 areas of codebase, each area is defined in /app/utils/phpstan/analysis in neon file, which includes ../../../phpstan.local.neon and defines paths for analysis
  • phpstan.local.neon includes common phpstan.neon and allows to define parallel or editorUrl per developer

Analysis and editorUrl result

When I run composer phpstan:src it executes phpstan --ansi analyse -cutils/phpstan/analysis/src.neon. If there are errors, rendered links are like phpstorm://open?file=/app/src/CryptoProvider.php&line=13, so it's actual path to the file inside container. But locally it does not exist, so clicking the link opens empty window in IDE.

@ondrejmirtes
Copy link
Member

Something like this could work:

parameters:
    editorUrlMapping:
        - .:/app

The . would resolve to the same directory where phpstan.neon is. This is basically the same argument as you're passing to -v for docker run I guess.

@Wirone
Copy link
Contributor Author

Wirone commented Apr 21, 2022

@ondrejmirtes it won't work because when PHPStan is executed inside container it does not have any context from localhost. For PHPStan in container, . is actually /app. I see some solutions:

  • as I wrote in the description: introduce new param %rel_file% so it can be used in editorUrl, so each user can explicitly set project's base path and add file's relative path to it
  • introduce editorUrlBasePath config option that would work similarly, but would be resolved slightly earlier (before passing path to %file%)
  • support environment variables for defining base path (like PHPSTAN_BASE_PATH), because it would be possible then to define it even for vcs-tracked files (as opposite to previous solutions that will work only if editorUrl is provided in non-tracked file because each developer can have different base path)

In the end it always requires user to configure explicit base path to prepare editorUrl because PHPStan config and/or env variable need to be adjusted as only developer knows where files are stored.

@Patabugen
Copy link

I came here looking for the same option.

Specifying an absolute path would also be fine in my case - especially since we have extensible config files.

Example from another app

For inspiration, the Ray app has a similar setting which works well for my setting:

https://spatie.be/docs/ray/v1/environment-specific-configuration/docker

My Suggestion

I'd suggest we can add a couple of config values under parameters alongside editorUrl, perhaps:

https://phpstan.org/config-reference#config-file

parameters:
     editorUrl: 'phpstorm://open?file=%%file%%&line=%%line%%'
     remotePath: '/app/'
     localPath: '\\wsl$\Ubuntu\home\user\my-app\'

And then in phpstan it is (I believe) simply a find & replace at the last minute.

@pscheit
Copy link

pscheit commented May 27, 2022

bit offtopic: note that this wont work if you click the link in WSL2. Custom protocols are not yet supported:
microsoft/terminal#7562

Wirone pushed a commit to Wirone/phpstan-src that referenced this issue Jun 11, 2022
This allows running PHPStan within Docker environment and output errors with links to files on the host (`editorUrl` must be configured using actual host's path to the project + `%rel_file%`).

Fixes phpstan/phpstan#7043
Wirone added a commit to Wirone/phpstan-src that referenced this issue Jun 11, 2022
This allows running PHPStan within Docker environment and output errors with links to files on the host (`editorUrl` must be configured using actual host's path to the project + `%rel_file%`).

Fixes phpstan/phpstan#7043
Wirone added a commit to Wirone/phpstan-src that referenced this issue Jun 15, 2022
This allows running PHPStan within Docker environment and output errors with links to files on the host (`editorUrl` must be configured using actual host's path to the project + `%rel_file%`).

Fixes phpstan/phpstan#7043
@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants