[DependencyInjection] Fix getArgument() for replaced definition arguments #2502

Merged
merged 2 commits into from Oct 28, 2011

Conversation

Projects
None yet
2 participants
Contributor

jmikola commented Oct 27, 2011

I was implementing something that worked with DefinitionDecorators before the container was compiled and ran into some missing functionality. While I was tinkering, I also added some additional tests for exceptions in both Definition and DefinitionDecorator.

jmikola added some commits Oct 27, 2011

@jmikola jmikola [DependencyInjection] Test Definition and DefinitionDecorator exceptions 4bbb685
@jmikola jmikola [DependencyInjection] Fix DefinitionDecorator::getArgument() for repl…
…acements

While Definition::getArgument() could be used to fetch replaced values, it relied upon bad comparison logic (e.g. "index_1" > 1). Additionally, storing original arguments and replacements in the same array makes Definition::getArguments()'s bounds check unreliable. A single argument and its replacement would count twice, allowing getArgument(2) to pass the bounds check and result in an array index error.

With this new method, fetching of replacement arguments is more straightforward and bounds checking functions as it should.
80f0b98

@fabpot fabpot added a commit that referenced this pull request Oct 28, 2011

@fabpot fabpot merged branch jmikola/2.0-DefinitionDecorator (PR #2502)
Commits
-------

80f0b98 [DependencyInjection] Fix DefinitionDecorator::getArgument() for replacements
4bbb685 [DependencyInjection] Test Definition and DefinitionDecorator exceptions

Discussion
----------

[DependencyInjection] Fix getArgument() for replaced definition arguments

I was implementing something that worked with DefinitionDecorators before the container was compiled and ran into some missing functionality. While I was tinkering, I also added some additional tests for exceptions in both Definition and DefinitionDecorator.
49ad487

@fabpot fabpot merged commit 80f0b98 into symfony:2.0 Oct 28, 2011

@jmikola jmikola added a commit to Exercise/JmikolaJsAssetsHelperBundle that referenced this pull request Nov 3, 2011

@jmikola jmikola Add readme note about symfony/symfony#2502 requirement afb7ad9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment