PHP_CodeSniffer icon indicating copy to clipboard operation
PHP_CodeSniffer copied to clipboard

Investigate use of `PHP_CODESNIFFER_IN_TESTS` in src code

Open jrfnl opened this issue 9 months ago • 0 comments

There are currently 8 places in the source code which check whether the global PHP_CODESNIFFER_IN_TESTS constant is defined and if so, change some behaviour.

Code behaving differently in test vs production situations makes the code risky to begin with.

It also makes it exponentially harder to write tests for some parts of the codebase and wastes a lot of dev time when devs are scratching their heads about tests not working, while all the while, the test is perfectly fine, but somewhere in the path to the code under test, the behaviour of PHPCS was changed via one of those PHP_CODESNIFFER_IN_TESTS checks.....

It should be investigated for each of the uses of PHP_CODESNIFFER_IN_TESTS in src, whether it can be removed and/or if there is a different solution possible to the problem the condition was trying to solve.

Ideally, the PHP_CODESNIFFER_IN_TESTS constant should be removed at the end of this exercise (but that's not strictly the goal of this ticket).

At the time of writing the following code refers to the constant
--------------------------------------------------
path/to/PHPCS/src/Config.php
--------------------------------------------------
385:      *
386:      * @return void
387:      */
388:     public function __construct(array $cliArgs=[], $dieOnUnknownArg=true)
389:     {
390:         if (defined('PHP_CODESNIFFER_IN_TESTS') === true) {
391:             // Let everything through during testing so that we can
392:             // make use of PHPUnit command line arguments as well.
393:             $this->dieOnUnknownArg = false;
394:         } else {
395:             $this->dieOnUnknownArg = $dieOnUnknownArg;

630:         $colors = self::getConfigData('colors');
631:         if ($colors !== null) {
632:             $this->colors = (bool) $colors;
633:         }
634: 
635:         if (defined('PHP_CODESNIFFER_IN_TESTS') === false) {
636:             $cache = self::getConfigData('cache');
637:             if ($cache !== null) {
638:                 $this->cache = (bool) $cache;
639:             }
640: 

793:         case 'cache':
794:             if (isset(self::$overriddenDefaults['cache']) === true) {
795:                 break;
796:             }
797: 
798:             if (defined('PHP_CODESNIFFER_IN_TESTS') === false) {
799:                 $this->cache = true;
800:                 self::$overriddenDefaults['cache'] = true;
801:             }
802:             break;
803:         case 'no-cache':

907:                     break;
908:                 }
909: 
910:                 $this->exclude = $this->parseSniffCodes(substr($arg, 8), 'exclude');
911:                 self::$overriddenDefaults['exclude'] = true;
912:             } else if (defined('PHP_CODESNIFFER_IN_TESTS') === false
913:                 && substr($arg, 0, 6) === 'cache='
914:             ) {
915:                 if ((isset(self::$overriddenDefaults['cache']) === true
916:                     && $this->cache === false)
917:                     || isset(self::$overriddenDefaults['cacheFile']) === true

---------------------------------------------------
path/to/PHPCS/src/Ruleset.php
---------------------------------------------------
198:                 }
199: 
200:                 Autoload::addSearchPath(dirname($standard), $namespace);
201:             }
202: 
203:             if (defined('PHP_CODESNIFFER_IN_TESTS') === true && empty($restrictions) === false) {
204:                 // In unit tests, only register the sniffs that the test wants and not the entire standard.
205:                 foreach ($restrictions as $restriction) {
206:                     $sniffs = array_merge($sniffs, $this->expandRulesetReference($restriction, dirname($standard)));
207:                 }
208: 

------------------------------------------------------
path/to/PHPCS/src/Files/File.php
------------------------------------------------------
342:             echo "\t*** START TOKEN PROCESSING ***".PHP_EOL;
343:         }
344: 
345:         $foundCode        = false;
346:         $listenerIgnoreTo = [];
347:         $inTests          = defined('PHP_CODESNIFFER_IN_TESTS');
348:         $checkAnnotations = $this->config->annotations;
349: 
350:         // Foreach of the listeners that have registered to listen for this
351:         // token, get them to process it.
352:         foreach ($this->tokens as $stackPtr => $token) {

------------------------------------------------------------------------------------------------
path/to/PHPCS/src/Standards/Generic/Sniffs/WhiteSpace/ScopeIndentSniff.php
------------------------------------------------------------------------------------------------
104:      *
105:      * @return array<int|string>
106:      */
107:     public function register()
108:     {
109:         if (defined('PHP_CODESNIFFER_IN_TESTS') === true) {
110:             $this->debug = false;
111:         }
112:
113:         return [T_OPEN_TAG];
114: 

----------------------------------------------------------------
path/to/PHPCS/src/Tokenizers/Tokenizer.php
----------------------------------------------------------------
172:     {
173:         $currColumn = 1;
174:         $lineNumber = 1;
175:         $eolLen     = strlen($this->eolChar);
176:         $ignoring   = null;
177:         $inTests    = defined('PHP_CODESNIFFER_IN_TESTS');
178: 
179:         $checkEncoding = false;
180:         if (function_exists('iconv_strlen') === true) {
181:             $checkEncoding = true;
182:         }

Action list:

  • [ ] Investigate and review use of PHP_CODESNIFFER_IN_TESTS in Config::__construct()
  • [x] Investigate and review use of PHP_CODESNIFFER_IN_TESTS in Config::restoreDefaults() - @jrfnl - Removed via #1068
  • [x] Investigate and review use of PHP_CODESNIFFER_IN_TESTS in Config::processLongArgument() (x2) - @jrfnl - Removed via #1068
  • [x] Investigate and review use of PHP_CODESNIFFER_IN_TESTS in Ruleset::__construct() - @jrfnl - Removed via #996
  • [ ] Investigate and review use of PHP_CODESNIFFER_IN_TESTS in File::process()
  • [x] Investigate and review use of PHP_CODESNIFFER_IN_TESTS in Generic\Sniffs\WhiteSpace\ScopeIndentSniff::register() - @jrfnl - Removed via #1036
  • [ ] Investigate and review use of PHP_CODESNIFFER_IN_TESTS in Tokenizer::createPositionMap()

Process

Leave a comment on this ticket if you want to "claim" one of the above actions.

jrfnl avatar Apr 13 '25 23:04 jrfnl