Package home | Report new bug | New search | Development Roadmap Status: Open | Feedback | All | Closed Since Version 3.4.2

Bug #20307 PHP_CodeSniffer_Standards_AbstractVariableSniff analyze traits
Submitted: 2014-06-22 15:45 UTC
From: aik099 Assigned: squiz
Status: Closed Package: PHP_CodeSniffer (version 1.5.3)
PHP Version: 5.4.20 OS: Linux
Roadmaps: (Not assigned)    

 [2014-06-22 15:45 UTC] aik099 (Alexander Obuhovich)
Description: ------------ The traits aren't currently analyzed by PHP_CodeSniffer_Standards_AbstractVariableSniff sniff. I think they should be .


 [2014-06-22 16:08 UTC] aik099 (Alexander Obuhovich)
Also the T_TRAIT should be added to Squiz check, because it's in Zend check. And id makes sense.
 [2014-06-24 06:43 UTC] squiz (Greg Sherwood)
-Status: Open +Status: Closed -Assigned To: +Assigned To: squiz
The AbstractVariableSniff now checks traits: 44ca115c7b97e Not sure what you mean by Zend and Squiz doing different things. You didn't specify what sniffs you were talking about. If this change doesn't fix it, please give me a bit more info and I'll take a look.
 [2014-06-24 16:38 UTC] aik099 (Alexander Obuhovich)
Ah, I wasn't aware that properties of the traits aren't being scanned at all. Thanks for fixing that. What I mean was the "hasCondition" lines of "Squiz_Sniffs_NamingConventions_ValidVariableNameSniff" sniff: - uiz/Sniffs/NamingConventions/ValidVariableNameSniff.php#L119-119 - uiz/Sniffs/NamingConventions/ValidVariableNameSniff.php#L228 As you can see when determining if variable is within a class the T_TRAIT token isn't checked. In contrast in the "Zend_Sniffs_NamingConventions_ValidVariableNameSniff" sniff this has been checked (2 places): nd/Sniffs/NamingConventions/ValidVariableNameSniff.php#L125-125 Also the "processVariableInString" implementation of both mentioned sniffs the leading "_" stripping code is never invoked because variables in string aren't tokenized so for: $var = "ups {$this->_test}"; code the variable name being analyzed is "this" not the "_test" as with regular tokenized code.
 [2014-06-30 12:29 UTC] squiz (Greg Sherwood)
Yep, you are right about those sniffs. I've committed some additional changes here: 84bef558566b2d
 [2014-06-30 13:42 UTC] aik099 (Alexander Obuhovich)
Maybe regex in processVariableInString can be improved to capture complex variable names as well, e.g. " ... {$this::ConstantName} ..." then we can tokenize each of them and determine what we need?