diff --git a/.gitignore b/.gitignore index 8484290..27d7e31 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,4 @@ /.idea /vendor /composer.lock -/old /.phpunit.result.cache \ No newline at end of file diff --git a/composer.json b/composer.json index ae8217a..88983da 100644 --- a/composer.json +++ b/composer.json @@ -6,14 +6,15 @@ "require": { "php": ">=7.4", "ext-json": "*", + "ext-mbstring": "*", "pimple/pimple": "~3.0", "symfony/yaml": "^2.4|~3|~4|~5", "psr/simple-cache": "^1.0", "ocramius/proxy-manager": "^2.8" }, "require-dev": { - "phpunit/phpunit": "~8", - "phpstan/phpstan": "^0.10.2", + "phpunit/phpunit": "~9", + "phpstan/phpstan": "~0", "mikey179/vfsstream": "^1.6", "cache/array-adapter": "^1.0", "cache/filesystem-adapter": "^1.0" diff --git a/phpunit.xml b/phpunit.xml index 6bceeb2..4829e95 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -1,9 +1,8 @@ - + tests - tests/bootstrap.php diff --git a/src/CompiledConfigBuilder.php b/src/CompiledConfigBuilder.php index 70dbdf5..94db0e1 100644 --- a/src/CompiledConfigBuilder.php +++ b/src/CompiledConfigBuilder.php @@ -98,7 +98,9 @@ public function build(MasterConfig $masterConfig, array $parameters = []) $tagMap = []; foreach ($parameters as $key => $value) { // Resolve all the parameters up front - $parameters[$key] = $parameterResolver->resolve($parameters, $value, $tagMap); + $parameters[$key] = is_array($value) ? + $parameterResolver->resolveArray($parameters, $value, $tagMap) : + $parameterResolver->resolve($parameters, $value, $tagMap); } foreach ($services as $serviceName => &$definition) { diff --git a/src/ContainerBuilder.php b/src/ContainerBuilder.php index 8966519..f522790 100644 --- a/src/ContainerBuilder.php +++ b/src/ContainerBuilder.php @@ -81,7 +81,7 @@ private function buildService(Container $container, array $definition) if ($isFactoryCreated) { $service = call_user_func_array( [ - $definition["factoryClass"] ?? $container->offsetGet(mb_substr($definition["factoryService"], 1)), + $definition["factoryClass"] ?? $container->offsetGet(\mb_substr($definition["factoryService"], 1)), $definition["factoryMethod"] ], $arguments @@ -103,9 +103,9 @@ protected function resolveArray(Container $container, array $array) { $arguments = []; foreach ($array as $k => $value) { - if (is_string($value) && strlen($value) > 0 && $value[0] === "\0") { - $arguments[$k] = $container->offsetGet(mb_substr($value, 1)); - } elseif (is_array($value)) { + if (\is_string($value) && \strlen($value) > 0 && $value[0] === "\0") { + $arguments[$k] = $container->offsetGet(\mb_substr($value, 1)); + } elseif (\is_array($value)) { $arguments[$k] = $this->resolveArray($container, $value); } else { $arguments[$k] = $value; diff --git a/src/FileConfig.php b/src/FileConfig.php index 98fba11..a9bfed6 100644 --- a/src/FileConfig.php +++ b/src/FileConfig.php @@ -237,26 +237,26 @@ protected function namespace(string $string) protected function isNamespaced(string $key) { - return mb_strpos($key, Token::NAMESPACE_SEPARATOR) !== false; + return \mb_strpos($key, Token::NAMESPACE_SEPARATOR) !== false; } protected function recursivelyNamespace($value) { - if (is_array($value)) { + if (\is_array($value)) { $return = []; foreach ($value as $k => $v) { $return[$k] = $this->recursivelyNamespace($v); } return $return; - } elseif (is_string($value) && strlen($value) > 0) { + } elseif (\is_string($value) && \strlen($value) > 0) { if ($value[0] === Token::SERVICE_CHAR) { return Token::SERVICE_CHAR . $this->namespace(mb_substr($value, 1)); - } elseif (mb_strpos($value, Token::PARAMETER_CHAR) !== false) { + } elseif (\mb_strpos($value, Token::PARAMETER_CHAR) !== false) { $replacements = []; $n = 0; - while (mb_substr_count($value, Token::PARAMETER_CHAR) > 1) { - $pos1 = mb_strpos($value, Token::PARAMETER_CHAR); - $pos2 = mb_strpos($value, Token::PARAMETER_CHAR, $pos1 + 1); + while (\mb_substr_count($value, Token::PARAMETER_CHAR) > 1) { + $pos1 = \mb_strpos($value, Token::PARAMETER_CHAR); + $pos2 = \mb_strpos($value, Token::PARAMETER_CHAR, $pos1 + 1); $placeholder = '|||'.$n.'|||'; @@ -265,12 +265,12 @@ protected function recursivelyNamespace($value) $this->namespace(mb_substr($value, $pos1 + 1, $pos2 - ($pos1 + 1))) . Token::PARAMETER_CHAR; - $value = mb_substr($value, 0, $pos1) . $placeholder . mb_substr($value, $pos2 + 1); + $value = \mb_substr($value, 0, $pos1) . $placeholder . \mb_substr($value, $pos2 + 1); $n++; } foreach ($replacements as $search => $replace) { - $value = str_replace($search, $replace, $value); + $value = \str_replace($search, $replace, $value); } } } diff --git a/src/FileStateCollection.php b/src/FileStateCollection.php index ed8a7a2..661880b 100644 --- a/src/FileStateCollection.php +++ b/src/FileStateCollection.php @@ -4,7 +4,7 @@ class FileStateCollection { - protected $state = []; + protected array $state = []; public function __construct(array $filenames) { diff --git a/src/Loader/YamlLoader.php b/src/Loader/YamlLoader.php index 791d8f1..f8fed05 100644 --- a/src/Loader/YamlLoader.php +++ b/src/Loader/YamlLoader.php @@ -9,9 +9,9 @@ class YamlLoader implements LoaderInterface { - protected $hasSymfony; - protected $hasExtension; - protected $forceSymfony; + protected bool $hasSymfony; + protected bool $hasExtension; + protected bool $forceSymfony; public function __construct(bool $useSymfony = false) { diff --git a/src/MasterConfig.php b/src/MasterConfig.php index 18f7384..a2c6c24 100644 --- a/src/MasterConfig.php +++ b/src/MasterConfig.php @@ -8,10 +8,10 @@ class MasterConfig { - protected $filenames = []; - protected $services = []; - protected $parameters = []; - protected $extensions = []; + protected array $filenames = []; + protected array $services = []; + protected array $parameters = []; + protected array $extensions = []; public function addFileConfig(FileConfig $fileConfig) { @@ -71,9 +71,8 @@ public function getExtensions() * Unfortunately, all of PHPs underlying sorting is done by QuickSort, so we have to do the sorting ourselves * * @param array $array - * @return array */ - protected function stableWeightSort(array &$array) + protected function stableWeightSort(array &$array) : void { foreach ($array as $i => &$value) { $value["key"] = $i; diff --git a/src/ParameterResolver.php b/src/ParameterResolver.php index 19f0a90..65b5599 100644 --- a/src/ParameterResolver.php +++ b/src/ParameterResolver.php @@ -17,14 +17,14 @@ class ParameterResolver public function resolveArray(array $parameters, array $array, array &$tagMap) { foreach ($array as $k => $v) { - $array[$k] = is_array($v) ? $this->resolveArray($parameters, $v, $tagMap) : $this->resolve($parameters, $v, $tagMap); + $array[$k] = \is_array($v) ? $this->resolveArray($parameters, $v, $tagMap) : $this->resolve($parameters, $v, $tagMap); } return $array; } public function resolve(array $parameters, $parameter, array &$tagMap) { - if (!is_string($parameter) || strlen($parameter) === 0) { + if (!\is_string($parameter) || \strlen($parameter) === 0) { return $parameter; } @@ -35,24 +35,24 @@ public function resolve(array $parameters, $parameter, array &$tagMap) return $parameter; case Token::SERVICE_CHAR: - return "\0" . mb_substr($parameter, 1); + return "\0" . \mb_substr($parameter, 1); case Token::TAG_CHAR: - $tagMap[mb_substr($parameter, 1)] = true; + $tagMap[\mb_substr($parameter, 1)] = true; return "\0" . $parameter; } $parameter = $this->replaceParameters($parameters, $parameter); - if (!is_string($parameter)) { - if (is_array($parameter)) { + if (!\is_string($parameter)) { + if (\is_array($parameter)) { return $this->resolveArray($parameters, $parameter, $tagMap); } return $parameter; } $parameter = $this->replaceConstants($parameter); - if (!is_string($parameter)) { - if (is_array($parameter)) { + if (!\is_string($parameter)) { + if (\is_array($parameter)) { return $this->resolveArray($parameters, $parameter, $tagMap); } return $parameter; @@ -63,13 +63,13 @@ public function resolve(array $parameters, $parameter, array &$tagMap) protected function replaceParameters(array $parameters, string $parameter) { - if (mb_strpos($parameter, Token::PARAMETER_CHAR) === false) { + if (\mb_strpos($parameter, Token::PARAMETER_CHAR) === false) { return $parameter; } return $this->replaceSurroundingToken($parameter, Token::PARAMETER_CHAR, function($value) use ($parameters) { // We need array_key_exists for if a value has been set as null, but isset is far far faster than array_key_exists - if (isset($parameters[$value]) || array_key_exists($value, $parameters)) { + if (isset($parameters[$value]) || \array_key_exists($value, $parameters)) { return $parameters[$value]; } @@ -79,7 +79,7 @@ protected function replaceParameters(array $parameters, string $parameter) protected function replaceConstants(string $parameter, array &$resolvedConstants = []) { - if (mb_strpos($parameter, Token::CONSTANT_CHAR) === false) { + if (\mb_strpos($parameter, Token::CONSTANT_CHAR) === false) { return $parameter; } @@ -96,12 +96,11 @@ protected function replaceConstants(string $parameter, array &$resolvedConstants protected function replaceEnvironment(string $parameter) { - if (mb_strpos($parameter, Token::ENV_CHAR) === false) { + if (\mb_strpos($parameter, Token::ENV_CHAR) === false) { return $parameter; } return $this->replaceSurroundingToken($parameter, Token::ENV_CHAR, function($value) { - if (($env = getenv($value)) !== false) { $this->resolvedEnvVars[$value] = $env; return $env; @@ -111,41 +110,60 @@ protected function replaceEnvironment(string $parameter) }); } - protected function replaceSurroundingToken(string $parameter, string $token, callable $callable) + protected function replaceSurroundingToken(string $parameter, string $token, callable $callable, array $stack = []) { $oldParameter = $parameter; - while (is_string($parameter) && mb_substr_count($parameter, $token) > 0) { - $pos1 = mb_strpos($parameter, $token); - $pos2 = mb_strpos($parameter, $token, $pos1 + 1); + while (\is_string($parameter) && \mb_substr_count($parameter, $token) > 0) { + // The passStack keeps track of what parameters we've had to resolve on this parameter and passes this + // down to ensure that we don't end up trying to do any recursive resolution + $passStack = $stack; + + // Find the first entry in the parameter + $pos1 = \mb_strpos($parameter, $token); + $pos2 = \mb_strpos($parameter, $token, $pos1 + 1); + // If they are next to each other, then they're escaped tokens and should be ignored if ($pos2 === $pos1 + 1) { - $parameter = mb_substr($parameter, 0, $pos1) . self::ESCAPED_TOKEN . mb_substr($parameter, $pos2 + 1); + $parameter = \mb_substr($parameter, 0, $pos1) . self::ESCAPED_TOKEN . \mb_substr($parameter, $pos2 + 1); continue; } + // If there's only one token, then someone has screwed up the config if ($pos2 === false) { throw new ConfigException("An uneven number of '{$token}' token bindings exists for '{$oldParameter}'"); } - $value = mb_substr($parameter, $pos1 + 1, $pos2 - ($pos1 + 1)); - $newValue = $callable($value); + // Get the value of the parameter, so with '%foo_bar% salad' this should return 'foo_bar' + $value = \mb_substr($parameter, $pos1 + 1, $pos2 - ($pos1 + 1)); + if (in_array($value, $stack, true)) { + throw new ConfigException("Infinite parameter loop detected: " . implode(" -> ", $stack)); + } + + // Add that we've resolved this key to the passStack + $passStack[] = $value; + $newValue = $callable($value); // If it took up the entire parameter - if (($pos1 === 0 && ($pos2 + 1) === mb_strlen($parameter))) { - $parameter = $newValue; + if (($pos1 === 0 && ($pos2 + 1) === \mb_strlen($parameter))) { + if (\is_string($newValue)) { + $parameter = $this->replaceSurroundingToken($newValue, $token, $callable, $passStack); + } else { + $parameter = $newValue; + } continue; } - if (!is_string($newValue) && !is_numeric($newValue)) { + if (!\is_string($newValue) && !\is_numeric($newValue)) { throw new ConfigException( "Parameter '{$value}' as part of '{$oldParameter}' resolved to a non-string. This is only permissible if the parameter attempts no interpolation" ); } - $parameter = mb_substr($parameter, 0, $pos1) . $newValue . mb_substr($parameter, $pos2 + 1); + + $parameter = \mb_substr($parameter, 0, $pos1) . $this->replaceSurroundingToken($newValue, $token, $callable, $passStack) . \mb_substr($parameter, $pos2 + 1); } - return is_string($parameter) ? str_replace(self::ESCAPED_TOKEN, $token, $parameter) : $parameter; + return \is_string($parameter) ? \str_replace(self::ESCAPED_TOKEN, $token, $parameter) : $parameter; } public function getResolvedConstants() : array diff --git a/src/TagCollection.php b/src/TagCollection.php index 44ff661..23bce02 100644 --- a/src/TagCollection.php +++ b/src/TagCollection.php @@ -7,10 +7,10 @@ class TagCollection implements \Iterator { - private $position = 0; - protected $services = []; - protected $aliases = []; - protected $container; + private int $position = 0; + protected array $services = []; + protected array $aliases = []; + protected Container $container; public function __construct(Container $container, array $services) { diff --git a/tests/bootstrap.php b/tests/bootstrap.php deleted file mode 100644 index 8cfe215..0000000 --- a/tests/bootstrap.php +++ /dev/null @@ -1,3 +0,0 @@ - [__DIR__], + "files" => ["file1.yml"] + ]); + + self::assertSame(["fridge", "magnet"], $container["my_parameter_9"]); + + self::assertSame(["potato", ["salad"]], $container["my_parameter_10"]); + } + public function testBoolAndInt() { $container = Syringe::build([ diff --git a/tests/integration/Basic/file1.yml b/tests/integration/Basic/file1.yml index 3a8a9f7..4e91c0e 100644 --- a/tests/integration/Basic/file1.yml +++ b/tests/integration/Basic/file1.yml @@ -22,4 +22,12 @@ parameters: my_parameter_6: 5003 my_parameter_7: "potato" - my_parameter_8: "salad" \ No newline at end of file + my_parameter_8: "salad" + + my_parameter_9: + - "fridge" + - "magnet" + + my_parameter_10: + - "%my_parameter_7%" + - ["%my_parameter_8%"] \ No newline at end of file diff --git a/tests/integration/Recursion/RecursionTest.php b/tests/integration/Recursion/RecursionTest.php new file mode 100644 index 0000000..b9e1f49 --- /dev/null +++ b/tests/integration/Recursion/RecursionTest.php @@ -0,0 +1,46 @@ + [__DIR__], + "files" => ["file1.yml"] + ]); + + self::assertSame(["fridge", "magnet"], $container["my_parameter_9"]); + } + + /** + * This is complicated, but it shouldn't trigger a recursion exception + * + * @throws ConfigException + * @throws \Psr\SimpleCache\InvalidArgumentException + * @throws \Silktide\Syringe\Exception\LoaderException + */ + public function testConfusingNonRecursive() + { + $container = Syringe::build([ + "paths" => [__DIR__], + "files" => ["file2.yml"] + ]); + + self::assertSame("hello hello", $container["my_parameter_1"]); + } + +} \ No newline at end of file diff --git a/tests/integration/Recursion/file1.yml b/tests/integration/Recursion/file1.yml new file mode 100644 index 0000000..fbe0d54 --- /dev/null +++ b/tests/integration/Recursion/file1.yml @@ -0,0 +1,8 @@ +parameters: + my_parameter_1: "%my_parameter_2% salad" + my_parameter_2: "%my_parameter_3%" + my_parameter_3: "%my_parameter_4%" + my_parameter_4: "%my_parameter_5%" + my_parameter_5: "%my_parameter_6%" + my_parameter_6: "%my_parameter_7%" + my_parameter_7: "%my_parameter_1% foo" diff --git a/tests/integration/Recursion/file2.yml b/tests/integration/Recursion/file2.yml new file mode 100644 index 0000000..dd0ed7a --- /dev/null +++ b/tests/integration/Recursion/file2.yml @@ -0,0 +1,7 @@ +parameters: + my_parameter_1: "%my_parameter_2%" + my_parameter_2: "%my_parameter_5% %my_parameter_3%" + my_parameter_3: "%my_parameter_4%" + my_parameter_4: "%my_parameter_5%" + my_parameter_5: "%my_parameter_6%" + my_parameter_6: "hello" diff --git a/tests/unit/Container/InvalidContainer.php b/tests/unit/Container/InvalidContainer.php index ba3e7ed..4096179 100644 --- a/tests/unit/Container/InvalidContainer.php +++ b/tests/unit/Container/InvalidContainer.php @@ -1,9 +1,7 @@