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

Bug #19694 Squiz.PHP.LowercasePHPFunctions incorrectly matches return by ref functions
Submitted: 2012-11-07 01:19 UTC Modified: 2012-11-07 07:24 UTC
From: happydog Assigned: squiz
Status: Closed Package: PHP_CodeSniffer (version 1.4.1)
PHP Version: Irrelevant OS: All
Roadmaps: (Not assigned)    
Subscription  


 [2012-11-07 01:19 UTC] happydog (Mark Clements)
Description: ------------ Squiz.PHP.LowercasePHPFunctions reports errors if any of the built-in PHP functions are used with the incorrect case. However, it gives false positives when you have class methods that happen to match an existing PHP function and your coding style is such that it is not in lower-case. An example that we use all over the place is providing a static Count() function for our data objects, which returns the number of objects of that type currently in the system. Any call to these functions is flagged as incorrect, even though it is not. Expected result: ---------------- That the sniff correctly identifies when a function call is actually a method call, i.e. to distinguish count() from ::count() or ->count() and to only check function calls for correct casing. Actual result: -------------- Class methods are also being checked, which can result in a lot of false positives if you use a coding style that is not lower-case.

Comments

 [2012-11-07 01:48 UTC] squiz (Greg Sherwood)
-Status: Open +Status: Feedback -Assigned To: +Assigned To: squiz
There is already code in there to make sure the function name doesnt have a -> or :: in front of it and that code has been there for 2 years, so I don't think you'd have an old file. Are you able to give me some sample code that produces the error and show me what output you get from running PHPCS on your file with the built-in Squiz standard.
 [2012-11-07 03:38 UTC] happydog (Mark Clements)
Hmmm... I can't seem to reproduce the problem as described, but I've found an example of a different false positive. I encountered the error in 1.4.0, so perhaps it has been fixed in 1.4.1, or perhaps I made a mistake and there was no error - or the error was this new problem and I got confused! The issue false positive I have found is when functions are declared as returning by reference: == Test case == function &Sort() { } == Result == Calls to inbuilt PHP functions must be lowercase; expected "sort" but found "Sort" I'm not sure whether to close this and log a new bug, or to update the summary, so I'll leave that to you.
 [2012-11-07 04:07 UTC] squiz (Greg Sherwood)
-Summary: Squiz.PHP.LowercasePHPFunctions incorrectly matches on class methods. +Summary: Squiz.PHP.LowercasePHPFunctions incorrectly matches return by ref functions
 [2012-11-07 04:10 UTC] squiz (Greg Sherwood)
-Status: Feedback +Status: Closed
Reusing this report is fine. I've committed a fix to the github repo: https://github.com/squizlabs/PHP_CodeSniffer/commit/f2c7e227fa217f6ddefcbc7f0e7 bda931feb7027
 [2012-11-07 07:24 UTC] happydog (Mark Clements)
Wow - I'm impressed by how quickly you've responded to and fixed these issues. Thank you!