[Feature request] Provide a way to test a ruleset.xml file

[VincentLanglet]

When writing custom sniffs, it's easy to test them thanks to the files AbstractSniffUnitTest and AllTest.php. For example, with this project just need to:

• Write a SniffUnitTest.inc with some errors
• Write a SniffUnitTest.inc.fixed with the fixed errors
• Write a SniffUnitTest.php with the list of the error.
• Run

  phpunit --filter WordPress --bootstrap="$(pwd)/vendor/squizlabs/php_codesniffer/tests/bootstrap.php" $(pwd)/vendor/squizlabs/php_codesniffer/tests/AllTests.php

But when we add a sniff into a custom ruleset.xml, I don't see an easy way to have a CI checking the ruleset lints exactly what we expect. I would love having the same way an AbstractRulesetTest working the exactly same way with inc, inc.fixed, php files but using all the Sniffs from a ruleset.xml instead of just one Sniff.

I could then:

• do TDD to find the right Sniff to use
• be sure there is no conflict between two different Sniffs
• easily check I can replace a custom Sniff by a provided one

Since you're both writing custom standards, WDYT about this @jrfnl, @michalbundyra ?

[jrfnl]

I'm not sure that an AbstractRulesetTest would be the way to go, but am happy to see a discussion about the topic.

In WPCS we basically already do the ruleset testing you are referring to by using PHPCS itself in the Travis script.

This involves two steps:

1. Test that all rules are covered. In the repo there is 100% clean file which should trigger each and every sniff: https://github.com/WordPress/WordPress-Coding-Standards/blob/develop/bin/class-ruleset-test.php PHPCS is run against that file with every ruleset WPCS offers and those runs should come back clean. The downside of this is the maintenance of the "clean" file. New rules are added, but the file isn't always (or rather: is rarely) updated when they are added, so it is easy for that file to become out of date.
2. Testing against fixer conflicts. As sniffs should function independently of each other, the unit test case files of most WPCS sniffs contain plenty of code which doesn't comply with the WPCS rules to safeguard that independence. phpcbf is run over all the .inc test case file with travis_retry. The first run should exit with 1 (= all fixable errors fixed) and then on the retry it should exit with 0 as there are no more fixable errors found.

While this isn't a 100% solution, it has worked well for us so far and catches most potential problems before they would have gone into the repo.

Over the years, I've worked on reducing the fixer conflicts in the rulesets of PHPCS itself (look up all my PRs marked with "fixer conflict") with the ultimate goal to introduce part 2 of the above checks into PHPCS itself. While I've managed to bring the number of issues down, it's not down to 0 yet (at least it wasn't last time I ran the checks), so more effort is needed before this can be introduced into the PHPCS native Travis run. Also see the comments about this in: https://github.com/squizlabs/PHP_CodeSniffer/issues/1809

For the Travis script in WPCS, see: https://github.com/WordPress/WordPress-Coding-Standards/blob/b82bc3eb3f7f911cd0bb8c9a2c6dfd81ccaaf028/.travis.yml#L73-L100

[VincentLanglet]

This involves two steps:

1. Test that all rules are covered. In the repo there is 100% clean file which should trigger each and every sniff: https://github.com/WordPress/WordPress-Coding-Standards/blob/develop/bin/class-ruleset-test.php PHPCS is run against that file with every ruleset WPCS offers and those runs should come back clean. The downside of this is the maintenance of the "clean" file. New rules are added, but the file isn't always (or rather: is rarely) updated when they are added, so it is easy for that file to become out of date.

You check this file is 100% clean, but if I remove all the sniff from the WPCS the file will still be clean. It's indeed a good idea to have this check, but I want also the opposite, a not-clean file in which I expect some errors.

2. Testing against fixer conflicts. As sniffs should function independently of each other, the unit test case files of most WPCS sniffs contain plenty of code which doesn't comply with the WPCS rules to safeguard that independence. phpcbf is run over all the .inc test case file with travis_retry. The first run should exit with 1 (= all fixable errors fixed) and then on the retry it should exit with 0 as there are no more fixable errors found.

While this isn't a 100% solution, it has worked well for us so far and catches most potential problems before they would have gone into the repo.

Over the years, I've worked on reducing the fixer conflicts in the rulesets of PHPCS itself (look up all my PRs marked with "fixer conflict") with the ultimate goal to introduce part 2 of the above checks into PHPCS itself. While I've managed to bring the number of issues down, it's not down to 0 yet (at least it wasn't last time I ran the checks), so more effort is needed before this can be introduced into the PHPCS native Travis run. Also see the comments about this in: #1809

I was thinking about two sort of conflict:

