-
Notifications
You must be signed in to change notification settings - Fork 8
Conversation
I completely missed that part. Here's an in depth review: https://blog.blackfire.io/php-7-performance-improvements-encapsed-strings-optimization.html |
docs/book/v2/ruleset.md
Outdated
$string = '\$var'; | ||
$query = "SELECT * FROM table WHERE name =''"; | ||
``` | ||
|
||
*Invalid: There are no variables inside double quote strings.* | ||
*Invalid: There are no variables, control characters inside double quote strings* |
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.
Change to: "Invalid: There are no variables or control characters inside double quote strings."
docs/book/v2/ruleset.md
Outdated
```php | ||
<?php | ||
$string = "Hello There\r\n"; | ||
$string = "Hello $there"; | ||
$string = 'Hello There'; | ||
$string = 'Hello'.' There'."\n"; | ||
$string = 'Hello' . ' There' . "\n"; |
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.
Maybe we should remove this example as interpolation would be better. In this case it would end up the same as the first example: "Hello There\n";
You need to update |
Thanks for the review @xtreamwayz . It'd be worth keeping an eye on squizlabs/PHP_CodeSniffer#2259 for a more comprehensive sniff. |
I saw that this morning. Looks like it's just an idea. No PR yet. |
I believe our rule is that double quoted string can not contain vars or other variable interpolation and |
The main reasons I've advocated usage of
Both of these were informed by performance of previous PHP versions with regards to string interpolation. Interpolation was known to be slow, and escaping rules tend to get messy. Concatenation is predictable, but hard to read. However, if PHP 7 solves most of the performance issues, string interpolation would likely be a viable choice again. As such, I've got an open question to Julien Pauli asking what the performance differences are between the two approaches. |
This repository has been closed and moved to laminas/laminas-coding-standard; a new issue has been opened at laminas/laminas-coding-standard#4. |
This repository has been moved to laminas/laminas-coding-standard. If you feel that this patch is still relevant, please re-open against that repository, and reference this issue. To re-open, we suggest the following workflow:
|
Squiz.Strings.DoubleQuoteUsage.ContainsVar prevents strings containing variables. This doesn't match the examples in the documentation and, given that PHP7s interpolation uses less memory than concatenation, probably isn't intended.
This PR adds an exclude for that check and tries to make the docs a little more exact.