-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Parameter generator backslash escaping #6023
Parameter generator backslash escaping #6023
Conversation
*/ | ||
public function testEscapesQuotes() | ||
{ | ||
$this->assertSame('\\\'', ValueGenerator::escape('\'', false)); |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
It's needed another test in ParameterGeneratorTest |
Found while trying different default parameters for methods. Currently,
Zend\Code
is not able to produce valid code when following is being done:Produces
Foo $bar = '\'
Which is obviously going to break method signature if used as a method parameter.