This PR was squashed before being merged into the 4.0-dev branch (closes#22763).
Discussion
----------
[DI] Remove deprecated isFrozen()
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | no
| New feature? | no
| BC breaks? | yes
| Deprecations? | no
| Tests pass? | should be
| Fixed tickets | #... <!-- #-prefixed issue number(s), if any -->
| License | MIT
| Doc PR | symfony/symfony-docs#... <!--highly recommended for new features-->
See #19673
Test failure seems unrelated.
Commits
-------
04b39a3 [DI] Remove deprecated isFrozen()
* 3.4:
bug #22814 [FrameworkBundle] FC with EventDispatcher 4.0 (xabbuh)
[PhpUnitBridge] remove unused use statement
do not used deprecated validator test case class
do not mock a deprecated interface
[DI] Added missing deprecation in changelog
[Ldap] add a changelog file
[Security][Serializer][DI] Add new arguments typehints in preparation for 4.0
[MonologBridge] Fix the Monlog ServerLogHandler from Hanging on Windows
[DependencyInjection] Fix dumping of RewindableGenerator with empty IteratorArgument
[DI][Serializer] Fix missing de(normalizer|coder) autoconfig
Use 0.0.0.0 as the server log host default.
This PR was merged into the 3.3-dev branch.
Discussion
----------
Fixing a bug where abstract classes were wired with the prototype loader
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | none
| License | MIT
| Doc PR | n/a
The prototype/PSR-4 loader currently tries to wire abstract classes. The problem is if, for example, you have, for example:
```php
abstract class BaseCommand extends Command
{
}
```
If this is registered as a service, and you have `autoconfigure`, then the console `Application` will try to use this a command.
Was there some reason abstract classes were originally allowed to be registered as services with the PSR4/prototype loader? I don't know if there is a real use-case for registering abstract classes. If you wanted to use that service as a parent service... then you'll probably be configuring it yourself anyways. We could also fix this by changing all tags compiler passes to skip classes that are abstract... *if* there is a use-case for Abstract classes being auto-registered.
Cheers!
Commits
-------
5326bab10a Fixing a bug where abstract classes were wired
This PR was merged into the 3.3-dev branch.
Discussion
----------
[DI] Do not throw autowiring exceptions for a service that will be removed
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | yes
| New feature? | no (arguable)
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | n/a
| License | MIT
| Doc PR | n/a
Hi guys!
tl;dr Do no throw a "Cannot autowire service id foo_bar" if that service (`foo_bar`) is private and is ultimately removed from the container.
I ran into a problem with the new PSR-4 service loader: our existing projects often contains directories with a mixture of services and model classes. In reality, that's not a problem: since the services are private, if any "extra" classes are registered as service, they're removed from the container because they're not referenced. In other words, the system is great: model classes do *not* become services naturally... because nobody tries to inject them as services.
However, if your model classes have constructor args... then things blow up on compilation. This fixes that: it delays autowiring errors until after `RemoveUnusedDefinitionsPass` runs and then does *not* throw those exceptions if the service is gone.
Cheers!
Commits
-------
f4913feaa8 Fixing a bug where services that were eventually removed could cause autowire errors
* 3.2:
[Console] Do not duplicate Helper::strlen() code
[FrameworkBundle] Adding the extension XML
[Form] Minor: Fix comment in ChoiceType
[FrameworkBundle] AbstractConfigCommand: do not try registering bundles twice
fixed CS
fixed CS
[DI] Fix PhpDumper blank lines around namespace
fixed CS
[Workflow] fix use directives
[Workflow] Move twig extension registration to twig bundle
Filesystem: annotate the one network test with a "network" group.
[DependencyInjection] Don't store default deprecation template in every service definition instance
* 2.8:
fixed CS
[DI] Fix PhpDumper blank lines around namespace
fixed CS
Filesystem: annotate the one network test with a "network" group.
[DependencyInjection] Don't store default deprecation template in every service definition instance
This PR was merged into the 3.3-dev branch.
Discussion
----------
[Config] Fix resource tracking with new GlobResource
| Q | A
| ------------- | ---
| Branch? | 3.3
| Bug fix? | yes
| New feature? | yes
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | -
| License | MIT
| Doc PR | -
Right now, resource tracking is done via tracking of directories mtimes, which means a container is rebuilt each time a file is either removed or added, but not when an existing file is modified.
This looks nice on the surface.
BUT.
Most code editors do create a temporary file when you open your code, thus change the parent dir mtime, thus trigger a container rebuild.
When working with PSR-4 loaders, this means each time you just open a file, most of you will trigger a container rebuild.
This is bad :(
Here is a new GlobResource to fix this issue.
Commits
-------
9190e108c1 [Config] Fix resource tracking with new GlobResource
Now that inherit_tags has been removed, 3.3 has the same functionality as 3.2: tags
are *never* cascaded from parent to child (but you tags do inherit from defaults
to a service and instanceof to a service).
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
This PR was squashed before being merged into the 3.3-dev branch (closes#22564).
Discussion
----------
Fixing problem where _defaults set to null was seen as a service
| 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
```yml
services:
_defaults:
```
If you leave `_defaults` empty (i.e. null), you got a bad error before. Now it's better :)
Before:
>The definition for "_defaults" has no class. If you intend to inject this service dynamicall
y at runtime, please mark it as synthetic=true. If this is an abstract definition solely use
d by child definitions, please add abstract=true, otherwise specify a class to get rid of th
is error.
After:
> Service "_defaults" key must be an array, "NULL" given in "/path/to/services.yml"
Commits
-------
4b7e148a9b Fixing problem where _defaults set to null was seen as a service
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.
This PR was merged into the 3.3-dev branch.
Discussion
----------
[DI] Allow service subscribers to leverage autowiring to know where their locator should be injected
| 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 | -
Commits
-------
e407b3d42e [DI] Allow service subscribers to leverage autowiring to know where the locator should be injected
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[]
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
This PR was squashed before being merged into the 3.3-dev branch (closes#22420).
Discussion
----------
[DI] Make tagged abstract services throw earlier
| Q | A
| ------------- | ---
| Branch? | 3.3
| Bug fix? | yes
| New feature? | yes
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | -
| License | MIT
| Doc PR | -
As spotted by @stof in https://github.com/symfony/symfony/pull/22388#issuecomment-293565243, skipping abstract tagged services removes an opportunity to report config mistakes to users.
Instead of skipping them, let's throw as done before (thus reverting #22039, ping @chalasr).
I made `$container->findTaggedServiceIds()` accept a 2nd arg to make this more systematic.
To keep the possibility to have abstract tagged services *for the purpose of tag inheritance*, `ResolveTagsInheritancePass` now resets their tags.
Commits
-------
388e4b3389 [DI] Make tagged abstract services throw earlier
cd06c1297b Revert "minor #22039 Skip abstract definitions in compiler passes (chalasr)"
This PR was merged into the 3.3-dev branch.
Discussion
----------
Enhancing integration test to show that "override" tags show up first
| 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 | not needed
Relates a bit to #22396, in that I'm clarifying and emphasizing the following types of situations:
```yml
services:
_instanceof:
Symfony\Component\Security\Core\Authorization\Voter\VoterInterface:
tags:
# you probably shouldn't set priority here, but let's pretend we did
- { name: security.voter, priority: 100 }
AppBundle\Security\CoolPersonVoter:
tags:
- { name: security.voter, priority: 50 }
```
In the final `Definition`, the tags will appear in this order:
* security.voter, priority 50
* security.voter, priority 100
It works the same for parent-child definitions.
tl;dr; If a service has the same tag multiple times, the one that should be used should appear *earlier* in the Definition. The code already works that way - this test emphasizes it.
Commits
-------
e9b96e5 Enhancing integration test to show that "override" tags always show up first