8000 [HttpKernel] Container class name should start with letter or underscore by jzawadzki · Pull Request #20750 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[HttpKernel] Container class name should start with letter or underscore #20750

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.

Sign up for GitHub

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

Closed
wants to merge 4 commits into from
Closed

[HttpKernel] Container class name should start with letter or underscore #20750

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1f49e43
Container class name should start with letter or underscore
jzawadzki Dec 4, 2016
e816655
fix tests for PHP 5.3
jzawadzki Dec 4, 2016
7b12b7d
Renamed a variable
javiereguiluz Feb 22, 2017
3f88f35
Change getName() instead of getContainerClass()
javiereguiluz Feb 27, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/Symfony/Component/HttpKernel/Kernel.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,12 @@ protected function initializeBundles()
*/
protected function getContainerClass()
{
return $this->name.ucfirst($this->environment).($this->debug ? 'Debug' : '').'ProjectContainer';
$name = $this->name.ucfirst($this->environment).($this->debug ? 'Debug' : '').'ProjectContainer';
if (!preg_match('/^[a-zA-Z_\x7f-\xff]/', $name)) { // class name must start with a letter or underscore
Copy link
Member

Choose a reason for hiding this comment

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

Instead, what about always adding a static prefix (like Symfony)? That would avoid executing this regex.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

wouldn't it be somehow considered as BC break? the default name (e.g. appDevDebugContainer) is used by e.g. symfony plugin for phpstorm?

Copy link
Member
@GromNaN GromNaN Dec 5, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

To avoid the regex, it can be replaced by the following:

if(false === stripos($name[0], 'abcdefghijklmnopqrstuvwxyz_'))

Copy link
Member
@yceruto yceruto Dec 6, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

@jzawadzki what about static prefix app then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@yceruto - I would like to avoid having default appappDevDebugProjectContainer
@fabpot - what if we would only add Symfony prefix if $this->name is different then app? then we would remove regex and let default to be appDevDebugProjectContainer

return '_'.$name;
}

return $name;
Copy link
Member
@yceruto yceruto Dec 6, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

to be consistent with the method name getContainerClass(), what do you think about to rename this one to $class ?

}

/**
Expand Down
18 changes: 18 additions & 0 deletions src/Symfony/Component/HttpKernel/Tests/KernelTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -761,6 +761,24 @@ public function testTerminateDelegatesTerminationOnlyForTerminableInterface()
$kernel->terminate(Request::create('/'), new Response());
}

public function testKernelRootDirNameStartingWithANumber()
{
$kernel = $this->getMockBuilder('Symfony\Component\HttpKernel\Tests\Fixtures\KernelForTest')
->disableOriginalConstructor()
->setMethods(array('getRootDir'))
->getMock();

$kernel
->expects($this->any())
->method('getRootDir')
->will($this->returnValue(__DIR__.'/Fixtures/123'));

$kernel->__construct('dev', false);

$kernel->boot();
$this->assertInstanceOf('Symfony\Component\DependencyInjection\ContainerInterface', $kernel->getContainer());
Copy link
Member

Choose a reason for hiding this comment

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

would be much better to write the test in a way which does not trigger a fatal error on failure, as this forbids running other tests and to get the proper PHPUnit reporting.

}

/**
* Returns a mock for the BundleInterface.
*
Expand Down
0