Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Parameter generator backslash escaping #6023

Merged
merged 5 commits into from

3 participants

@Ocramius
Collaborator

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 Code bug labels
@EvanDotPro EvanDotPro was assigned by Ocramius
@Ocramius Ocramius added this to the 2.3.1 milestone
tests/ZendTest/Code/Generator/ValueGeneratorTest.php
@@ -138,4 +138,20 @@ public function testPropertyDefaultValueCanHandleArrayWithUnsortedKeys()
$this->assertEquals($expectedSource, $valueGenerator->generate());
}
+
+ /**
+ * @group 6023
+ */
+ public function testEscapesBackslash()
+ {
+ $this->assertSame('\\\\', ValueGenerator::escape('\\', false));
+ }
+
+ /**
+ * @group 6023
+ */
+ public function testEscapesQuotes()
+ {
+ $this->assertSame('\\\'', ValueGenerator::escape('\'', false));
@Maks3w Collaborator
Maks3w added a note

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

@Maks3w Collaborator
Maks3w added a note

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

@Ocramius Collaborator

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?

@Maks3w Collaborator
Maks3w added a note

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Maks3w
Collaborator

It's needed another test in ParameterGeneratorTest

@EvanDotPro EvanDotPro was unassigned by Maks3w
@Maks3w Maks3w was assigned by Ocramius
@Maks3w Maks3w merged commit 44d47fd into zendframework:master

1 check passed

Details default The Travis CI build passed
@weierophinney weierophinney referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@weierophinney weierophinney referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
@weierophinney weierophinney referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
2  library/Zend/Code/Generator/ValueGenerator.php
@@ -398,7 +398,7 @@ public function generate()
*/
public static function escape($input, $quote = true)
{
- $output = addcslashes($input, "'");
+ $output = addcslashes($input, "\\'");
// adds quoting strings
if ($quote) {
View
16 tests/ZendTest/Code/Generator/ParameterGeneratorTest.php
@@ -227,4 +227,20 @@ public function testTypehintsWithNamespaceInNamepsacedClassReturnTypewithBacksla
$this->assertEquals('\OtherNamespace\ParameterClass', $param->getType());
}
+
+ /**
+ * @group 6023
+ *
+ * @coversNothing
+ */
+ public function testGeneratedParametersHaveEscapedDefaultValues()
+ {
+ $parameter = new ParameterGenerator();
+
+ $parameter->setName('foo');
+ $parameter->setDefaultValue("\\'");
+ $parameter->setType('stdClass');
+
+ $this->assertSame("stdClass \$foo = '\\\\\\''", $parameter->generate());
+ }
}
View
26 tests/ZendTest/Code/Generator/ValueGeneratorTest.php
@@ -14,6 +14,8 @@
/**
* @group Zend_Code_Generator
* @group Zend_Code_Generator_Php
+ *
+ * @covers \Zend\Code\Generator\ValueGenerator
*/
class ValueGeneratorTest extends \PHPUnit_Framework_TestCase
{
@@ -138,4 +140,28 @@ public function testPropertyDefaultValueCanHandleArrayWithUnsortedKeys()
$this->assertEquals($expectedSource, $valueGenerator->generate());
}
+
+ /**
+ * @group 6023
+ *
+ * @dataProvider getEscapedParameters
+ */
+ public function testEscaping($input, $expectedEscapedValue)
+ {
+ $this->assertSame($expectedEscapedValue, ValueGenerator::escape($input, false));
+ }
+
+ /**
+ * Data provider for escaping tests
+ *
+ * @return string[][]
+ */
+ public function getEscapedParameters()
+ {
+ return array(
+ array('\\', '\\\\'),
+ array("'", "\\'"),
+ array("\\'", "\\\\\\'"),
+ );
+ }
}
Something went wrong with that request. Please try again.