• Two different Sniffs are asking for a different way to write some code (this is checked by you Travis script)
• Two different Sniffs returning a similar error message for the same part of code. These duplicates can't be found without a list of expected errors for the "not-clean" file.

Since this repository is maintaining different Standard PSR1, PSR2, PSR12, ..., adding functional tests with a correct example (https://www.php-fig.org/psr/psr-12/#example) and wrong one seems for me a nice improvement.

[VincentLanglet]

If you're still interested about this discussion @jrfnl

Just by commenting 4 lines in the testSniff function of the AbstractSniffUnitTest

    final public function testSniff()
    {
        // Skip this test if we can't run in this environment.
        if ($this->shouldSkipTest() === true) {
            $this->markTestSkipped();
        }

        $sniffCode = Common::getSniffCode(get_class($this));
        list($standardName, $categoryName, $sniffName) = explode('.', $sniffCode);

        $testFileBase = $this->testsDir.$categoryName.DIRECTORY_SEPARATOR.$sniffName.'UnitTest.';

        // Get a list of all test files to check.
        $testFiles = $this->getTestFiles($testFileBase);
        $GLOBALS['PHP_CODESNIFFER_SNIFF_CASE_FILES'][] = $testFiles;

        if (isset($GLOBALS['PHP_CODESNIFFER_CONFIG']) === true) {
            $config = $GLOBALS['PHP_CODESNIFFER_CONFIG'];
        } else {
            $config        = new Config();
            $config->cache = false;
            $GLOBALS['PHP_CODESNIFFER_CONFIG'] = $config;
        }

        $config->standards = [$standardName];
// COMMENTING THE FOLLOWING LINE
//        $config->sniffs    = [$sniffCode];
        $config->ignored   = [];

        if (isset($GLOBALS['PHP_CODESNIFFER_RULESETS']) === false) {
            $GLOBALS['PHP_CODESNIFFER_RULESETS'] = [];
        }

        if (isset($GLOBALS['PHP_CODESNIFFER_RULESETS'][$standardName]) === false) {
            $ruleset = new Ruleset($config);
            $GLOBALS['PHP_CODESNIFFER_RULESETS'][$standardName] = $ruleset;
        }

        $ruleset = $GLOBALS['PHP_CODESNIFFER_RULESETS'][$standardName];

        $sniffFile = $this->standardsDir.DIRECTORY_SEPARATOR.'Sniffs'.DIRECTORY_SEPARATOR.$categoryName.DIRECTORY_SEPARATOR.$sniffName.'Sniff.php';

        $sniffClassName = substr(get_class($this), 0, -8).'Sniff';
        $sniffClassName = str_replace('\Tests\\', '\Sniffs\\', $sniffClassName);
        $sniffClassName = Common::cleanSniffClass($sniffClassName);

// COMMENTING THE THREE FOLLOWING LINES
//        $restrictions = [strtolower($sniffClassName) => true];
//        $ruleset->registerSniffs([$sniffFile], $restrictions, []);
//        $ruleset->populateTokenListeners();

        $failureMessages = [];
        foreach ($testFiles as $testFile) {
            $filename  = basename($testFile);
            $oldConfig = $config->getSettings();

            try {
                $this->setCliValues($filename, $config);
                $phpcsFile = new LocalFile($testFile, $ruleset, $config);
                $phpcsFile->process();
            } catch (RuntimeException $e) {
                $this->fail('An unexpected exception has been caught: '.$e->getMessage());
            }

            $failures        = $this->generateFailureMessages($phpcsFile);
            $failureMessages = array_merge($failureMessages, $failures);

            if ($phpcsFile->getFixableCount() > 0) {
                // Attempt to fix the errors.
                $phpcsFile->fixer->fixFile();
                $fixable = $phpcsFile->getFixableCount();
                if ($fixable > 0) {
                    $failureMessages[] = "Failed to fix $fixable fixable violations in $filename";
                }

                // Check for a .fixed file to check for accuracy of fixes.
                $fixedFile = $testFile.'.fixed';
                if (file_exists($fixedFile) === true) {
                    $diff = $phpcsFile->fixer->generateDiff($fixedFile);
                    if (trim($diff) !== '') {
                        $filename          = basename($testFile);
                        $fixedFilename     = basename($fixedFile);
                        $failureMessages[] = "Fixed version of $filename does not match expected version in $fixedFilename; the diff is\n$diff";
                    }
                }
            }

            // Restore the config.
            $config->setSettings($oldConfig);
        }//end foreach

        if (empty($failureMessages) === false) {
            $this->fail(implode(PHP_EOL, $failureMessages));
        }

    }//end testSniff()

I succeed to run a complete ruleset instead of just a sniff over my file. What do you think about adding an overridable option to not restrict the test to only a Sniff ? cc @gsherwood