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

Bug #17331 Possible parse error: interfaces may not include member vars
Submitted: 2010-04-21 14:27 UTC
From: ogmueller Assigned: squiz
Status: Closed Package: PHP_CodeSniffer (version 1.2.2)
PHP Version: 5.2.12 OS:
Roadmaps: (Not assigned)    
Subscription  


 [2010-04-21 14:27 UTC] ogmueller (Oliver Mueller)
Description: ------------ The following Interface will add warnings ("Possible parse error: interfaces may not include member vars"), even though it should be ok, because there are no member vars. They are just function params. The file File.php in PHPCS triggers this warning in function getMemberProperties in line 2040. It might want to check "nested_parenthesis" as well to exclude this case. Test script: --------------- <?php /** * Interface for WHERE condition */ /** * Interface for WHERE condition */ interface MyInterface { /** * Useful stuff * * @param string $param Something * @return bollean TRUE or FALSE */ public function func( $param ); } ?> Expected result: ---------------- No warning Actual result: -------------- Possible parse error: interfaces may not include member vars

Comments

 [2010-04-22 05:24 UTC] squiz (Greg Sherwood)
-Status: Open +Status: Feedback -Assigned To: +Assigned To: squiz
I've tried this on PHP 5.2 and 5.3 with PHPCS 1.2.2 and the SVN version and I cannot get your sample code to report that possible parse error. Can you please run your PHPCS on that sample code with the -vv command line argument and paste the output so I can compre the tokenizing: phpcs -vv sample_code.php
 [2010-04-22 13:30 UTC] ogmueller (Oliver Mueller)
