Skip to content

Prepare for CodeSniffer 4 - #236

Open
ziegenberg wants to merge 12 commits into
moodlehq:mainfrom
ziegenberg:prepare-cs-4
Open

ziegenberg wants to merge 12 commits into
moodlehq:mainfrom
ziegenberg:prepare-cs-4

Conversation

@ziegenberg

@ziegenberg ziegenberg commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor

This adds support for PHP_CodeSniffer version 4:

  • Updated the moodle-extra and moodle rulesets to use the new PHPCS ruleset schema URL.
  • Removed support for JS scanning (removed upstream in PHPCS 4.0).
  • moodle.Commenting.InlineComment: detect tabs in comments using the token orig_content as PHPCS 4 expands tabs.
  • moodle.Commenting.ValidTags: flag the legacy @codingStandards* docblock tags as invalid, as that annotation syntax was removed in PHPCS 4.0.
  • moodle.Files.BoilerplateComment: account for the fact that the T_OPEN_TAG token no longer contains trailing whitespace.
  • moodle.Namespaces.NamespaceStatement: support the new PHPCS 4 name tokenization (T_NAME_FULLY_QUALIFIED).
  • moodle.NamingConventions.ValidFunctionName: skip anonymous class scopes as T_ANON_CLASS is now a scope token.
  • moodle.PHPUnit.TestCaseNames: retrieve namespace names in a cross-version compatible way (via PHPCSUtils).
  • moodle.WhiteSpace.WhiteSpaceInStrings: detect tabs in whitespace using the token orig_content as PHPCS 4 expands tabs.
  • Updated the test suite for PHPCS 4 (instance-based Config::setConfigData(), removed JS-specific test, tab/namespace handling).
  • Updated the GitHub Actions integration tests to expect the new PHPCS 4 exit codes.

@codecov

codecov Bot commented Sep 14, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.28%. Comparing base (ce5ef30) to head (f2d6120).

Additional details and impacted files
@@            Coverage Diff            @@
##               main     #236   +/-   ##
=========================================
  Coverage     98.27%   98.28%           
- Complexity      994      997    +3     
=========================================
  Files            42       42           
  Lines          2961     2967    +6     
=========================================
+ Hits           2910     2916    +6     
  Misses           51       51           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ziegenberg
ziegenberg force-pushed the prepare-cs-4 branch 3 times, most recently from 79962e5 to 42166c8 Compare September 19, 2026 07:36
PHP_CodeSniffer 3.x is no longer receiving updates save for
security and compatibility fixes for newer PHP versions. Also,
the 4.x branch brings with it the only ongoing tokenizer
and syntax support.

`phpcsextra` and `phpcodesniffer-composer-installer` are bumped to the
first releases which are compatible with PHPCS 4.

Signed-off-by: Daniel Ziegenberg <daniel@ziegenberg.at>
The `master` branch of the PHP_CodeSniffer repository has been renamed
to `3.x`, making the raw GitHub permalink to the ruleset XSD invalid.
Use the new dedicated permanent location for the schema instead.

Signed-off-by: Daniel Ziegenberg <daniel@ziegenberg.at>
`PHP_CodeSniffer\Config::setConfigData()` is no longer a static method
as of PHPCS 4.0, so the test framework and the MoodleUtilTest had to
stop calling it statically.

A small helper was added to the base test case which creates a Config
instance and calls the method on it, keeping the behaviour unchanged.

Signed-off-by: Daniel Ziegenberg <daniel@ziegenberg.at>
@ziegenberg
ziegenberg force-pushed the prepare-cs-4 branch 5 times, most recently from 346d273 to 0259d7f Compare September 19, 2026 08:17
@ziegenberg
ziegenberg marked this pull request as ready for review September 19, 2026 08:20
Copilot AI lite review requested due to automatic review settings September 19, 2026 08:20

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

PHP_CodeSniffer 4.0 has dropped support for the JS and CSS tokenizers
completely, along with all JS-only tokens such as `T_PROPERTY`,
`T_OBJECT` and `T_PROTOTYPE`, and the `File::tokenizerType` property.

