Skip to content

Commit

Permalink
Deprecations and type inference improvements for template resolvers
Browse files Browse the repository at this point in the history 
The return type for a template resolver _should_ simply be `string`. Any kind of failure should cause an exception, ideally something like `TemplateNotFound`.

All existing resolvers are marked as `@final` and there are several additional deprecations to clean up un-documented behaviour.

Signed-off-by: George Steel <[email protected]>
  • Loading branch information
@gsteel
gsteel committed Jul 20, 2023
1 parent b3ae82d commit fdc1706
Show file tree
Hide file tree
Showing 9 changed files with 211 additions and 144 deletions.
118 changes: 59 additions & 59 deletions psalm-baseline.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1584,9 +1584,15 @@
<code>gettype($helpers)</code>
<code>gettype($variables)</code>
</DocblockTypeContradiction>
<FalsableReturnStatement>
<code><![CDATA[$this->__templateResolver->resolve($name, $this)]]></code>
</FalsableReturnStatement>
<ImplementedParamTypeMismatch>
<code>$values</code>
</ImplementedParamTypeMismatch>
<InvalidNullableReturnType>
<code>string|Resolver</code>
</InvalidNullableReturnType>
<MixedArgument>
<code>$__vars</code>
<code><![CDATA[$this->__template]]></code>
Expand Down Expand Up @@ -1623,7 +1629,6 @@
</MixedClone>
<MixedInferredReturnType>
<code>string</code>
<code>string|Resolver</code>
</MixedInferredReturnType>
<MixedMethodCall>
<code>getArrayCopy</code>
Expand All @@ -1634,7 +1639,6 @@
</MixedOperand>
<MixedReturnStatement>
<code><![CDATA[$this->__filterChain->filter($this->__content)]]></code>
<code><![CDATA[$this->__templateResolver->resolve($name, $this)]]></code>
</MixedReturnStatement>
<NullableReturnStatement>
<code><![CDATA[$this->__templateResolver]]></code>
Expand Down Expand Up @@ -1670,57 +1674,56 @@
</UnresolvableInclude>
</file>
<file src="src/Resolver/AggregateResolver.php">
<MissingTemplateParam>
<code>IteratorAggregate</code>
</MissingTemplateParam>
<MixedAssignment>
<code>$resolver</code>
<code>$resource</code>
<DeprecatedProperty>
<code><![CDATA[$this->lastLookupFailure]]></code>
<code><![CDATA[$this->lastLookupFailure]]></code>
<code><![CDATA[$this->lastLookupFailure]]></code>
<code><![CDATA[$this->lastLookupFailure]]></code>
<code><![CDATA[$this->lastSuccessfulResolver]]></code>
</MixedAssignment>
<MixedInferredReturnType>
<code>false|string</code>
</MixedInferredReturnType>
<MixedMethodCall>
<code>resolve</code>
</MixedMethodCall>
<MixedReturnStatement>
<code>$resource</code>
</MixedReturnStatement>
<PossiblyNullPropertyAssignmentValue>
<code>null</code>
</PossiblyNullPropertyAssignmentValue>
<PropertyNotSetInConstructor>
<code>$lastSuccessfulResolver</code>
</PropertyNotSetInConstructor>
<code><![CDATA[$this->lastSuccessfulResolver]]></code>
<code><![CDATA[$this->lastSuccessfulResolver]]></code>
</DeprecatedProperty>
</file>
<file src="src/Resolver/PrefixPathStackResolver.php">
<MixedArgumentTypeCoercion>
<code>$prefix</code>
</MixedArgumentTypeCoercion>
<MixedAssignment>
<code>$result</code>
</MixedAssignment>
<InvalidNullableReturnType>
<code>resolve</code>
</InvalidNullableReturnType>
<RedundantCastGivenDocblockType>
<code>(string) $prefix</code>
</RedundantCastGivenDocblockType>
</file>
<file src="src/Resolver/TemplateMapResolver.php">
<DocblockTypeContradiction>
<code><![CDATA[! is_array($map) && ! $map instanceof Traversable]]></code>
<code><![CDATA[! is_array($map) && ! $map instanceof Traversable]]></code>
<code>is_string($nameOrMap)</code>
<code>is_iterable($map)</code>
<code>is_iterable($map)</code>
</DocblockTypeContradiction>
<MissingTemplateParam>
<code>IteratorAggregate</code>
</MissingTemplateParam>
<MixedInferredReturnType>
<code>false|string</code>
</MixedInferredReturnType>
<MixedReturnStatement>
<code><![CDATA[$this->map[$name]]]></code>
</MixedReturnStatement>
<MixedPropertyTypeCoercion>
<code><![CDATA[array_replace_recursive($this->map, $map)]]></code>
</MixedPropertyTypeCoercion>
<RedundantConditionGivenDocblockType>
<code><![CDATA[is_string($nameOrMap) && is_string($path)]]></code>
<code>is_string($path)</code>
</RedundantConditionGivenDocblockType>
</file>
<file src="src/Resolver/TemplatePathStack.php">
<DeprecatedConstant>
<code>static::FAILURE_NOT_FOUND</code>
<code>static::FAILURE_NO_PATHS</code>
</DeprecatedConstant>
<DeprecatedMethod>
<code>setUseStreamWrapper</code>
</DeprecatedMethod>
<DeprecatedProperty>
<code><![CDATA[$this->lastLookupFailure]]></code>
<code><![CDATA[$this->lastLookupFailure]]></code>
<code><![CDATA[$this->lastLookupFailure]]></code>
<code><![CDATA[$this->lastLookupFailure]]></code>
<code><![CDATA[$this->useStreamWrapper]]></code>
<code><![CDATA[$this->useStreamWrapper]]></code>
<code><![CDATA[$this->useViewStream]]></code>
<code><![CDATA[$this->useViewStream]]></code>
<code><![CDATA[$this->useViewStream]]></code>
</DeprecatedProperty>
<DocblockTypeContradiction>
<code>is_string($path)</code>
</DocblockTypeContradiction>
Expand All @@ -1729,19 +1732,11 @@
<code>false</code>
</FalsableReturnStatement>
<MixedArgument>
<code>$value</code>
<code>$value</code>
<code>$value</code>
<code>$value</code>
<code><![CDATA[$options['default_suffix']]]></code>
<code><![CDATA[$options['lfi_protection']]]></code>
<code><![CDATA[$options['script_paths']]]></code>
<code><![CDATA[$options['use_stream_wrapper']]]></code>
</MixedArgument>
<MixedAssignment>
<code><![CDATA[$this->lastLookupFailure]]></code>
<code><![CDATA[$this->lastLookupFailure]]></code>
<code>$value</code>
</MixedAssignment>
<NullArgument>
<code><![CDATA[$this->paths]]></code>
</NullArgument>
<RedundantCastGivenDocblockType>
<code>(bool) $flag</code>
<code>(bool) $flag</code>
Expand Down Expand Up @@ -2841,11 +2836,16 @@
<code>$model</code>
</MixedArgument>
</file>
<file src="test/Resolver/RelativeFallbackResolverTest.php">
<MixedAssignment>
<code>$test</code>
<code>$test</code>
</MixedAssignment>
<file src="test/Resolver/AggregateResolverTest.php">
<DocblockTypeContradiction>
<code>assertNull</code>
</DocblockTypeContradiction>
</file>
<file src="test/Resolver/TemplatePathStackTest.php">
<DeprecatedConstant>
<code>TemplatePathStack::FAILURE_NOT_FOUND</code>
<code>TemplatePathStack::FAILURE_NO_PATHS</code>
</DeprecatedConstant>
</file>
<file src="test/Strategy/FeedStrategyTest.php">
<MixedAssignment>
Expand Down
35 changes: 28 additions & 7 deletions src/Resolver/AggregateResolver.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<int, ResolverInterface>
*/
class AggregateResolver implements Countable, IteratorAggregate, Resolver
{
public const FAILURE_NO_RESOLVERS = 'AggregateResolver_Failure_No_Resolvers';
Expand All @@ -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<ResolverInterface, int> */
protected $queue;

/**
Expand All @@ -38,7 +49,10 @@ class AggregateResolver implements Countable, IteratorAggregate, Resolver
*/
public function __construct()
{
$this->queue = new PriorityQueue();
/** @var PriorityQueue<ResolverInterface, int> $priorityQueue */
$priorityQueue = new PriorityQueue();

$this->queue = $priorityQueue;
}

/**
Expand All @@ -55,7 +69,7 @@ public function count()
/**
* IteratorAggregate: return internal iterator
*
* @return PriorityQueue
* @return Traversable<int, ResolverInterface>
*/
#[ReturnTypeWillChange]
public function getIterator()
Expand All @@ -67,7 +81,7 @@ public function getIterator()
* Attach a resolver
*
* @param int $priority
* @return AggregateResolver
* @return $this
*/
public function attach(Resolver $resolver, $priority = 1)
{
Expand All @@ -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
Expand All @@ -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()
{
Expand All @@ -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()
Expand Down
18 changes: 14 additions & 4 deletions src/Resolver/PrefixPathStackResolver.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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, list<string>|string|ResolverInterface>
*/
private array $prefixes = [];

/**
* Constructor
*
* @param string[]|string[][]|ResolverInterface[] $prefixes Set of path prefixes
* @param array<string, list<string>|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 = [])
Expand All @@ -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
}
}
2 changes: 2 additions & 0 deletions src/Resolver/RelativeFallbackResolver.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
4 changes: 3 additions & 1 deletion src/Resolver/ResolverInterface.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Loading

0 comments on commit fdc1706

Please sign in to comment.