chore(lint): run PHPStan and Rector isolated from the machine's php.ini

.github/workflows/build.yml

@@ -46,6 +46,9 @@ jobs:
         with:
           php-version: '8.4'
           coverage: none
+          # bin/lint.sh runs PHPStan and Rector with no php.ini, so the analysis needs
+          # FFI back explicitly (phpstan.neon analyses FFI\CData); see AGENTS.md.
+          extensions: ffi
           tools: composer:v2
 
       - name: Install dependencies

AGENTS.md

@@ -266,6 +266,51 @@ hadolint on Dockerfiles (none today). A missing tool is a failure. Needs
 `hadolint`. Rector skips are documented in `rector.php`; do not "fix" them (for example
 `ZVecDocIterator::$collection` is never read on purpose).
 
+PHPStan and Rector are run with the machine's `php.ini` replaced by an empty one, for the
+same reason the test suite needs `-n` (#188): this repository also builds a PHP extension
+(`php-ext/`, the legacy `zvec`), and a copy of it can be installed locally and enabled in
+`php.ini`. Both tools work inside a PHP process, so whatever that process has loaded is
+what they see — without the isolation the lint result depends on the machine instead of on
+the working tree, which is how a Rector finding in #246 showed up in CI but not locally.
+
+`php -n` is **not** how this is done, for two reasons:
+
+- PHPStan needs the `Phar` class and Rector needs `PhpToken` from the tokenizer
+  extension. On the CI image those come from conf.d inis, which `-n` drops: the first
+  push of #259 failed `lint` with `Class "Phar" not found` and
+  `Class "PhpToken" not found`.
+- `-n` does not reach the processes PHPStan starts for itself. It restarts with
+  `PHP_BINARY -d …` and starts parallel workers, and those load the machine's
+  `php.ini` again — verified with `ps` while it runs. `PHPRC` is an environment
+  variable, so it is inherited.
+
+conf.d is still scanned, so an extension installed that way stays available — **as long
+as the generated ini carries `extension_dir`**, because those entries name their
+extension without a path and on a Homebrew/pecl setup `extension_dir` is set in
+`php.ini`. `bin/lint.sh` copies it, which is also what makes the FFI fallback work for a
+shared `ffi.so`.
+
+An `extension=zvec.so` line in `php.ini` — the #188 case of a `make install` plus a manual
+line — no longer reaches the tools; one in conf.d still would.
+
+What `bin/lint.sh` does, and how to repeat it by hand:
+
+```bash
+# The ini PHP is told to load instead of the machine's php.ini.
+i="$(mktemp)"; printf 'extension_dir="%s"\n' "$(php -r 'echo ini_get("extension_dir");')" > "$i"
+
+env PHPRC="$i" php -d memory_limit=-1 vendor/bin/phpstan analyse --no-progress
+env PHPRC="$i" php -d memory_limit=-1 vendor/bin/rector process --dry-run
+```
+
+`-d memory_limit=-1` replaces the built-in 128M, because the replaced `php.ini` is where
+the real limit usually is. Where the analysis would have no FFI at all, `extension=ffi`
+goes into the generated ini — `phpstan.neon` ignores undefined FFI methods and properties,
+not a missing FFI class, so without FFI the analysis reports errors on `src/`. The ini is
+what PHPStan's restarted processes load too, which is why FFI is not also passed as `-d`:
+PHP would load it twice and warn. The current PHP is asked whether it has FFI in exactly
+this environment first, so nothing is loaded twice on any machine.
+
 Setting up a fresh worktree: `bin/worktree.sh <issue>` runs `bin/worktree-setup.sh`
 (`composer install`, then `./build_zvec.sh` for the FFI adapter the tests load). Nothing
 binds a host port, so worktrees do not collide.

CHANGELOG.md

@@ -115,6 +115,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+- **PHPStan and Rector no longer see the machine's `php.ini`** (#258)
+  - `bin/lint.sh` starts both with `PHPRC` pointing at an ini generated per run, so a copy of the `zvec` PHP extension installed on the machine — the #188 case of a `make install` plus an `extension=` line — can no longer change what the analysis sees. conf.d is still scanned, so an extension installed that way stays available.
+  - `php -n` was tried first and does not work here: PHPStan needs the `Phar` class and Rector needs `PhpToken` from the tokenizer extension, which the CI image provides through conf.d (`lint` failed with `Class "Phar" not found` and `Class "PhpToken" not found`), and `-n` does not reach the processes PHPStan starts for itself anyway — it restarts with `PHP_BINARY -d …` and starts parallel workers, both of which load the machine's `php.ini` again. `PHPRC` is an environment variable and is inherited.
+  - What the replaced `php.ini` took with it is added back explicitly: `extension_dir` (on a Homebrew/pecl setup it is set in `php.ini`, and the conf.d entries name their extension without a path), `memory_limit=-1` instead of the built-in 128M, and FFI (`extension=ffi` in the generated ini, which every one of those processes loads) only where the current PHP has none in that same environment. `phpstan.neon` only ignores undefined FFI methods and properties, not a missing FFI class, so without FFI the analysis would report errors on `src/`.
+  - No step, level or rule changed. PHP-CS-Fixer is untouched: it works on the token stream and never resolves the project's classes.
+  - The CI `lint` job now asks for FFI explicitly, so the analysis has it whether the runner provides it compiled in or through an ini.
+  - `AGENTS.md` documents the flags, and why `php -n` is not the answer here.
+
 - **Repository standard rolled out** (process and tooling, no API change)
   - `LICENSE` (MIT, Crazy Goat Software), `docs/workflow.md`, `docs/release-workflow.md`, `CONTRIBUTING.md` and `bin/pick-issue.sh`, `bin/worktree.sh`, `bin/worktree-done.sh`, `bin/worktree-setup.sh`.
   - `bin/lint.sh` (also `composer lint`) runs PHP-CS-Fixer, PHPStan (level 2), Rector, clang-format, shellcheck and hadolint. The PHP sources were reformatted to PSR-12 and the C++ sources with clang-format (whitespace only), plus a few Rector dead-code simplifications.

bin/lint.sh

@@ -33,15 +33,61 @@ shell_scripts() {
     done < <(git ls-files -z)
 }
 
+# Isolation for PHPStan and Rector. Both run inside a PHP process, so they see whatever
+# that process has loaded — and this repository also builds a PHP extension (php-ext/,
+# the legacy `zvec`), which a contributor can have installed and enabled in php.ini. The
+# lint result would then depend on the machine instead of on the working tree, which is
+# what happened in #246.
+#
+# `-n` is not usable here. PHPStan needs the Phar class and Rector needs `PhpToken` from
+# the tokenizer extension, and on the CI image those come from conf.d inis, which `-n`
+# drops: `lint` failed in #259 with `Class "Phar" not found` and `Class "PhpToken" not
+# found`. So instead of dropping every ini, the machine's php.ini is replaced by one that
+# is generated per run and holds what the analysis needs. `PHPRC` is an environment
+# variable and is inherited, so this also reaches the processes PHPStan starts for itself
+# — it restarts with `PHP_BINARY -d ...` and starts parallel workers, which drop `-n`.
+#
+# What the replaced php.ini took with it has to be put back explicitly:
+#   - extension_dir: on a Homebrew/pecl setup it is set in php.ini, while the conf.d
+#     entries name their extension without a path, so without it every extension
+#     installed that way fails to load.
+#   - memory_limit: without it the limit is the built-in 128M, far less than what the
+#     analysis gets today.
+#   - FFI, but only where the analysis would otherwise have none: phpstan.neon ignores
+#     undefined FFI methods and properties, not a missing FFI class, so without FFI the
+#     analysis reports errors on src/. Probed in the same isolated environment the tools
+#     run in, and then written to the ini — which every one of those processes loads, so
+#     it is not also passed as `-d`, or PHP would load it twice and warn.
+ANALYSIS_INI="$(mktemp "${TMPDIR:-/tmp}/zvec-lint-ini.XXXXXX")"
+trap 'rm -f "$ANALYSIS_INI"' EXIT
+: >"$ANALYSIS_INI"
+php_extension_dir="$(php -r 'echo (string) ini_get("extension_dir");' 2>/dev/null || true)"
+# Quoted, so a path with a space is fine; a quote or a newline cannot be expressed in an
+# ini value at all, and such a path is left out rather than written out broken.
+case "$php_extension_dir" in
+    *'"'* | *$'\n'*) php_extension_dir="" ;;
+esac
+if [ -n "$php_extension_dir" ]; then
+    printf 'extension_dir="%s"\n' "$php_extension_dir" >"$ANALYSIS_INI"
+fi
+PHP_ANALYSIS_FLAGS=(-d memory_limit=-1)
+if ! env PHPRC="$ANALYSIS_INI" php -r 'exit(extension_loaded("FFI") ? 0 : 1);' 2>/dev/null; then
+    printf 'extension=ffi\n' >>"$ANALYSIS_INI"
+fi
+
+php_analysis() {
+    env PHPRC="$ANALYSIS_INI" php "${PHP_ANALYSIS_FLAGS[@]}" "$@"
+}
+
 if [ "$FIX" = 1 ]; then
-    vendor/bin/rector process --no-progress-bar || true
+    php_analysis vendor/bin/rector process --no-progress-bar || true
     vendor/bin/php-cs-fixer fix || true # exits non-zero when it changed files
     cpp_sources | xargs -0 -r clang-format -i || true
 fi
 
 step "php-cs-fixer" vendor/bin/php-cs-fixer fix --dry-run --diff
-step "phpstan" vendor/bin/phpstan analyse --no-progress
-step "rector" vendor/bin/rector process --dry-run --no-progress-bar
+step "phpstan" php_analysis vendor/bin/phpstan analyse --no-progress
+step "rector" php_analysis vendor/bin/rector process --dry-run --no-progress-bar
 step "clang-format" bash -c "$(declare -f cpp_sources); cpp_sources | xargs -0 -r clang-format --dry-run --Werror"
 step "shellcheck" bash -c "$(declare -f shell_scripts); shell_scripts | xargs -0 -r shellcheck"
 step "hadolint" bash -c "git ls-files -z '*Dockerfile*' | xargs -0 -r hadolint"