Skip to content

Make Docker and Apple container actually work with flycheck-phpstan - #63

Merged
zonuexe merged 3 commits into
feature/flycheck-generic-checkerfrom
fix/container-path-normalization
Jul 17, 2026
Merged

Make Docker and Apple container actually work with flycheck-phpstan#63
zonuexe merged 3 commits into
feature/flycheck-generic-checkerfrom
fix/container-path-normalization

Conversation

@zonuexe

@zonuexe zonuexe commented Jul 17, 2026

Copy link
Copy Markdown
Member

Makes Docker and Apple container actually work with flycheck-phpstan. Both were broken before #62 as well — #62 wired the runtimes up correctly, but three separate defects downstream meant a containerized check still reported nothing.

Stacked on #62. Please merge that first; this branch targets it.

The three defects

1. The analyzed file was passed as a host path. phpstan-get-command-args translated the config file with phpstan-normalize-path but not the target, so PHPStan answered Path /Users/.../test-docker.php does not exist and reported no errors. flymake-phpstan dodged this for modified buffers by normalizing the temp file itself, but was affected for unmodified ones.

Normalization now happens in phpstan-get-command-args, the one place that already knows about the mount point, and the redundant call in flymake-phpstan--create-temp-file is dropped so there is a single source of truth.

The --tmp-file branch needed more than translation: :temp-file creates the copy in the system temporary directory, which is not mounted and therefore invisible to the container. It now uses the in-place copy when containerized. That branch is only reachable for containers with phpstan-activate-editor-mode set to enabled, since version detection currently disables editor mode for them, but it was wrong either way.

2. The JSON report was ignored when prefixed. phpstan--parse-json deliberately skips everything before the first line starting with {, because the checker process merges STDERR into STDOUT. Its caller disagreed and only took the JSON branch when the whole output started with {. Docker stays quiet once the image is local, so this went unnoticed — but Apple container reports progress on STDERR for every run:

[0/6] [0s]
[1/6] Fetching image [0s]
...

so the JSON was never parsed and every check came back clean.

3. Failures were discarded. When there was no JSON at all, the fallback built a warning holding the raw output and then threw it away — data is a list of flycheck-error, so (plist-get data :files) returned nil. Every failure that stopped PHPStan from producing a report (broken config, bad -c path, a crash) was shown as a buffer with no errors. This is why the two defects above were silent rather than loud.

Verification

Ran Flycheck for real against ghcr.io/phpstan/phpstan on both runtimes:

case before after
docker, unmodified buffer 0 errors 2 errors ✅
container, unmodified buffer 0 errors 2 errors ✅
container, modified buffer (in-place copy) 0 errors 3 errors ✅
container, modified, phpstan-activate-editor-mode = enabled (--tmp-file) 0 errors 3 errors ✅
("docker" "run" ...) explicit form 0 errors 2 errors ✅

All match what local vendor/bin/phpstan reports for the same fixture, with no leftover temporary files. Local (non-container) execution is unchanged. With a config file PHPStan cannot use, the checker now reports Bootstrap file /.../tests/bootstrap.php does not exist. instead of nothing.

Still open

phpstan-editor-mode-available-p runs docker --version and parses d1c06ef out of Docker version 29.5.3, build d1c06ef as the PHPStan version, so editor mode stays silently disabled for container users — they get the in-place copy branch, which works. Fixing it needs a design decision (running IMAGE --version costs a container start per check, and the phpstan-executable-versions-alist cache is keyed by executable string), so it is left for a follow-up.

zonuexe added 3 commits July 17, 2026 21:51
`phpstan-get-command-args' translated the config file with
`phpstan-normalize-path', but handed the analyzed file to PHPStan as a
host path.  A containerized PHPStan only sees the project through its
mount point, so it answered

    Path /Users/.../test-docker.php does not exist

and reported no errors at all.  `flycheck-phpstan' was therefore unusable
with `(phpstan-executable . docker)'; `flymake-phpstan' worked only for
modified buffers, because it normalized the temporary file itself while
passing an unnormalized path for the unmodified case.

Normalize the paths in `phpstan-get-command-args', which is the one place
that already knows about the mount point, and drop the now-redundant call
in `flymake-phpstan--create-temp-file' so the translation lives in a
single place.

The `--tmp-file' branch needs more than translation: `:temp-file' creates
the copy in the system temporary directory, which is not mounted into the
container and hence invisible to it.  Use the in-place copy instead when
running containerized, so the file lands inside the project.  That branch
is only reachable for containers with `phpstan-activate-editor-mode' set
to `enabled', since version detection currently disables editor mode for
them, but it is wrong either way.
`phpstan--parse-json' deliberately skips everything before the first line
starting with `{', so that whatever the process wrote to STDERR is
ignored -- the checker process merges STDERR into STDOUT.

Its caller disagreed: it only chose the JSON branch when the *whole*
output started with `{'.  Docker happens to stay quiet once the image is
local, so this went unnoticed, but Apple container reports its progress
on STDERR for every run:

    [0/6] [0s]
    [1/6] Fetching image [0s]
    ...

That prefix meant the JSON was never parsed, and every check came back
with no errors.  Look for a line starting with `{' instead, which is the
same condition `phpstan--parse-json' then acts on.
When the output held no JSON report, `flycheck-phpstan-parse-output' built
a warning carrying the raw output -- and then dropped it on the floor:

    (data   (if json? (phpstan--parse-json ...)
              (list (flycheck-error-new-at 1 1 'warning ...))))
    (errors (phpstan--plist-to-alist (plist-get data :files)))

In the fallback branch DATA is a list of `flycheck-error', so `plist-get'
found no `:files' and returned nil, and the warning never reached
flycheck.  Every failure that stopped PHPStan from producing a report --
a broken configuration file, a bad `-c' path, a crash -- was shown to the
user as a buffer with no errors.

Return the warning directly.  With a config file PHPStan cannot use, the
checker now says

    Bootstrap file /.../tests/bootstrap.php does not exist.

instead of nothing at all.
@zonuexe
zonuexe merged commit ace4531 into feature/flycheck-generic-checker Jul 17, 2026
12 of 14 checks passed
zonuexe added a commit that referenced this pull request Jul 17, 2026
#63 and #64 were merged into their base branches rather than master:
#62 landed on master first, which left the rest of the stack behind on
feature/flycheck-generic-checker and fix/container-path-normalization.

Bring in the four commits that never reached master.
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

Successfully merging this pull request may close these issues.

1 participant