Merge branch 'main' into forbid-unserialize

Author: andrewnicols

.phplint.yml

@@ -2,3 +2,4 @@ path: ./
 exclude:
   - vendor
   - moodle/Tests/fixtures
+  - moodle/Tests/Sniffs/Namespaces/fixtures

CHANGELOG.md

@@ -7,6 +7,16 @@ The format of this change log follows the advice given at [Keep a CHANGELOG](htt
 
 ## [Unreleased]
 
+## [v3.3.6] - 2023-09-15
+### Added
+- A new `moodle-extra` coding standard which moves towards a more PSR-12 compliant coding style.
+- Enforce the use of the short array syntax (`[]`), warning about the long alternative (`array()`): `Generic.Arrays.DisallowLongArraySyntax`. This will be raised from `warning` to `error` in 1 year.
+
+## [v3.3.5] - 2023-08-28
+### Changed
+- Update composer dependencies to current versions, notably PHPCompatibility (0a17f9ed).
+- Enforce the use of `&&` and `||` logical operators, **now erroring** (after a grace period of 1 year) with `and` and `or` uses: `Squiz.Operators.ValidLogicalOperators`
+
 ## [v3.3.4] - 2023-05-28
 ### Changed
 - Update composer dependencies to current versions, notably PHPCompatibility (70e4ca24).
@@ -51,7 +61,9 @@ All features are maintained and no new features have been introduced to either t
 
 All the details about [previous releases] can be found in [local_codechecker](https://github.com/moodlehq/moodle-local_codechecker) own change log.
 
-[Unreleased]: https://github.com/moodlehq/moodle-cs/compare/v3.3.4...main
+[Unreleased]: https://github.com/moodlehq/moodle-cs/compare/v3.3.6...main
+[v3.3.6]: https://github.com/moodlehq/moodle-cs/compare/v3.3.5...v3.3.6
+[v3.3.5]: https://github.com/moodlehq/moodle-cs/compare/v3.3.4...v3.3.5
 [v3.3.4]: https://github.com/moodlehq/moodle-cs/compare/v3.3.3...v3.3.4
 [v3.3.3]: https://github.com/moodlehq/moodle-cs/compare/v3.3.2...v3.3.3
 [v3.3.2]: https://github.com/moodlehq/moodle-cs/compare/v3.3.1...v3.3.2

README.md

@@ -16,81 +16,72 @@
 
 ## Information
 
-This repository contains the Moodle Coding Style configuration.
+This repository contains the Moodle Coding Style configurations, written as [PHP CodeSniffer](https://github.com/squizlabs/PHP_CodeSniffer) rulesets.
 
-Currently this only includes the configuration for PHP Coding style, but this
-may be extended to include custom rules for JavaScript, and any other supported
-languages or syntaxes.
+Two coding styles are included:
 
+- `moodle` - the main ruleset for the [Moodle Coding Style](https://moodledev.io/general/development/policies/codingstyle)
+- `moodle-extra` - extended ruleset which includes recommended best practices
+  - extends the main `moodle` ruleset
+
+Currently this only includes the configuration for PHP Coding style, but this may be extended to include custom rules for JavaScript, and any other supported languages or syntaxes.
 
 ## Installation
 
-### Using Composer
+### Using Composer (recommended)
 
-You can include these coding style rules using Composer to make them available
-globally across your system.
+You can install these coding style rules using Composer to make them available globally across your system.
 
-This will install the correct version of phpcs, with the Moodle rules, and their
-dependencies.
+This will install the correct version of phpcs, with the Moodle rules, and their dependencies.
 
-```
+```shell
 composer global require moodlehq/moodle-cs
 ```
 
-### As a part of moodle-local_codechecker
-
-This plugin is included as part of the [moodle-local_codechecker
-plugin](https://github.com/moodlehq/moodle-local_codechecker).
-
-
 ## Configuration
 
-You can set the Moodle standard as the system default:
-```
-phpcs --config-set default_standard moodle
-```
-
-This will inform most IDEs automatically.
-Alternatively you can configuration your IDE to use phpcs with the Moodle
-ruleset as required.
-
+Typically configuration is not required. Recent versions of Moodle (3.11 onwards) include a configuration file for the PHP CodeSniffer, which will set the standard when run within a Moodle directory.
 
-### IDE Integration
+Additional configuration can be generated automatically to have PHP CodeSniffer ignore any third-party library code. This can be generated by running:
 
-#### PhpStorm
-
-1. Open PhpStorm preferences
-2. Go to Inspections > PHP > PHP Code Sniffer Validation
-3. In the 'coding standard' dropdown, select 'moodle'
+```shell
+npx grunt ignorefiles
+```
 
-#### Sublime Text
+### Using the `moodle-extra` coding style
 
-Find documentation [here](https://docs.moodle.org/dev/Setting_up_Sublime2#Sublime_PHP_CS).
+The recommended way of configuring PHP CodeSniffer to use the `moodle-extra` coding style is to provide an additional configuration file.
 
-1. Go in your Sublime Text to Preferences -> Package Control -> Package Control: Install Package
-2. Write 'phpcs' in the search field, if you see Phpcs and SublimeLinter-phpcs, click on them to install them.
-3. If not, check if they are already installed Preferences -> Package Control -> Package Control: Remove Package.
-4. To set your codecheck to moodle standards go to Preferences -> Package Settings -> PHP Code Sniffer -> Settings-User and write:
+For Moodle 3.11 onwards you can create a file named `.phpcs.xml` with the following contents:
 
-        { "phpcs_additional_args": {
-                "--standard": "moodle",
-                "-n": "
-            },
-        }
+```xml
+<?xml version="1.0" encoding="UTF-8"?>
+<ruleset name="MoodleCore">
+  <rule ref="./phpcs.xml"/>
+  <rule ref="moodle-extra"/>
+</ruleset>
+```
 
-5. If you don’t have the auto-save plugin turned on, YOU’RE DONE!
-6. If you have the auto-save plugin turned on, because the codecheck gets triggered on save, the quick panel will keep popping making it impossible to type.
-   To stop quick panel from showing go to Settings-User file and add:
+This will load the `phpcs.xml` file (generated by `npx grunt ignorefiles`), and apply the `moodle-extra` configuration on top.
 
-        "phpcs_show_quick_panel": false,
+### Moodle 3.10 and earlier
 
-   The line with the error will still get marked and if you’ll click on it you’ll see the error text in the status bar.
+The easiset way to have PHP CodeSniffer pick up your preferred style, you can create a file named `phpcs.xml` with the following contents:
 
-#### VSCode
+```xml
+<?xml version="1.0" encoding="UTF-8"?>
+<ruleset name="MoodleCore">
+  <rule ref="moodle"/>
+</ruleset>
+```
 
-Find documentation [here](https://docs.moodle.org/dev/Setting_up_VSCode#PHP_CS).
+If you wish to use the `moodle-extra` coding style, then you can use the following content:
 
-1. Install [PHPSniffer](https://marketplace.visualstudio.com/items?itemName=wongjn.php-sniffer).
-2. Open VSCode settings.json and add the following setting to define standard PHP CS (if you haven't set it as default in your system):
+```xml
+<?xml version="1.0" encoding="UTF-8"?>
+<ruleset name="MoodleCore">
+  <rule ref="moodle-extra"/>
+</ruleset>
+```
 
-        "phpSniffer.standard": "moodle",
+Note: Third-party library code will not be ignored with these versions of Moodle.

composer.json

@@ -12,12 +12,17 @@
         {
             "name": "Andrew Lyons",
             "email": "andrew@nicols.co.uk"
+        },
+        {
+            "name": "Eloy Lafuente",
+            "email": "stronk7@moodle.com"
         }
     ],
     "require": {
         "dealerdirect/phpcodesniffer-composer-installer": "^1.0.0",
         "squizlabs/php_codesniffer": "^3.7.2",
-        "phpcompatibility/php-compatibility": "dev-develop#70e4ca24"
+        "phpcsstandards/phpcsextra": "^1.1.0",
+        "phpcompatibility/php-compatibility": "dev-develop#0a17f9ed"
     },
     "config": {
         "allow-plugins": {

moodle-extra/ruleset.xml

@@ -0,0 +1,99 @@
+<?xml version="1.0"?>
+<ruleset xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xsi:noNamespaceSchemaLocation="https://raw.githubusercontent.com/squizlabs/PHP_CodeSniffer/master/phpcs.xsd"
+    name="moodle-strict"
+    namespace="MoodleHQ\MoodleCS\MoodleExtra">
+
+    <description>Best Practices for Moodle development beyond the core Coding Standards</description>
+
+    <!-- Extend the standard Moodle coding style -->
+    <rule ref="moodle"/>
+
+    <!-- This is an error in moodle-extra. TODO: Remove as part of #58 -->
+    <rule ref="Generic.Arrays.DisallowLongArraySyntax">
+        <type>error</type>
+    </rule>
+
+    <!-- Include the PSR-12 ruleset with relevant Moodle exclusions -->
+    <rule ref="PSR12">
+
+        <!-- Moodle has a header manager and places its copyright on the first line after the opening tag -->
+        <exclude name="PSR12.Files.FileHeader.SpacingAfterBlock"/>
+
+        <!-- Moodle already defines its own line length, so remove this from the PSR-12 standard -->
+        <exclude name="Generic.Files.LineLength.TooLong"/>
+
+        <!-- Moodle has its own custom sniff for side effects  -->
+        <exclude name="PSR1.Files.SideEffects.FoundWithSymbols"/>
+
+        <!-- Moodle does not support camel case at all -->
+        <exclude name="PSR1.Methods.CamelCapsMethodName.NotCamelCaps"/>
+
+        <!-- Moodle contains a lot of code which pre-dates PHP 7.1 and did not support constant visibility -->
+        <exclude name="PSR12.Properties.ConstantVisibility.NotFound"/>
+
+        <!-- Moodle does not place the opening brace on a new line -->
+        <exclude name="PSR2.Classes.ClassDeclaration.OpenBraceNewLine"/>
+
+        <!-- Moodle allows use of else if-->
+        <exclude name="PSR2.ControlStructures.ElseIfDeclaration.NotAllowed"/>
+
+        <!-- Moodle casing rules do not allow camel case at all -->
+        <exclude name="Squiz.Classes.ValidClassName.NotCamelCaps"/>
+
+        <!-- Moodle casing currently places the brace in the same line -->
+        <exclude name="Squiz.Functions.MultiLineFunctionDeclaration.BraceOnSameLine"/>
+    </rule>
+
+    <!-- Detect duplicate array keys -->
+    <rule ref="Universal.Arrays.DuplicateArrayKey"/>
+
+    <!-- Disallow use of list() instead of [] -->
+    <rule ref="Universal.Lists.DisallowLongListSyntax"/>
+
+    <!-- Enusre that ::class is lower-cased -->
+    <rule ref="Universal.Constants.LowercaseClassResolutionKeyword"/>
+
+    <!-- Require a consistent modifier keyword order for OO constant declarations -->
+    <rule ref="Universal.Constants.ModifierKeywordOrder"/>
+
+    <!-- Enforce that the names used in a class/enum "implements" statement or an interface "extends" statement are listed in alphabetic order -->
+    <rule ref="Universal.OOStructures.AlphabeticExtendsImplements"/>
+
+    <!-- Enforce the use of a single space after the use, function, const keywords and both before and after the as keyword in import use statements -->
+    <rule ref="Universal.UseStatements.KeywordSpacing"/>
+
+    <!-- Enforce lowercase function/const -->
+    <rule ref="Universal.UseStatements.LowercaseFunctionConst"/>
+
+    <!-- Detect useless class imports (aliases) -->
+    <rule ref="Universal.UseStatements.NoUselessAliases"/>
+
+    <!-- Enfore comma, spacing, like, this -->
+    <rule ref="Universal.WhiteSpace.CommaSpacing"/>
+
+    <!--
+        TODO
+
+        PER-2.0 support (https://github.com/squizlabs/PHP_CodeSniffer/issues/3793), including:
+        - Trailing commas in function parameters https://github.com/squizlabs/PHP_CodeSniffer/issues/2030
+        - Multi-line implements/extends changes
+
+    -->
+
+    <!--
+        Detect issues with Unit Test dataProviders:
+        - private providers
+        - providers which do not exist
+        - providers whose name is prefixed with _test
+        - incorrect casing of dataProvider
+        - dataProviders which do not return an array or Iterable
+        - dataProviders which can be converted to a static method (PHPUnit 10 compatibility)
+    -->
+    <rule ref="moodle.PHPUnit.TestCaseProvider">
+        <properties>
+            <property name="autofixStaticProviders" value="true"/>
+        </properties>
+    </rule>
+
+</ruleset>

moodle/Sniffs/Namespaces/NamespaceStatementSniff.php

@@ -0,0 +1,71 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * Checks that each file contains the standard GPL comment.
+ *
+ * @package    moodle-cs
+ * @copyright  2023 Andrew Lyons <andrew@nicols.co.uk>
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+namespace MoodleHQ\MoodleCS\moodle\Sniffs\Namespaces;
+
+use PHP_CodeSniffer\Sniffs\Sniff;
+use PHP_CodeSniffer\Files\File;
+use PHP_CodeSniffer\Util\Tokens;
+
+// phpcs:disable moodle.NamingConventions
+
+class NamespaceStatementSniff implements Sniff {
+    public function register()
+    {
+        return [
+            T_NAMESPACE,
+        ];
+    }
+
+    public function process(File $file, $stackPtr)
+    {
+        $tokens = $file->getTokens();
+        // Format should be:
+        // - T_NAMESPACE
+        // - T_WHITESPACE
+        // - T_STRING
+
+        $checkPtr = $stackPtr + 2;
+        $token = $tokens[$checkPtr];
+        if ($token['code'] === T_NS_SEPARATOR) {
+            $fqdn = '';
+            $stop = $file->findNext(Tokens::$emptyTokens, ($stackPtr + 2));
+            for ($i = $stackPtr + 2; $i < $stop; $i++) {
+                $fqdn .= $tokens[$i]['content'];
+            }
+            $fix = $file->addFixableError(
+                'Namespace should not start with a slash: %s',
+                $checkPtr,
+                'LeadingSlash',
+                [$fqdn]
+            );
+
+            if ($fix) {
+                $file->fixer->beginChangeset();
+                $file->fixer->replaceToken($checkPtr, '');
+                $file->fixer->endChangeset();
+            }
+        }
+    }
+}

moodle/Sniffs/PHPUnit/TestCaseCoversSniff.php

@@ -49,11 +49,16 @@ public function process(File $file, $pointer) {
 
         // Before starting any check, let's look for various things.
 
-        // Get the moodle branch being analysed.
-        $moodleBranch = MoodleUtil::getMoodleBranch($file);
+        // If we aren't checking Moodle 4.0dev (400) and up, nothing to check.
+        // Make and exception for codechecker phpunit tests, so they are run always.
+        if (!MoodleUtil::meetsMinimumMoodleVersion($file, 400) && !MoodleUtil::isUnitTestRunning()) {
+            return; // @codeCoverageIgnore
+        }
 
-        // Detect if we are running PHPUnit.
-        $runningPHPUnit = defined('PHPUNIT_TEST') && PHPUNIT_TEST;
+        // If the file is not a unit test file, nothing to check.
+        if (!MoodleUtil::isUnitTest($file) && !MoodleUtil::isUnitTestRunning()) {
+            return; // @codeCoverageIgnore
+        }
 
         // We have all we need from core, let's start processing the file.
 
@@ -70,24 +75,6 @@ public function process(File $file, $pointer) {
             return; // @codeCoverageIgnore
         }
 
-        // If we aren't checking Moodle 4.0dev (400) and up, nothing to check.
-        // Make and exception for codechecker phpunit tests, so they are run always.
-        if (isset($moodleBranch) && $moodleBranch < 400 && !$runningPHPUnit) {
-            return; // @codeCoverageIgnore
-        }
-
-        // If the file isn't under tests directory, nothing to check.
-        if (stripos($file->getFilename(), '/tests/') === false) {
-            return; // @codeCoverageIgnore
-        }
-
-        // If the file isn't called, _test.php, nothing to check.
-        // Make an exception for codechecker own phpunit fixtures here, allowing any name for them.
-        $fileName = basename($file->getFilename());
-        if (substr($fileName, -9) !== '_test.php' && !$runningPHPUnit) {
-            return; // @codeCoverageIgnore
-        }
-
         // Iterate over all the classes (hopefully only one, but that's not this sniff problem).
         $cStart = $pointer;
         while ($cStart = $file->findNext(T_CLASS, $cStart + 1)) {