Tweaking class not found autowiring error #24599
Conversation
| @@ -485,7 +485,7 @@ private function createAutowiredDefinition($type) | ||
| private function createTypeNotFoundMessage(TypedReference $reference, $label) | ||
| { | ||
| if (!$r = $this->container->getReflectionClass($type = $reference->getType(), false)) { | ||
| $message = sprintf('has type "%s" but this class cannot be loaded.', $type); | ||
| $message = sprintf('is type-hinted with "%s" but this class was not found.', $type); |
There was a problem hiding this comment.
The "cannot be loaded" case is possible when the parent class is not found. Maybe and "or the/a parent class is not found"? (Not sure about the wording but that's explaining the current one.)
There was a problem hiding this comment.
Is there any issue with addition a conditional?
if (class_exists($type)) {
// we know a parent class must not exist
} else {
// we know that the class itself does not exist
}There was a problem hiding this comment.
class_exists($type) would trigger a fatal error when the parent is missing. container->getReflectionClass() contains a protection for that situation. Maybe passing "true" as 2nd argument to getReflectionClass and dealing with the exception could help make the message better?
There was a problem hiding this comment.
also please don't use type "hints". those are not hints like phpdocs.
There was a problem hiding this comment.
"hint" is used everywhere, in Symfony, in the PHP doc, on google. Technically is a bit more than a "hint" I agree, but that's what ppl are used to now IMHO.
There was a problem hiding this comment.
Not at all. It was explicitly avoided to call them hints in the php doc. See http://php.net/manual/en/functions.arguments.php#functions.arguments.type-declaration
And also the RFC about type declarations does not use the term hints for this reason.
109a91c to
425e41b
Compare
parent class may not exist.
|
it looks like PHP 7.2 may behave differently, that could be the reason for the failure on deps=low |
|
Just tested with php7.2 locally, green. |
|
So, Here is the full patch accounting for my comments: diff --git a/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php b/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php
index d4abf04..58c1a9c 100644
--- a/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php
+++ b/src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php
@@ -491,12 +491,12 @@ class AutowirePass extends AbstractRecursivePass
$resource = new ClassExistenceResource($type, false);
// isFresh() will explode ONLY if a parent class/trait does not exist
$resource->isFresh(0);
- $classExists = false;
+ $parentMsg = false;
} catch (\ReflectionException $e) {
- $classExists = true;
+ $parentMsg = $e->getMessage();
}
- $message = sprintf('has type "%s" but this class %s.', $type, $classExists ? 'cannot be loaded: its parent class may be missing' : 'was not found');
+ $message = sprintf('has type "%s" but this class %s.', $type, $parentMsg ? sprintf('is missing a parent class (%s)', $parentMsg) : 'was not found');
} else {
$message = $this->container->has($type) ? 'this service is abstract' : 'no such service exists';
$message = sprintf('references %s "%s" but %s.%s', $r->isInterface() ? 'interface' : 'class', $type, $message, $this->createTypeAlternatives($reference));
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php
index 6687583..cae761e 100644
--- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php
+++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php
@@ -374,7 +374,7 @@ class AutowirePassTest extends TestCase
/**
* @expectedException \Symfony\Component\DependencyInjection\Exception\AutowiringFailedException
- * @expectedExceptionMessage Cannot autowire service "a": argument "$r" of method "Symfony\Component\DependencyInjection\Tests\Compiler\BadParentTypeHintedArgument::__construct()" has type "Symfony\Component\DependencyInjection\Tests\Compiler\OptionalServiceClass" but this class cannot be loaded: its parent class may be missing.
+ * @expectedExceptionMessage Cannot autowire service "a": argument "$r" of method "Symfony\Component\DependencyInjection\Tests\Compiler\BadParentTypeHintedArgument::__construct()" has type "Symfony\Component\DependencyInjection\Tests\Compiler\OptionalServiceClass" but this class is missing a parent class (Class Symfony\Bug\NotExistClass not found).
*/
public function testParentClassNotFoundThrowsException()
{
diff --git a/src/Symfony/Component/DependencyInjection/composer.json b/src/Symfony/Component/DependencyInjection/composer.json
index 0ce0ffe..5f79b19 100644
--- a/src/Symfony/Component/DependencyInjection/composer.json
+++ b/src/Symfony/Component/DependencyInjection/composer.json
@@ -32,7 +32,7 @@
"symfony/proxy-manager-bridge": "Generate service proxies to lazy load them"
},
"conflict": {
- "symfony/config": "<3.3.1",
+ "symfony/config": "<3.3.7",
"symfony/finder": "<3.3",
"symfony/yaml": "<3.3"
}, |
|
@nicolas-grekas That patch looks great - the message is even better! I've made the changes. Fabbot failure is a false-positive. |
|
Rock-n-Roll :) |
|
Thank you @weaverryan. |
This PR was squashed before being merged into the 3.3 branch (closes #24599). Discussion ---------- Tweaking class not found autowiring error | Q | A | ------------- | --- | Branch? | 3.3 | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | none | License | MIT | Doc PR | n/a A trainee in my workshop today hit this error, and it struck me as not as clear as it could be - it's really a "Class Not Found" error. Commits ------- 48832e6 Tweaking class not found autowiring error
A trainee in my workshop today hit this error, and it struck me as not as clear as it could be - it's really a "Class Not Found" error.