Skip to content
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

[Service Container] Container camelize strange behavior #7431

Closed
eduardosoliv opened this issue Mar 20, 2013 · 4 comments
Closed

[Service Container] Container camelize strange behavior #7431

eduardosoliv opened this issue Mar 20, 2013 · 4 comments
Labels
Good first issue Ideal for your first contribution! (some Symfony experience may be required)

Comments

@eduardosoliv
Copy link
Contributor

At Container the method camelize:

    /**
     * Camelizes a string.
     *
     * @param string $id A string to camelize
     *
     * @return string The camelized string
     */
    public static function camelize($id)
    {
        return preg_replace_callback('/(^|_|\.)+(.)/', function ($match) { return ('.' === $match[1] ? '_' : '').strtoupper($match[2]); }, $id);
    }

I don't understand why if the first char is . or _ is kept:

php > function camelize($id) { return preg_replace_callback('/(^|_|\.)+(.)/', function ($match) { return ('.' === $match[1] ? '_' : '').strtoupper($match[2]); }, $id); }
php > echo camelize('test.test2') . "\n";
Test_Test2
php > echo camelize('.testtest2') . "\n";
.testtest2
php > echo camelize('test_test2') . "\n";
TestTest2
php > echo camelize('_test_test2') . "\n";
_testTest2

If the camelize is used to transform for example an array on several variables it will fail:

php > $.t = 5;
PHP Parse error:  syntax error, unexpected '.', expecting variable (T_VARIABLE) or '$' in php shell code on line 1

Parse error: syntax error, unexpected '.', expecting variable (T_VARIABLE) or '$' in php shell code on line 1

And is really strange that after you camelize a string it still keep underscore. My suggestion is that if a _ or . exists as first character would disappear on camelize, so instead of the below behavior both would return TestDa

php > echo camelize('test_da') . "\n";
TestDa
php > echo camelize('_test_da') . "\n";
_testDa
@jfsimon
Copy link
Contributor

jfsimon commented Apr 2, 2013

Why should heading . or _ be replaced with something? With what?

@eduardosoliv
Copy link
Contributor Author

Camelize will remove all _ except the first one, in my opinion it should remove all the _ without any exceptions.

php > echo camelize('_test_test2') . "\n";
_testTest2

About the . it is replaced with _ in all positions except in the first position that is not replaced, eg:

php > echo camelize('test.test2') . "\n";
Test_Test2
php > echo camelize('.testtest2') . "\n";
.testtest2

Keeping the first . doesn't make a lot of sense to me, why that "exception"? For sure a lot of people use camelize to invoke methods dynamic, and a method with . as start is invalid, but that is not the main point.

The main point is that I don't see why a camelize that remove all the _ and switch all . into _ have exceptions to first chars. If that behavior will be kept at least the PHPdoc of the method should be clear about that.

@radutopala
Copy link

Guys, I just found this issue using symfony/symfony 2.2.x-dev with commit 3c44d48 and PHP5.4.13.

On composer.phar install:

Generating autoload files
PHP Parse error:  syntax error, unexpected '('/(^|_|\.)+(.)/', fu' in .../vendor/symfony/symfony/src/Symfony/Component/DependencyInjection/Container.php on line 489
Script Sensio\Bundle\DistributionBundle\Composer\ScriptHandler::buildBootstrap handling the post-install-cmd event terminated with an exception

  [RuntimeException]                                     
  An error occurred when generating the bootstrap file.

@dlsniper
Copy link
Contributor

dlsniper commented May 9, 2013

@fabpot can you give your input on this? I'd like to solve the issue but I need some rules for how this should work. Thanks :)

fabpot added a commit that referenced this issue Jul 21, 2013
This PR was merged into the 2.2 branch.

Discussion
----------

[DependencyInjection] Fix Container::camelize to convert beginning and ending . and _

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

I'm using strtr to make the conversion in order to ensure that the behavior is the same as `Container::get`.

From the test cases I've added, the following were not passing:

Commits
-------

485d53a [DependencyInjection] Fix Container::camelize to convert beginning and ending chars
@fabpot fabpot closed this as completed Jul 21, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good first issue Ideal for your first contribution! (some Symfony experience may be required)
Projects
None yet
Development

No branches or pull requests

5 participants