Merge pull request #146 from andrewnicols/phpunitTags

Tags for file-specific paths should normalise pathnames

.github/workflows/phpcs.yml

@@ -4,18 +4,26 @@ on: [push, pull_request]
 
 jobs:
   test:
-    runs-on: ubuntu-latest
+    runs-on: ${{ matrix.os }}
 
     strategy:
       fail-fast: false
       matrix:
-        include:
-          - php: 8.3
-          - php: 8.2
-          - php: 8.1
-          - php: 8.0
-          - php: 7.4
+        php:
+          - 8.3
+          - 8.2
+          - 8.1
+          - 8.0
+          - 7.4
+        os:
+          - ubuntu-latest
+          - windows-latest
     steps:
+      - name: Set git to use LF
+        run: |
+          git config --global core.autocrlf false
+          git config --global core.eol lf
+
       - name: Check out repository code
         uses: actions/checkout@v4
 
@@ -55,7 +63,7 @@ jobs:
         run: ./vendor/bin/phpunit-coverage-check -t 80 clover.xml
 
       - name: Integration tests
-        if: ${{ !cancelled() }}
+        if: ${{ (!cancelled()) && (runner.os == 'ubuntu-latest') }}
         run: |
           # There is one failure (exit with error)
           vendor/bin/phpcs --standard=moodle moodle/Tests/fixtures/integration_test_ci.php | tee output.txt || [[ $? = 1 ]]

moodle/Sniffs/PHPUnit/TestCaseNamesSniff.php