The InlineComment sniff is not really a "mixed" sniff, so the JS-only
branches are removed rather than made conditional, and the `T_PROPERTY`
references in the Docblocks utility and the attribute test are dropped
as the JS member-property token no longer exists.

Signed-off-by: Daniel Ziegenberg <daniel@ziegenberg.at>
PHP_CodeSniffer 4.0 expands tabs within tokens to spaces according to
the configured tab-width. The tokenizer keeps the original content in
the `orig_content` index, so use it, when present, to detect a real tab
between comment markers and within whitespace-only tokens. Otherwise
tabs go unnoticed and both sniffs lose a check they were able to
perform in PHPCS 3.x.

Signed-off-by: Daniel Ziegenberg <daniel@ziegenberg.at>
In PHPCS 4.0 the opening tag no longer includes a trailing newline; the
whitespace now becomes a regular `T_WHITESPACE` token of its own.

The boilerplate check therefore can no longer rely on the comment
living at `openTag + 1` to know that it is "at the first line" and on
the fixer keeping the newline when inserting or moving a boilerplate.
Instead it compares line numbers, and the insert/move helpers account
for the now separate whitespace token.

Signed-off-by: Daniel Ziegenberg <daniel@ziegenberg.at>
Since PHP 8.0 (and consistently in PHPCS 4.0) identifier names are
tokenized as single name tokens (`T_NAME_QUALIFIED`,
`T_NAME_FULLY_QUALIFIED`, ...) rather than as a chain of `T_STRING` and
`T_NS_SEPARATOR` tokens.

The NamespaceStatement sniff now detects a leading backslash on the
fully-qualified name token, and the TestCaseNames sniff delegates
namespace name extraction to PHPCSUtils, which handles both old and
new tokenization.

Signed-off-by: Daniel Ziegenberg <daniel@ziegenberg.at>
As of PHPCS 4.0 `T_ANON_CLASS` is part of the OO scope tokens, so the
AbstractScopeSniff starts registering anonymous classes as a scope.
`File::getDeclarationName()` rejects anonymous constructs by design, so
guard against the scope being an anonymous class before asking for its
name. Method checks inside anonymous classes are unaffected; only the
(impossible) class-name mention in the error message is skipped.

Signed-off-by: Daniel Ziegenberg <daniel@ziegenberg.at>
Two things changed in PHPCS 4.0 which affect this test:

- JS support has been removed, so the `.js` fixture is scanned as PHP
  and produces "no PHP code found" instead of inline comment errors.
  The dedicated JS test is removed as there is nothing left to test.
- The `--sniffs` selection is now honoured strictly, so tests for
  `Generic.Files.EndFileNewline` and `Generic.Classes.OpeningBraceSameLine`
  need the `Generic` standard, as those sniffs are not part of the
  `moodle` standard.

Signed-off-by: Daniel Ziegenberg <daniel@ziegenberg.at>
The moodle standard already enforces the end-of-file newline via
PSR12.Files.EndFileNewline, which is a strict superset of the Generic
sniff (it also reports multiple trailing newlines and hardcodes the
unix EOL). Generic.Files.EndFileNewline is not part of the moodle
standard, so this test provided no coverage of a moodle rule and
required special-casing the "Generic" standard as host.

Drop the test and its fixture, discovered while validating the
MoodleStandardTest changes for PHPCS 4.

Signed-off-by: Daniel Ziegenberg <daniel@ziegenberg.at>
PHP_CodeSniffer 4.0 renumbered its exit codes so that `phpcbf` can
return `0` once all fixable issues have been fixed:

- fixable-only issues now return `1` (was `2`)
- a successful `phpcbf` run returns `0` (was `1`)

The CI integration test asserted the old codes, so update the expected
exit codes accordingly.

Signed-off-by: Daniel Ziegenberg <daniel@ziegenberg.at>
Signed-off-by: Daniel Ziegenberg <daniel@ziegenberg.at>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants