Commit Graph

452 Commits

Author SHA1 Message Date
Fabien Potencier
89979ca1eb feature #22563 Not allowing autoconfigure, instanceofConditionals or defaults for ChildDefinition (weaverryan)
This PR was merged into the 3.3-dev branch.

Discussion
----------

Not allowing autoconfigure, instanceofConditionals or defaults for ChildDefinition

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes (removing risky behavior)
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | see #22530
| License       | MIT
| Doc PR        | n/a

This PR *prohibits* using `autoconfigure`, `_instanceof` and `_defaults` for ChildDefinition.

Additionally, I added many "integration" test cases: we need to test and prove all edge cases. These are in the `integration/` directory: the `main.yml` file is parsed and compared to `expected.yml`. Both are in YAML to ease comparing the before/after. We need to check these out and make sure they're right and we're not missing anything else.

This PR removes MANY of the "wtf" cases, but there are still 4 that I know of... and of course they all deal with parent-child stuff :).

A) [MAJOR] [autoconfigure_parent_child_tags](https://github.com/symfony/symfony/pull/22563/files#diff-fd6cf15470c5abd40156e4e7dc4e7f6d) `instanceof` tags from autoconfigure are NEVER applied to the child (you can't set `autoconfigure` directly on a Child, but you still can set it on a parent and inherit it... sneaky). We could throw an Exception I suppose to prevent this `autoconfigure` from cascading from parent to child... but it's tricky due to `instanceof`.

B( [MAJOR] [instanceof_parent_child](https://github.com/symfony/symfony/pull/22563/files#diff-14666e9a25322d44b3c2c583b6814dc2) `instanceof` tags that are applied to the parent, are not applied to the child. Again, you can't set `instanceof` directly on a Child, but you *can* set it on a parent, and have that cascade to the child. Like before, we could maybe throw an exception to prevent this.

C) [MINOR] ([autoconfigure_child_not_applied](https://github.com/symfony/symfony/pull/22563/files#diff-3372a1dcaf3af30d14a7d0a6c8bfa988))  automatic `instanceof` will not be applied to the child when the parent class has a different (non-instanceof-ed) class. If we could throw an exception for (A), then it would cover this too.

D) `_tags` from defaults are never used (unless you have inherit_tags) - fixed in #22530

A, B & C are effectively caused by there being a "sneaky" way to re-enable `autoconfigure` and `instanceof` for ChildDefinition... which opens up wtf cases.

## Wait, why not support `_defaults`, `autoconfigure` and `_instanceof` for child definitions?

1 big reason: reduction of wtf moments where we arbitrarily decide override logic. PLUS, since `_defaults`, `instanceof` and `autoconfigure` *are* applied to parent definitions, in practice (other than tags), this makes no difference: the configuration will still pass from parent down to child.

Also, using parent-child definitions is already an edge case, and this *simply* prevents *just* those services from using the new features.

## Longer reasons why

The reason behind this is that parent-child definitions are a different mechanism for "inheritance"
than `_instanceof` and `_defaults`... creating some edge cases when trying to figure out which settings "win". For example:

```yml
# file1.yml
services:
    _defaults:
        public: false

    ChildService:
        parent: parent_service

# file2.yml
services:
    _defaults:
        public: true

    ParentService: ~
```

Is `ChildDefinition` `public: true` (so the parent
overrides the child, even though it only came from _defaults) or `public: false` (where
the child wins... even though it was only set from its _defaults)?

Or, if ParentService is explicitly set to `public: true`, should that override the `public: false` of ChildService (which it got from its `_defaults`)? On one hand, ParentService is being explicitly
set. On the other hand, ChildService is explicitly in a file settings `_defaults` `public: false`
There's no correct answer.

There are also problems with `_instanceof`. The importance goes:

> defaults < instanceof < service definition

But how do parent-child relationships fit into that? If a child has public: false
from an _instanceof, but the parent explicitly sets public: true, which wins? Should
we assume the parent definition wins because it's explicitly set? Or would the
_instanceof win, because that's being explicitly applied to the child definition's
class by an _instanceof that lives in the same file as that class (whereas the parent
definition may live in a different file).

Because of this, @nicolas-grekas and I (we also talked a bit to Fabien) decided that
the complexity was growing too much. The solution is to not allow any of these
new feature to be used by ChildDefinition objects. In other words, when you want some
sort of "inheritance" for your service, you should *either* giving your service a
parent *or* using defaults and instanceof. And instead of silently not applying
defaults and instanceof to child definitions, I think it's better to scream that it's
not supported.

Commits
-------

a943b96d42 Not allowing autoconfigure, instanceofConditionals or defaults for ChildDefinition
2017-04-29 08:25:33 -07:00
Ryan Weaver
4b7e148a9b Fixing problem where _defaults set to null was seen as a service 2017-04-29 12:32:04 +02:00
Ryan Weaver
a943b96d42 Not allowing autoconfigure, instanceofConditionals or defaults for ChildDefinition
Also, not allowing arguments or method calls for autoconfigure. This is a safety
mechanism, since we don't have merging logic. It will allow us to add this in the
future if we want to.

The reason is that parent-child definitions are a different mechanism for "inheritance"
than instanceofConditionas and defaults... creating some edge cases when trying to
figure out which settings "win". For example:

Suppose a child and parent definitions are defined in different YAML files. The
child receives public: false from its _defaults, and the parent receives public: true
from its _defaults. Should the final child definition be public: true (so the parent
overrides the child, even though it only came from _defaults) or public: false (where
the child wins... even though it was only set from its _defaults). Or, if the parent
is explicitly set to public: true, should that override the public: false of the
child (which it got from its _defaults)? On one hand, the parent is being explicitly
set. On the other hand, the child is explicitly in a file settings _defaults public
to false. There's no correct answer.

There are also problems with instanceof. The importance goes:
  defaults < instanceof < service definition

But how does parent-child relationships fit into that? If a child has public: false
from an _instanceof, but the parent explicitly sets public: true, which wins? Should
we assume the parent definition wins because it's explicitly set? Or would the
_instanceof win, because that's being explicitly applied to the child definition's
class by an _instanceof that lives in the same file as that class (whereas the parent
definition may live in a different file).

Because of this, @nicolas-grekas and I (we also talked a bit to Fabien) decided that
the complexity was growing too much. The solution is to not allow any of these
new feature to be used by ChildDefinition objects. In other words, when you want some
sort of "inheritance" for your service, you should *either* giving your service a
parent *or* using defaults and instanceof. And instead of silently not applying
defaults and instanceof to child definitions, I think it's better to scream that it's
not supported.
2017-04-28 17:09:21 -04:00
Ryan Weaver
e85bcc9e8d Throwing an exception if the class is not found 2017-04-27 10:37:33 -04:00
Fabien Potencier
3471b58318 bug #22496 [DI] Fix inlining conflict by restricting IteratorArgument to Reference[] (nicolas-grekas)
This PR was merged into the 3.3-dev branch.

Discussion
----------

[DI] Fix inlining conflict by restricting IteratorArgument to Reference[]

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

`Reference` found in `ArgumentInterface::getValue()` are currently not inlined.
While trying to do so (hint: I failed), I noticed that the current code is broken for `IteratorArgument` which can contain anonymous `Definition` for now, which are then not inlined correctly.

This PR restricts `IteratorArgument` to arrays of `Reference`, and improves a few related things found while doing it.

(fabbot failure is false positive)

Commits
-------

4d3dce1c0f [DI] Fix inlining conflict by restricting IteratorArgument to Reference[]
2017-04-23 15:36:34 -07:00
Nicolas Grekas
4d3dce1c0f [DI] Fix inlining conflict by restricting IteratorArgument to Reference[] 2017-04-21 14:38:43 +02:00
Fabien Potencier
f730ffae49 feature #22234 [DI] Introducing autoconfigure: automatic _instanceof configuration (weaverryan)
This PR was squashed before being merged into the 3.3-dev branch (closes #22234).

Discussion
----------

[DI] Introducing autoconfigure: automatic _instanceof configuration

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes (mostly, a continuation of a new feature)
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | https://github.com/symfony/symfony-docs/issues/7538

This is a proposal to allow the user to opt into some automatic `_instanceof` config. Suppose I want to auto-tag all of my voters and event subscribers

```yml
# current
services:
    _defaults:
        autowire: true

    _instanceof:
        Symfony\Component\Security\Core\Authorization\Voter\VoterInterface:
            tags: [security.voter]

        Symfony\Component\EventDispatcher\EventSubscriberInterface:
            tags: [kernel.event_subscriber]

    # services using the above tags
    AppBundle\Security\PostVoter: ~
    AppBundle\EventListener\CheckRequirementsSubscriber: ~
```

If I'm registering a service with a class that implements `VoterInterface`, when would I ever *not* want that to be tagged with `security.voter`? Here's the proposed code:

```yml
# proposed
services:
    _defaults:
        autowire: true
        autoconfigure: true

    # services using the auto_configure_instanceof functionality
    AppBundle\Security\PostVoter: ~
    AppBundle\EventListener\CheckRequirementsSubscriber: ~
```

The user must opt into this and it only applies locally to this configuration file. It works because each enabled bundle would have the opportunity to add one or more "automatic instanceof" definitions - e.g. SecurityBundle would add the `security.voter` instanceof config, FrameworkBundle would add the `kernel.event_subscriber` instanceof config, etc.

For another example, you can check out the proposed changes to `symfony-demo` - symfony/symfony-demo#483 - the `_instanceof` section is pretty heavy: 81694ac21e/app/config/services.yml (L20)

Thanks!

Commits
-------

18627bf9f6 [DI] Introducing autoconfigure: automatic _instanceof configuration
2017-04-20 11:20:32 -06:00
Ryan Weaver
18627bf9f6 [DI] Introducing autoconfigure: automatic _instanceof configuration 2017-04-20 11:20:30 -06:00
Nicolas Grekas
1dd3285c91 [DI] Reduce memory overhead of id normalization 2017-04-20 10:02:26 +02:00
Ryan Weaver
e9b96e5f44 Enhancing integration test to show that "override" tags always show up first 2017-04-12 10:34:49 -04:00
Ryan Weaver
6d6116b920 Adding an integration test for the hirarchy of defaults, instanceof, child, parent definitions 2017-04-11 16:40:02 +02:00
Ryan Weaver
cbaee55223 [DI] Track changes at the "Definition" level 2017-04-10 18:14:17 +02:00
Nicolas Grekas
9c53b3deb0 [DI] Restrict autowired registration to "same-vendor" namespaces 2017-04-06 11:28:30 +02:00
Nicolas Grekas
cc5e582dcf [BC BREAK][DI] Always autowire "by id" instead of using reflection against all existing services 2017-04-05 23:48:42 +02:00
Nicolas Grekas
12bb392a39 Merge branch '3.2'
* 3.2:
  move provider after test
  update dataProvider function name
  cast substr result to string and remove empty function use
  rename dataset provider
  Add a test to prevent future regressions
  Switch to `empty` native function to check emptiness
  remove non relevant test case
  Switch to `is_string` native method
  Remove unnecessary parentheses
  Add a test case to prevent future regressions
  Move empty condition in return statement
  Use LF line separator
  fix coding standard to comply with fabbot
  Remove malformed EmailValidatorTest + Update UrlValidator test
  Add empty check on host in other methods + add unit tests
  [Validator] Allow checkMX() to return false when $host is empty
  [DI] Prevent AutowirePass from triggering irrelevant deprecations
  [DI] Fix the xml schema
  [Translation] avoid creating cache files for fallback locales.
2017-04-05 23:43:54 +02:00
Nicolas Grekas
32f264f3c8 Merge branch '2.8' into 3.2
* 2.8:
  move provider after test
  update dataProvider function name
  cast substr result to string and remove empty function use
  rename dataset provider
  Add a test to prevent future regressions
  Switch to `empty` native function to check emptiness
  remove non relevant test case
  Switch to `is_string` native method
  Remove unnecessary parentheses
  Add a test case to prevent future regressions
  Move empty condition in return statement
  Use LF line separator
  fix coding standard to comply with fabbot
  Remove malformed EmailValidatorTest + Update UrlValidator test
  Add empty check on host in other methods + add unit tests
  [Validator] Allow checkMX() to return false when $host is empty
  [DI] Prevent AutowirePass from triggering irrelevant deprecations
  [DI] Fix the xml schema
  [Translation] avoid creating cache files for fallback locales.
2017-04-05 23:38:45 +02:00
Robin Chalas
77927f4b33 [DI] Prevent AutowirePass from triggering irrelevant deprecations 2017-04-05 16:41:09 +02:00
Nicolas Grekas
8ff764be82 [DI] add ServiceLocatorTagPass::register() to share service locators 2017-04-01 13:57:21 +02:00
Nicolas Grekas
4da8884ca4 [DI] Throw on "configured-keys <> getSubscribedServices()" mismatch 2017-03-28 08:17:58 +02:00
Fabien Potencier
6cc9dc7586 feature #22060 [DI] Add "by-id" autowiring: a side-effect free variant of it based on the class<>id convention (nicolas-grekas)
This PR was merged into the 3.3-dev branch.

Discussion
----------

[DI] Add "by-id" autowiring: a side-effect free variant of it based on the class<>id convention

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

This PR adds a new autowiring mode, based only on the class <> id convention.
This way of autowiring is free from any conflicting behavior, which is what I was looking for to begin with.

The expected DX is a bit more involving than the current way we do autowiring. But it's worth it to me, because it's plain predictable - a lot less "magic" imho.

So in this mode, for each `App\Foo` type hint, a reference to an "App\Foo" service will be created. If no such service exists, an exception will be thrown. To me, this opens a nice DX: when type hinting interfaces (which is the best practice), this will tell you when you need to create the explicit interface <> id mapping that is missing - thus encourage things to be made explicit, but only when required, and gradually, in a way that will favor discoverability by devs.

Of course, this is opt-in, and BC. You'd need to do eg in yaml: `autowire: by_id`.
For consistency, the current mode (`autowire: true`) can be configured using `autowire: by_type`.

Commits
-------

c298f2a90c [DI] Add "by-id" autowiring: a side-effect free variant of it based on the class<>id convention
2017-03-25 13:18:59 -07:00
Nicolas Grekas
23fa3a09bf Revert "feature #20973 [DI] Add getter injection (nicolas-grekas)"
This reverts commit 2183f98f54, reversing
changes made to b465634a55.
2017-03-25 15:07:47 +01:00
Nicolas Grekas
c298f2a90c [DI] Add "by-id" autowiring: a side-effect free variant of it based on the class<>id convention 2017-03-24 15:54:23 +01:00
Nicolas Grekas
9b7df39865 [DI] Add and wire ServiceSubscriberInterface 2017-03-22 20:26:09 +01:00
Roland Franssen
6abd312800 [DI] Deprecate Container::isFrozen and introduce isCompiled 2017-03-22 19:49:34 +01:00
Nicolas Grekas
08c2ee32f1 [*Bundle] Add autowiring aliases for common services 2017-03-21 22:34:27 +01:00
Nicolas Grekas
b3f494f5da [DI] Restore skipping logic when autowiring getters 2017-03-21 09:09:26 +01:00
Nicolas Grekas
4d48b58d19 Merge branch '3.2'
* 3.2:
  [Yaml] CS
  [DI] Fix PhpDumper generated doc block
  #20411 fix Yaml parsing for very long quoted strings
  [Workflow] add Phpdoc for better IDE support
  fix package name in conflict rule
  improve message when workflows are missing
  [Doctrine Bridge] fix priority for doctrine event listeners
  Use PHP functions as array_map callbacks when possible
  [Validator] revert wrong Phpdoc change
  Use proper line endings
2017-03-20 11:06:58 +01:00
Nicolas Grekas
d7e74b964f Merge branch '2.8' into 3.2
* 2.8:
  [DI] Fix PhpDumper generated doc block
  #20411 fix Yaml parsing for very long quoted strings
  [Doctrine Bridge] fix priority for doctrine event listeners
  Use PHP functions as array_map callbacks when possible
  [Validator] revert wrong Phpdoc change
  Use proper line endings
2017-03-20 10:32:19 +01:00
Nicolas Grekas
58b3ee7616 [DI] Fix PhpDumper generated doc block 2017-03-20 09:19:13 +01:00
Nicolas Grekas
e41064f65a feature #22030 [DI] Remove skipping magic for autowired methods (nicolas-grekas)
This PR was merged into the 3.3-dev branch.

Discussion
----------

[DI] Remove skipping magic for autowired methods

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

Wildcard based autowiring made it required to auto-skip methods that were not wireable.
Now that things need to be explicit (ie via the `@required` annotation, or via configuration), this "automagic" behavior is not required anymore.
Since it can lead to wtf moments ("*I* did *put that `@required` annotation, why is it ignored by autowiring?*"), I think we should remove it.
This also fixes another issue where configured method calls had their optional arguments wired, while we want only the constructor's to behave as such.

Commits
-------

a6bfe1c [DI] Remove skipping magic for autowired methods
2017-03-19 18:25:10 +01:00
Nicolas Grekas
a6bfe1c6c5 [DI] Remove skipping magic for autowired methods 2017-03-18 00:31:45 +01:00
Nicolas Grekas
5d230b5871 [DI] Introduce "container.service_locator" tag, replaces ServiceLocatorArgument 2017-03-17 17:49:32 +01:00
Guilhem Niot
9b7138545e
[DependencyInjection] Support anonymous services in Yaml 2017-03-14 21:40:20 +01:00
Nicolas Grekas
b77d97a6a3 bug #21937 [DependencyInjection] Handle void return types in closure-proxy (pierredup)
This PR was squashed before being merged into the 3.3-dev branch (closes #21937).

Discussion
----------

[DependencyInjection] Handle void return types in closure-proxy

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

I recently got an error when registering an event listener that specifies a `void` return type. Dumping the container generates a closure proxy that always returns a value, which then conflicts with the return type hint.

E.G the following code is generated (some class names removed for readability)

```
$instance->addListener('kernel.view', /** @closure-proxy ... */ function (...\GetResponseForControllerResultEvent $event): void {
    return ${($_ = isset($this->services[listener']) ? $this->services['listener'] : $this->get('listener')) && false ?: '_'}->onKernelView($event);
}, 128);
```

This then causes the error `A void function must not return a value in ...`

So void return types should be handled by removing the `return` inside the closure

Commits
-------

a5c5ad1 [DependencyInjection] Handle void return types in closure-proxy
2017-03-10 10:24:40 +01:00
Pierre du Plessis
a5c5ad1db1 [DependencyInjection] Handle void return types in closure-proxy 2017-03-10 10:24:38 +01:00
Nicolas Grekas
35977fdf0a [DI] Fix dumping null ServiceClosureArgument 2017-03-09 09:35:59 +01:00
Nicolas Grekas
372ff7ce7d [DI] Replace PHP7-conditional return-type checks by regular type-hints that work on PHP5 2017-03-06 09:56:20 +01:00
Nicolas Grekas
1d9663326e [DI] Allow creating ServiceLocator-based services in extensions 2017-03-05 14:36:05 +01:00
Fabien Potencier
3fa8a0571d feature #21763 [DI] Replace wildcard-based methods autowiring by @required annotation (nicolas-grekas)
This PR was squashed before being merged into the 3.3-dev branch (closes #21763).

Discussion
----------

[DI] Replace wildcard-based methods autowiring by `@required` annotation

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no (affects things that are only on master)
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

While playing a bit with new features in master around DI configuration, several people around me got bitten by wildcard-based autowiring. The typical example is adding `autowire: [set*]` in `_defaults`: use that on `resource: ../src/Command/` PSR4-based loading and boom, `setApplication` and `setHelperSet` will now be wrongly called. You could tell me "of course, don't to that" - but being bitten so early on a master-only feature makes me really unconfident that this will be easy enough for people after the release.

If wildcard-based autowiring is removed, then I don't see anymore the need for allowing arrays as in `autowire: [setFoo,getBar]`. Moreover, this array syntax has a core DX issue: it's a dead end as far as the learning curve is concerned. You learn it, then when becoming a more advanced dev, someone teaches you that you'd better use another syntax: explicit wiring.

And in fact, we don't need it at all, because something else already exists: just declare a method call, but don't define its arguments. If `autowire: true` is set, then the AutowiringPass already fills in the holes. There is only one tweak required to make this work: don't autowire optional arguments for method calls - or that'd be a BC break. To my PoV that's even better: this makes autowiring fit a "do the minimum to make it work" strategy. A really good one to me.

But there is still an issue: wildcard-based autowiring fits a need. Namely, it allows one to define a convention (eg. `'set*'`), and have all such methods that follow the convention be autowired. To me, this looks like doing it reverse (the DI config should adapt to the code, not reverse). So, to fill this need, let the declaration be in the source: just use an annotation!

This PR adds support for the `@required` annotation, borrowed from the Spring framework:
https://www.tutorialspoint.com/spring/spring_required_annotation.htm

Using the annotation is totally optional of course. If you do, *and if autowiring is on*, then it'll be autowired. If you don't, nothing changes: do manual wiring.

Even when not using autowiring, the annotation is still a nice hint for the consumer of your classes: it tells the reader that this method needs to be called for correct instantiation - thus lowering one drawback of setter injection (discoverability).

The implementation of the annotation parsing is done using a few regexp (no dep on any complex parser) - and works with inheritance, by leveraging the `@inheritdoc` tag (the default behavior being to *not* inherit anything from parent methods).

All in all, looking at the diff stats, it makes everything simpler. Good sign, isn't it?

Commits
-------

f286fcc25f [DI] Replace wildcard-based methods autowiring by `@required` annotation
9081699980 Revert "minor #21315 [DI][FrameworkBundle] Show autowired methods in descriptors (ogizanagi)"
2017-03-01 12:47:24 -08:00
Fabien Potencier
ae52490f13 minor #21808 Fix DI test (chalasr)
This PR was merged into the 3.3-dev branch.

Discussion
----------

Fix DI test

| Q             | A
| ------------- | ---
| Branch?       | master
| Tests pass?   | yes

Should fix appveyor/travis builds

Commits
-------

8740e44086 Fix DI test
2017-02-28 17:01:26 -08:00
Robin Chalas
8740e44086 Fix DI test 2017-03-01 01:03:24 +01:00
Fabien Potencier
526d396ccd bug #21627 [DependencyInjection] make the service container builder register its own self referencing definition (hhamon)
This PR was merged into the 3.3-dev branch.

Discussion
----------

[DependencyInjection] make the service container builder register its own self referencing definition

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

Commits
-------

9c97496b5f [DependencyInjection] make the service container builder register the definition of its related service container service (and aliases) in order to make compiler passes be able to reference the special service_container service.
2017-02-28 14:49:08 -08:00
Romain Neutron
671694d0d6
[master] Fix issues reported by static analyse 2017-02-28 16:38:18 +01:00
Nicolas Grekas
6bede45549 Merge branch '3.2'
* 3.2:
  [2.7] Fix issues reported by static analyse
  [Serializer] Reduce nesting in YamlFileLoader
  [DependencyInjection] add missing dumped private services list in a container frozen constructor.
2017-02-28 14:19:28 +01:00
Nicolas Grekas
f286fcc25f [DI] Replace wildcard-based methods autowiring by @required annotation 2017-02-28 10:00:46 +01:00
Fabien Potencier
e37bff942a bug #21602 [DI] Always consider abstract getters as autowiring candidates (nicolas-grekas)
This PR was merged into the 3.3-dev branch.

Discussion
----------

[DI] Always consider abstract getters as autowiring candidates

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes (a missing part of getter autowiring really)
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

When a definition is set to be autowired with no method explicitly configured, we already wire the constructor.
We should also autowire abstract getters - with the same reasoning that makes us autowire the constructor: without concrete getters, the class is unusable. This just makes it usable again.

Commits
-------

8f246bde1d [DI] Always consider abstract getters as autowiring candidates
2017-02-27 14:16:56 -08:00
Hugo Hamon
9c97496b5f [DependencyInjection] make the service container builder register the definition of its related service container service (and aliases) in order to make compiler passes be able to reference the special service_container service. 2017-02-27 17:26:21 +01:00
Hugo Hamon
838d9ca6c0 [DependencyInjection] add missing dumped private services list in a container frozen constructor. 2017-02-27 16:48:04 +01:00
Kévin Dunglas
773eca7794 [DependencyInjection] Tests + refacto for "instanceof" definitions 2017-02-17 19:36:36 +01:00
Fabien Potencier
d7aec48fa8 Merge branch '3.2'
* 3.2:
  Revert "bug #21436 [DependencyInjection] check for circular refs caused by method calls (xabbuh)"
  Static code analysis with Php Inspections (EA Extended)
  [VarDumper] Added missing persistent stream cast
  remove unused translation file
  reverted usage of isNan
2017-02-16 14:50:29 -08:00
Fabien Potencier
ea12123bcf Merge branch '2.8' into 3.2
* 2.8:
  Revert "bug #21436 [DependencyInjection] check for circular refs caused by method calls (xabbuh)"
  Static code analysis with Php Inspections (EA Extended)
  [VarDumper] Added missing persistent stream cast
2017-02-16 14:46:52 -08:00
Fabien Potencier
f53672f82b Merge branch '2.7' into 2.8
* 2.7:
  Revert "bug #21436 [DependencyInjection] check for circular refs caused by method calls (xabbuh)"
  Static code analysis with Php Inspections (EA Extended)
  [VarDumper] Added missing persistent stream cast
2017-02-16 14:43:37 -08:00
Fabien Potencier
68d6415955 Revert "bug #21436 [DependencyInjection] check for circular refs caused by method calls (xabbuh)"
This reverts commit 3441b1586f, reversing
changes made to d1f4cb27fd.
2017-02-16 14:39:07 -08:00
Fabien Potencier
6580a49706 Merge branch '3.2'
* 3.2:
  Permit empty suffix on Windows
  fixed CS
  [FrameworkBundle] Remove unused import
  [Console][Table] fixed render when using multiple rowspans.
  add docblocks for Twig url and path function to improve ide completion
  check for circular refs caused by method calls
  [Serializer] fix upper camel case conversion (see #21399)
  [DI] Auto register extension configuration classes as a resource
  [Console] Updated phpdoc on return types
2017-02-16 06:15:30 -08:00
Fabien Potencier
b3ba37d862 Merge branch '2.8' into 3.2
* 2.8:
  Permit empty suffix on Windows
  [Console][Table] fixed render when using multiple rowspans.
  add docblocks for Twig url and path function to improve ide completion
  check for circular refs caused by method calls
  [Serializer] fix upper camel case conversion (see #21399)
  [DI] Auto register extension configuration classes as a resource
  [Console] Updated phpdoc on return types
2017-02-16 06:07:22 -08:00
Fabien Potencier
e103e1d1db Merge branch '2.7' into 2.8
* 2.7:
  Permit empty suffix on Windows
  [Console][Table] fixed render when using multiple rowspans.
  add docblocks for Twig url and path function to improve ide completion
  check for circular refs caused by method calls
  [Serializer] fix upper camel case conversion (see #21399)
  [DI] Auto register extension configuration classes as a resource
  [Console] Updated phpdoc on return types
2017-02-16 06:06:24 -08:00
Fabien Potencier
7c9a5c1475 feature #21484 [DI] Deprecate underscore-services in YamlFileLoader (nicolas-grekas)
This PR was merged into the 3.3-dev branch.

Discussion
----------

[DI] Deprecate underscore-services in YamlFileLoader

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

As discussed when introducing `_defaults`

Commits
-------

7781082587 [DI] Deprecate underscore-services in YamlFileLoader
2017-02-16 04:51:31 -08:00
Nicolas Grekas
8f246bde1d [DI] Always consider abstract getters as autowiring candidates 2017-02-14 16:19:09 +01:00
Maxime Steinhausser
aa9074d25a [DI][FrameworkBundle] ServiceLocator: Tests dumpers & update descriptors 2017-02-13 20:22:27 +01:00
Fabien Potencier
9ac3a7e010 feature #21383 [DependencyInjection] Add support for named arguments (dunglas, nicolas-grekas)
This PR was merged into the 3.3-dev branch.

Discussion
----------

[DependencyInjection] Add support for named arguments

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

This PR introduces named arguments for services definitions. It's especially useful to inject parameters in an autowired service. It is (at least partially) an alternative to #21376 and #20738.

Usage:

```yml
services:
    _defaults: { autowire: true }
    Acme\NewsletterManager: { $apiKey: "%mandrill_api_key%" }

# Alternative (traditional) syntax
services:
    newsletter_manager:
        class: Acme\NewsletterManager
        arguments:
            $apiKey: "%mandrill_api_key%"
        autowire: true
```
```php
use Doctrine\ORM\EntityManager;
use Psr\Log\LoggerInterface;

namespace Acme;

class NewsletterManager
{
    private $logger;
    private $em;
    private $apiKey;

    public function __construct(LoggerInterface $logger, EntityManager $em, $apiKey)
    {
        $this->logger = $logger;
        $this->em = $em;
        $this->apiKey = $apiKey;
    }
}
```

Commits
-------

8a126c8537 [DI] Deprecate string keys in arguments
2ce36a6074 [DependencyInjection] Add a new pass to check arguments validity
6e501296f9 [DependencyInjection] Add support for named arguments
2017-02-13 15:38:11 +01:00
Robin Chalas
e7935c0adb
[DI] Replace container injection by explicit service locators
[SecurityBundle] Avoid container injection in FirewallMap
2017-02-13 11:05:06 +01:00
Kévin Dunglas
2ce36a6074
[DependencyInjection] Add a new pass to check arguments validity 2017-02-13 10:03:44 +01:00
Kévin Dunglas
6e501296f9
[DependencyInjection] Add support for named arguments 2017-02-13 10:03:43 +01:00
Fabien Potencier
91904af902 feature #21289 [DI] Add prototype services for PSR4-based discovery and registration (nicolas-grekas)
This PR was merged into the 3.3-dev branch.

Discussion
----------

[DI] Add prototype services for PSR4-based discovery and registration

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

Talking with @dunglas, we wondered if this could be a good idea, as a more general approach to folder-based service registration as done in [DunglasActionBundle](https://github.com/dunglas/DunglasActionBundle/blob/master/DependencyInjection/DunglasActionExtension.php).

So here is the implementation.

This allows one to define a set of services as such:
```yaml
services:
    App\:
        resources: ../src/{Controller,Command} # relative to the current file as usual
        autowire: true # or any other attributes really
```

This looks for php files in the "src" folder, derivates PSR-4 class names from them, and uses `class_exists` for final discovery. The resulting services are named after the classes found this way.

The "resource" attribute can be a glob to select only a subset of the classes/files.

This approach has several advantages over [DunglasActionBundle](https://github.com/dunglas/DunglasActionBundle/blob/master/DependencyInjection/DunglasActionExtension.php):
- it is resilient to missing parent classes (see test case)
- it loads classes using the normal code path (the autoloader), thus play well with them (e.g. if one registered a special autoloader).
- it is more predictable, because it uses discovered paths as the only source of ids/classes to register - vs relying on `get_declared_classes`, which would make things context sensitive.

Fits well with current initiatives to me.

Commits
-------

03470b788e [DI] Add "psr4" service attribute for PSR4-based discovery and registration
2017-02-13 06:22:10 +01:00
Nicolas Grekas
03470b788e [DI] Add "psr4" service attribute for PSR4-based discovery and registration 2017-02-12 23:09:17 +01:00
Fabien Potencier
915cca84b6 feature #21533 [DI] Deprecate (un)setting pre-defined services (ro0NL)
This PR was merged into the 3.3-dev branch.

Discussion
----------

[DI] Deprecate (un)setting pre-defined services

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

This PR is the subset of #19668 that fixes #19192: it deprecates (un)setting pre-defined services.
This opens the path to some optimizations in the dumped container in 4.0.

Commits
-------

fdb2140b81 [DI] Deprecate (un)setting pre-defined services
2017-02-12 12:52:54 +01:00
Nicolas Grekas
5a388042bb feature #21194 [Yaml] Add tags support (GuilhemN)
This PR was squashed before being merged into the 3.3-dev branch (closes #21194).

Discussion
----------

[Yaml] Add tags support

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets | https://github.com/symfony/symfony/issues/21185
| License       | MIT
| Doc PR        |

This PR adds custom tags support to the Yaml component.
Symfony tags (`!!binary`, `!str`, etc.) are still managed in the parser to have a lighter diff but we'll be able to convert them later if we want to.

The primary addition of this PR is the `TagInterface`:
```php
interface TagInterface
{
    public function construct(mixed $value): mixed;
}
```

It can be used to register custom tags. An example that could be used to convert [the syntax `=iterator`](https://github.com/symfony/symfony/pull/20907#issuecomment-270728216) to a tag:
```php
final class IteratorTag implements TagInterface
{
    public function construct(mixed $value): mixed
    {
        return new IteratorArgument($value);
    }
}

$parser = new Parser(['iterator' => new IteratorTag()]);
```

If you think this is too complex, @nicolas-grekas [proposed an alternative](https://github.com/symfony/symfony/issues/21185#issuecomment-271074840) to my proposal externalizing this support by introducing a new class `TaggedValue`.

Commits
-------

4744107 [Yaml] Add tags support
2017-02-10 15:54:52 +01:00
Guilhem N
47441070e4 [Yaml] Add tags support 2017-02-10 15:48:44 +01:00
Nicolas Grekas
ad5b5b5d93 [DI] Refacto / cleanup / minor fixes 2017-02-08 13:32:07 +01:00
Roland Franssen
fdb2140b81 [DI] Deprecate (un)setting pre-defined services 2017-02-05 11:30:27 +01:00
Kévin Dunglas
c48c36be8f [DI] Add support for getter autowiring 2017-02-02 20:37:38 +01:00
Nicolas Grekas
7781082587 [DI] Deprecate underscore-services in YamlFileLoader 2017-02-02 16:30:45 +01:00
Nicolas Grekas
caba97a62b feature #21455 [DI] Allow to count on lazy collection arguments (ogizanagi)
This PR was merged into the 3.3-dev branch.

Discussion
----------

[DI] Allow to count on lazy collection arguments

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | https://github.com/symfony/symfony/pull/21450#issuecomment-275931764
| License       | MIT
| Doc PR        | todo (with https://github.com/symfony/symfony-docs/issues/7336)

When using the new iterator feature of the DI component to lazy load collection, we always know the number of arguments in the collection (only the invalidBehavior set to `IGNORE_ON_INVALID_REFERENCE` may change this number). So we are able to generate and use a `RewindableGenerator` implementing `\Countable` by computing this value ahead.

So, in a service accepting `array|iterable`, like in the `GuardAuthenticationListener` (see #21450):

```php
class GuardAuthenticationListener implements ListenerInterface
{
    private $guardAuthenticators;

    /**
       * @param iterable|GuardAuthenticatorInterface[]  $guardAuthenticators   The authenticators, with keys that match what's passed to GuardAuthenticationProvider
       * @param LoggerInterface                         $logger                A LoggerInterface instance
    */
    public function __construct($guardAuthenticators, LoggerInterface $logger = null)
    {
          // ...
    }

    public function handle(GetResponseEvent $event)
    {
        if (null !== $this->logger) {
            $context = array()
            if (is_array($this->guardAuthenticators) || $this->guardAuthenticators instanceof \Countable) {
                $context['authenticators'] = count($this->guardAuthenticators);
            }
            $this->logger->debug('Checking for guard authentication credentials.', $context);
        }
        // ...
    }
}
```

we still keep the ability to call count without loosing the lazy load benefits.

Commits
-------

f23e460 [DI] Allow to count on lazy collection arguments
2017-02-02 15:11:42 +01:00
Jean Pasqualini
2440b0f05d [DI] : Fix bad generation of proxy class when use overriden getter on class with constructor
- [X] Add test fail
- [X] Fix bug
- [X] Run test pass
2017-02-02 11:50:54 +01:00
Nicolas Grekas
b11d391cb7 [DI] Deprecate autowiring-types in favor of aliases 2017-02-01 19:42:39 +01:00
Maxime Steinhausser
f23e460fad [DI] Allow to count on lazy collection arguments 2017-01-31 10:40:15 +01:00
Fabien Potencier
2183f98f54 feature #20973 [DI] Add getter injection (nicolas-grekas)
This PR was merged into the 3.3-dev branch.

Discussion
----------

[DI] Add getter injection

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #20657
| License       | MIT
| Doc PR        | symfony/symfony-docs#7300

Getter overriding by the container will allow a new kind of dependency injection which enables easier laziness and more immutable classes, by not requiring any corresponding setter. See linked issue for more.

This is WIP:
- [x] wire the concept
- [x] dump anonymous classes with PhpDumper
- [x] generate at runtime in ContainerBuilder::createService
- [x] tests
- [x] make it work on PHP 5

Commits
-------

cb498580d1 [DI] Add getter injection
2017-01-30 11:43:35 -08:00
Maxime Steinhausser
bb870304f0 [DI][DX] Do not map id to class for global classes 2017-01-29 23:41:20 +01:00
Christian Flothmann
fe4f7eccf7 check for circular refs caused by method calls 2017-01-28 09:01:30 +01:00
Nicolas Grekas
cb498580d1 [DI] Add getter injection 2017-01-28 08:49:57 +01:00
Nicolas Grekas
29b6e3215e minor #21348 [DependencyInjection] Yaml: check if $tags is an array before using it (dunglas)
This PR was squashed before being merged into the 3.3-dev branch (closes #21348).

Discussion
----------

[DependencyInjection] Yaml: check if $tags is an array before using it

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

Commits
-------

1c9b5c9 [DependencyInjection] Yaml: check if  is an array before using it
2017-01-24 13:43:57 +01:00
Kévin Dunglas
1c9b5c9916 [DependencyInjection] Yaml: check if is an array before using it 2017-01-24 13:43:55 +01:00
Maxime Steinhausser
e6d85700d5 [DependencyInjection] Use current class as default class for factory declarations 2017-01-24 00:01:39 +01:00
Fabien Potencier
4e665548c2 feature #21313 [DI] Add Yaml syntax for short services definition (ogizanagi)
This PR was merged into the 3.3-dev branch.

Discussion
----------

[DI] Add Yaml syntax for short services definition

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

In my experience, most (at least, a lot) of the service definitions in an end-application only require the class and constructor arguments.

#21133 allows to get rid of the `class` attribute by using the service id.
Which means only arguments remain for most use-cases. Hence, we could support this syntax:

#### Before:

```yml
services:
    App\Foo\Bar:
        arguments: ['@baz', 'foo', '%qux%']
```

#### After:

```yml
services:
    App\Foo\Bar: ['@baz', 'foo', '%qux%']
```

It works especially well along with services `_defaults` introduced in #21071 :

```yml
services:
    _defaults:
        public: false
        autowire: ['set*']
        tags: ['serializer.normalizer']

    App\Serializer\FooNormalizer: ['@baz', 'foo', '%qux%']
```

Commits
-------

83b599c6ad [DI] Add Yaml syntax for short services definition
2017-01-23 13:43:33 -08:00
Fabien Potencier
3f8aa7b220 feature #21327 [DI] Factorize compiler passes around new AbstractRecursivePass (nicolas-grekas)
This PR was merged into the 3.3-dev branch.

Discussion
----------

[DI] Factorize compiler passes around new AbstractRecursivePass

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

This PR introduces an AbstractRecursivePass that is able to visit the definition tree recursively everywhere a Definition or a Reference can be nested.

All existing compiler passes that need recursivity are updated to leverage this new class.
This remove a bunch of boilerplate that was previously repeated all over.
It also fixes compiler passes that eg missed looking at configurators for no reason (this "fix" is a new feature really).
It then applies recursivity to AutowirePass, that does not handle it today, but should.

I'm happy that the net result is a loss of 153 lines :)

Commits
-------

6acb80f48f [DI] Factorize compiler passes around new AbstractRecursivePass
2017-01-23 13:11:24 -08:00
Maxime Steinhausser
83b599c6ad [DI] Add Yaml syntax for short services definition 2017-01-23 11:41:40 +01:00
Nicolas Grekas
61475b5596 Merge branch '3.2'
* 3.2:
  fix getMock usage
  fix merge
  [DependencyInjection] Fixed variadic method parameter in autowired classes
  update German translation
  [Validator] Improved error message for missing upload_tmp_dir
2017-01-23 09:28:52 +01:00
Nicolas Grekas
ba41e706ad Merge branch '3.1' into 3.2
* 3.1:
  fix getMock usage
  [DependencyInjection] Fixed variadic method parameter in autowired classes
  update German translation
  [Validator] Improved error message for missing upload_tmp_dir
2017-01-23 09:25:37 +01:00
matze
a7f63de414 [DependencyInjection] Fixed variadic method parameter in autowired classes 2017-01-22 12:28:21 -08:00
Fabien Potencier
735d0a6ce7 Revert "fixed typo"
This reverts commit 6830d9f4c9.
2017-01-21 08:37:26 -08:00
Fabien Potencier
6830d9f4c9 fixed typo 2017-01-21 08:33:48 -08:00
Nicolas Grekas
6acb80f48f [DI] Factorize compiler passes around new AbstractRecursivePass 2017-01-18 21:44:53 +01:00
Fabien Potencier
f7679f74ae feature #21244 [DI] Remove synthetic services from methodMap + generated methods (nicolas-grekas)
This PR was merged into the 3.3-dev branch.

Discussion
----------

[DI] Remove synthetic services from methodMap + generated methods

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

For synthetic services, the generated methods are just dead code to fill opcache ;)
And having them in "methodMap" prevents using the property for checking if a service comes from a (non-synthetic) definition or not.
This prepares some changes that we'd like to do in 4.0, see #19668.

Commits
-------

c1e1e999f3 [DI] Remove synthetic services from methodMap + generated methods
2017-01-15 08:42:11 -08:00
Nicolas Grekas
c1e1e999f3 [DI] Remove synthetic services from methodMap + generated methods 2017-01-15 16:01:29 +01:00
Nicolas Grekas
ce0ee1e484 [DI] Mark generated containers as final 2017-01-13 08:23:38 +01:00
Fabien Potencier
7aeb31e78d feature #21223 [DI] Deprecate case insentivity of service identifiers (nicolas-grekas)
This PR was merged into the 3.3-dev branch.

Discussion
----------

[DI] Deprecate case insentivity of service identifiers

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | minor (see UPGRADE note)
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets | #21193
| License       | MIT
| Doc PR        | -

As discussed in linked RFC.

Commits
-------

d08f1101df [DI] Deprecate case insentivity of service identifiers
2017-01-11 13:31:22 -08:00
Nicolas Grekas
d08f1101df [DI] Deprecate case insentivity of service identifiers 2017-01-11 16:25:44 +01:00
Fabien Potencier
4d916c63dd feature #20611 [DI] FileLoaders: Allow to explicit type to load (ogizanagi)
This PR was merged into the 3.3-dev branch.

Discussion
----------

[DI] FileLoaders: Allow to explicit type to load

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | https://github.com/symfony/symfony/issues/20308
| License       | MIT
| Doc PR        | Not yet

(fabbot will scream out regarding the PR fixtures)

Commits
-------

6b660c2114 [DI] FileLoaders: Allow to explicit type to load
2017-01-10 07:50:45 -08:00
Nicolas Grekas
16d33e1fff Merge branch '3.2'
* 3.2:
  [DI] Add missing legacy group on testLegacy
  Minor tweaks
  Fix merge
  [DI] Dont share service when no id provided
  Fix Container and PhpDumper test inaccuracies
  [DI] Fix missing new line after private alias
  [ClassLoader] Throw an exception if the cache is not writeable
  Fixing regression in TwigEngine exception handling.
2017-01-10 15:32:56 +01:00
Nicolas Grekas
783abab7ae Merge branch '3.1' into 3.2
* 3.1:
  Fix merge
  [DI] Dont share service when no id provided
  Fix Container and PhpDumper test inaccuracies
  [DI] Fix missing new line after private alias
  [ClassLoader] Throw an exception if the cache is not writeable
  Fixing regression in TwigEngine exception handling.
2017-01-10 15:14:38 +01:00