diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 0acb49a4f..62ed9e6a9 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -1584,9 +1584,15 @@ gettype($helpers) gettype($variables) + + __templateResolver->resolve($name, $this)]]> + $values + + string|Resolver + $__vars __template]]> @@ -1623,7 +1629,6 @@ string - string|Resolver getArrayCopy @@ -1634,7 +1639,6 @@ __filterChain->filter($this->__content)]]> - __templateResolver->resolve($name, $this)]]> __templateResolver]]> @@ -1670,57 +1674,56 @@ - - IteratorAggregate - - - $resolver - $resource + + lastLookupFailure]]> + lastLookupFailure]]> lastLookupFailure]]> lastLookupFailure]]> lastSuccessfulResolver]]> - - - false|string - - - resolve - - - $resource - - - null - - - $lastSuccessfulResolver - + lastSuccessfulResolver]]> + lastSuccessfulResolver]]> + - - $prefix - - - $result - + + resolve + + + (string) $prefix + - - - is_string($nameOrMap) + is_iterable($map) + is_iterable($map) - - IteratorAggregate - - - false|string - - - map[$name]]]> - + + map, $map)]]> + + + + is_string($path) + + + static::FAILURE_NOT_FOUND + static::FAILURE_NO_PATHS + + + setUseStreamWrapper + + + lastLookupFailure]]> + lastLookupFailure]]> + lastLookupFailure]]> + lastLookupFailure]]> + useStreamWrapper]]> + useStreamWrapper]]> + useViewStream]]> + useViewStream]]> + useViewStream]]> + is_string($path) @@ -1729,19 +1732,11 @@ false - $value - $value - $value - $value + + + + - - lastLookupFailure]]> - lastLookupFailure]]> - $value - - - paths]]> - (bool) $flag (bool) $flag @@ -2841,11 +2836,16 @@ $model - - - $test - $test - + + + assertNull + + + + + TemplatePathStack::FAILURE_NOT_FOUND + TemplatePathStack::FAILURE_NO_PATHS + diff --git a/src/Resolver/AggregateResolver.php b/src/Resolver/AggregateResolver.php index 11a3627f6..6146d2f5f 100644 --- a/src/Resolver/AggregateResolver.php +++ b/src/Resolver/AggregateResolver.php @@ -9,10 +9,15 @@ use Laminas\Stdlib\PriorityQueue; use Laminas\View\Renderer\RendererInterface as Renderer; use Laminas\View\Resolver\ResolverInterface as Resolver; -use ReturnTypeWillChange; // phpcs:ignore +use ReturnTypeWillChange; +use Traversable; use function count; +/** + * @final + * @implements IteratorAggregate + */ class AggregateResolver implements Countable, IteratorAggregate, Resolver { public const FAILURE_NO_RESOLVERS = 'AggregateResolver_Failure_No_Resolvers'; @@ -21,14 +26,20 @@ class AggregateResolver implements Countable, IteratorAggregate, Resolver /** * Last lookup failure * + * @deprecated This property will be removed in v3.0 of this component. + * * @var false|string */ protected $lastLookupFailure = false; - /** @var Resolver */ + /** + * @deprecated This property will be removed in v3.0 of this component. + * + * @var Resolver|null + */ protected $lastSuccessfulResolver; - /** @var PriorityQueue */ + /** @var PriorityQueue */ protected $queue; /** @@ -38,7 +49,10 @@ class AggregateResolver implements Countable, IteratorAggregate, Resolver */ public function __construct() { - $this->queue = new PriorityQueue(); + /** @var PriorityQueue $priorityQueue */ + $priorityQueue = new PriorityQueue(); + + $this->queue = $priorityQueue; } /** @@ -55,7 +69,7 @@ public function count() /** * IteratorAggregate: return internal iterator * - * @return PriorityQueue + * @return Traversable */ #[ReturnTypeWillChange] public function getIterator() @@ -67,7 +81,7 @@ public function getIterator() * Attach a resolver * * @param int $priority - * @return AggregateResolver + * @return $this */ public function attach(Resolver $resolver, $priority = 1) { @@ -92,6 +106,9 @@ public function resolve($name, ?Renderer $renderer = null) } foreach ($this->queue as $resolver) { + /** + * @todo This loop should be modified to try { return resolve } catch { continue } in v3.0 + */ $resource = $resolver->resolve($name, $renderer); if ($resource) { // Resource found; return it @@ -107,7 +124,9 @@ public function resolve($name, ?Renderer $renderer = null) /** * Return the last successful resolver, if any * - * @return Resolver + * @deprecated This method will be removed in v3.0 of this component + * + * @return Resolver|null */ public function getLastSuccessfulResolver() { @@ -117,6 +136,8 @@ public function getLastSuccessfulResolver() /** * Get last lookup failure * + * @deprecated This method will be removed in v3.0 of this component + * * @return false|string */ public function getLastLookupFailure() diff --git a/src/Resolver/PrefixPathStackResolver.php b/src/Resolver/PrefixPathStackResolver.php index 9c98402fa..816aaf856 100644 --- a/src/Resolver/PrefixPathStackResolver.php +++ b/src/Resolver/PrefixPathStackResolver.php @@ -10,22 +10,23 @@ use function strpos; use function substr; +/** @final */ final class PrefixPathStackResolver implements ResolverInterface { /** * Array containing prefix as key and "template path stack array" as value * - * @var string[]|string[][]|ResolverInterface[] + * @var array|string|ResolverInterface> */ private array $prefixes = []; /** * Constructor * - * @param string[]|string[][]|ResolverInterface[] $prefixes Set of path prefixes + * @param array|string|ResolverInterface> $prefixes Set of path prefixes * to be matched (array keys), with either a path or an array of paths - * to use for matching as in the {@see \Laminas\View\Resolver\TemplatePathStack}, - * or a {@see \Laminas\View\Resolver\ResolverInterface} + * to use for matching as in the {@see TemplatePathStack}, + * or a {@see ResolverInterface} * to use for view path starting with that prefix */ public function __construct(array $prefixes = []) @@ -47,11 +48,20 @@ public function resolve($name, ?Renderer $renderer = null) $resolver = new TemplatePathStack(['script_paths' => (array) $resolver]); } + /** + * @todo In V3, this should just try,return and catch,continue. + * It relies on internal knowledge that some resolvers return false when really + * they should always return a string or throw an exception. + */ if ($result = $resolver->resolve(substr($name, strlen($prefix)), $renderer)) { return $result; } } + /** + * @todo This should be exceptional. All resolvers are exhausted and no template can be found. + * It further deviates from the previously un-documented norm, that the return type is false-able + */ return; // phpcs:ignore } } diff --git a/src/Resolver/RelativeFallbackResolver.php b/src/Resolver/RelativeFallbackResolver.php index 1c6bb9c56..86aa010b8 100644 --- a/src/Resolver/RelativeFallbackResolver.php +++ b/src/Resolver/RelativeFallbackResolver.php @@ -20,6 +20,8 @@ * * This allows for usage of partial template paths such as `some/partial`, resolving to * `my/module/script/path/some/partial.phtml`, while rendering template `my/module/script/path/my-view` + * + * @final */ class RelativeFallbackResolver implements ResolverInterface { diff --git a/src/Resolver/ResolverInterface.php b/src/Resolver/ResolverInterface.php index 7c7e1036d..dec63d90f 100644 --- a/src/Resolver/ResolverInterface.php +++ b/src/Resolver/ResolverInterface.php @@ -11,8 +11,10 @@ interface ResolverInterface /** * Resolve a template/pattern name to a resource the renderer can consume * + * In version 3.0 of this component, this method will guarantee a string return type or an exception will be thrown. + * * @param string $name - * @return mixed + * @return string|false */ public function resolve($name, ?Renderer $renderer = null); } diff --git a/src/Resolver/TemplateMapResolver.php b/src/Resolver/TemplateMapResolver.php index 0e57fc998..5a8f5aea5 100644 --- a/src/Resolver/TemplateMapResolver.php +++ b/src/Resolver/TemplateMapResolver.php @@ -9,20 +9,26 @@ use Laminas\Stdlib\ArrayUtils; use Laminas\View\Exception; use Laminas\View\Renderer\RendererInterface as Renderer; -use ReturnTypeWillChange; // phpcs:ignore +use ReturnTypeWillChange; use Traversable; use function array_key_exists; use function array_replace_recursive; -use function gettype; -use function is_array; -use function is_object; +use function get_debug_type; +use function is_iterable; use function is_string; use function sprintf; +use function trigger_error; +use const E_USER_DEPRECATED; + +/** + * @implements IteratorAggregate + * @final + */ class TemplateMapResolver implements IteratorAggregate, ResolverInterface { - /** @var array */ + /** @var array */ protected $map = []; /** @@ -30,7 +36,7 @@ class TemplateMapResolver implements IteratorAggregate, ResolverInterface * * Instantiate and optionally populate template map. * - * @param array|Traversable $map + * @param iterable $map */ public function __construct($map = []) { @@ -40,7 +46,7 @@ public function __construct($map = []) /** * IteratorAggregate: return internal iterator * - * @return Traversable + * @return Traversable */ #[ReturnTypeWillChange] public function getIterator() @@ -53,17 +59,17 @@ public function getIterator() * * Maps should be arrays or Traversable objects with name => path pairs * - * @param array|Traversable $map + * @param iterable $map * @throws Exception\InvalidArgumentException - * @return TemplateMapResolver + * @return $this */ public function setMap($map) { - if (! is_array($map) && ! $map instanceof Traversable) { + if (! is_iterable($map)) { throw new Exception\InvalidArgumentException(sprintf( '%s: expects an array or Traversable, received "%s"', __METHOD__, - is_object($map) ? $map::class : gettype($map) + get_debug_type($map), )); } @@ -78,51 +84,54 @@ public function setMap($map) /** * Add an entry to the map * - * @param string|array|Traversable $nameOrMap - * @param null|string $path + * @param string|iterable $nameOrMap + * @param null|string $path * @throws Exception\InvalidArgumentException - * @return TemplateMapResolver + * @return $this */ public function add($nameOrMap, $path = null) { - if (is_array($nameOrMap) || $nameOrMap instanceof Traversable) { - $this->merge($nameOrMap); + if (is_string($nameOrMap) && ($path === null || $path === '')) { + trigger_error( + 'Using add() to remove individual templates is deprecated and will be removed in version 3.0', + E_USER_DEPRECATED, + ); + unset($this->map[$nameOrMap]); + return $this; } - if (! is_string($nameOrMap)) { + $map = is_string($nameOrMap) && is_string($path) + ? [$nameOrMap => $path] + : $nameOrMap; + + if (! is_iterable($map)) { throw new Exception\InvalidArgumentException(sprintf( '%s: expects a string, array, or Traversable for the first argument; received "%s"', __METHOD__, - is_object($nameOrMap) ? $nameOrMap::class : gettype($nameOrMap) + get_debug_type($map), )); } - if (empty($path)) { - if (isset($this->map[$nameOrMap])) { - unset($this->map[$nameOrMap]); - } - return $this; - } + $this->merge($map); - $this->map[$nameOrMap] = $path; return $this; } /** * Merge internal map with provided map * - * @param array|Traversable $map + * @param iterable $map * @throws Exception\InvalidArgumentException - * @return TemplateMapResolver + * @return $this */ public function merge($map) { - if (! is_array($map) && ! $map instanceof Traversable) { + if (! is_iterable($map)) { throw new Exception\InvalidArgumentException(sprintf( '%s: expects an array or Traversable, received "%s"', __METHOD__, - is_object($map) ? $map::class : gettype($map) + get_debug_type($map), )); } @@ -155,15 +164,17 @@ public function has($name) public function get($name) { if (! $this->has($name)) { + // @TODO This should be exceptional return false; } + return $this->map[$name]; } /** * Retrieve the template map * - * @return array + * @return array */ public function getMap() { diff --git a/src/Resolver/TemplatePathStack.php b/src/Resolver/TemplatePathStack.php index 8d07726e0..6ddeb1f06 100644 --- a/src/Resolver/TemplatePathStack.php +++ b/src/Resolver/TemplatePathStack.php @@ -4,6 +4,7 @@ namespace Laminas\View\Resolver; +use Laminas\Stdlib\ArrayUtils; use Laminas\Stdlib\SplStack; use Laminas\View\Exception; use Laminas\View\Renderer\RendererInterface as Renderer; @@ -11,6 +12,7 @@ use SplFileInfo; use Traversable; +use function array_change_key_case; use function count; use function file_exists; use function gettype; @@ -27,7 +29,6 @@ use function stream_get_wrappers; use function stream_wrapper_register; use function strpos; -use function strtolower; use const DIRECTORY_SEPARATOR; use const PATHINFO_EXTENSION; @@ -36,10 +37,19 @@ * Resolves view scripts based on a stack of paths * * @psalm-type PathStack = SplStack + * @psalm-type Options = array{ + * lfi_protection?: bool, + * script_paths?: list, + * default_suffix?: string, + * use_stream_wrapper?: bool, + * } + * @final */ class TemplatePathStack implements ResolverInterface { - public const FAILURE_NO_PATHS = 'TemplatePathStack_Failure_No_Paths'; + /** @deprecated */ + public const FAILURE_NO_PATHS = 'TemplatePathStack_Failure_No_Paths'; + /** @deprecated */ public const FAILURE_NOT_FOUND = 'TemplatePathStack_Failure_Not_Found'; /** @@ -57,6 +67,8 @@ class TemplatePathStack implements ResolverInterface /** * Reason for last lookup failure * + * @deprecated This property will be removed in v3.0 of this component. + * * @var false|string */ protected $lastLookupFailure = false; @@ -72,18 +84,22 @@ class TemplatePathStack implements ResolverInterface * Flags used to determine if a stream wrapper should be used for enabling short tags */ - /** @var bool */ + /** + * @deprecated Stream wrapper functionality will be removed in version 3.0 of this component + * + * @var bool + */ protected $useViewStream = false; - /** @var bool */ + /** + * @deprecated Stream wrapper functionality will be removed in version 3.0 of this component + * + * @var bool + */ protected $useStreamWrapper = false; /**@-*/ - /** - * Constructor - * - * @param null|array|Traversable $options - */ + /** @param null|Options|Traversable $options */ public function __construct($options = null) { $this->useViewStream = (bool) ini_get('short_open_tag'); @@ -105,7 +121,7 @@ public function __construct($options = null) /** * Configure object * - * @param array|Traversable $options + * @param Options|Traversable $options * @return void * @throws Exception\InvalidArgumentException */ @@ -119,24 +135,23 @@ public function setOptions($options) )); } - foreach ($options as $key => $value) { - switch (strtolower($key)) { - case 'lfi_protection': - $this->setLfiProtection($value); - break; - case 'script_paths': - $this->addPaths($value); - break; - case 'use_stream_wrapper': - /** @psalm-suppress DeprecatedMethod */ - $this->setUseStreamWrapper($value); - break; - case 'default_suffix': - $this->setDefaultSuffix($value); - break; - default: - break; - } + $options = $options instanceof Traversable ? ArrayUtils::iteratorToArray($options) : $options; + $options = array_change_key_case($options); + + if (isset($options['lfi_protection'])) { + $this->setLfiProtection($options['lfi_protection']); + } + + if (isset($options['script_paths'])) { + $this->addPaths($options['script_paths']); + } + + if (isset($options['use_stream_wrapper'])) { + $this->setUseStreamWrapper($options['use_stream_wrapper']); + } + + if (isset($options['default_suffix'])) { + $this->setDefaultSuffix($options['default_suffix']); } } @@ -144,7 +159,7 @@ public function setOptions($options) * Set default file suffix * * @param string $defaultSuffix - * @return TemplatePathStack + * @return $this */ public function setDefaultSuffix($defaultSuffix) { @@ -167,7 +182,7 @@ public function getDefaultSuffix() * Add many paths to the stack at once * * @param list $paths - * @return TemplatePathStack + * @return $this */ public function addPaths(array $paths) { @@ -178,7 +193,7 @@ public function addPaths(array $paths) } /** - * Rest the path stack to the paths provided + * Reset the path stack to the paths provided * * @param PathStack|list $paths * @return TemplatePathStack @@ -223,7 +238,7 @@ public static function normalizePath($path) * Add a single path to the stack * * @param string $path - * @return TemplatePathStack + * @return $this * @throws Exception\InvalidArgumentException */ public function addPath($path) @@ -234,7 +249,7 @@ public function addPath($path) gettype($path) )); } - $this->paths[] = static::normalizePath($path); + $this->paths->push(static::normalizePath($path)); return $this; } @@ -330,6 +345,7 @@ public function resolve($name, ?Renderer $renderer = null) if (! count($this->paths)) { $this->lastLookupFailure = static::FAILURE_NO_PATHS; + // @TODO In version 3, this should become an exception return false; } @@ -360,12 +376,16 @@ public function resolve($name, ?Renderer $renderer = null) } $this->lastLookupFailure = static::FAILURE_NOT_FOUND; + // @TODO This should become an exception in v3.0 return false; } /** * Get the last lookup failure message, if any * + * @deprecated In version 3.0, this resolver will throw exceptions instead of + * incorrectly returning false from resolve() + * * @return false|string */ public function getLastLookupFailure() diff --git a/test/Resolver/AggregateResolverTest.php b/test/Resolver/AggregateResolverTest.php index e34ccc1ca..7afe221d7 100644 --- a/test/Resolver/AggregateResolverTest.php +++ b/test/Resolver/AggregateResolverTest.php @@ -8,23 +8,21 @@ use Laminas\View\Resolver; use PHPUnit\Framework\TestCase; -use function count; - class AggregateResolverTest extends TestCase { public function testAggregateIsEmptyByDefault(): void { $resolver = new Resolver\AggregateResolver(); - $this->assertEquals(0, count($resolver)); + $this->assertCount(0, $resolver); } public function testCanAttachResolvers(): void { $resolver = new Resolver\AggregateResolver(); $resolver->attach(new Resolver\TemplateMapResolver()); - $this->assertEquals(1, count($resolver)); + $this->assertCount(1, $resolver); $resolver->attach(new Resolver\TemplateMapResolver()); - $this->assertEquals(2, count($resolver)); + $this->assertCount(2, $resolver); } public function testReturnsNonFalseValueWhenAtLeastOneResolverSucceeds(): void diff --git a/test/Resolver/TemplatePathStackTest.php b/test/Resolver/TemplatePathStackTest.php index 636366e26..f62100fdc 100644 --- a/test/Resolver/TemplatePathStackTest.php +++ b/test/Resolver/TemplatePathStackTest.php @@ -18,11 +18,14 @@ use const DIRECTORY_SEPARATOR; +/** + * @psalm-import-type Options from TemplatePathStack + */ class TemplatePathStackTest extends TestCase { private TemplatePathStack $stack; - /** @var string[] */ + /** @var list */ private array $paths; private string $baseDir; @@ -197,7 +200,7 @@ public function testSettingOptionsWithInvalidArgumentRaisesException(mixed $opti } /** - * @return array|ArrayObject}> + * @return array */ public static function validOptions(): array { @@ -213,7 +216,7 @@ public static function validOptions(): array } /** - * @param array|ArrayObject $options + * @param Options|ArrayObject $options */ #[DataProvider('validOptions')] public function testAllowsSettingOptions($options): void @@ -226,13 +229,13 @@ public function testAllowsSettingOptions($options): void /** @psalm-suppress DeprecatedMethod */ $this->assertSame($expected, $this->stack->useStreamWrapper()); - $this->assertSame($options['default_suffix'], $this->stack->getDefaultSuffix()); + $this->assertSame($options['default_suffix'] ?? null, $this->stack->getDefaultSuffix()); $this->assertEquals(array_reverse($this->paths), $this->stack->getPaths()->toArray()); } /** - * @param array|ArrayObject $options + * @param Options|ArrayObject $options */ #[DataProvider('validOptions')] public function testAllowsPassingOptionsToConstructor($options): void