diff --git a/src/DependencyInjection/Configurator.php b/src/DependencyInjection/Configurator.php index 7865956266..bcd15989b5 100644 --- a/src/DependencyInjection/Configurator.php +++ b/src/DependencyInjection/Configurator.php @@ -12,19 +12,25 @@ use PHPStan\File\FileReader; use PHPStan\File\FileWriter; use PHPStan\Turbo\TurboExtensionEnabler; +use function array_fill_keys; +use function array_intersect_key; use function array_keys; use function count; use function error_reporting; use function explode; +use function file_get_contents; use function hash_file; use function implode; use function in_array; use function is_dir; use function is_file; use function ksort; +use function preg_match; +use function preg_match_all; use function restore_error_handler; use function set_error_handler; use function sprintf; +use function str_contains; use function str_ends_with; use function substr; use function time; @@ -38,6 +44,13 @@ final class Configurator extends \Nette\Bootstrap\Configurator { + /** + * Env vars PHPStan reads at build time in a CompilerExtension (FnsrExtension reads PHPSTAN_FNSR), + * so the container depends on them and they must stay in the cache key. ContainerCacheKeyEnvGuardTest + * fails if a CompilerExtension reads an env var not listed here or referenced via %env.*%. + */ + public const BUILD_TIME_ENV_VARIABLES = ['PHPSTAN_FNSR']; + /** @var string[] */ private array $allConfigFiles = []; @@ -97,6 +110,19 @@ public function loadContainer(): string // make sure invocations via blackfire use the same container unset($staticParameters['env']['BLACKFIRE_AGENT_SOCKET']); + // Keep only the env vars the container actually depends on in the cache key, so unrelated env + // changes (CI/shell) don't force a full recompile - phpstan/phpstan#14072. The full env stays + // in the container parameters, so %env.*% resolution is unaffected; this only narrows the key. + if (isset($staticParameters['env'])) { + $relevantEnvVariableNames = $this->relevantEnvVariableNamesForCacheKey(); + if ($relevantEnvVariableNames !== null) { + $staticParameters['env'] = array_intersect_key( + $staticParameters['env'], + array_fill_keys($relevantEnvVariableNames, true), + ); + } + } + $containerKey = [ $staticParameters, array_keys($this->dynamicParameters), @@ -231,6 +257,41 @@ public function createContainer(bool $initialize = true): OriginalNetteContainer return $container; } + /** + * Env vars that can change the generated container, so they must stay in the cache key: every + * %env.NAME% referenced across the loaded configs, plus BUILD_TIME_ENV_VARIABLES. Returns null + * when a config references the whole %env% array, in which case all of it must be kept. + * + * @return list|null + */ + private function relevantEnvVariableNamesForCacheKey(): ?array + { + $names = self::BUILD_TIME_ENV_VARIABLES; + foreach ($this->allConfigFiles as $file) { + $contents = @file_get_contents($file); + if ($contents === false || !str_contains($contents, '%env')) { + continue; + } + + // A bare %env% reference exposes the whole environment to the container; keep all of it. + if (preg_match('~%env%~', $contents) === 1) { + return null; + } + + // Match Nette's parameter-name grammar (%([\w.-]*)% in NeonAdapter) so a valid reference like + // %env.MY-VAR% is not missed, which would drop MY-VAR from the key and reuse a stale container. + if (preg_match_all('~%env\.([\w.-]+)%~', $contents, $matches) === 0) { + continue; + } + + foreach ($matches[1] as $name) { + $names[] = $name; + } + } + + return $names; + } + /** * @return string[] */ diff --git a/tests/PHPStan/DependencyInjection/ContainerCacheKeyEnvGuardTest.php b/tests/PHPStan/DependencyInjection/ContainerCacheKeyEnvGuardTest.php new file mode 100644 index 0000000000..f0c6e79415 --- /dev/null +++ b/tests/PHPStan/DependencyInjection/ContainerCacheKeyEnvGuardTest.php @@ -0,0 +1,68 @@ +isFile() || $file->getExtension() !== 'php') { + continue; + } + + $contents = file_get_contents($file->getPathname()); + if ($contents === false || !str_contains($contents, 'extends CompilerExtension')) { + continue; + } + + if (preg_match_all('~(?:getenv\(|\$_ENV\[)\s*[\'"]([A-Za-z_][A-Za-z0-9_]*)[\'"]~', $contents, $matches) === 0) { + continue; + } + + foreach ($matches[1] as $envName) { + if (in_array($envName, Configurator::BUILD_TIME_ENV_VARIABLES, true)) { + continue; + } + + $offenders[] = $file->getFilename() . ': ' . $envName; + } + } + + sort($offenders); + + $this->assertSame( + [], + $offenders, + 'A CompilerExtension reads an environment variable at build time that is not declared in ' + . 'Configurator::BUILD_TIME_ENV_VARIABLES. Add it there (so it stays in the container cache ' + . 'key and a change recompiles), or read it via %env.* in a config instead:' . PHP_EOL + . implode(PHP_EOL, $offenders), + ); + } + +} diff --git a/tests/PHPStan/DependencyInjection/ContainerCacheKeyEnvTest.php b/tests/PHPStan/DependencyInjection/ContainerCacheKeyEnvTest.php new file mode 100644 index 0000000000..7adaa383e2 --- /dev/null +++ b/tests/PHPStan/DependencyInjection/ContainerCacheKeyEnvTest.php @@ -0,0 +1,176 @@ +envBackup = getenv(self::ENV_VAR); + $this->tmpDir = sys_get_temp_dir() . '/phpstan-container-cache-key-' . md5(uniqid()); + $this->removeDirectory($this->tmpDir); + } + + #[Override] + protected function tearDown(): void + { + if ($this->envBackup === false) { + putenv(self::ENV_VAR); + } else { + putenv(sprintf('%s=%s', self::ENV_VAR, $this->envBackup)); + } + + $this->removeDirectory($this->tmpDir); + } + + public function testEnvironmentIsDroppedFromCacheKeyWhenNoConfigReferencesIt(): void + { + // No loaded config uses %env.*%, so an unrelated env change must NOT recompile the container. + putenv(sprintf('%s=first', self::ENV_VAR)); + $this->buildContainer(__DIR__ . '/containerCacheKey-noenv.neon'); + + putenv(sprintf('%s=second', self::ENV_VAR)); + $this->buildContainer(__DIR__ . '/containerCacheKey-noenv.neon'); + + $this->assertSame( + 1, + $this->countCompiledContainers(), + 'Changing an unrelated environment variable should reuse the cached container.', + ); + } + + public function testEnvironmentStaysInCacheKeyWhenAConfigReferencesIt(): void + { + // A loaded config uses %env.*%, so the environment is relevant and a change must recompile. + putenv(sprintf('%s=first', self::ENV_VAR)); + $this->buildContainer(__DIR__ . '/containerCacheKey-withenv.neon'); + + putenv(sprintf('%s=second', self::ENV_VAR)); + $this->buildContainer(__DIR__ . '/containerCacheKey-withenv.neon'); + + $this->assertSame( + 2, + $this->countCompiledContainers(), + 'When a config references %env.*%, changing it must recompile the container.', + ); + } + + public function testDashedEnvVariableReferenceStaysInCacheKey(): void + { + // %env.MY-VAR% is a valid Nette reference (grammar %([\w.-]*)%): extraction must keep dashed + // names too, otherwise changing the referenced var would reuse a stale container. + $envName = 'PHPSTAN_TEST_W1-DASH'; + $backup = getenv($envName); + try { + putenv(sprintf('%s=first', $envName)); + $this->buildContainer(__DIR__ . '/containerCacheKey-withenv-dashed.neon'); + + putenv(sprintf('%s=second', $envName)); + $this->buildContainer(__DIR__ . '/containerCacheKey-withenv-dashed.neon'); + + $this->assertSame( + 2, + $this->countCompiledContainers(), + 'A %env.* reference whose name contains a dash must keep that var in the cache key.', + ); + } finally { + if ($backup === false) { + putenv($envName); + } else { + putenv(sprintf('%s=%s', $envName, $backup)); + } + } + } + + public function testBuildTimeEnvVariableStaysInCacheKey(): void + { + // FnsrExtension reads getenv('PHPSTAN_FNSR') at build time and rewires the container, so it + // depends on the var even with no %env.*% in config; toggling it must recompile. + $backup = getenv('PHPSTAN_FNSR'); + try { + putenv('PHPSTAN_FNSR=0'); + $this->buildContainer(__DIR__ . '/containerCacheKey-noenv.neon'); + + putenv('PHPSTAN_FNSR=1'); + $this->buildContainer(__DIR__ . '/containerCacheKey-noenv.neon'); + + $this->assertSame( + 2, + $this->countCompiledContainers(), + 'Changing PHPSTAN_FNSR (read at build time by FnsrExtension) must recompile the container.', + ); + } finally { + if ($backup === false) { + putenv('PHPSTAN_FNSR'); + } else { + putenv(sprintf('PHPSTAN_FNSR=%s', $backup)); + } + } + } + + private function buildContainer(string $additionalConfigFile): void + { + $rootDir = __DIR__ . '/../../..'; + $fileHelper = new FileHelper($rootDir); + $rootDir = $fileHelper->normalizePath($rootDir, '/'); + $containerFactory = new ContainerFactory($rootDir); + $containerFactory->create( + $this->tmpDir, + [ + $containerFactory->getConfigDirectory() . '/config.level8.neon', + $additionalConfigFile, + ], + [], + ); + } + + private function countCompiledContainers(): int + { + $containers = glob($this->tmpDir . '/cache/nette.configurator/Container_*.php'); + + return $containers === false ? 0 : count($containers); + } + + private function removeDirectory(string $directory): void + { + if (!is_dir($directory)) { + return; + } + + $entries = glob($directory . DIRECTORY_SEPARATOR . '*'); + foreach ($entries === false ? [] : $entries as $entry) { + if (is_dir($entry)) { + $this->removeDirectory($entry); + } else { + @unlink($entry); + } + } + + @rmdir($directory); + } + +} diff --git a/tests/PHPStan/DependencyInjection/containerCacheKey-noenv.neon b/tests/PHPStan/DependencyInjection/containerCacheKey-noenv.neon new file mode 100644 index 0000000000..f3ba79b4ae --- /dev/null +++ b/tests/PHPStan/DependencyInjection/containerCacheKey-noenv.neon @@ -0,0 +1,2 @@ +parameters: + customRulesetUsed: true diff --git a/tests/PHPStan/DependencyInjection/containerCacheKey-withenv-dashed.neon b/tests/PHPStan/DependencyInjection/containerCacheKey-withenv-dashed.neon new file mode 100644 index 0000000000..834eaae582 --- /dev/null +++ b/tests/PHPStan/DependencyInjection/containerCacheKey-withenv-dashed.neon @@ -0,0 +1,5 @@ +# References an env var whose name contains a dash (%env.PHPSTAN_TEST_W1-DASH%) - a valid Nette +# reference (Nette's parameter-name grammar is %([\w.-]*)%). The env-name extraction must keep such +# names in the container cache key too, otherwise changing this var would reuse a stale container. +parameters: + tmpDir: %env.PHPSTAN_TEST_W1-DASH% diff --git a/tests/PHPStan/DependencyInjection/containerCacheKey-withenv.neon b/tests/PHPStan/DependencyInjection/containerCacheKey-withenv.neon new file mode 100644 index 0000000000..70813de3d6 --- /dev/null +++ b/tests/PHPStan/DependencyInjection/containerCacheKey-withenv.neon @@ -0,0 +1,6 @@ +# References an environment variable via %env.%, so that env var must stay in the container +# cache key - otherwise changing it would wrongly reuse a stale container. The resolved tmpDir value +# is irrelevant here (ContainerFactory::create() overrides tmpDir with its own argument); the +# reference itself is what keeps PHPSTAN_TEST_W1_CACHE_KEY in the cache key. +parameters: + tmpDir: %env.PHPSTAN_TEST_W1_CACHE_KEY%