Do not error normalizing a nil path under a container - #67
Merged
Conversation
`phpstan-get-command-args' starts with
(config (or config (phpstan-normalize-path (phpstan-get-config-file))))
and `phpstan-get-config-file' returns nil when the project has no
phpstan.neon and `phpstan-config-file' is unset. For a container
executable `phpstan-normalize-path' then passed that nil to
`replace-regexp-in-string', raising `(wrong-type-argument arrayp nil)'.
The local case was unaffected, since it returns `(or source
source-original)' without touching the path.
So `M-x phpstan-analyze-project', `phpstan-analyze-this-file', flycheck,
and flymake all errored under `(phpstan-executable . docker)' /
`container' whenever the project had no config file -- which
`phpstan-enable-on-no-config-file' explicitly allows (Composer autoload is
enough to enable PHPStan).
Guard `phpstan-normalize-path' at the source: a nil path falls through to
the `(or source source-original)' branch instead of the container
rewrite, so it returns nil (or the optional SOURCE fallback) rather than
erroring. Fixing it in the one function protects every caller. With
config nil the command line now simply omits `-c', which is correct.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes a container run erroring when the project has no configuration file — found while writing the test suite in #66.
The bug
phpstan-get-command-argsopens with:(config (or config (phpstan-normalize-path (phpstan-get-config-file))))phpstan-get-config-filereturns nil when the project has nophpstan.neon(.dist)andphpstan-config-fileis unset. For a container executable,phpstan-normalize-paththen passed that nil toreplace-regexp-in-string, raising(wrong-type-argument arrayp nil). The local case was unaffected — it returns(or source source-original)without touching the path.So
M-x phpstan-analyze-project,phpstan-analyze-this-file, flycheck, and flymake all errored under(phpstan-executable . docker)/containerwhenever the project had no config file. That combination is real:phpstan-enable-on-no-config-fileexplicitly enables PHPStan from a Composer autoload alone.The fix
Guard
phpstan-normalize-pathat the source: a nil path falls through to the(or source source-original)branch instead of the container rewrite, returning nil (or the optionalSOURCEfallback) rather than erroring. Fixing the one function protects every caller. With config nil, the command line now simply omits-c, which is correct.Tests
Two regression tests in
test/phpstan-test.el, both verified to fail (with the originalwrong-type-argument) when the fix is reverted:phpstan-test-normalize-path-nil— nil in, nil out across docker / container / explicit-command / local / executable forms, and that the optionalSOURCEfallback still applies.phpstan-test-command-args-without-config-under-container— a docker run withphpstan-get-config-filestubbed to nil produces a command line with no-cand no error.15/15 pass on system Emacs and Emacs 27.2; compile is warning-free on both.