From e75835268e960c34181986d29792340237442f54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20=C5=A0korupa?= Date: Thu, 17 Sep 2026 16:04:23 +0200 Subject: [PATCH 1/3] feat: update --- .github/dependabot.yml | 2 + .github/workflows/grumphp.yaml | 43 ++ .php-cs-fixer.dist.php | 4 +- AGENTS.md | 8 + CHANGELOG.md | 121 +++++ README.md | 316 ++++------- Taskfile.yaml | 12 +- bin/grumphp_hooks/environment_spinup | 13 + composer.json | 27 +- config/grumphp.yaml | 8 - docker-compose.yml | 14 +- grumphp.yml | 35 +- phpcs.ruleset.83.non-ddd.xml | 19 - phpcs.ruleset.83.xml | 513 ------------------ phpcs.ruleset.84.xml | 513 +++++++++++++++++- phpcs.ruleset.85.non-ddd.xml | 10 +- phpcs.ruleset.85.xml | 2 +- phpcs.xml.dist | 4 +- phpmd.dist.xml | 21 - phpunit.xml | 9 +- src/GrumPHP/Linter/Xml/XmlLinter.php | 97 ++-- src/GrumPHP/Task/ComposerInstallCheckTask.php | 2 +- src/GrumPHP/Task/PhpMdExtendedTask.php | 126 ----- .../Classes/RequireAbstractOrFinalSniff.php | 23 + tests/Example/Bar.php | 6 +- tests/Example/Foo.php | 4 +- .../ForbiddenAnnotations/Annotations.php | 14 + tests/Functional/ForbiddenAnnotationsTest.php | 29 + tests/Functional/PhpcsTestCase.php | 110 ++-- .../RequireAbstractOrFinal/Classes.php | 23 + .../Functional/RequireAbstractOrFinalTest.php | 58 ++ .../Bar.SuperfluousWhitespace.after.php | 8 +- .../Bar.SuperfluousWhitespace.before.php | 8 +- .../Functional/SuperfluousWhitespaceTest.php | 3 - tests/Functional/XmlLinter/external-dtd.xml | 3 + tests/Functional/XmlLinter/external.dtd | 1 + tests/Functional/XmlLinter/schema/child.xsd | 7 + tests/Functional/XmlLinter/schema/root.xsd | 15 + .../XmlLinter/valid-multiple-namespaces.xml | 7 + tests/Functional/XmlLinterTest.php | 50 ++ 40 files changed, 1221 insertions(+), 1067 deletions(-) create mode 100644 .github/workflows/grumphp.yaml create mode 100644 AGENTS.md create mode 100644 CHANGELOG.md create mode 100755 bin/grumphp_hooks/environment_spinup delete mode 100644 phpcs.ruleset.83.non-ddd.xml delete mode 100644 phpcs.ruleset.83.xml delete mode 100644 phpmd.dist.xml delete mode 100644 src/GrumPHP/Task/PhpMdExtendedTask.php create mode 100644 src/PixelFederationCodingStandard/Sniffs/Classes/RequireAbstractOrFinalSniff.php create mode 100644 tests/Functional/ForbiddenAnnotations/Annotations.php create mode 100644 tests/Functional/ForbiddenAnnotationsTest.php create mode 100644 tests/Functional/RequireAbstractOrFinal/Classes.php create mode 100644 tests/Functional/RequireAbstractOrFinalTest.php create mode 100644 tests/Functional/XmlLinter/external-dtd.xml create mode 100644 tests/Functional/XmlLinter/external.dtd create mode 100644 tests/Functional/XmlLinter/schema/child.xsd create mode 100644 tests/Functional/XmlLinter/schema/root.xsd create mode 100644 tests/Functional/XmlLinter/valid-multiple-namespaces.xml create mode 100644 tests/Functional/XmlLinterTest.php diff --git a/.github/dependabot.yml b/.github/dependabot.yml index a839255..1949fb4 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -7,5 +7,7 @@ version: 2 updates: - package-ecosystem: "composer" directory: "/" + cooldown: + default-days: 7 schedule: interval: "daily" diff --git a/.github/workflows/grumphp.yaml b/.github/workflows/grumphp.yaml new file mode 100644 index 0000000..c27b742 --- /dev/null +++ b/.github/workflows/grumphp.yaml @@ -0,0 +1,43 @@ +name: Grumphp + +on: + push: + branches: + - master + pull_request: + branches: + - master + +jobs: + production-build: + runs-on: ubuntu-latest + timeout-minutes: 15 + strategy: + matrix: + php-version: [ '8.4', '8.5' ] + + steps: + - name: Checkout current repo + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + - name: Setup .env file + run: echo "php-container=coding-standards-php${{ matrix.php-version }}" >> $GITHUB_ENV + + - name: Start containers + run: docker compose -f docker-compose.yml up -d ${{ env.php-container }} + + - name: Check PHP Version + run: docker exec ${{ env.php-container }} php -v + + - name: Validate composer.json + run: docker exec ${{ env.php-container }} composer validate --strict --no-check-lock + + - name: Composer update + run: docker exec ${{ env.php-container }} composer update --prefer-dist --no-interaction + + - name: Run static analysis (GrumPHP) + run: docker exec ${{ env.php-container }} vendor/bin/grumphp run --testsuite=php${{ matrix.php-version }} + + - name: Stop containers + if: always() + run: docker compose -f docker-compose.yml down -v diff --git a/.php-cs-fixer.dist.php b/.php-cs-fixer.dist.php index 3d17e50..3a96fd4 100644 --- a/.php-cs-fixer.dist.php +++ b/.php-cs-fixer.dist.php @@ -5,11 +5,11 @@ use PhpCsFixer\Config; use PhpCsFixer\Finder; -$finder = (new Finder()) +$finder = new Finder() ->in(__DIR__) ->exclude('var'); -return (new Config()) +return new Config() ->setRiskyAllowed(true) ->setRules([ 'array_syntax' => [ diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..a51c314 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,8 @@ +# Project instructions + +Run project commands, including tests, static analysis, code-style checks, and Composer commands, inside the project's Docker containers. + +- Use the `coding-standards-php-min` container by default. +- When a command verifies behavior specific to a PHP version, use the corresponding container, such as `coding-standards-php8.4` or `coding-standards-php8.5`. +- Start the required service with Docker Compose if it is not already running. +- Do not use the host PHP or Composer installation for project validation. diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..79a7ecd --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,121 @@ +# Changelog + +All notable changes from version 4.0.0 onward are documented in this file. + +## 6.0.0 + +### Added + +- Added PHP 8.4 and PHP 8.5 GrumPHP test suites and a GitHub Actions workflow that tests against the latest allowed + dependency versions. +- Added the `ClassKeywordOrder`, `ReadonlyClass`, `TraitUseOrder`, `CatchExceptionsOrder`, and + `ThrowsAnnotationsOrder` Slevomat rules. +- Added a project `RequireAbstractOrFinal` sniff that accepts native `final`, `abstract`, and the `@final` + annotation. +- Added functional coverage for forbidden annotations, abstract or final classes, whitespace fixes, and XML schema + validation. + +### Changed + +- Raised the minimum PHP version to 8.4. +- Updated PHP_CodeSniffer to 4.0.4, Slevomat Coding Standard to 8.31.1, and the development dependencies to their + current supported versions. +- Made PHP 8.4 the base ruleset and made PHP 8.5 extend it. +- Limited forbidden annotations to `@author`, `@copyright`, and `@license`; `@throws` remains allowed. +- Improved the XML linter to validate multi-namespace documents through their root schema and to restore libxml + error handling after exceptions. +- Prevented functional PHPCS tests from modifying source fixtures. + +### Removed + +- Removed PHP 8.3 rulesets and runtime support. +- Removed the PHP Mess Detector dependency and the `phpmd_extended` GrumPHP task. + +## 5.5.1 - 2026-02-03 + +### Fixed + +- Fixed XML linting and task option handling. +- Improved compatibility of custom GrumPHP tasks with current GrumPHP APIs. + +## 5.5.0 - 2026-01-07 + +### Added + +- Added the `phpstan_extended` task with chunked PHPStan execution. +- Added the `xmllint_extended` task with strict schema validation. + +## 5.4.0 - 2025-12-15 + +### Added + +- Added PHP 8.5 rulesets and a PHP 8.5 development container. + +## 5.3.1 - 2025-12-11 + +### Added + +- Added explicit end-of-file newline enforcement. +- Extended functional test helpers for PHPCS fix verification. + +## 5.3.0 - 2025-12-09 + +### Added + +- Added stricter whitespace rules. +- Added functional tests for PHPCS fixes. + +## 5.2.0 - 2025-11-14 + +### Added + +- Added the chunked `phpmd_extended` GrumPHP task. + +## 5.1.1 - 2025-11-14 + +### Added + +- Added enforcement for whitespace on otherwise empty lines. + +## 5.1.0 - 2025-11-13 + +### Added + +- Added GrumPHP integration and custom tasks for Composer installation checks and Doctrine schema validation. +- Added project configuration for PHP-CS-Fixer, PHPCS, and PHPStan. + +## 5.0.0 - 2025-10-22 + +### Changed + +- Reworked the version-specific ruleset hierarchy. +- Updated the development environment and project tooling. + +### Removed + +- Removed obsolete PHP 8.1 and PHP 8.2 rulesets. +- Removed sniffs deprecated by newer PHP_CodeSniffer and Slevomat Coding Standard releases. + +## 4.1.1 - 2025-02-25 + +### Fixed + +- Fixed the installed path configuration for both local development and Composer-installed usage. + +## 4.1.0 - 2025-02-24 + +### Added + +- Added PHP 8.4 support and version-specific PHP 8.4 rulesets. +- Added the Pixel Federation custom sniff standard and the switch-statement restriction. + +### Changed + +- Updated package dependencies and reorganized example files. +- Replaced the generic ruleset with PHP-version-specific rulesets. + +## 4.0.0 - 2024-11-26 + +### Added + +- Added PHP 8.3 support. diff --git a/README.md b/README.md index ea6a4ce..e08d85a 100644 --- a/README.md +++ b/README.md @@ -1,271 +1,175 @@ # Coding standards -This package provides PHPCS rule set for coding standards in Pixel Federation. It should be included into -each project maintained by Pixel Federation that uses PHP Code Sniffer [PHPCS](https://github.com/squizlabs/PHP_CodeSniffer). +This package provides the shared PHP_CodeSniffer rules and GrumPHP tasks used by Pixel Federation PHP projects. -## Migration from v4 to v4.1.0 -Generic ruleset was removed. Now there are only rulesets for specific PHP versions. +## Requirements -Replace (in your ruleset) reference to file -```vendor/pixelfederation/coding-standards/phpcs.ruleset.xml``` -with -```vendor/pixelfederation/coding-standards/phpcs.ruleset.84.xml``` -for PHP 8.4. +- PHP 8.4 or 8.5 +- PHP_CodeSniffer 4 +- Slevomat Coding Standard 8 -## How to use - -### Install composer dependencies +## Installation ```bash -composer require --dev pixelfederation/coding-standards:^5.0 +composer require --dev pixelfederation/coding-standards ``` -### Supported versions - -For each php version there are 2 versions of the ruleset. One for DDD projects and one for Non-DDD projects. +## PHP_CodeSniffer -For example for PHP 8.4: -``` -vendor/pixelfederation/coding-standards/phpcs.ruleset.84.xml -OR -vendor/pixelfederation/coding-standards/phpcs.ruleset.84.non-ddd.xml -``` +The package contains rulesets for PHP 8.4 and PHP 8.5. Each PHP version has a standard ruleset and a less +restrictive ruleset for projects that do not use DDD: -### Ruleset creation +| PHP | Standard ruleset | Non-DDD ruleset | +| --- | --- | --- | +| 8.4 | `phpcs.ruleset.84.xml` | `phpcs.ruleset.84.non-ddd.xml` | +| 8.5 | `phpcs.ruleset.85.xml` | `phpcs.ruleset.85.non-ddd.xml` | -Create a file named `phpcs.ruleset.xml` in the root folder of your project with the following content: +Create `phpcs.ruleset.xml` in the project root: ```xml - + - PixelFederation rule set. - + Project coding standard. + tests/ - - - + + + ``` -### Running checks - -In your project directory run this command: +Run the checks with: ```bash vendor/bin/phpcs --standard=phpcs.ruleset.xml src ``` -### Automatically fixing errors - -In your project directory run this command: +Automatically fix supported violations with: ```bash vendor/bin/phpcbf --standard=phpcs.ruleset.xml src ``` +The complete Slevomat sniff documentation is available in the +[Slevomat Coding Standard repository](https://github.com/slevomat/coding-standard). -## Additional links +## GrumPHP -Sniffs documentation for slevomat coding standards are here: -https://github.com/slevomat/coding-standard +Custom tasks require the Composer installation of GrumPHP. They do not work with `phpro/grumphp-shim` when +parallel execution is enabled because PHAR task classes cannot be serialized. -# GrumPHP +Register the extension in the project's `grumphp.yml`: -❗**Note:** Custom tasks don’t work when using `phpro/grumphp-shim` (PHAR) with `parallel.enabled: true` due to class serialization limitations. If you need custom tasks, install GrumPHP via Composer. - -## Tasks - -### Installation - -````YAML -# grumphp.yml +```yaml grumphp: - extensions: - - PixelFederation\CodingStandards\GrumPHP\ExtensionLoader -```` + extensions: + - PixelFederation\CodingStandards\GrumPHP\ExtensionLoader +``` -### Doctrine ORM Mapping Validation +### Doctrine ORM mapping validation -````YAML -# grumphp.yml -grumphp: - tasks: - doctrine_schema_validate: - skip_mapping: false - skip_sync: false - skip_property_types: false - em: default - triggered_by: ['php', 'xml', 'yml'] -```` - -For multiple entity managers you can specify the entity manager to be used: -````YAML -# grumphp.yml +```yaml grumphp: - tasks: - doctrine_schema_validate_application: - em: application - metadata: - task: doctrine_schema_validate - doctrine_schema_validate_reporting: - em: reporting - metadata: - task: doctrine_schema_validate -```` - -**console_path** - -*Default: 'bin/console'* - -With this parameter you can set the path of the console to be used. - -**skip_mapping** - -*Default: false* - -With this parameter you can skip the mapping validation check. - -**skip_sync** - -*Default: false* - -With this parameter you can skip checking if the mapping is in sync with the database. - -**triggered_by** - -*Default: [php, xml, yml]* - -This is a list of extensions that should trigger the Doctrine task. - -**em** - -*Default: null* - -Require `doctrine/orm >= 3.0`. -Specify the entity manager to be used. If not set, the default entity manager will be used. - -**skip_property_types** - -*Default: null* - -Require `doctrine/orm >= 3.0`. -With this parameter you can skip checking if property types match the Doctrine types. + tasks: + doctrine_schema_validate: + console_path: bin/console + em: default + skip_mapping: false + skip_property_types: false + skip_sync: false + triggered_by: [php, xml, yml] +``` -### Composer Install Check +For multiple entity managers, configure multiple tasks using the shared task implementation: -````YAML -# grumphp.yml +```yaml grumphp: - tasks: - composer_install_check: - script: './vendor/pixelfederation/coding-standards/bin/composer_install_check.sh', - ignore_patterns: [] - triggered_by: ['php', 'yml', 'yaml', 'xml'] - whitelist_patterns: [] - metadata: - priority: 900 -```` - -**script** - -*Default: './bin/composer_install_check.sh'* - -Path to check script. - -**ignore_patterns** - -*Default: []* - -This is a list of patterns that will be ignored by phpcs. With this option you can skip files like tests. Leave this option blank to run phpcs for every php file. - -**triggered_by** - -*Default: ['php', 'yml', 'yaml', 'xml']* - -This is a list of extensions to be sniffed. - -**whitelist_patterns** - -*Default: []* - -This is a list of regex patterns that will filter files to validate. With this option you can skip files like tests. This option is used in relation with the parameter `triggered_by`. + tasks: + doctrine_schema_validate_application: + em: application + metadata: + task: doctrine_schema_validate + doctrine_schema_validate_reporting: + em: reporting + metadata: + task: doctrine_schema_validate +``` -### PhpMd Extended +Options: -Extends the default [PhpMd task](vendor/phpro/grumphp/doc/tasks/phpmd.md) and splits the files into smaller chunks to prevent the `Argument list too long` error. +| Option | Default | Description | +| --- | --- | --- | +| `console_path` | `bin/console` | Path to the Symfony console. | +| `em` | `null` | Entity manager name. Requires Doctrine ORM 3 or newer. | +| `skip_mapping` | `false` | Skip mapping validation. | +| `skip_property_types` | `null` | Skip the Doctrine property type check. Requires Doctrine ORM 3 or newer. | +| `skip_sync` | `false` | Skip the database synchronization check. | +| `triggered_by` | `[php, xml, yml]` | File extensions that trigger the task. | -***Config*** +### Composer install check -The task lives under the `phpmd_extended` namespace and has following configurable parameters: +This task checks whether changes to Composer files require dependencies to be installed again. ```yaml -# grumphp.yml grumphp: - tasks: - phpmd_extended: - whitelist_patterns: [] - exclude: [] - report_format: text - ruleset: ['cleancode', 'codesize', 'naming'] - triggered_by: ['php'] - chunks_size: 1000 + tasks: + composer_install_check: + script: ./vendor/pixelfederation/coding-standards/bin/composer_install_check.sh + ignore_patterns: [] + triggered_by: [json, lock, php, xml, yaml, yml] + whitelist_patterns: [] + metadata: + priority: 900 ``` -**chunk_size** - -*Default: 1000* +Options: -This parameter defines how many files will be checked in one execution of phpmd. This can help with performance on large codebases. +| Option | Default | Description | +| --- | --- | --- | +| `script` | `./bin/composer_install_check.sh` | Path to the check script. | +| `ignore_patterns` | `[]` | Patterns excluded from the check. | +| `triggered_by` | `[json, lock, php, xml, yaml, yml]` | File extensions that trigger the task. | +| `whitelist_patterns` | `[]` | Patterns limiting which changed files are checked. | ### PHPStan Extended -Extends the default [PHPStan task](vendor/phpro/grumphp/doc/tasks/phpstan.md) and splits the files into smaller chunks to prevent the `Argument list too long` error. - -***Config*** - -The task lives under the `phpstan_extended` namespace and has following configurable parameters: +This task extends the standard PHPStan task and splits files into smaller chunks to avoid operating-system command +length limits. ```yaml -# grumphp.yml grumphp: - tasks: - phpstan_extended: - autoload_file: ~ - chunk_size: 1000 - configuration: ~ - level: null - force_patterns: [] - ignore_patterns: [] - triggered_by: ['php'] - memory_limit: "-1" - use_grumphp_paths: true + tasks: + phpstan_extended: + autoload_file: ~ + chunk_size: 1000 + configuration: phpstan.neon + force_patterns: [] + ignore_patterns: [] + level: max + memory_limit: "-1" + triggered_by: [php] + use_grumphp_paths: true ``` -**chunk_size** - -*Default: 1000* - -This parameter defines how many files will be checked in one execution of phpstan. This can help with performance on large codebases. +`chunk_size` determines the maximum number of files processed by one PHPStan execution. -### XmlLint Extended +### XMLLint Extended -Extends the default [XmlLint task](vendor/phpro/grumphp/doc/tasks/xmllint.md) with strict schema validation. -Require `ext-dom` and `ext-libxml` extensions. - -***Config*** - -It lives under the `xmllint_extended` namespace and has following configurable parameters: +This task extends the standard XMLLint task with DTD, XInclude, and XML Schema validation. It requires the `dom` +and `libxml` PHP extensions. ```yaml -# grumphp.yml grumphp: - tasks: - xmllint_extended: - ignore_patterns: [] - load_from_net: false - x_include: false - dtd_validation: false - scheme_validation: false - triggered_by: ['xml'] + tasks: + xmllint_extended: + dtd_validation: false + ignore_patterns: [] + load_from_net: false + scheme_validation: false + triggered_by: [xml] + x_include: false ``` + +When schema validation is enabled, the linter supports `xsi:noNamespaceSchemaLocation` and multi-namespace +`xsi:schemaLocation` documents. Imported namespaces are resolved by the root document schema. diff --git a/Taskfile.yaml b/Taskfile.yaml index 911b261..20dcc9a 100644 --- a/Taskfile.yaml +++ b/Taskfile.yaml @@ -53,20 +53,20 @@ tasks: - ds # PHP containers - container:php83: + container:php84: cmds: - task: docker:exec vars: - CONTAINER: coding-standards-php8.3 + CONTAINER: coding-standards-php8.4 COMMAND: '{{default "/bin/bash" .CLI_ARGS}}' aliases: - - php83 + - php84 - container:php84: + container:php85: cmds: - task: docker:exec vars: - CONTAINER: coding-standards-php8.4 + CONTAINER: coding-standards-php8.5 COMMAND: '{{default "/bin/bash" .CLI_ARGS}}' aliases: - - php84 + - php85 diff --git a/bin/grumphp_hooks/environment_spinup b/bin/grumphp_hooks/environment_spinup new file mode 100755 index 0000000..60ef907 --- /dev/null +++ b/bin/grumphp_hooks/environment_spinup @@ -0,0 +1,13 @@ +#!/bin/bash + +export PATH="/usr/local/bin:$PATH" + +# +# Run the hook command. +# Note: this will be replaced by the real command during copy. +# +DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" + +if [[ -z `docker compose ps -q coding-standards-php-min` ]] || [[ -z `docker ps -q --no-trunc | grep $(docker compose ps -q coding-standards-php-min)` ]]; then + docker compose up -d +fi diff --git a/composer.json b/composer.json index 9df3762..4e26d16 100644 --- a/composer.json +++ b/composer.json @@ -11,21 +11,20 @@ ], "homepage": "https://github.com/pixelfederation/coding-standards", "require": { - "php": "^8.3", - "slevomat/coding-standard": "^8.24", - "squizlabs/php_codesniffer": "^4.0" + "php": "^8.4", + "slevomat/coding-standard": "^8.31.1", + "squizlabs/php_codesniffer": "^4.0.4" }, "require-dev": { "ext-dom": "*", "ext-libxml": "*", - "ergebnis/composer-normalize": "^2.45", - "friendsofphp/php-cs-fixer": "^3.89", - "nikic/php-parser": "^5.6", + "ergebnis/composer-normalize": "^2.53", + "friendsofphp/php-cs-fixer": "^3.95.25", + "nikic/php-parser": "^5.9", "php-parallel-lint/php-parallel-lint": "^1.4", - "phpmd/phpmd": "^2.15", - "phpro/grumphp": "^2.19", - "phpstan/phpstan": "^2.1", - "phpunit/phpunit": "^12.5" + "phpro/grumphp": "^2.23", + "phpstan/phpstan": "^2.2.14", + "phpunit/phpunit": "^13.3.4" }, "autoload": { "psr-4": { @@ -34,9 +33,7 @@ }, "autoload-dev": { "psr-4": { - "PixelFederation\\CodingStandards\\Tests\\": [ - "tests/" - ] + "PixelFederation\\CodingStandards\\Tests\\": "tests/" } }, "config": { @@ -57,10 +54,10 @@ "auto-scripts": { "normalizer": "composer normalize" }, - "phpcbf8.3": "phpcbf --standard=phpcs.ruleset.83.xml tests/Example", "phpcbf8.4": "phpcbf --standard=phpcs.ruleset.84.xml tests/Example", - "phpcs8.3": "phpcs --standard=phpcs.ruleset.83.xml tests/Example", + "phpcbf8.5": "phpcbf --standard=phpcs.ruleset.85.xml tests/Example", "phpcs8.4": "phpcs --standard=phpcs.ruleset.84.xml tests/Example", + "phpcs8.5": "phpcs --standard=phpcs.ruleset.85.xml tests/Example", "phpunit": "vendor/bin/phpunit tests/Functional --configuration=phpunit.xml --colors=always" } } diff --git a/config/grumphp.yaml b/config/grumphp.yaml index 48865d6..4df4ce6 100644 --- a/config/grumphp.yaml +++ b/config/grumphp.yaml @@ -15,14 +15,6 @@ services: tags: - { name: grumphp.task, task: composer_install_check } - PixelFederation\CodingStandards\GrumPHP\Task\PhpMdExtendedTask: - class: PixelFederation\CodingStandards\GrumPHP\Task\PhpMdExtendedTask - arguments: - - "@process_builder" - - "@formatter.raw_process" - tags: - - { name: grumphp.task, task: phpmd_extended } - PixelFederation\CodingStandards\GrumPHP\Task\PhpStanExtendedTask: class: PixelFederation\CodingStandards\GrumPHP\Task\PhpStanExtendedTask arguments: diff --git a/docker-compose.yml b/docker-compose.yml index fe9d7d4..7a19669 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,4 +1,5 @@ x-php-common: &php_common + pull_policy: always volumes: - .gitconfig:/root/.gitconfig - .:/srv/www:delegated @@ -10,14 +11,6 @@ x-php-build: &php_build dockerfile: ./docker/php/Dockerfile services: - coding-standards-php8.3: - <<: *php_common - container_name: coding-standards-php8.3 - build: - <<: *php_build - args: - PHP_VERSION: 8.3 - coding-standards-php8.4: <<: *php_common container_name: coding-standards-php8.4 @@ -33,3 +26,8 @@ services: <<: *php_build args: PHP_VERSION: 8.5 + + coding-standards-php-min: + extends: + service: coding-standards-php8.4 + container_name: coding-standards-php-min diff --git a/grumphp.yml b/grumphp.yml index 34f0054..fdad78e 100644 --- a/grumphp.yml +++ b/grumphp.yml @@ -2,6 +2,8 @@ grumphp: ascii: failed: assets/grumphp/pixel-bad.txt succeeded: assets/grumphp/pixel-good.txt + git_hook_variables: + EXEC_GRUMPHP_COMMAND: './bin/grumphp_hooks/environment_spinup && docker exec -i coding-standards-php-min' fixer: enabled: false parallel: @@ -9,17 +11,31 @@ grumphp: max_workers: 16 process_timeout: 1800 stop_on_failure: false + testsuites: + php8.4: + tasks: &tasks + - composer + - composer_install_check + - composer_normalize + - jsonlint + - phpcs + - phpcsfixer2 + - phplint + - phpparser + - phpstan_extended + - phpunit + - xmllint_extended + - yamllint + php8.5: + tasks: *tasks tasks: composer: ~ - composer_normalize: ~ composer_install_check: script: './bin/composer_install_check.sh' metadata: priority: 900 + composer_normalize: ~ jsonlint: ~ - phpunit: - always_execute: true - config_file: phpunit.xml phpcs: standard: 'phpcs.xml.dist' tab_width: 4 @@ -41,11 +57,9 @@ grumphp: diff: true triggered_by: [ 'php' ] phplint: ~ - phpmd_extended: - ruleset: [ 'phpmd.dist.xml' ] phpparser: ignore_patterns: [ ] - php_version: '8.3' + php_version: '8.4' visitors: declare_strict_types: ~ no_exit_statements: ~ @@ -68,10 +82,11 @@ grumphp: configuration: 'phpstan.dist.neon' level: max memory_limit: "-1" - ignore_patterns: [ - 'tests/', - ] + ignore_patterns: [ ] triggered_by: [ 'php' ] + phpunit: + always_execute: true + config_file: phpunit.xml xmllint_extended: ignore_patterns: [ ] load_from_net: true diff --git a/phpcs.ruleset.83.non-ddd.xml b/phpcs.ruleset.83.non-ddd.xml deleted file mode 100644 index cf4fe56..0000000 --- a/phpcs.ruleset.83.non-ddd.xml +++ /dev/null @@ -1,19 +0,0 @@ - - - PixelFederation ruleset for PHP 8.3 (Non-DDD). - - - - - - - - - - - - - - diff --git a/phpcs.ruleset.83.xml b/phpcs.ruleset.83.xml deleted file mode 100644 index 388cad6..0000000 --- a/phpcs.ruleset.83.xml +++ /dev/null @@ -1,513 +0,0 @@ - - - - - PixelFederation ruleset for PHP 8.3. - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - 5 - - - 5 - - - 5 - - - 5 - - - 5 - - - 5 - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/phpcs.ruleset.84.xml b/phpcs.ruleset.84.xml index f581104..d93dae7 100644 --- a/phpcs.ruleset.84.xml +++ b/phpcs.ruleset.84.xml @@ -1,11 +1,518 @@ - + + xsi:noNamespaceSchemaLocation="vendor/squizlabs/php_codesniffer/phpcs.xsd"> + + PixelFederation ruleset for PHP 8.4. - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + 5 + + + 5 + + + 5 + + + 5 + + + 5 + + + 5 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/phpcs.ruleset.85.non-ddd.xml b/phpcs.ruleset.85.non-ddd.xml index 982186d..84b014d 100644 --- a/phpcs.ruleset.85.non-ddd.xml +++ b/phpcs.ruleset.85.non-ddd.xml @@ -7,13 +7,5 @@ - - - - - - - - - + diff --git a/phpcs.ruleset.85.xml b/phpcs.ruleset.85.xml index a796cff..ba6a4d5 100644 --- a/phpcs.ruleset.85.xml +++ b/phpcs.ruleset.85.xml @@ -7,5 +7,5 @@ - + diff --git a/phpcs.xml.dist b/phpcs.xml.dist index e8f7350..3ab41d2 100644 --- a/phpcs.xml.dist +++ b/phpcs.xml.dist @@ -8,14 +8,14 @@ - + config/ src/ vendor/ - + diff --git a/phpmd.dist.xml b/phpmd.dist.xml deleted file mode 100644 index 7ddb8a6..0000000 --- a/phpmd.dist.xml +++ /dev/null @@ -1,21 +0,0 @@ - - - - Custom rule set for project - - tests/ - - - - - - - - - - - diff --git a/phpunit.xml b/phpunit.xml index e055cf5..b880218 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -1,9 +1,14 @@ - + - + diff --git a/src/GrumPHP/Linter/Xml/XmlLinter.php b/src/GrumPHP/Linter/Xml/XmlLinter.php index 970f02c..a2cf373 100644 --- a/src/GrumPHP/Linter/Xml/XmlLinter.php +++ b/src/GrumPHP/Linter/Xml/XmlLinter.php @@ -32,27 +32,29 @@ public function lint(SplFileInfo $file): LintErrorsCollection $useInternalErrors = $this->useInternalXmlLogging(true); $this->flushXmlErrors(); - $document = $this->loadDocument($file); - if (!$document) { - $this->collectXmlErrors($errors, null); - $this->useInternalXmlLogging($useInternalErrors); + try { + $document = $this->loadDocument($file); + if (!$document) { + $this->collectXmlErrors($errors, null); - return $errors; - } - - if ($this->xInclude && $document->xinclude() === -1) { - $this->collectXmlErrors($errors, $document); - } + return $errors; + } - if ($this->dtdValidation && !$this->validateDTD($document)) { - $this->collectXmlErrors($errors, $document); - } + if ($this->xInclude && $document->xinclude() === -1) { + $this->collectXmlErrors($errors, $document); + } - $this->checkInternalSchemes($file, $document, $errors); + if ($this->dtdValidation && !$this->validateDTD($document)) { + $this->collectXmlErrors($errors, $document); + } - $this->useInternalXmlLogging($useInternalErrors); + $this->checkInternalSchemes($file, $document, $errors); - return $errors; + return $errors; + } finally { + $this->flushXmlErrors(); + $this->useInternalXmlLogging($useInternalErrors); + } } #[Override] @@ -310,51 +312,44 @@ private function addSchemasFromSchemaLocation( return $schemas; } - foreach ($parts as $key => $value) { - $schemas = $this->addSchemasFromSchemaLocationPart( - $file, - $document, - $errors, - $schemas, - $key, - $value, - ); + $documentNamespace = $document->documentElement->namespaceURI ?? ''; + $schema = $this->findSchemaForNamespace($parts, $documentNamespace); + if ($schema === null) { + $this->addMissingSchemaError($file, $errors, $documentNamespace); + + return $schemas; } + $schemas[] = $schema; + return $schemas; } /** - * @param array $schemas - * @return array + * @param array $parts */ - private function addSchemasFromSchemaLocationPart( - SplFileInfo $file, - DOMDocument $document, - LintErrorsCollection $errors, - array $schemas, - int $key, - string $value, - ): array { - if ($key & 1) { - $schemas[] = $value; - - return $schemas; + private function findSchemaForNamespace(array $parts, string $documentNamespace): ?string + { + for ($key = 0; $key < count($parts); $key += 2) { + if ($parts[$key] === $documentNamespace) { + return $parts[$key + 1]; + } } - if ($value !== $document->documentElement?->namespaceURI) { - $this->addError( - $errors, - LintError::TYPE_FATAL, - sprintf( - 'Namespace "%s" from schemaLocation is not declared in the document', - $value, - ), - $file->getPathname(), - ); - } + return null; + } - return $schemas; + private function addMissingSchemaError( + SplFileInfo $file, + LintErrorsCollection $errors, + string $documentNamespace, + ): void { + $this->addError( + $errors, + LintError::TYPE_FATAL, + sprintf('Missing schema for document namespace "%s"', $documentNamespace), + $file->getPathname(), + ); } private function locateScheme(SplFileInfo $xmlFile, string $scheme, bool $loadFromNet): ?string diff --git a/src/GrumPHP/Task/ComposerInstallCheckTask.php b/src/GrumPHP/Task/ComposerInstallCheckTask.php index b3b03c2..1485042 100644 --- a/src/GrumPHP/Task/ComposerInstallCheckTask.php +++ b/src/GrumPHP/Task/ComposerInstallCheckTask.php @@ -28,7 +28,7 @@ public static function getConfigurableOptions(): ConfigOptionsResolver $resolver->setDefaults([ 'ignore_patterns' => [], 'script' => './vendor/pixelfederation/coding-standards/bin/composer_install_check.sh', - 'triggered_by' => ['php', 'yml', 'yaml', 'xml'], + 'triggered_by' => ['json', 'lock', 'php', 'xml', 'yaml', 'yml'], 'whitelist_patterns' => [], ]); diff --git a/src/GrumPHP/Task/PhpMdExtendedTask.php b/src/GrumPHP/Task/PhpMdExtendedTask.php deleted file mode 100644 index 5bbccf0..0000000 --- a/src/GrumPHP/Task/PhpMdExtendedTask.php +++ /dev/null @@ -1,126 +0,0 @@ -, - * report_format: string, - * ruleset: array, - * triggered_by: array, - * whitelist_patterns: array, - * } - * @extends AbstractExternalTask - */ -final class PhpMdExtendedTask extends AbstractExternalTask -{ - #[Override] - public static function getConfigurableOptions(): ConfigOptionsResolver - { - $resolver = new OptionsResolver(); - $resolver->setDefaults([ - 'chunk_size' => 1000, - 'exclude' => [], - 'report_format' => 'text', - 'ruleset' => ['cleancode', 'codesize', 'naming'], - 'triggered_by' => ['php'], - 'whitelist_patterns' => [], - ]); - - $resolver->addAllowedTypes('whitelist_patterns', ['array']); - $resolver->addAllowedTypes('exclude', ['array']); - $resolver->addAllowedTypes('report_format', ['string']); - $resolver->addAllowedValues('report_format', ['text', 'ansi']); - $resolver->addAllowedTypes('ruleset', ['array']); - $resolver->addAllowedTypes('triggered_by', ['array']); - $resolver->setAllowedValues( - 'chunk_size', - static function (mixed $value): bool { - return false !== filter_var($value, FILTER_VALIDATE_INT, ['options' => ['min_range' => 1]]); - }, - ); - - return ConfigOptionsResolver::fromClosure( - static fn (array $options): array => $resolver->resolve($options), - ); - } - - #[Override] - public function canRunInContext(ContextInterface $context): bool - { - return $context instanceof GitPreCommitContext || $context instanceof RunContext; - } - - #[Override] - public function run(ContextInterface $context): TaskResultInterface - { - // phpcs:ignore SlevomatCodingStandard.PHP.RequireExplicitAssertion.RequiredExplicitAssertion - /** @var ConfigType $config */ - $config = $this->getConfig()->getOptions(); - $files = $this->getFiles($context, $config); - if (count($files) === 0) { - return TaskResult::createSkipped($this, $context); - } - - $chunkSize = $config['chunk_size']; - $chunks = array_chunk($files->toArray(), $chunkSize); - $totalChunks = count($chunks); - foreach ($chunks as $index => $chunk) { - $arguments = $this->processBuilder->createArgumentsForCommand('phpmd'); - $arguments->addCommaSeparatedFiles(new FilesCollection($chunk)); - $arguments->add($config['report_format']); - $arguments->addOptionalCommaSeparatedArgument('%s', $config['ruleset']); - $arguments->addOptionalArgument('--exclude', $config['exclude'] !== []); - $arguments->addOptionalCommaSeparatedArgument('%s', $config['exclude']); - - $arguments->addOptionalArgument('--suffixes', $config['triggered_by'] !== []); - $arguments->addOptionalCommaSeparatedArgument('%s', $config['triggered_by']); - - $process = $this->processBuilder->buildProcess($arguments); - $process->run(); - - if (!$process->isSuccessful()) { - $message = sprintf( - 'Chunk %d/%d failed:%s%s', - $index + 1, - $totalChunks, - PHP_EOL, - $this->formatter->format($process), - ); - - return TaskResult::createFailed($this, $context, $message); - } - } - - return TaskResult::createPassed($this, $context); - } - - /** - * @param ConfigType $config - */ - private function getFiles(ContextInterface $context, array $config): FilesCollection - { - $files = $context->getFiles(); - if (count($config['whitelist_patterns']) > 0) { - $files = $files->paths($config['whitelist_patterns']); - } - - return $files->extensions($config['triggered_by']); - } -} diff --git a/src/PixelFederationCodingStandard/Sniffs/Classes/RequireAbstractOrFinalSniff.php b/src/PixelFederationCodingStandard/Sniffs/Classes/RequireAbstractOrFinalSniff.php new file mode 100644 index 0000000..90e99dc --- /dev/null +++ b/src/PixelFederationCodingStandard/Sniffs/Classes/RequireAbstractOrFinalSniff.php @@ -0,0 +1,23 @@ +&1', + escapeshellarg(self::getPath('PATH_PHPCS')), + escapeshellarg(self::getPath('PATH_PHPCS_RULESET')), + escapeshellarg('SlevomatCodingStandard.Commenting.ForbiddenAnnotations'), + escapeshellarg(__DIR__ . '/ForbiddenAnnotations/Annotations.php'), + ); + + exec($command, $output, $exitCode); + $report = implode("\n", $output); + + self::assertSame(1, $exitCode, $report); + self::assertStringContainsString('Use of annotation @author is forbidden.', $report); + self::assertStringContainsString('Use of annotation @copyright is forbidden.', $report); + self::assertStringContainsString('Use of annotation @license is forbidden.', $report); + self::assertStringNotContainsString('Use of annotation @throws is forbidden.', $report); + self::assertSame(3, substr_count($report, 'AnnotationForbidden')); + } +} diff --git a/tests/Functional/PhpcsTestCase.php b/tests/Functional/PhpcsTestCase.php index dff7730..7848052 100644 --- a/tests/Functional/PhpcsTestCase.php +++ b/tests/Functional/PhpcsTestCase.php @@ -9,19 +9,6 @@ abstract class PhpcsTestCase extends TestCase { - public static function trimEndFileNewline(string $file): void - { - $file = __DIR__ . '/' . $file; - $content = file_get_contents($file); - if ($content === false) { - throw new RuntimeException('Could not read file: ' . $file); - } - - $trimmedContent = rtrim($content, "\n"); - - file_put_contents($file, $trimmedContent); - } - public static function assertPhpcbf( string $fileBefore, string $fileAfter, @@ -38,60 +25,87 @@ public static function assertPhpcbf( self::assertTrue(mkdir($tmpDir, 0777, true), $message); $tmpFile = $tmpDir . '/' . $tmpFilename; - self::assertNotFalse($tmpFile, $message); - file_put_contents($tmpFile, file_get_contents($before)); + $beforeContent = file_get_contents($before); + if ($beforeContent === false) { + throw new RuntimeException('Could not read file: ' . $before); + } - $cmdCbf = sprintf( - '%s --standard=%s %s 2>&1', - escapeshellarg(self::getPath('PATH_PHPCBF')), - escapeshellarg(self::getPath('PATH_PHPCS_RULESET')), - escapeshellarg($tmpFile), - ); + try { + self::assertNotFalse(file_put_contents($tmpFile, $beforeContent), $message); - $beforeContent = file_get_contents($before); - file_put_contents($tmpFile, $beforeContent); + [$exitCodeCbf, $outputCbf] = self::runPhpcsCommand('PATH_PHPCBF', $tmpFile); + self::assertSame(0, $exitCodeCbf, implode("\n", $outputCbf) . $message); - exec($cmdCbf, $outputCbf, $exitCodeCbf); - self::assertSame(0, $exitCodeCbf, $message); + self::assertFixedContent($tmpFile, $expected, $beforeContent, $message); - $actual = file_get_contents($tmpFile); + [$exitCodeCs, $outputCs] = self::runPhpcsCommand('PATH_PHPCS', $tmpFile); + self::assertSame( + 0, + $exitCodeCs, + sprintf( + "Expected no PHPCS errors after PHPCBF fix, got exit code %s.\nOutput:\n%s %s", + $exitCodeCs, + implode("\n", $outputCs), + $message, + ), + ); + } finally { + self::removeTemporaryFiles($tmpFile, $tmpDir); + } + } + + protected static function getPath(string $env): string + { + return __DIR__ . '/../../' . getenv($env); + } + + private static function assertFixedContent( + string $actualFile, + string $expectedFile, + string $beforeContent, + string $message, + ): void { + $actualContent = file_get_contents($actualFile); self::assertNotSame( $beforeContent, - $actual, + $actualContent, 'Expected PHPCBF to change the file, but contents are identical.' . $message, ); - self::assertSame( - file_get_contents($expected), - $actual, + file_get_contents($expectedFile), + $actualContent, "PHPCBF output does not match expected 'after' file. " . $message, ); + } - $cmdCs = sprintf( + /** + * @return array{int, list} + */ + private static function runPhpcsCommand(string $binaryPathEnv, string $file): array + { + $command = sprintf( '%s --standard=%s %s 2>&1', - escapeshellarg(self::getPath('PATH_PHPCS')), + escapeshellarg(self::getPath($binaryPathEnv)), escapeshellarg(self::getPath('PATH_PHPCS_RULESET')), - escapeshellarg($tmpFile), + escapeshellarg($file), ); - exec($cmdCs, $outputCs, $exitCodeCs); + $output = []; + exec($command, $output, $exitCode); - self::assertSame( - 0, - $exitCodeCs, - sprintf( - "Expected no PHPCS errors after PHPCBF fix, got exit code %s.\nOutput:\n%s %s", - $exitCodeCs, - implode("\n", $outputCs), - $message, - ), - ); - - @unlink($tmpFile); + return [$exitCode, $output]; } - protected static function getPath(string $env): string + private static function removeTemporaryFiles(string $file, string $directory): void { - return __DIR__ . '/../../' . getenv($env); + if (is_file($file)) { + unlink($file); + } + + if (!is_dir($directory)) { + return; + } + + rmdir($directory); } } diff --git a/tests/Functional/RequireAbstractOrFinal/Classes.php b/tests/Functional/RequireAbstractOrFinal/Classes.php new file mode 100644 index 0000000..75e9528 --- /dev/null +++ b/tests/Functional/RequireAbstractOrFinal/Classes.php @@ -0,0 +1,23 @@ +&1', + escapeshellarg(self::getPath('PATH_PHPCS')), + escapeshellarg(self::getPath('PATH_PHPCS_RULESET')), + escapeshellarg('PixelFederationCodingStandard.Classes.RequireAbstractOrFinal'), + escapeshellarg(__DIR__ . '/RequireAbstractOrFinal/Classes.php'), + ); + + exec($command, $output, $exitCode); + + self::assertSame(1, $exitCode, implode("\n", $output)); + + $report = self::requireArray(json_decode(implode("\n", $output), true), 'Could not decode PHPCS JSON report.'); + $totals = self::requireArray($report['totals'] ?? null, 'PHPCS JSON report does not contain totals.'); + + self::assertSame(1, $totals['errors'] ?? null); + self::assertSame(0, $totals['warnings'] ?? null); + + $reportFiles = self::requireArray($report['files'] ?? null, 'PHPCS JSON report does not contain files.'); + $files = array_values($reportFiles); + self::assertCount(1, $files); + + $file = self::requireArray($files[0] ?? null, 'PHPCS JSON report contains an invalid file result.'); + $messages = self::requireArray($file['messages'] ?? null, 'PHPCS JSON file result does not contain messages.'); + self::assertCount(1, $messages); + + $message = self::requireArray($messages[0] ?? null, 'PHPCS JSON report contains an invalid message.'); + self::assertSame(21, $message['line'] ?? null); + self::assertSame( + 'PixelFederationCodingStandard.Classes.RequireAbstractOrFinal.ClassNeitherAbstractNorFinal', + $message['source'] ?? null, + ); + } + + /** + * @return array + */ + private static function requireArray(mixed $value, string $errorMessage): array + { + if (!is_array($value)) { + throw new RuntimeException($errorMessage); + } + + return $value; + } +} diff --git a/tests/Functional/SuperfluousWhitespace/Bar.SuperfluousWhitespace.after.php b/tests/Functional/SuperfluousWhitespace/Bar.SuperfluousWhitespace.after.php index a4ef9f7..3e7fda0 100644 --- a/tests/Functional/SuperfluousWhitespace/Bar.SuperfluousWhitespace.after.php +++ b/tests/Functional/SuperfluousWhitespace/Bar.SuperfluousWhitespace.after.php @@ -4,16 +4,16 @@ namespace PixelFederation\CodingStandards\Tests\Functional\SuperfluousWhitespace; -final class Bar +final readonly class Bar { - public readonly int $superNumber; + public int $superNumber; /** * Superfluous docblock parameters */ public function __construct( - public readonly int $width, - public readonly int $height, + public int $width, + public int $height, ) { $total = $this->width + $this->height; diff --git a/tests/Functional/SuperfluousWhitespace/Bar.SuperfluousWhitespace.before.php b/tests/Functional/SuperfluousWhitespace/Bar.SuperfluousWhitespace.before.php index 068a9ab..3267c80 100644 --- a/tests/Functional/SuperfluousWhitespace/Bar.SuperfluousWhitespace.before.php +++ b/tests/Functional/SuperfluousWhitespace/Bar.SuperfluousWhitespace.before.php @@ -5,10 +5,10 @@ namespace PixelFederation\CodingStandards\Tests\Functional\SuperfluousWhitespace; -final class Bar +final readonly class Bar { - public readonly int $superNumber; + public int $superNumber; /** @@ -18,9 +18,9 @@ final class Bar * @param int $height */ public function __construct( - public readonly int $width , + public int $width , - public readonly int $height, + public int $height, ) { $total = $this->width + $this->height; diff --git a/tests/Functional/SuperfluousWhitespaceTest.php b/tests/Functional/SuperfluousWhitespaceTest.php index 0377d56..5656808 100644 --- a/tests/Functional/SuperfluousWhitespaceTest.php +++ b/tests/Functional/SuperfluousWhitespaceTest.php @@ -11,9 +11,6 @@ public function testPhpcbfFixesWhitespaceAsExpected(): void $fileBefore = 'SuperfluousWhitespace/Bar.SuperfluousWhitespace.before.php'; $fileAfter = 'SuperfluousWhitespace/Bar.SuperfluousWhitespace.after.php'; - // Ensure no trailing newlines interfere with the comparison - self::trimEndFileNewline($fileBefore); - self::assertPhpcbf($fileBefore, $fileAfter); } } diff --git a/tests/Functional/XmlLinter/external-dtd.xml b/tests/Functional/XmlLinter/external-dtd.xml new file mode 100644 index 0000000..66ebf24 --- /dev/null +++ b/tests/Functional/XmlLinter/external-dtd.xml @@ -0,0 +1,3 @@ + + + diff --git a/tests/Functional/XmlLinter/external.dtd b/tests/Functional/XmlLinter/external.dtd new file mode 100644 index 0000000..b7e2d0b --- /dev/null +++ b/tests/Functional/XmlLinter/external.dtd @@ -0,0 +1 @@ + diff --git a/tests/Functional/XmlLinter/schema/child.xsd b/tests/Functional/XmlLinter/schema/child.xsd new file mode 100644 index 0000000..87f0268 --- /dev/null +++ b/tests/Functional/XmlLinter/schema/child.xsd @@ -0,0 +1,7 @@ + + + + diff --git a/tests/Functional/XmlLinter/schema/root.xsd b/tests/Functional/XmlLinter/schema/root.xsd new file mode 100644 index 0000000..ab1977c --- /dev/null +++ b/tests/Functional/XmlLinter/schema/root.xsd @@ -0,0 +1,15 @@ + + + + + + + + + + + diff --git a/tests/Functional/XmlLinter/valid-multiple-namespaces.xml b/tests/Functional/XmlLinter/valid-multiple-namespaces.xml new file mode 100644 index 0000000..43a0405 --- /dev/null +++ b/tests/Functional/XmlLinter/valid-multiple-namespaces.xml @@ -0,0 +1,7 @@ + + + value + diff --git a/tests/Functional/XmlLinterTest.php b/tests/Functional/XmlLinterTest.php new file mode 100644 index 0000000..ac8fff7 --- /dev/null +++ b/tests/Functional/XmlLinterTest.php @@ -0,0 +1,50 @@ +setLoadFromNet(false); + $linter->setXInclude(false); + $linter->setDtdValidation(false); + $linter->setSchemeValidation(true); + + $errors = $linter->lint(new SplFileInfo(__DIR__ . '/XmlLinter/valid-multiple-namespaces.xml')); + + self::assertCount(0, $errors, (string) $errors); + } + + public function testInternalErrorSettingIsRestoredWhenLoadingThrows(): void + { + $previousUseInternalErrors = libxml_use_internal_errors(false); + libxml_set_external_entity_loader( + static fn (): never => throw new RuntimeException('Expected external entity loader failure.'), + ); + + try { + $linter = new XmlLinter(); + $linter->setLoadFromNet(true); + + self::expectException(RuntimeException::class); + + try { + $linter->lint(new SplFileInfo(__DIR__ . '/XmlLinter/external-dtd.xml')); + } finally { + self::assertFalse(libxml_use_internal_errors()); + } + } finally { + libxml_set_external_entity_loader(null); + libxml_use_internal_errors($previousUseInternalErrors); + libxml_clear_errors(); + } + } +} From 4e6f0fd98940689caa378cf44d0d41db4fccb183 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20=C5=A0korupa?= Date: Fri, 18 Sep 2026 09:01:11 +0200 Subject: [PATCH 2/3] fix: apply code review --- README.md | 2 +- bin/grumphp_hooks/environment_spinup | 2 +- grumphp.yml | 3 +- src/GrumPHP/Linter/Xml/XmlLinter.php | 43 +++++++++++++------ .../XmlLinter/invalid-multiple-namespaces.xml | 7 +++ .../XmlLinter/schema/no-namespace-root.xsd | 13 ++++++ ...lid-no-namespace-with-namespaced-child.xml | 7 +++ tests/Functional/XmlLinterTest.php | 39 ++++++++++++++--- 8 files changed, 96 insertions(+), 20 deletions(-) create mode 100644 tests/Functional/XmlLinter/invalid-multiple-namespaces.xml create mode 100644 tests/Functional/XmlLinter/schema/no-namespace-root.xsd create mode 100644 tests/Functional/XmlLinter/valid-no-namespace-with-namespaced-child.xml diff --git a/README.md b/README.md index e08d85a..c49530a 100644 --- a/README.md +++ b/README.md @@ -127,7 +127,7 @@ Options: | Option | Default | Description | | --- | --- | --- | -| `script` | `./bin/composer_install_check.sh` | Path to the check script. | +| `script` | `./vendor/pixelfederation/coding-standards/bin/composer_install_check.sh` | Path to the check script. | | `ignore_patterns` | `[]` | Patterns excluded from the check. | | `triggered_by` | `[json, lock, php, xml, yaml, yml]` | File extensions that trigger the task. | | `whitelist_patterns` | `[]` | Patterns limiting which changed files are checked. | diff --git a/bin/grumphp_hooks/environment_spinup b/bin/grumphp_hooks/environment_spinup index 60ef907..ccfe648 100755 --- a/bin/grumphp_hooks/environment_spinup +++ b/bin/grumphp_hooks/environment_spinup @@ -9,5 +9,5 @@ export PATH="/usr/local/bin:$PATH" DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" if [[ -z `docker compose ps -q coding-standards-php-min` ]] || [[ -z `docker ps -q --no-trunc | grep $(docker compose ps -q coding-standards-php-min)` ]]; then - docker compose up -d + docker compose up -d coding-standards-php-min fi diff --git a/grumphp.yml b/grumphp.yml index fdad78e..572b245 100644 --- a/grumphp.yml +++ b/grumphp.yml @@ -88,7 +88,8 @@ grumphp: always_execute: true config_file: phpunit.xml xmllint_extended: - ignore_patterns: [ ] + ignore_patterns: + - 'tests/Functional/XmlLinter/invalid-multiple-namespaces.xml' load_from_net: true x_include: true dtd_validation: true diff --git a/src/GrumPHP/Linter/Xml/XmlLinter.php b/src/GrumPHP/Linter/Xml/XmlLinter.php index a2cf373..dfafcc4 100644 --- a/src/GrumPHP/Linter/Xml/XmlLinter.php +++ b/src/GrumPHP/Linter/Xml/XmlLinter.php @@ -5,6 +5,7 @@ namespace PixelFederation\CodingStandards\GrumPHP\Linter\Xml; use DOMDocument; +use DOMNode; use GrumPHP\Collection\LintErrorsCollection; use GrumPHP\Linter\LinterInterface; use GrumPHP\Linter\LintError; @@ -283,22 +284,11 @@ private function addSchemasFromSchemaLocation( LintErrorsCollection $errors, array $schemas, ): array { - $schemaLocation = $document->documentElement?->attributes->getNamedItem('schemaLocation'); + $schemaLocation = $this->getSchemaLocation($file, $document, $errors); if ($schemaLocation === null) { return $schemas; } - if ($schemaLocation->namespaceURI !== self::XSI_NAMESPACE) { - $this->addError( - $errors, - LintError::TYPE_FATAL, - 'schemaLocation attribute is not in the XML Schema Instance namespace', - $file->getPathname(), - ); - - return $schemas; - } - /** @var array $parts */ $parts = preg_split('/\s+/', trim($schemaLocation->textContent)); // @phpstan-ignore varTag.nativeType if (count($parts) % 2 !== 0) { @@ -313,6 +303,10 @@ private function addSchemasFromSchemaLocation( } $documentNamespace = $document->documentElement->namespaceURI ?? ''; + if ($documentNamespace === '') { + return $schemas; + } + $schema = $this->findSchemaForNamespace($parts, $documentNamespace); if ($schema === null) { $this->addMissingSchemaError($file, $errors, $documentNamespace); @@ -325,6 +319,31 @@ private function addSchemasFromSchemaLocation( return $schemas; } + private function getSchemaLocation( + SplFileInfo $file, + DOMDocument $document, + LintErrorsCollection $errors, + ): ?DOMNode { + $schemaLocation = $document->documentElement?->attributes->getNamedItemNS( + self::XSI_NAMESPACE, + 'schemaLocation', + ); + if ($schemaLocation !== null) { + return $schemaLocation; + } + + if ($document->documentElement?->attributes->getNamedItem('schemaLocation') !== null) { + $this->addError( + $errors, + LintError::TYPE_FATAL, + 'schemaLocation attribute is not in the XML Schema Instance namespace', + $file->getPathname(), + ); + } + + return null; + } + /** * @param array $parts */ diff --git a/tests/Functional/XmlLinter/invalid-multiple-namespaces.xml b/tests/Functional/XmlLinter/invalid-multiple-namespaces.xml new file mode 100644 index 0000000..9632366 --- /dev/null +++ b/tests/Functional/XmlLinter/invalid-multiple-namespaces.xml @@ -0,0 +1,7 @@ + + + value + diff --git a/tests/Functional/XmlLinter/schema/no-namespace-root.xsd b/tests/Functional/XmlLinter/schema/no-namespace-root.xsd new file mode 100644 index 0000000..5410cab --- /dev/null +++ b/tests/Functional/XmlLinter/schema/no-namespace-root.xsd @@ -0,0 +1,13 @@ + + + + + + + + + + + diff --git a/tests/Functional/XmlLinter/valid-no-namespace-with-namespaced-child.xml b/tests/Functional/XmlLinter/valid-no-namespace-with-namespaced-child.xml new file mode 100644 index 0000000..bf00fce --- /dev/null +++ b/tests/Functional/XmlLinter/valid-no-namespace-with-namespaced-child.xml @@ -0,0 +1,7 @@ + + + value + diff --git a/tests/Functional/XmlLinterTest.php b/tests/Functional/XmlLinterTest.php index ac8fff7..2ca2dee 100644 --- a/tests/Functional/XmlLinterTest.php +++ b/tests/Functional/XmlLinterTest.php @@ -12,17 +12,35 @@ final class XmlLinterTest extends PhpcsTestCase { public function testValidDocumentWithMultipleNamespacesPassesSchemaValidation(): void { - $linter = new XmlLinter(); - $linter->setLoadFromNet(false); - $linter->setXInclude(false); - $linter->setDtdValidation(false); - $linter->setSchemeValidation(true); + $linter = self::createSchemaValidatingLinter(); $errors = $linter->lint(new SplFileInfo(__DIR__ . '/XmlLinter/valid-multiple-namespaces.xml')); self::assertCount(0, $errors, (string) $errors); } + public function testInvalidDocumentWithMultipleNamespacesFailsSchemaValidation(): void + { + $linter = self::createSchemaValidatingLinter(); + + $errors = $linter->lint(new SplFileInfo(__DIR__ . '/XmlLinter/invalid-multiple-namespaces.xml')); + + self::assertGreaterThan(0, count($errors)); + self::assertStringContainsString( + "Element '{urn:child}unexpected': This element is not expected.", + (string) $errors, + ); + } + + public function testNoNamespaceDocumentWithNamespacedChildPassesSchemaValidation(): void + { + $linter = self::createSchemaValidatingLinter(); + + $errors = $linter->lint(new SplFileInfo(__DIR__ . '/XmlLinter/valid-no-namespace-with-namespaced-child.xml')); + + self::assertCount(0, $errors, (string) $errors); + } + public function testInternalErrorSettingIsRestoredWhenLoadingThrows(): void { $previousUseInternalErrors = libxml_use_internal_errors(false); @@ -47,4 +65,15 @@ public function testInternalErrorSettingIsRestoredWhenLoadingThrows(): void libxml_clear_errors(); } } + + private static function createSchemaValidatingLinter(): XmlLinter + { + $linter = new XmlLinter(); + $linter->setLoadFromNet(false); + $linter->setXInclude(false); + $linter->setDtdValidation(false); + $linter->setSchemeValidation(true); + + return $linter; + } } From dcb7dfb1305316c6ce2750a5d3283bd30bb46b01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20=C5=A0korupa?= Date: Fri, 18 Sep 2026 09:06:44 +0200 Subject: [PATCH 3/3] fix: apply code review --- docker/php/Dockerfile | 1 + 1 file changed, 1 insertion(+) diff --git a/docker/php/Dockerfile b/docker/php/Dockerfile index e666340..e070925 100644 --- a/docker/php/Dockerfile +++ b/docker/php/Dockerfile @@ -4,6 +4,7 @@ FROM php:${PHP_VERSION}-cli RUN apt-get update && apt-get install -y git-core zlib1g-dev libzip-dev zip unzip RUN docker-php-ext-install zip +RUN php -r 'foreach (["dom", "libxml"] as $extension) { if (!extension_loaded($extension)) { fwrite(STDERR, sprintf("Required PHP extension %s is not loaded.\n", $extension)); exit(1); } }' RUN php -r "copy('https://getcomposer.org/installer', 'composer-setup.php');" && \ php -r "if (hash_file('SHA384', 'composer-setup.php') === trim(file_get_contents('https://composer.github.io/installer.sig'))) { echo 'Installer verified'; } else { echo 'Installer corrupt'; unlink('composer-setup.php'); } echo PHP_EOL;" && \