Skip to content

Commit

Permalink
minor #22531 Throwing an exception if the class is not found (weaverr…
Browse files Browse the repository at this point in the history
…yan)

This PR was squashed before being merged into the 3.3-dev branch (closes #22531).

Discussion
----------

Throwing an exception if the class is not found

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | n/a

We now throw an exception if the user makes a mistake with their PSR-4 prefix and namespace. For example:

```yml
AppBundle\Controller\:
    resource: '../../src/AppBundle/{Controller}'
    public: true
```

I should not have the `\Controller` at the end of the key. Previously, it would silently not import any services from the directory. Now it throws:

> Expected to find class "AppBundle\Controller\Controller\Admin\BlogController" in file "/path/to/project/src/AppBundle/Controller/Admin/BlogController.php" while importing services from resource "../../src/AppBundle/{Controller}", but it was not found! Check the namespace prefix used with the resource.

The only "downside" is that this prevents someone from importing files from a resource that has a file with no class in it (`functions.php`). @nicolas-grekas and I decided today that we can throw an exception now to be safe, and see if anyone has that valid use-case.

Cheers!

Commits
-------

e85bcc9 Throwing an exception if the class is not found
  • Loading branch information
nicolas-grekas committed Apr 27, 2017
2 parents 8974b52 + e85bcc9 commit 9d9f628
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 9 deletions.
Expand Up @@ -103,8 +103,9 @@ private function findClasses($namespace, $resource)
if (!preg_match('/^[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*+(?:\\\\[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*+)*+$/', $class)) {
continue;
}
if (!$r = $this->container->getReflectionClass($class, true)) {
continue;
// check to make sure the expected class exists
if (!$r = $this->container->getReflectionClass($class)) {
throw new InvalidArgumentException(sprintf('Expected to find class "%s" in file "%s" while importing services from resource "%s", but it was not found! Check the namespace prefix used with the resource.', $class, $path, $resource));
}
if (!$r->isInterface() && !$r->isTrait()) {
$classes[] = $class;
Expand Down

This file was deleted.

Expand Up @@ -86,6 +86,19 @@ public function testRegisterClasses()

$this->assertTrue($container->has(Bar::class));
}

/**
* @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException
* @expectedExceptionMessageRegExp /Expected to find class "Symfony\\Component\\DependencyInjection\\Tests\\Fixtures\\Prototype\\Bar" in file ".+" while importing services from resource "Prototype\/Sub\/\*", but it was not found\! Check the namespace prefix used with the resource/
*/
public function testRegisterClassesWithBadPrefix()
{
$container = new ContainerBuilder();
$loader = new TestFileLoader($container, new FileLocator(self::$fixturesPath.'/Fixtures'));

// the Sub is missing from namespace prefix
$loader->registerClasses(new Definition(), 'Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\\', 'Prototype/Sub/*');
}
}

class TestFileLoader extends FileLoader
Expand Down

0 comments on commit 9d9f628

Please sign in to comment.