8000 [DX] [DI] Improve exception for invalid setter injection arguments by curry684 · Pull Request #25663 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[DX] [DI] Improve exception for invalid setter injection arguments #25663

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 3, 2018

Conversation

curry684
Copy link
Contributor
@curry684 curry684 commented Jan 3, 2018
Q A
Branch? 4.0
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
License MIT

Improve the exception message for when you accidentally write setter injection wrong as I did too many times:

My\Logging\Service:
  calls: [ [ setLogger, "@logger" ] ]

Used to throw:

Type error: Argument 2 passed to Symfony\Component\DependencyInjection\Definition::addMethodCall() must be of the type array, object given, called in /var/www/project/vendor/symfony/dependency-injection/Loader/YamlFileLoader.php on line 457

Now throws:

The second parameter for function call "setLogger" must be an array of its arguments for service "My\Namespace\Service" in /var/www/project/config/services.yaml. Check your YAML syntax in /var/www/project/config/services.yaml (which is loaded in resource "/var/www/project/config/services.yaml").

(semi-offtopic: why isn't LoggerAwareInterface in the default autoconfigure for FrameworkExtension?)

@carsonbot carsonbot added Status: Needs Review DX DX = Developer eXperience (anything that improves the experience of using Symfony) DependencyInjection labels Jan 3, 2018
@curry684
Copy link
Contributor Author
curry684 commented Jan 3, 2018

Come to think of it now - we could just also force-wrap the second argument into an array so the currently failing syntax also works. Don't see a reason why not.

Copy link
Member
@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK as is for 3.4 IMHO

@nicolas-grekas nicolas-grekas added this to the 3.4 milestone Jan 3, 2018
@curry684 curry684 changed the base branch from master to 3.4 January 3, 2018 10:42
@curry684 curry684 changed the base branch from 3.4 to master January 3, 2018 10:43
@curry684
Copy link
Contributor Author
curry684 commented Jan 3, 2018

@nicolas-grekas before I rebase - what's your take on just force-wrapping the array. I think it's harmless convenience and it makes the exception obsolete.

@nicolas-grekas
Copy link
Member
nicolas-grekas commented Jan 3, 2018

Alternative ways to write configuration create dead end for user experience (people need to learn two ways of doing things, so they can just read config from different people)
which means it's not as obvious as it seems to me
the exception is for 3.4
the discussion about the new syntax is for master for sure

@curry684 curry684 force-pushed the setter-injection-arguments branch from ee2fea3 to 6850a22 Compare January 3, 2018 11:24
@curry684 curry684 changed the base branch from master to 3.4 January 3, 2018 11:24
@curry684
Copy link
Contributor Author
curry684 commented Jan 3, 2018

Rebased.

@fabpot
Copy link
Member
fabpot commented Jan 3, 2018

Thank you @curry684.

@fabpot fabpot merged commit 6850a22 into symfony:3.4 Jan 3, 2018
fabpot added a commit that referenced this pull request Jan 3, 2018
… arguments (curry684)

This PR was merged into the 3.4 branch.

Discussion
----------

[DX] [DI] Improve exception for invalid setter injection arguments

| Q             | A
| ------------- | ---
| Branch?       | 4.0
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no <!-- don't forget to update UPGRADE-*.md files -->
| Tests pass?   | yes
| License       | MIT

Improve the exception message for when you accidentally write setter injection wrong as I did too many times:
```yaml
My\Logging\Service:
  calls: [ [ setLogger, "@logger" ] ]
```
Used to throw:

> Type error: Argument 2 passed to Symfony\Component\DependencyInjection\Definition::addMethodCall() must be of the type array, object given, called in /var/www/project/vendor/symfony/dependency-injection/Loader/YamlFileLoader.php on line 457

Now throws:

> The second parameter for function call "setLogger" must be an array of its arguments for service "My\Namespace\Service" in /var/www/project/config/services.yaml. Check your YAML syntax in /var/www/project/config/services.yaml (which is loaded in resource "/var/www/project/config/services.yaml").

(semi-offtopic: why isn't `LoggerAwareInterface` in the default autoconfigure for FrameworkExtension?)

Commits
-------

6850a22 [DX] [DI] Improve exception for invalid setter injection arguments
@curry684 curry684 deleted the setter-injection-arguments branch January 3, 2018 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DependencyInjection DX DX = Developer eXperience (anything that improves the experience of using Symfony) Status: Reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0