Hi Greg, sorry, I think I forgot the most important information: we used our own Sniff. It looks like this: <?php require_once 'PHP/CodeSniffer/Standards/AbstractScopeSniff.php'; /** * Valid Variable Name sniff. * * Checks for valid variable naming. This must be camel style. * Private and protected variable have to start with an underscore. * * @access public * @package TEQlibs * @subpackage quality * @author Timo Buhmann <buhmann@teqneers.de> * @author Oliver G. Mueller <mueller@teqneers.de> * @version $Revision: 11065 $ * @internal $Id: ValidVariableNameSniff.php 11065 2009-10-27 13:09:21Z oliver $ * @copyright Copyright (C) 2003-2010 TEQneers GmbH & Co. KG. All rights reserved. */ class TEQneers_Sniffs_PHP_ValidVariableNameSniff extends PHP_CodeSniffer_Standards_AbstractScopeSniff { /** * constructor * * we have to define this scope sniff to know, if the function will be * in or outside of class/interface scope. */ public function __construct() { parent::__construct( array(T_CLASS, T_INTERFACE), array(T_VARIABLE), true ); } /** * Processes function names within the scope. * * @param PHP_CodeSniffer_File $phpcsFile The file being processed. * @param integer $stackPtr The position where this token was found. * @param integer $currScope The position of the current scope. * @return void */ protected function processTokenWithinScope( PHP_CodeSniffer_File $phpcsFile, $stackPtr, $currScope ) { if( $phpcsFile->hasCondition( $stackPtr, T_FUNCTION ) ) { return $this->processTokenOutsideScope( $phpcsFile, $stackPtr ); } $className = $phpcsFile->getDeclarationName( $currScope ); $classProps = $phpcsFile->getMemberProperties($stackPtr); $token = $phpcsFile->getTokens(); $propName = substr( $token[$stackPtr]['content'], 1 ); $isPublic = $classProps['scope'] === 'public' ? true : false; $scope = $classProps['scope']; $scopeSpecified = $classProps['scope_specified']; // check if the variable is defined as function parameter if( isset($token[$stackPtr]['nested_parenthesis']) ) { $parenthesis = reset($token[$stackPtr]['nested_parenthesis']); if( $token[$token[$parenthesis]['parenthesis_owner']]['code'] == T_FUNCTION ) { if ( PHP_CodeSniffer::isCamelCaps( $propName, false, true, true ) === false ) { $methodName = $phpcsFile- >getDeclarationName($token[$parenthesis]['parenthesis_owner']); $error = 'Method parameter "'.$className.'->'.$methodName.'( $'.$propName.' )" is not in camel caps format'; $phpcsFile->addError($error, $stackPtr); return; } return; } } // methods have to have a scope like public, protected or private assigned if ( $scopeSpecified === false ) { $error = 'Attribute "'.$className.'::$'.$propName.'" has no scope defined'; $phpcsFile->addError( $error, $stackPtr ); return; } // private and protected methods, it must have an underscore on the front. if ( $isPublic === false && strpos( $propName, '_' ) !== 0 ) { $error = ucfirst($scope).' attribute "'.$className.'::$'.$propName.'" must be prefixed with an underscore'; $phpcsFile->addError( $error, $stackPtr ); return; } // If it's not a private method, it must not have an underscore on the front. if ( $isPublic === true && $scopeSpecified === true && strpos( $propName, '_' ) === 0 ) { $error = ucfirst($scope).' attributes "'.$className.'::$'.$propName.'" must not be prefixed with an underscore'; $phpcsFile->addError( $error, $stackPtr ); return; } if ( PHP_CodeSniffer::isCamelCaps( $propName, false, $isPublic, true ) === false ) { $error = 'Attribute "'.$className.'::$'.$propName.'" is not in camel caps format'; $phpcsFile->addError($error, $stackPtr); return; } } /** * Processes function names outside scope. * * @param PHP_CodeSniffer_File $phpcsFile The file being processed. * @param integer $stackPtr The position where this token was found. * @param integer $currScope The position of the current scope. * @return void */ protected function processTokenOutsideScope( PHP_CodeSniffer_File $phpcsFile, $stackPtr ) { $tokens = $phpcsFile->getTokens(); $propName = substr( $tokens[$stackPtr]['content'], 1 ); // Ignore assignments used in a condition, like an IF or FOR. if (isset($tokens[$stackPtr]['nested_parenthesis']) === true) { foreach ($tokens[$stackPtr]['nested_parenthesis'] as $start => $end) { if (isset($tokens[$start]['parenthesis_owner']) === true) { return; } } } // ignore self::$_var // TODO: we should detect self with whitespaces as well as "self :: $_var" if( $stackPtr-2 >= 0 && $tokens[$stackPtr-2]['code'] == T_SELF ) { return; } // do not check camel caps on capitalized variables if( preg_match( '([a-z])', $propName ) && PHP_CodeSniffer::isCamelCaps( $propName, false, true, true ) === false ) { $error = 'Variable "$'.$propName.'" is not in camel caps format'; $phpcsFile->addError($error, $stackPtr); return; } } } ?> I was pretty sure, that I copied it from one of the existing Sniffs (SQUIZ, PEAR, ZEND, ...?). Now I saw, that the ValidVariableNameSniff in those had change a lot. I guess, I should copy it again and redo my work. But you might want to change the given behavior anyway? sorry for the effort.
 [2010-04-23 04:28 UTC] squiz (Greg Sherwood)
I'll let you have a go and updating it because I'm not really sure what I should be changing here. If you can be a bit clearer about what you think I should change, I'll take another look. Otherwise, I'll just close the bug. And if you copied that code, please keep the licence header as that sniff uses the BSD licence.
 [2010-04-23 11:34 UTC] ogmueller (Oliver Mueller)
Hi Greg, I added your copyright to all our copied and modified sniffs to fulfill the BSD license. We just replaced the header with our standard one and by doing so, the license was removed, which didn't happen on purpose. Sorry! The problem is the following. If someone uses "$phpcsFile->getMemberProperties($stackPtr);" on an interface, this Method assumes, that method parameters are class members, which they are not. You can try it out with the MyInterface example. getMemberProperties will tell you "Possible parse error: interfaces may not include member vars" when it analyses the function "func" with "$param".
 [2010-05-05 04:32 UTC] squiz (Greg Sherwood)
-Status: Feedback +Status: Closed
This bug has been fixed in SVN. If this was a documentation problem, the fix will appear on pear.php.net by the end of next Sunday (CET). If this was a problem with the pear.php.net website, the change should be live shortly. Otherwise, the fix will appear in the package's next release. Thank you for the report and for helping us make PEAR better.