@@ -233,9 +233,10 @@ public function process(File $file, $pointer) {
             if ($bspos !== false) { // Only if there are level2 and down namespace.
                 $relns = str_replace('\\', '/', substr(trim($namespace, ' \\'), $bspos + 1));
 
+                $filename = MoodleUtil::getStandardisedFilename($file);
                 // Calculate the relative path under tests directory.
-                $dirpos = strripos(trim(dirname($file->getFilename()), ' /') . '/', '/tests/');
-                $reldir = str_replace('\\', '/', substr(trim(dirname($file->getFilename()), ' /'), $dirpos + 7));
+                $dirpos = strripos(trim(dirname($filename), ' /') . '/', '/tests/');
+                $reldir = str_replace('\\', '/', substr(trim(dirname($filename), ' /'), $dirpos + 7));
 
                 // Warning if the relative namespace does not match the relative directory.
                 if ($reldir !== $relns) {

moodle/Tests/MoodleCSBaseTestCase.php

@@ -221,9 +221,13 @@ protected function verifyCsResults() {
         // Let's process the fixture.
         try {
             if ($this->fixtureFileName !== null) {
+                $fixtureFilename = $this->fixtureFileName;
+                if (DIRECTORY_SEPARATOR !== '/') {
+                    $fixtureFilename = str_replace('/', DIRECTORY_SEPARATOR, $fixtureFilename);
+                }
                 $fixtureSource = file_get_contents($this->fixture);
                 $fixtureContent = <<<EOF
-                phpcs_input_file: {$this->fixtureFileName}
+                phpcs_input_file: {$fixtureFilename}
                 {$fixtureSource}
                 EOF;
                 $phpcsfile = new \PHP_CodeSniffer\Files\DummyFile($fixtureContent, $ruleset, $config);

moodle/Tests/Sniffs/Commenting/ValidTagsSniffTest.php

@@ -55,11 +55,11 @@ public static function provider(): array {
                 'fixturePath' => 'lib/tests/example_test.php',
                 'fixtureSource' => 'unit_test',
                 'errors' => [
-                    11 => 'Incorrect docblock tag "@returns". Should be "@return"',
-                    12 => 'Invalid docblock tag "@void"',
-                    55 => 'Invalid docblock tag "@small"',
-                    56 => 'Invalid docblock tag "@zzzing"',
-                    57 => 'Invalid docblock tag "@inheritdoc"',
+                    12 => 'Incorrect docblock tag "@returns". Should be "@return"',
+                    13 => 'Invalid docblock tag "@void"',
+                    58 => 'Invalid docblock tag "@small"',
+                    59 => 'Invalid docblock tag "@zzzing"',
+                    60 => 'Invalid docblock tag "@inheritdoc"',
                 ],
                 'warnings' => [],
             ],

moodle/Tests/Sniffs/Commenting/fixtures/ValidTags/unit_test.php

@@ -2,6 +2,7 @@
 
 /**
  * @covers \some_class
+ * @group some_group
  */
 class some_test extends \advanced_testcase {
     /**
@@ -42,6 +43,8 @@ public function also_all_valid_tags() {
 
     /**
      * Some more valid tags, because we are under tests area.
+     *
+     * @group some_group
      */
     public function valid_inline_tags() {
         // @codeCoverageIgnoreStart

moodle/Tests/Sniffs/Commenting/fixtures/ValidTags/unit_test.php.fixed

@@ -2,6 +2,7 @@
 
 /**
  * @covers \some_class
+ * @group some_group
  */
 class some_test extends \advanced_testcase {
     /**
@@ -41,6 +42,8 @@ class some_test extends \advanced_testcase {
 
     /**
      * Some more valid tags, because we are under tests area.
+     *
+     * @group some_group
      */
     public function valid_inline_tags() {
         // @codeCoverageIgnoreStart

moodle/Util/Docblocks.php

@@ -32,7 +32,7 @@ abstract class Docblocks
      *
      * @var array<string, bool>
      * @link http://manual.phpdoc.org/HTMLSmartyConverter/HandS/ */
-    public static array $validTags = [
+    private static array $validTags = [
         // Behat tags.
         'Given' => true,
         'Then' => true,
@@ -100,7 +100,7 @@ abstract class Docblocks
      *
      * @var string[]
      */
-    public static array $invalidTagsToRemove = [
+    private static array $invalidTagsToRemove = [
         'void',
     ];
 
@@ -109,7 +109,7 @@ abstract class Docblocks
      *
      * @var string[string]
      */
-    public static array $renameTags = [
+    private static array $renameTags = [
         // Rename returns to return.
         'returns' => 'return',
     ];
@@ -120,7 +120,7 @@ abstract class Docblocks
      *
      * @var array(string => array(string))
      */
-    public static array $pathRestrictedTags = [
+    private static array $pathRestrictedTags = [
         'Given' => ['#.*/tests/behat/.*#'],
         'Then' => ['#.*/tests/behat/.*#'],
         'When' => ['#.*/tests/behat/.*#'],
@@ -324,7 +324,7 @@ public static function isValidTag(
         $tag = ltrim($tokens[$tagPtr]['content'], '@');
         if (array_key_exists($tag, self::$validTags)) {
             if (array_key_exists($tag, self::$pathRestrictedTags)) {
-                $file = $phpcsFile->getFilename();
+                $file = MoodleUtil::getStandardisedFilename($phpcsFile);
                 foreach (self::$pathRestrictedTags[$tag] as $path) {
                     if (preg_match($path, $file)) {
                         return true;

moodle/Util/MoodleUtil.php

@@ -242,14 +242,17 @@ public static function getMoodleComponent(File $file, $selfPath = true): ?string
             }
         }
 
+        $filepath = MoodleUtil::getStandardisedFilename($file);
         // Let's find the first component that matches the file path.
         foreach ($components as $component => $componentPath) {
             // Only components with path.
             if (empty($componentPath)) {
                 continue;
             }
+
             // Look for component paths matching the file path.
-            if (strpos($file->path, $componentPath . '/') === 0) {
+            $componentPath = str_replace('\\', '/', $componentPath . DIRECTORY_SEPARATOR);
+            if (strpos($filepath, $componentPath) === 0) {
                 // First match found should be the better one always. We are done.
                 return $component;
             }
@@ -450,14 +453,15 @@ public static function getMoodleRoot(?File $file = null, bool $selfPath = true):
      */
     public static function isLangFile(File $phpcsFile): bool
     {
+        $filename = MoodleUtil::getStandardisedFilename($phpcsFile);
         // If the file is not under a /lang/[a-zA-Z0-9_-]+/ directory, nothing to check.
         // (note that we are using that regex because it's what PARAM_LANG does).
-        if (preg_match('~/lang/[a-zA-Z0-9_-]+/~', $phpcsFile->getFilename()) === 0) {
+        if (preg_match('~/lang/[a-zA-Z0-9_-]+/~', $filename) === 0) {
             return false;
         }
 
         // If the file is not a PHP file, nothing to check.
-        if (substr($phpcsFile->getFilename(), -4) !== '.php') {
+        if (substr($filename, -4) !== '.php') {
             return false;
         }
 
@@ -476,28 +480,29 @@ public static function isLangFile(File $phpcsFile): bool
      */
     public static function isUnitTest(File $phpcsFile): bool
     {
+        $filename = MoodleUtil::getStandardisedFilename($phpcsFile);
         // If the file isn't called, _test.php, nothing to check.
         if (stripos(basename($phpcsFile->getFilename()), '_test.php') === false) {
             return false;
         }
 
         // If the file isn't under tests directory, nothing to check.
-        if (stripos($phpcsFile->getFilename(), '/tests/') === false) {
+        if (stripos($filename, '/tests/') === false) {
             return false;
         }
 
         // If the file is in a fixture directory, ignore it.
-        if (stripos($phpcsFile->getFilename(), '/tests/fixtures/') !== false) {
+        if (stripos($filename, '/tests/fixtures/') !== false) {
             return false;
         }
 
         // If the file is in a generator directory, ignore it.
-        if (stripos($phpcsFile->getFilename(), '/tests/generator/') !== false) {
+        if (stripos($filename, '/tests/generator/') !== false) {
             return false;
         }
 
         // If the file is in a behat directory, ignore it.
-        if (stripos($phpcsFile->getFilename(), '/tests/behat/') !== false) {
+        if (stripos($filename, '/tests/behat/') !== false) {
             return false;
         }
 
@@ -609,4 +614,14 @@ public static function getTokensOnLine(
             ARRAY_FILTER_USE_BOTH
         );
     }
+
+    /**
+     * Get the standardised filename for the file.
+     *
+     * @param File @phpcsFile
+     * @return string
+     */
+    public static function getStandardisedFilename(File $phpcsFile): string {
+        return str_replace('\\', '/', $phpcsFile->getFilename());
+    }
 }