diff --git a/src/DependencyInjection/Configurator.php b/src/DependencyInjection/Configurator.php index 7865956266b..e60d49b8ce7 100644 --- a/src/DependencyInjection/Configurator.php +++ b/src/DependencyInjection/Configurator.php @@ -6,25 +6,36 @@ use Nette\DI\Config\Loader; use Nette\DI\Container as OriginalNetteContainer; use Nette\DI\ContainerLoader; +use Nette\DI\Definitions\Statement; +use Nette\Neon\Entity; use Override; use PHPStan\File\CouldNotReadFileException; use PHPStan\File\CouldNotWriteFileException; use PHPStan\File\FileReader; use PHPStan\File\FileWriter; use PHPStan\Turbo\TurboExtensionEnabler; +use Throwable; +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_array; use function is_dir; use function is_file; +use function is_string; 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 +49,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 +115,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 +262,96 @@ 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 (or its references can't be safely enumerated), + * in which case all of it must be kept. + * + * References are enumerated from the config as parsed by PHPStan's own NeonAdapter - the same + * parser the container compiler uses - rather than from raw config text: comments are ignored and + * service entities become Statements, so a %env.NAME% in a parameter value, a service argument or + * a factory is found the same way it is at compile time. The %env.NAME% grammar mirrors Nette's + * parameter-name grammar (%([\w.-]*)%), so dashed names like %env.MY-VAR% are handled too. + * + * @return list|null + */ + private function relevantEnvVariableNamesForCacheKey(): ?array + { + $names = self::BUILD_TIME_ENV_VARIABLES; + $adapter = new NeonAdapter([]); + + foreach ($this->allConfigFiles as $file) { + $contents = @file_get_contents($file); + if ($contents === false || !str_contains($contents, '%env')) { + continue; + } + + try { + $data = $adapter->load($file); + } catch (Throwable) { + // A config we can't parse as NEON here (e.g. a .php config) might reference any env + // var, so we can't prove which ones the container needs: keep the whole environment. + return null; + } + + $referencesWholeEnv = false; + $referencedNames = $this->collectReferencedEnvVariableNames($data, $referencesWholeEnv); + if ($referencesWholeEnv) { + return null; + } + + foreach ($referencedNames as $name) { + $names[] = $name; + } + } + + return $names; + } + + /** + * Recursively collect the env-variable names referenced by %env.NAME% in a parsed config node. + * Recurses into arrays and into the service definitions the NeonAdapter produces (Statements, and + * any remaining Neon entities) so references in service arguments and factories are not missed. + * Sets $referencesWholeEnv on a bare %env% (the whole environment), which forces keeping all of it. + * + * @param mixed $node + * @return list + */ + private function collectReferencedEnvVariableNames($node, bool &$referencesWholeEnv): array + { + if (is_string($node)) { + if (preg_match('~%env%~', $node) === 1) { + $referencesWholeEnv = true; + } + + if (preg_match_all('~%env\.([\w.-]+)%~', $node, $matches) > 0) { + return $matches[1]; + } + + return []; + } + + if ($node instanceof Statement) { + $node = [$node->getEntity(), $node->arguments]; + } elseif ($node instanceof Entity) { + $node = [$node->value, $node->attributes]; + } + + if (!is_array($node)) { + return []; + } + + $names = []; + foreach ($node as $value) { + foreach ($this->collectReferencedEnvVariableNames($value, $referencesWholeEnv) 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 00000000000..667683e9898 --- /dev/null +++ b/tests/PHPStan/DependencyInjection/ContainerCacheKeyEnvGuardTest.php @@ -0,0 +1,189 @@ +isFile() || $file->getExtension() !== 'php') { + continue; + } + + $contents = file_get_contents($file->getPathname()); + if ($contents === false || !str_contains($contents, 'extends CompilerExtension')) { + continue; + } + + foreach ($this->buildTimeEnvReads($contents) 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), + ); + } + + /** + * Literal env-var names read via getenv('NAME') or $_ENV['NAME'], found through the tokenizer so + * that occurrences in comments or string literals are not matched. + * + * @return list + */ + private function buildTimeEnvReads(string $contents): array + { + $tokens = token_get_all($contents); + $names = []; + + for ($i = 0, $count = count($tokens); $i < $count; $i++) { + $token = $tokens[$i]; + if (!is_array($token)) { + continue; + } + + if ($token[0] === T_STRING && strtolower($token[1]) === 'getenv') { + $previous = $this->previousSignificantTokenId($tokens, $i); + if (in_array($previous, [T_OBJECT_OPERATOR, T_DOUBLE_COLON], true)) { + continue; // a method or static call, not the global getenv() + } + + $name = $this->literalStringArgument($tokens, $i, '('); + } elseif ($token[0] === T_VARIABLE && $token[1] === '$_ENV') { + $name = $this->literalStringArgument($tokens, $i, '['); + } else { + continue; + } + + if ($name === null) { + continue; + } + + $names[] = $name; + } + + return $names; + } + + /** + * The literal name in ` 'NAME'` that follows token $index (skipping whitespace and + * comments), or null when the first argument is not a plain literal, which means a dynamic name + * that has to be routed through %env.* anyway. + * + * @param list $tokens + */ + private function literalStringArgument(array $tokens, int $index, string $opener): ?string + { + $openerIndex = $this->nextSignificantTokenIndex($tokens, $index + 1); + if ($openerIndex === null || $tokens[$openerIndex] !== $opener) { + return null; + } + + $argumentIndex = $this->nextSignificantTokenIndex($tokens, $openerIndex + 1); + if ($argumentIndex === null) { + return null; + } + + $argument = $tokens[$argumentIndex]; + if (!is_array($argument) || $argument[0] !== T_CONSTANT_ENCAPSED_STRING) { + return null; + } + + $name = substr($argument[1], 1, -1); + if (preg_match('~^[A-Za-z_][A-Za-z0-9_]*$~', $name) !== 1) { + return null; + } + + return $name; + } + + /** + * @param list $tokens + */ + private function nextSignificantTokenIndex(array $tokens, int $from): ?int + { + for ($i = $from, $count = count($tokens); $i < $count; $i++) { + $token = $tokens[$i]; + if (is_array($token) && in_array($token[0], [T_WHITESPACE, T_COMMENT, T_DOC_COMMENT], true)) { + continue; + } + + return $i; + } + + return null; + } + + /** + * @param list $tokens + */ + private function previousSignificantTokenId(array $tokens, int $index): ?int + { + for ($i = $index - 1; $i >= 0; $i--) { + $token = $tokens[$i]; + if (is_array($token) && in_array($token[0], [T_WHITESPACE, T_COMMENT, T_DOC_COMMENT], true)) { + continue; + } + + return is_array($token) ? $token[0] : null; + } + + return null; + } + +} diff --git a/tests/PHPStan/DependencyInjection/ContainerCacheKeyEnvTest.php b/tests/PHPStan/DependencyInjection/ContainerCacheKeyEnvTest.php new file mode 100644 index 00000000000..2ca635efd0d --- /dev/null +++ b/tests/PHPStan/DependencyInjection/ContainerCacheKeyEnvTest.php @@ -0,0 +1,214 @@ +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 (parameter-name grammar %([\w.-]*)%). The env-name + // extraction must keep dash/dot/digit-start names too - otherwise changing such a referenced + // var would reuse a stale container, the regression class W1 fixes. + $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') in beforeCompile() and rewires the container, so + // the compiled container depends on it even though no config references %env.*%. Toggling it + // must recompile - otherwise the change would be silently ignored (a stale container). + $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)); + } + } + } + + public function testEnvironmentReferencedInServiceFactoryStaysInCacheKey(): void + { + // %env.* referenced inside a service factory (a Neon entity), not a plain parameter value. + // Enumerating references through Nette's expander reaches this position; dropping it would + // reuse a stale container when the variable changes. + putenv(sprintf('%s=first', self::ENV_VAR)); + $this->buildContainer(__DIR__ . '/containerCacheKey-withenv-factory.neon'); + + putenv(sprintf('%s=second', self::ENV_VAR)); + $this->buildContainer(__DIR__ . '/containerCacheKey-withenv-factory.neon'); + + $this->assertSame( + 2, + $this->countCompiledContainers(), + 'A %env.* reference inside a service definition must keep that var in the cache key.', + ); + } + + public function testEnvironmentReferencedOnlyInACommentIsDroppedFromCacheKey(): void + { + // %env.* appearing only in a comment is not a real reference. Nette's parser ignores comments, + // so the variable is dropped from the key and an unrelated change does not recompile - a raw + // text scan would instead treat the comment as a reference and recompile spuriously. + putenv(sprintf('%s=first', self::ENV_VAR)); + $this->buildContainer(__DIR__ . '/containerCacheKey-comment-env.neon'); + + putenv(sprintf('%s=second', self::ENV_VAR)); + $this->buildContainer(__DIR__ . '/containerCacheKey-comment-env.neon'); + + $this->assertSame( + 1, + $this->countCompiledContainers(), + 'A %env.* mentioned only in a comment must not keep that var in the cache key.', + ); + } + + 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-comment-env.neon b/tests/PHPStan/DependencyInjection/containerCacheKey-comment-env.neon new file mode 100644 index 00000000000..15c2fccb56d --- /dev/null +++ b/tests/PHPStan/DependencyInjection/containerCacheKey-comment-env.neon @@ -0,0 +1,5 @@ +# Mentions %env.PHPSTAN_TEST_W1_CACHE_KEY% only in this comment - it is not a real reference, so the +# variable must not be kept in the container cache key (Nette's parser ignores comments, unlike a raw +# text scan which would treat this as a reference and recompile spuriously). +parameters: + customRulesetUsed: true diff --git a/tests/PHPStan/DependencyInjection/containerCacheKey-noenv.neon b/tests/PHPStan/DependencyInjection/containerCacheKey-noenv.neon new file mode 100644 index 00000000000..f3ba79b4ae7 --- /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 00000000000..834eaae5829 --- /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-factory.neon b/tests/PHPStan/DependencyInjection/containerCacheKey-withenv-factory.neon new file mode 100644 index 00000000000..412c4006b90 --- /dev/null +++ b/tests/PHPStan/DependencyInjection/containerCacheKey-withenv-factory.neon @@ -0,0 +1,5 @@ +# References %env.* inside a service factory entity - a position that a scan limited to plain +# parameter/argument string values would miss. Toggling the referenced var must still recompile. +services: + envFactoryService: + factory: PhpParser\Node\Name(%env.PHPSTAN_TEST_W1_CACHE_KEY%) diff --git a/tests/PHPStan/DependencyInjection/containerCacheKey-withenv.neon b/tests/PHPStan/DependencyInjection/containerCacheKey-withenv.neon new file mode 100644 index 00000000000..70813de3d69 --- /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%