Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-76727

phpcs configuration does not allow for local overrides

XMLWordPrintable

    • MOODLE_311_STABLE, MOODLE_400_STABLE, MOODLE_401_STABLE
    • MOODLE_311_STABLE, MOODLE_400_STABLE, MOODLE_401_STABLE
    • MDL-76727-401
    • MDL-76727-master
    • Hide
      Setup
      1. Install `phpcs`, and the moodlehq coding standards, globally:

        composer global require moodlehq/moodle-cs
        

      2. Ensure that your global vendor binaries directory is in your $PATH as noted at https://getcomposer.org/doc/03-cli.md#global - this will depend on your installation, but usually it's a case of updating your .bashrc or .zshrc (or whatever shell you use) to add:

        export PATH=~/.composer/vendor/bin:$PATH
        

      Test
      1. Go to your moodle instance's root folder.
      2. Make sure you're using the correct nodejs version and that grunt-cli is installed. Assuming you have nvm:

        nvm install
        npm i
        

      3. Ensure you do not already have the phpcs.xml or .phpcs.xml files:

        rm phpcs.xml .phpcs.xml
        

      4. Remove any default standard for phpcs:

        phpcs --config-delete default_standard
        

      5. Run phpcs against:
        1. 4.0, 4.1, master: course/format/classes:
        2. 3.11: admin/antiviruses.php

          # 4.0+
          phpcs -sp course/format/classes
          # 3.11:
          phpcs -sp admin/antiviruses.php
          

        3. Confirm that there were some warnings about Squiz.Operators.ValidLogicalOperators.NotAllowed (which is not allowed)
          Note: There are also some other warnings
      6. Run phpcs against an external file, for example lib/zipstream:

        phpcs -sp lib/zipstream
        

        1. Confirm that loads of Moodle-specific errors/warnings are shown (e.g. header)
          (At this point we don't know how to ignore it)
      7. Generate the ignorefile:

        npx grunt ignorefiles
        

      8. Run phpcs against an external file again, for example lib/zipstream:

        phpcs -sp lib/zipstream
        

        1. Confirm that there was no output at all
      9. Run phpcs with increased verbosity to check that files are appropriately ignored:

        phpcs -spvv lib/zipstream
        

        1. Confirm that the file was loaded: (You will see something similar to the following)

          Processing ruleset /path/to/your/moodle/phpcs.xml
          

        2. Confirm that the Moodle ruleset was then used:
          (The following will be in the output)

           	Processing rule "moodle"
          

        3. Confirm that a number of global ignore patterns were added, including lib/zipstream:
          (The following will be in the output)

          	=> added global absolute ignore pattern: lib/zipstream/
          

      10. Remove the Moodle standard but ensure that phpcs is still installed

        composer global remove moodlehq/moodle-cs
        composer global require squizlabs/php_codesniffer
        

      11. Run phpcs on a file again

        phpcs index.php
        

        1. Confirm that it complained that no sniffs were available:

          ERROR: No sniffs were registered
           
          Run "phpcs --help" for usage information
          

      12. Install the moodlehq coding standards and my stricter standards, globally:

        composer global remove squizlabs/php_codesniffer
        composer global require moodlehq/moodle-cs andrewnicols/moodle-cs-strict
        

      13. Fix a standard Moodle file using the normal Moodle standard:

        phpcbf lib/externallib.php
        

      14. Have git stage it so you can detect the changes more easily

        git add lib/externallib.php
        

      15. Now create a new .phpcs.xml file with your new, stricter, configuration:

        <?xml version="1.0" encoding="UTF-8"?>
        <ruleset name="MoodleCore">
          <rule ref="./phpcs.xml"/>
          <rule ref="MoodleStrict"/>
        </ruleset>
        

      16. Add to your local git ignore:

        echo .phpcs.xml >> .git/info/exclude
        

      17. Run phpcbf against the file again:

        phpcbf -sp lib/externallib.php
        

      18. Compare the changes:

        git diff
        

        1. Confirm that the diff showed additional changes to the code
      Resetting after the test
      1. Remove the local configuration

        rm .phpcs.xml
        

      2. Reset git

        git reset
        git checkout .
        git clean -f
        

      Show
      Setup Install `phpcs`, and the moodlehq coding standards, globally: composer global require moodlehq/moodle-cs Ensure that your global vendor binaries directory is in your $PATH as noted at https://getcomposer.org/doc/03-cli.md#global - this will depend on your installation, but usually it's a case of updating your .bashrc or .zshrc (or whatever shell you use) to add: export PATH=~/.composer/vendor/bin:$PATH Test Go to your moodle instance's root folder. Make sure you're using the correct nodejs version and that grunt-cli is installed. Assuming you have nvm : nvm install npm i Ensure you do not already have the phpcs.xml or .phpcs.xml files: rm phpcs.xml .phpcs.xml Remove any default standard for phpcs: phpcs --config-delete default_standard Run phpcs against: 4.0, 4.1, master: course/format/classes : 3.11: admin/antiviruses.php # 4.0+ phpcs -sp course/format/classes # 3.11: phpcs -sp admin/antiviruses.php Confirm that there were some warnings about Squiz.Operators.ValidLogicalOperators.NotAllowed (which is not allowed) Note: There are also some other warnings Run phpcs against an external file, for example lib/zipstream : phpcs -sp lib/zipstream Confirm that loads of Moodle-specific errors/warnings are shown (e.g. header) (At this point we don't know how to ignore it) Generate the ignorefile: npx grunt ignorefiles Run phpcs against an external file again, for example lib/zipstream : phpcs -sp lib/zipstream Confirm that there was no output at all Run phpcs with increased verbosity to check that files are appropriately ignored: phpcs -spvv lib/zipstream Confirm that the file was loaded: (You will see something similar to the following) Processing ruleset /path/to/your/moodle/phpcs.xml Confirm that the Moodle ruleset was then used: (The following will be in the output) Processing rule "moodle" Confirm that a number of global ignore patterns were added, including lib/zipstream : (The following will be in the output) => added global absolute ignore pattern: lib/zipstream/ Remove the Moodle standard but ensure that phpcs is still installed composer global remove moodlehq/moodle-cs composer global require squizlabs/php_codesniffer Run phpcs on a file again phpcs index.php Confirm that it complained that no sniffs were available : ERROR: No sniffs were registered   Run "phpcs --help" for usage information Install the moodlehq coding standards and my stricter standards, globally: composer global remove squizlabs/php_codesniffer composer global require moodlehq/moodle-cs andrewnicols/moodle-cs-strict Fix a standard Moodle file using the normal Moodle standard: phpcbf lib/externallib.php Have git stage it so you can detect the changes more easily git add lib/externallib.php Now create a new .phpcs.xml file with your new, stricter, configuration: <?xml version="1.0" encoding="UTF-8"?> <ruleset name="MoodleCore"> <rule ref="./phpcs.xml"/> <rule ref="MoodleStrict"/> </ruleset> Add to your local git ignore: echo .phpcs.xml >> .git/info/exclude Run phpcbf against the file again: phpcbf -sp lib/externallib.php Compare the changes: git diff Confirm that the diff showed additional changes to the code Resetting after the test Remove the local configuration rm .phpcs.xml Reset git git reset git checkout . git clean -f

      I introduced per-repository phpcs configuration in MDL-74511

      The order of loading of files is:

      • .phpcs.xml
      • phpcs.xml
      • .phpcs.xml.dist
      • phpcs.xml.dist

      At the moment we have two files:

      • .phpcs.xml.dist
      • .phpcs.xml

      The .phpcs.xml file generated by the grunt ignorefiles command is the most dominant of the possible configuration options and, as such, cannot be overridden by local configuration.

      I woudl propose that we reduce the priority of these files so that it is possible for local rules to be applied (or alternative and complementary rulesets to be applied).

      I suggest that we move:

      • .phpcs.xml.dist to phpcs.xml.dist (the base ruleset definition)
      • .phpcs.xml to phpcs.xml (the one generated by the ignorefiles)

      This will allow people to use stricter standards, for example I have been writing a MoodleStrict standard for use in my own code which applies more opinionated preferences.

      By making the proposed change, I can then create my own .phpcs.xml file with contents:

      <?xml version="1.0" encoding="UTF-8"?>
      <ruleset name="MoodleCore">
        <rule ref="./phpcs.xml"/>
        <rule ref="MoodleStrict"/>
      </ruleset>
      

      This configuration loads the configuration generated by grunt ignorefiles and further applies my own, stricter, configuration on top of it. That means that my end configuration is:

      • moodle-cs
      • ignored files
      • MoodleStrict

        1. 2022-12-21 18_28_02-Window.png
          2022-12-21 18_28_02-Window.png
          71 kB
        2. 311.png
          311.png
          1.00 MB
        3. 400.png
          400.png
          1.53 MB
        4. 401.png
          401.png
          1.49 MB
        5. Master.png
          Master.png
          1.65 MB

            dobedobedoh Andrew Lyons
            dobedobedoh Andrew Lyons
            Eloy Lafuente (stronk7) Eloy Lafuente (stronk7)
            Ilya Tregubov Ilya Tregubov
            Ron Carl Alfon Yu Ron Carl Alfon Yu
            Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

              Created:
              Updated:
              Resolved:

                Estimated:
                Original Estimate - Not Specified
                Not Specified
                Remaining:
                Remaining Estimate - 0 minutes
                0m
                Logged:
                Time Spent - 1 week, 3 days, 3 hours, 1 minute
                1w 3d 3h 1m

                  Error rendering 'clockify-timesheets-time-tracking-reports:timer-sidebar'. Please contact your Jira administrators.