PHPCSStandards / PHP_CodeSniffer

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

Review all sniffs/code doing label name comparisons #372

Closed jrfnl closed 5 months ago

jrfnl commented 6 months ago

Bug Description

Label names, i.e. names of namespaces, class/trait/interface/enum name and function names are case-insensitive for the ASCII letters, but case-sensitive for char 0x80 and up.

For the record: variable names in PHP are case-sensitive and so are constant names (unless explicitly declared not to be so via define()).

There are a number of sniffs in PHP_CodeSniffer which compare names, be it comparing a class name with another class name (Generic.Classes.DuplicateClassName), be it comparing a class name with a method name (Generic.NamingConventions.ConstructorName).

I'm fairly sure that, in most cases, these comparisons will use code akin to strtolower($nameA) === strtolower($nameB). This is incorrect and these comparisons should be fixed to use strcasecmp() instead, as strcasecmp() only compares the ASCII letters case-insensitively (which is exactly what is needed).

Notes about this task:

Task List

The below is a list of sniffs in which strtolower() was found and which need to be reviewed.

Each of these will need to be reviewed, though I expect that by far not all of them will need changes.

To do

Claimed

Nothing yet

Done

Nothing yet

Want to contribute ?

Leave a comment below to claim one or more files you'll be reviewing.

PRs related to this task should preferably only address one file - or even one instance of the issue in one file - per PR. This will make reviewing the changes more straight forward and will allow for merging PRs promptly.

jrfnl commented 5 months ago

Hmm.. interestingly enough, it seems that strtolower() actually ignores multibyte characters/non-ASCII characters completely, so we should actually be fine.

Also see: https://3v4l.org/AQ93s

Closing this issue as not needed.