Skip to content

Commit

Permalink
feature #20651 [DependencyInjection] Added Yaml syntax shortcut for n…
Browse files Browse the repository at this point in the history
…ame-only tags (wouterj)

This PR was merged into the 3.3-dev branch.

Discussion
----------

[DependencyInjection] Added Yaml syntax shortcut for name-only tags

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

This PR adds a little shorcut for tags without any attributes. There are increasingly more name-only tags in Symfony and having to do `{ name: twig.extension }` for these seems way too verbose to me.

**Before**
```yaml
services:
    app.twig_extension:
        class: AppBundle\Twig\AppExtension
        tags:
            - { name: twig.extension }
```

**After**
```yaml
services:
    app.twig_extension:
        class: AppBundle\Twig\AppExtension
        tags: [twig.extension]
        # or
        #    - twig.extension
```

This of course means we introduce a new format to achieve the same goal. I believe this isn't a big problem as the decision is distinctive and simple: If you configure tag attributes, use the long format, otherwise use the short format.

Backwards compatibility
---

In this PR, an exception was removed to allow this new shortcut format. The BC promise doesn't cover exceptions and I think removing the exception here should cause anything to break:

 * Applications shouldn't rely on exceptions
 * If code was triggering this exception before, it would not cause any behaviour change after this PR: The service just retrieves an unused tag, which is simply ignored by the container.

Commits
-------

7fa8c8a Added Yaml syntax shortcut for name-only tags
  • Loading branch information
fabpot committed Dec 13, 2016
2 parents 22586ca + 7fa8c8a commit 902d9ed
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 17 deletions.
Expand Up @@ -267,7 +267,7 @@ private function parseDefinition($id, $service, $file)

foreach ($service['tags'] as $tag) {
if (!is_array($tag)) {
throw new InvalidArgumentException(sprintf('A "tags" entry must be an array for service "%s" in %s. Check your YAML syntax.', $id, $file));
$tag = array('name' => $tag);
}

if (!isset($tag['name'])) {
Expand Down

This file was deleted.

@@ -0,0 +1,5 @@
services:
foo_service:
class: FooClass
tags:
- foo
Expand Up @@ -223,16 +223,6 @@ public function testNonArrayTagsThrowsException()
}
}

/**
* @expectedException \Symfony\Component\DependencyInjection\Exception\InvalidArgumentException
* @expectedExceptionMessage A "tags" entry must be an array for service
*/
public function testNonArrayTagThrowsException()
{
$loader = new YamlFileLoader(new ContainerBuilder(), new FileLocator(self::$fixturesPath.'/yaml'));
$loader->load('badtag4.yml');
}

public function testTagWithoutNameThrowsException()
{
$loader = new YamlFileLoader(new ContainerBuilder(), new FileLocator(self::$fixturesPath.'/yaml'));
Expand All @@ -245,6 +235,15 @@ public function testTagWithoutNameThrowsException()
}
}

public function testNameOnlyTagsAreAllowedAsString()
{
$container = new ContainerBuilder();
$loader = new YamlFileLoader($container, new FileLocator(self::$fixturesPath.'/yaml'));
$loader->load('tag_name_only.yml');

$this->assertCount(1, $container->getDefinition('foo_service')->getTag('foo'));
}

public function testTagWithAttributeArrayThrowsException()
{
$loader = new YamlFileLoader(new ContainerBuilder(), new FileLocator(self::$fixturesPath.'/yaml'));
Expand Down

0 comments on commit 902d9ed

Please sign in to comment.