squizlabs / PHP_CodeSniffer

PHP_CodeSniffer tokenizes PHP files and detects violations of a defined set of coding standards.
BSD 3-Clause "New" or "Revised" License
10.66k stars 1.48k forks source link

PSR-12 is not checking underscore in variable name #2707

Closed Eydamos closed 4 years ago

Eydamos commented 4 years ago

According to PSR-12 section 4.3 Properties and Constants

Property names MUST NOT be prefixed with a single underscore to indicate protected or private visibility. That is, an underscore prefix explicitly has no meaning.

I have some old legacy code which still uses underscores like $_aResult = []; but phpcs --standard=PSR12 is not complaining about the underscore.

I'm using phpcs version 3.5.2

jrfnl commented 4 years ago

If I read this:

That is, an underscore prefix explicitly has no meaning.

It basically means that underscore prefixes are allowed in all circumstances and that no check should be done against the property visibility to determine whether an underscore prefix is allowed or not.

So AFAICS, PSR12 handles this correctly.

Eydamos commented 4 years ago

But it states in the first sentence Property names MUST NOT be prefixed with a single underscore so for my understanding you should never ever have an underscore as the first character. The second sentence just clarifies why there MUST NOT be an underscore as an underscore has no meaning

jrfnl commented 4 years ago

You can't just take part of that sentence - please read the sentence as a whole (emphasis in the below quote is mine):

Property names MUST NOT be prefixed with a single underscore to indicate protected or private visibility.

In other words, property names should not be prefixed with an underscore to indicate visibility, but prefixing property names with an underscore for other reasons is fine.

michalbundyra commented 4 years ago

@TrvTimWerdin

Example:

<?php

namespace MyTest;

class A
{
    protected $_a;
}
$ ./bin/phpcs ./example.php --standard=PSR12

FILE: /Users/michalbundyra/Code/PHP_CodeSniffer/example.php
----------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------------------------------------
 7 | WARNING | Property name "$_a" should not be prefixed with an underscore to indicate visibility
----------------------------------------------------------------------------------------------------

Time: 343ms; Memory: 6MB
Eydamos commented 4 years ago

So this is only checked for class variables but not for other variables?

<?php

namespace MyTest;

class A
{
    public function foo(): void
    {
        $_result = [];
    }
}

phpcs --standard=PSR-12 ./test.php

Does not give any errors at all

jrfnl commented 4 years ago

So this is only checked for class variables but not for other variables?

The PSR12 rule only applies to properties either way.

Eydamos commented 4 years ago

Ok I will write my own sniff to fix missing rules. Thanks for the quick response

michalbundyra commented 4 years ago

@TrvTimWerdin see Squiz.NamingConventions.ValidVariableName :)