Skip to content

Commit

Permalink
minor #32143 use proper return types in ErrorHandler and ArgumentReso…
Browse files Browse the repository at this point in the history
…lver (Tobion)

This PR was merged into the 4.4 branch.

Discussion
----------

use proper return types in ErrorHandler and ArgumentResolver

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | tiny
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets |
| License       | MIT
| Doc PR        |

Found those things while reviewing #31996 which missed some return types due to using `return` instead of `return null`.
It's part of fixing #17201 (due to #10717). See also #30869 that somebody was stumbling over.

Commits
-------

2f9121b use proper return types in ErrorHandler and ArgumentResolver
  • Loading branch information
fabpot committed Jun 26, 2019
2 parents aaea4b0 + 2f9121b commit f15722d
Show file tree
Hide file tree
Showing 6 changed files with 23 additions and 34 deletions.
9 changes: 2 additions & 7 deletions src/Symfony/Component/Debug/ErrorHandler.php
Expand Up @@ -369,18 +369,13 @@ private function reRegister($prev)
/**
* Handles errors by filtering then logging them according to the configured bit fields.
*
* @param int $type One of the E_* constants
* @param string $message
* @param string $file
* @param int $line
*
* @return bool Returns false when no handling happens so that the PHP engine can handle the error itself
*
* @throws \ErrorException When $this->thrownErrors requests so
*
* @internal
*/
public function handleError($type, $message, $file, $line)
public function handleError(int $type, string $message, string $file, int $line): bool
{
// @deprecated to be removed in Symfony 5.0
if (\PHP_VERSION_ID >= 70300 && $message && '"' === $message[0] && 0 === strpos($message, '"continue') && preg_match('/^"continue(?: \d++)?" targeting switch is equivalent to "break(?: \d++)?"\. Did you mean to use "continue(?: \d++)?"\?$/', $message)) {
Expand Down Expand Up @@ -443,7 +438,7 @@ public function handleError($type, $message, $file, $line)
self::$silencedErrorCache[$id][$message] = $errorAsException;
}
if (null === $lightTrace) {
return;
return true;
}
} else {
$errorAsException = new \ErrorException($logMessage, 0, $type, $file, $line);
Expand Down
10 changes: 6 additions & 4 deletions src/Symfony/Component/HttpKernel/Controller/ArgumentResolver.php
Expand Up @@ -55,14 +55,16 @@ public function getArguments(Request $request, $controller)

$resolved = $resolver->resolve($request, $metadata);

if (!$resolved instanceof \Generator) {
throw new \InvalidArgumentException(sprintf('%s::resolve() must yield at least one value.', \get_class($resolver)));
}

$atLeastOne = false;
foreach ($resolved as $append) {
$atLeastOne = true;
$arguments[] = $append;
}

if (!$atLeastOne) {
throw new \InvalidArgumentException(sprintf('%s::resolve() must yield at least one value.', \get_class($resolver)));
}

// continue to the next controller argument
continue 2;
}
Expand Down
Expand Up @@ -41,8 +41,6 @@ public function resolve(Request $request, ArgumentMetadata $argument)
throw new \InvalidArgumentException(sprintf('The action argument "...$%1$s" is required to be an array, the request attribute "%1$s" contains a type of "%2$s" instead.', $argument->getName(), \gettype($values)));
}

foreach ($values as $value) {
yield $value;
}
yield from $values;
}
}
Expand Up @@ -37,7 +37,7 @@ public function supports(Request $request, ArgumentMetadata $argument);
* @param Request $request
* @param ArgumentMetadata $argument
*
* @return \Generator
* @return iterable
*/
public function resolve(Request $request, ArgumentMetadata $argument);
}
Expand Up @@ -42,30 +42,24 @@ public function createArgumentMetadata($controller)

/**
* Returns an associated type to the given parameter if available.
*
* @param \ReflectionParameter $parameter
*
* @return string|null
*/
private function getType(\ReflectionParameter $parameter, \ReflectionFunctionAbstract $function)
private function getType(\ReflectionParameter $parameter, \ReflectionFunctionAbstract $function): ?string
{
if (!$type = $parameter->getType()) {
return;
return null;
}
$name = $type->getName();
$lcName = strtolower($name);

if ('self' !== $lcName && 'parent' !== $lcName) {
return $name;
}
if (!$function instanceof \ReflectionMethod) {
return;
}
if ('self' === $lcName) {
return $function->getDeclaringClass()->name;
}
if ($parent = $function->getDeclaringClass()->getParentClass()) {
return $parent->name;
if ($function instanceof \ReflectionMethod) {
$lcName = strtolower($name);
switch ($lcName) {
case 'self':
return $function->getDeclaringClass()->name;
case 'parent':
return ($parent = $function->getDeclaringClass()->getParentClass()) ? $parent->name : null;
}
}

return $name;
}
}
Expand Up @@ -185,7 +185,7 @@ public function testGetArgumentWithoutArray()
$resolver = new ArgumentResolver($factory, [$valueResolver]);

$valueResolver->expects($this->any())->method('supports')->willReturn(true);
$valueResolver->expects($this->any())->method('resolve')->willReturn('foo');
$valueResolver->expects($this->any())->method('resolve')->willReturn([]);

$request = Request::create('/');
$request->attributes->set('foo', 'foo');
Expand Down

0 comments on commit f15722d

Please sign in to comment.