Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

Parameter generator backslash escaping #6023

Conversation

Ocramius
Copy link
Member

Found while trying different default parameters for methods. Currently, Zend\Code is not able to produce valid code when following is being done:

$generator = new Zend\Code\Generator\ParameterGenerator();

$generator->setType('Foo');
$generator->setDefaultValue('\\');
$generator->setName('bar');

var_dump($generator->generator());

Produces

Foo $bar = '\'

Which is obviously going to break method signature if used as a method parameter.

@Ocramius Ocramius added this to the 2.3.1 milestone Mar 22, 2014
*/
public function testEscapesQuotes()
{
$this->assertSame('\\\'', ValueGenerator::escape('\'', false));
Copy link
Member

Choose a reason for hiding this comment

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

CS: If the use of single quotes implies escapping then use double quotes

Copy link
Member

Choose a reason for hiding this comment

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

You could write a data provider instead write unique tests for each value

Copy link
Member Author

Choose a reason for hiding this comment

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

CS: If the use of single quotes implies escapping then use double quotes

Hmm, always used single quotes in any possible case - is that a new one that I missed?

Copy link
Member

Choose a reason for hiding this comment

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

It's a rule of the outdate ZF CS guidelines. It's more readable a string without escaping characters.

Anyway. CS is always subject to readability.

@Maks3w
Copy link
Member

Maks3w commented Mar 22, 2014

It's needed another test in ParameterGeneratorTest

Maks3w added a commit that referenced this pull request Mar 23, 2014
Maks3w added a commit that referenced this pull request Mar 23, 2014
Maks3w added a commit that referenced this pull request Mar 23, 2014
@Maks3w Maks3w merged commit 44d47fd into zendframework:master Mar 23, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants