This is just bad advice.

1.) Indentation here is messed up.

$array = array('a', 
                  'b',
                 );
$array2 = ['c', 
             'd',
            ];

What is going on there? :)

2.) You have a copy paste error in The following pattern is considered legit:

$array = array('a', 
                  'b'
                 );

$array2 = ['c', 'd', ];

I guess that $array2 was supposed to have the , removed?

Either way, trailing commas on multi-line array definitions are awesome. They are also supported in pretty much every language I've used, but are banned in JSON.

They are a style guide choice for sure, but a) they don't hurt at all, and b) they reduce friction and potential conflicts in diffs. I've seen a LOT of conflicts caused by just this, and since asking developers on various teams to use trailing diffs those problems have gone away.

I'd like to see this one deleted entirely.

We shouldn't encourage people to include a closing php by including it in the examples. This is because if someone writes a php class and then puts a new line character after the closing tag, php will send the response - not the desired behaviour at all!

The title in the document is wrong, however, the rule is missing one important point: Interfaces

Members of an interface are per language specification always public and any other visibility makes no sense. Hence, requiring the visibility to be defined is redundant and (imho) should actually be avoided for clarity.

Changes are along the lines of:

• Correcting grammer and spelling errors
• Fix some blatant copy/paste errors
• Removing PHP closing tags if not needed to demonstrate a point (to be in line with the no-closing-tags rule)
• Removing unused/commented out headers (to be in line with no-commented-out-code rule - lead by example)
• Small markdown use improvements, mainly for consistency:
  • adding code backticks
  • removing code backticks from non code phrases, replacing them by underscores to make the text cursive
  • make functions in phrases always have (), and remove them for most language constructs
  • consistent use of bullets and fixed where they would not be recognized as such
  • simplify table layouts
• Added some additional Further Reading links
• Make links to PHP manual language agnostic

As this is quite a huge PR, I have purposefully split it in functional commits for easier review. Please see the notes on the individual commits for more information.

How should someone e.g. remove in_array() with isset() when the first function operates on values, while the other could only check for a key. You might advice someone trying to use the isset(), but it's not really a possible replacement. Same is true of diff/intersect on arrays.

Replacing array_walk() and array_map() with a foreach-by-reference could cause some side effects as mentioned in the PHP Pitfalls document you linked to. Although the foreach-by-reference is a lot faster, the usage of the other functions is necessary if you use clean and readable functional programming techniques.

Not entirely sure what the point of this is.

https://github.com/dseguy/clearPHP/blob/master/rules/no-eval.md#rule-details

It doesn't really explain what is happening, and if its good or bad.

Also you say:

Such code has to be systematically sanitized before it is used.

It's impossible to sanitize eval from user input securely.

http://stackoverflow.com/questions/5922762/eval-base64-decode-php-virus

http://stackoverflow.com/a/3697776

Best to just NEVER put user input anywhere near eval.

In https://github.com/dseguy/clearPHP/blob/master/rules/imported-collision.md#rule-details block last code sample with $a,$b,$c,$d,$e,$f is obviously not considered warnings

Just before the first code sample we have (https://github.com/dseguy/clearPHP/blob/master/rules/avoid-redefining-properties.md#avoid-redefining-properties) Check the code below : $lock is defined in a class But there's no $lock variable. And nothing about vehicle too,

Things like https://github.com/vlucas/phpdotenv are a great solution to this sort of problem, and it's a framework agnostic (or no framework) solution. Laravel builds on top of it, as do others no doubt.

Maybe mention you don't want to put sleep in web facing code specifically, but it's ok in CLI stuff.

https://github.com/dseguy/clearPHP/blob/master/rules/no-sleep.md

Sleep can be pretty handy in big cron jobs when you're doing batch imports and dont want to utterly flood your database.

I'm not sure where "kitsched" came from, but I'm confident that "derickr" is the contributor that was intended to be included.

Made these quick edits through github, sorry for the commit spam.

In https://github.com/dseguy/clearPHP/blob/master/rules/use-smart-autoload.md example:

function my_autoloader($class) { include 'classes/' . $class . '.class.php'; }

For every include that fail, will generate a warning and increase your log. If you have 4 functions (methods,..) registered using include directly, will create 4 warnings for every request.

It is worse with require, that create a fatal error and stop the execution.

It 's a shame than spl_autoload_register don't work like include_path, that try every path and show the error if all fail.

When going through everything, I noticed that these two are essentially the same. If they are not (supposed to be), then maybe the difference between the two rules should be made clearer. If they are, I suggest merging them into one rule.

https://github.com/dseguy/clearPHP/blob/master/rules/no-buried-assignation.md https://github.com/dseguy/clearPHP/blob/master/rules/no-implied-if.md

Currently are all examples, pattern without space between brackets and var. I think is also a good practice to write readable code. Maybe I would check all examples for this and send a pull request with the changes - if you like it?

Example:

if (!$foo) {

much better

if ( ! $foo ) {

Also I would add a chapter about space, whitespace in code for better legibility, if you like?

While you have linked to Nikita's article Disproving the Single Quotes Performance Myth, your advice is still incredibly generalized and not entirely accurate.

I wrote this up on PHP The Right Way: Strings

Scroll down to this bit:

Which is quicker? There is a myth floating around that single quote strings are fractionally quicker than double quote strings. This is fundamentally not true.

If you are defining a single string and not trying to concatenate values or anything complicated, then either a single or double quoted string will be entirely identical. Neither are quicker.

If you are concatenating multiple strings of any type, or interpolate values into a double quoted string, then the results can vary. If you are working with a small number of values, concatenation is minutely faster. With a lot of values, interpolating is minutely faster.

Regardless of what you are doing with strings, none of the types will ever have any noticeable impact on your application. Trying to rewrite code to use one or the other is always an exercise in futility, so avoid this micro- optimization unless you really understand the meaning and impact of the differences.