Skip to content

Investigate use of PHP_CODESNIFFER_IN_TESTS in src code #966

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
4 of 7 tasks
jrfnl opened this issue Apr 13, 2025 · 0 comments
Open
4 of 7 tasks

Investigate use of PHP_CODESNIFFER_IN_TESTS in src code #966

jrfnl opened this issue Apr 13, 2025 · 0 comments

Comments

@jrfnl
Copy link
Member

jrfnl commented Apr 13, 2025

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:

Process

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant