diff --git a/.editorconfig b/.editorconfig index 30288000..69ef6731 100644 --- a/.editorconfig +++ b/.editorconfig @@ -12,3 +12,9 @@ [*.md] trim_trailing_whitespace = false + + # Preserve the UTF-8 byte order mark in the BOM ruleset test fixtures, so that + # editors honouring EditorConfig do not silently strip it (which would + # invalidate the Generic.Files.ByteOrderMark ruleset test). + [*-bom.inc] + charset = utf-8-bom diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 8a17c256..bb5f3fdf 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -61,7 +61,7 @@ This package contains Composer scripts to quickly run the developer checks which After `composer install`, you can do: -- `composer lint`: Lint PHP and XML files in against parse errors. +- `composer lint`: Lint PHP files against parse errors. - `composer cs`: Check the code style and code quality of the codebase via PHPCS. - `composer test`: Run the unit tests for the VIPCS sniffs. - `composer test-coverage`: Run the unit tests for the VIPCS sniffs with coverage enabled. @@ -91,6 +91,7 @@ When you introduce new `public` sniff properties, or your sniff extends a class ### Pre-requisites * VIP Coding Standards * WordPress-Coding-Standards +* VariableAnalysis * PHPCSUtils 1.x * PHP_CodeSniffer 3.x * PHPUnit 4.x - 9.x @@ -144,15 +145,18 @@ The easiest way to do this is to add a `phpunit.xml` file to the root of your VI Expected output: ``` -PHPUnit 9.6.15 by Sebastian Bergmann and contributors. +Note: Tests are running in "CS" mode -........................................ 40 / 40 (100%) +PHPUnit 9.6.23 by Sebastian Bergmann and contributors. -45 sniff test files generated 175 unique error codes; 0 were fixable (0%) +Runtime: PHP 8.4.10 +Configuration: phpunit.xml.dist -Time: 150 ms, Memory: 20.00 MB +....................................... 39 / 39 (100%) -OK (40 tests, 0 assertions) +52 sniff test files generated 172 unique error codes; 0 were fixable (0%) + +Time: 00:00.862, Memory: 58.00 MB ``` ### Unit Testing conventions diff --git a/.github/workflows/basics.yml b/.github/workflows/basics.yml index 8a6c8fa1..ba8886e7 100644 --- a/.github/workflows/basics.yml +++ b/.github/workflows/basics.yml @@ -22,51 +22,65 @@ jobs: name: 'Basic CS and QA checks' runs-on: ubuntu-latest + env: + XMLLINT_INDENT: ' ' # This is a tab character. + steps: - name: Checkout code - uses: actions/checkout@v4 + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # 7.0.1 - name: Install PHP - uses: shivammathur/setup-php@v2 + uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # 2.37.2 with: php-version: 'latest' coverage: none tools: cs2pr - # Updating the lists can fail intermittently, typically after Microsoft has released a new package. - # This should not be blocking for this job, so ignore any errors from this step. - # Ref: https://github.com/dotnet/core/issues/4167 - - name: Update the available packages list - continue-on-error: true - run: sudo apt-get update - - - name: Install xmllint - run: sudo apt-get install --no-install-recommends -y libxml2-utils - - # Show XML violations inline in the file diff. - # @link https://github.com/marketplace/actions/xmllint-problem-matcher - - name: Enable showing XML issues inline - uses: korelstar/xmllint-problem-matcher@v1 - # Validate the composer.json file. # @link https://getcomposer.org/doc/03-cli.md#validate - name: Validate Composer installation run: composer validate --no-check-all --strict - name: 'Composer: adjust dependencies' - # Using PHPCS `master` as an early detection system for bugs upstream. - run: composer require --no-update --no-scripts squizlabs/php_codesniffer:"dev-master" --no-interaction + # Using PHPCS `3.x` as an early detection system for bugs upstream. + run: composer require --no-update --no-scripts squizlabs/php_codesniffer:"3.x-dev" --no-interaction # Install dependencies and handle caching in one go. # @link https://github.com/marketplace/actions/install-php-dependencies-with-composer - name: Install Composer dependencies - uses: "ramsey/composer-install@v3" + uses: "ramsey/composer-install@65e4f84970763564f46a70b8a54b90d033b3bdda" # 4.0.0 with: # Bust the cache at least once a month - output format: YYYY-MM. custom-cache-suffix: $(date -u "+%Y-%m") - - name: 'Validate XML against schema and check code style' - run: ./bin/xml-lint + # Validate the XML file. + - name: Validate rulesets against schema + uses: phpcsstandards/xmllint-validate@5189514594c8d5f4cf21b7e5af50f54d697973d7 # v2.0.0 + with: + pattern: "./*/ruleset.xml" + xsd-file: "vendor/squizlabs/php_codesniffer/phpcs.xsd" + + # Check the code-style consistency of the XML file. + # Note: this needs xmllint, but that will be installed via the phpcsstandards/xmllint-validate action runner in the previous step. + - name: Check XML code style + run: | + diff -B --tabsize=4 ./WordPressVIPMinimum/ruleset.xml <(xmllint --format "./WordPressVIPMinimum/ruleset.xml") + diff -B --tabsize=4 ./WordPress-VIP-Go/ruleset.xml <(xmllint --format "./WordPress-VIP-Go/ruleset.xml") + + # Validate dev tool related XML files. + - name: Validate Project PHPCS ruleset against schema + uses: phpcsstandards/xmllint-validate@5189514594c8d5f4cf21b7e5af50f54d697973d7 # v2.0.0 + with: + pattern: ".phpcs.xml.dist" + xsd-file: "vendor/squizlabs/php_codesniffer/phpcs.xsd" + + # Note: PHPUnit 9.3 introduced some new configuration options and deprecated the old versions of those. + # For cross-version compatibility, the validation is done against the PHPUnit 9.2 schema. + - name: "Validate PHPUnit config for use with PHPUnit 9" + uses: phpcsstandards/xmllint-validate@5189514594c8d5f4cf21b7e5af50f54d697973d7 # v2.0.0 + with: + pattern: "phpunit.xml.dist" + xsd-file: "vendor/phpunit/phpunit/schema/9.2.xsd" # Check the code-style consistency of the PHP files. - name: Check PHP code style @@ -90,10 +104,10 @@ jobs: steps: - name: Checkout code - uses: actions/checkout@v4 + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # 7.0.1 - name: Install PHP - uses: shivammathur/setup-php@v2 + uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # 2.37.2 with: php-version: 'latest' coverage: none @@ -103,7 +117,7 @@ jobs: # Dependencies need to be installed to make sure the PHPCS and PHPUnit classes are recognized. # @link https://github.com/marketplace/actions/install-php-dependencies-with-composer - name: Install Composer dependencies - uses: "ramsey/composer-install@v3" + uses: "ramsey/composer-install@65e4f84970763564f46a70b8a54b90d033b3bdda" # 4.0.0 with: # Bust the cache at least once a month - output format: YYYY-MM. custom-cache-suffix: $(date -u "+%Y-%m") diff --git a/.github/workflows/quicktest.yml b/.github/workflows/quicktest.yml index fff3cfa4..76bbffc0 100644 --- a/.github/workflows/quicktest.yml +++ b/.github/workflows/quicktest.yml @@ -26,9 +26,9 @@ jobs: strategy: matrix: include: - - php: '5.4' + - php: '7.4' dependencies: 'stable' - - php: '5.4' + - php: '7.4' dependencies: 'lowest' - php: 'latest' @@ -40,10 +40,10 @@ jobs: steps: - name: Checkout code - uses: actions/checkout@v4 + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # 7.0.1 - name: Set up PHP - uses: shivammathur/setup-php@v2 + uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # 2.37.2 with: php-version: ${{ matrix.php }} # With stable PHPCS dependencies, allow for PHP deprecation notices. @@ -51,10 +51,14 @@ jobs: ini-values: error_reporting=E_ALL & ~E_DEPRECATED, display_errors=On coverage: none + - name: Enable creation of `composer.lock` file + if: ${{ matrix.dependencies == 'lowest' }} + run: composer config --unset lock + # Install dependencies and handle caching in one go. # @link https://github.com/marketplace/actions/install-php-dependencies-with-composer - name: Install Composer dependencies - uses: "ramsey/composer-install@v3" + uses: "ramsey/composer-install@65e4f84970763564f46a70b8a54b90d033b3bdda" # 4.0.0 with: # Bust the cache at least once a month - output format: YYYY-MM. custom-cache-suffix: $(date -u "+%Y-%m") diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index ab08a031..afa59cdc 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -28,24 +28,24 @@ jobs: strategy: matrix: - php: ['5.4', 'latest', '8.4'] + php: ['7.4', 'latest', '8.5'] name: "Lint: PHP ${{ matrix.php }}" - continue-on-error: ${{ matrix.php == '8.4' }} + continue-on-error: ${{ matrix.php == '8.5' }} steps: - name: Checkout code - uses: actions/checkout@v4 + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # 7.0.1 - name: Install PHP - uses: shivammathur/setup-php@v2 + uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # 2.37.2 with: php-version: ${{ matrix.php }} coverage: none tools: cs2pr - name: Install Composer dependencies - uses: "ramsey/composer-install@v3" + uses: "ramsey/composer-install@65e4f84970763564f46a70b8a54b90d033b3bdda" # 4.0.0 with: # Bust the cache at least once a month - output format: YYYY-MM. custom-cache-suffix: $(date -u "+%Y-%m") @@ -64,36 +64,29 @@ jobs: # - php: The PHP versions to test against. # - dependencies: The PHPCS dependencies versions to test against. # IMPORTANT: test runs shouldn't fail because of PHPCS being incompatible with a PHP version. - # - PHPCS will run without errors on PHP 5.4 - 7.4 on any supported version. - # - PHP 8.0 needs PHPCS 3.5.7+ to run without errors, and we require a higher minimum version. - # - PHP 8.1 needs PHPCS 3.6.1+ to run without errors, but works best with 3.7.1+, and we require at least this minimum version. - # - PHP 8.2 and 8.3 need PHPCS 3.8.0+ to run without errors (though the errors don't affect the tests). + # - PHP 8.2, 8.3 and 8.4 need PHPCS 3.8.0+ to run without errors (though the errors don't affect the tests). matrix: - php: ['5.4', '5.5', '5.6', '7.0', '7.1', '7.2', '7.3', '7.4', '8.0', '8.1', '8.2', '8.3'] + php: ['7.4', '8.0', '8.1', '8.2', '8.3', '8.4'] dependencies: ['lowest', 'stable'] include: # Test against dev versions of all dependencies with select PHP versions for early detection of issues. - - php: '5.4' - dependencies: 'dev' - - php: '7.0' - dependencies: 'dev' - php: '7.4' dependencies: 'dev' - - php: '8.3' + - php: '8.4' dependencies: 'dev' # Test against upcoming PHP version. - - php: '8.4' + - php: '8.5' dependencies: 'dev' name: "Test: PHP ${{ matrix.php }} - PHPCS ${{ matrix.dependencies }}" - continue-on-error: ${{ matrix.php == '8.4' }} + continue-on-error: ${{ matrix.php == '8.5' }} steps: - name: Checkout code - uses: actions/checkout@v4 + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # 7.0.1 # With stable PHPCS dependencies, allow for PHP deprecation notices. # Unit tests don't need to fail on those for stable releases where those issues won't get fixed anymore. @@ -101,13 +94,13 @@ jobs: id: set_ini run: | if [[ "${{ matrix.dependencies }}" != "dev" ]]; then - echo 'PHP_INI=error_reporting=E_ALL & ~E_DEPRECATED, display_errors=On' >> $GITHUB_OUTPUT + echo 'PHP_INI=error_reporting=E_ALL & ~E_DEPRECATED, display_errors=On' >> "$GITHUB_OUTPUT" else - echo 'PHP_INI=error_reporting=-1, display_errors=On' >> $GITHUB_OUTPUT + echo 'PHP_INI=error_reporting=-1, display_errors=On' >> "$GITHUB_OUTPUT" fi - name: Install PHP - uses: shivammathur/setup-php@v2 + uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # 2.37.2 with: php-version: ${{ matrix.php }} ini-values: ${{ steps.set_ini.outputs.PHP_INI }} @@ -117,16 +110,20 @@ jobs: if: ${{ matrix.dependencies == 'dev' }} run: > composer require --no-update --no-scripts --no-interaction - squizlabs/php_codesniffer:"dev-master" + squizlabs/php_codesniffer:"3.x-dev" phpcsstandards/phpcsutils:"dev-develop" phpcsstandards/phpcsextra:"dev-develop" sirbrillig/phpcs-variable-analysis:"2.x" wp-coding-standards/wpcs:"dev-develop" + - name: Enable creation of `composer.lock` file + if: ${{ matrix.dependencies == 'lowest' }} + run: composer config --unset lock + # Install dependencies and handle caching in one go. # @link https://github.com/marketplace/actions/install-php-dependencies-with-composer - name: Install Composer dependencies - uses: "ramsey/composer-install@v3" + uses: "ramsey/composer-install@65e4f84970763564f46a70b8a54b90d033b3bdda" # 4.0.0 with: # Bust the cache at least once a month - output format: YYYY-MM. custom-cache-suffix: $(date -u "+%Y-%m") diff --git a/.gitignore b/.gitignore index 843c4b83..f80e3fee 100644 --- a/.gitignore +++ b/.gitignore @@ -7,3 +7,4 @@ phpunit.xml phpcs.cache phpstan.neon .phpunit.result.cache +ruleset-tests-report.json diff --git a/.phpcs.xml.dist b/.phpcs.xml.dist index 098e5acb..ea93917a 100644 --- a/.phpcs.xml.dist +++ b/.phpcs.xml.dist @@ -1,5 +1,5 @@ - + The custom ruleset for the VIP Coding Standards itself. . @@ -21,6 +21,7 @@ + @@ -43,21 +44,32 @@ - + - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + diff --git a/CHANGELOG.md b/CHANGELOG.md index 3e04b2ac..be060469 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,99 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [3.1.0] - 2026-07-27 + +Props: @GaryJones, @jrfnl, @mahangu, @mchanDev, @mujuonly, @rebeccahum + +This release raises the minimum requirements and reworks a large number of the VIPCS sniffs. + +The minimum [WordPressCS](https://github.com/WordPress/WordPress-Coding-Standards/releases/tag/3.4.1) requirement is raised to 3.4.1 (from 3.2.0) — a security release which also brings numerous accuracy improvements to sniffs that VIPCS bundles — along with PHP_CodeSniffer 3.13.5, PHPCSUtils 1.2.3, PHPCSExtra 1.5.1 and VariableAnalysis 2.13.0. The minimum PHP version is now 7.4. Many sniffs have been reworked to fix false positives, add support for modern PHP syntaxes and adopt PHPCSUtils. The JavaScript-specific sniffs and the `DynamicCalls` sniff have been hard-deprecated ahead of their removal in 4.0.0, and the unused `Security.Twig` sniff has been removed. + +Please ensure you run `composer update automattic/vipwpcs --with-dependencies` to benefit from this. + +### Added +- [#882](https://github.com/Automattic/VIP-Coding-Standards/pull/882): Performance/NoPaging: flag `posts_per_page` and `numberposts` set to `-1` (WordPress' "no limit" value), which previously slipped past the WPCS `> 100` check. +- [#883](https://github.com/Automattic/VIP-Coding-Standards/pull/883): Hooks/AlwaysReturnInFilter: `exit`, `die` or `throw` in a filter callback now raises a dedicated `TerminatingInsteadOfReturn` warning, instead of the generic `MissingReturnStatement` error. +- [#890](https://github.com/Automattic/VIP-Coding-Standards/pull/890): Add a Byte Order Mark (BOM) check to the `WordPressVIPMinimum` ruleset. + +### Changed +- [#824](https://github.com/Automattic/VIP-Coding-Standards/pull/824): Disable `WordPress.Security.EscapeOutput.ExceptionNotEscaped` in the `WordPressVIPMinimum` ruleset, as throwing an exception with a translated message triggered it and the check is considered controversial. +- [#854](https://github.com/Automattic/VIP-Coding-Standards/pull/854): Functions/StripTags: always flag use of `strip_tags()`, as the function should never be used on the VIP platform. +- As VIPCS bundles a number of WordPressCS sniffs and the minimum WordPressCS version has been raised to 3.4.1, users will now receive more accurate results from the following WPCS sniffs (see the [WordPressCS 3.3.0](https://github.com/WordPress/WordPress-Coding-Standards/releases/tag/3.3.0), [3.4.0](https://github.com/WordPress/WordPress-Coding-Standards/releases/tag/3.4.0) and [3.4.1](https://github.com/WordPress/WordPress-Coding-Standards/releases/tag/3.4.1) release notes for full details): + * `WordPress.DB.DirectDatabaseQuery`: recognises more caching functions (such as the `wp_cache_*_multiple()` and `wp_cache_*_salted()` functions), and has fewer false positives when caching functions are called using a non-canonical function name. + * `WordPress.DB.PreparedSQL`: fewer false positives for correctly escaped SQL called using a non-canonical function name, and for static method calls to a non-global class named `wpdb`. + * `WordPress.Security.EscapeOutput`: adds support for attributes on anonymous classes (PHP 8.0), `readonly` anonymous classes (PHP 8.3) and `exit` as a function call (PHP 8.4); expands `*::class` false-positive protection; fixes false positives/negatives for `get_search_query()` and `_deprecated_file()` used with non-standard casing; and no longer treats `wp_kses_allowed_html()` as an escaping function, which may surface a new warning where that function's return value was being output directly. + * `WordPress.Security.NonceVerification`: fewer false positives when the nonce-checking function is called using a non-canonical function name. + * `WordPress.Security.ValidatedSanitizedInput`: clearer error message for the `InputNotValidated` error code. + * `WordPress.WP.AlternativeFunctions`: fixes a false negative when class members share a name with select global WP functions/constants, and a false positive for the fully qualified stream constants `\STDIN`, `\STDOUT` and `\STDERR`. + * `WordPress.WP.CronInterval`: fixes a false positive when the callback reference used a different case to the function declaration. + +### Deprecated +- [#839](https://github.com/Automattic/VIP-Coding-Standards/pull/839): Hard-deprecate all JavaScript-specific sniffs (`WordPressVIPMinimum.JS.*`). They are excluded from the rulesets and will be removed in VIPCS 4.0.0. +- [#865](https://github.com/Automattic/VIP-Coding-Standards/pull/865): Hard-deprecate the `WordPressVIPMinimum.Functions.DynamicCalls` sniff. It is excluded from the rulesets and will be removed in VIPCS 4.0.0. + +### Removed +- [#864](https://github.com/Automattic/VIP-Coding-Standards/pull/864): Remove the unused `WordPressVIPMinimum.Security.Twig` sniff. + +### Fixed +- [#840](https://github.com/Automattic/VIP-Coding-Standards/pull/840): Fix dangerous comparisons against the value of token constants. +- [#842](https://github.com/Automattic/VIP-Coding-Standards/pull/842): Security/Mustache: fix potential false positives on a delimiter change, and examine double-quoted strings with interpolation and nowdocs which were previously skipped. +- [#846](https://github.com/Automattic/VIP-Coding-Standards/pull/846), [#848](https://github.com/Automattic/VIP-Coding-Standards/pull/848): Security/PHPFilterFunctions: fix false positives for method calls, namespaced function calls and attributes which share the function name. +- [#847](https://github.com/Automattic/VIP-Coding-Standards/pull/847): Security/StaticStrreplace: fix flawed detection of `str_replace()` by extending the WordPressCS `AbstractFunctionParameterSniff`; method calls, namespaced calls, first-class callables and argument unpacking are no longer flagged. +- [#850](https://github.com/Automattic/VIP-Coding-Standards/pull/850): Variables/ServerVariables: fix incorrect quote stripping and expand safeguards against false positives. +- [#851](https://github.com/Automattic/VIP-Coding-Standards/pull/851): Constants/ConstantString: fix flawed detection of `define()`/`defined()` by extending `AbstractFunctionParameterSniff`, and clarify the error message. +- [#852](https://github.com/Automattic/VIP-Coding-Standards/pull/852): Hooks/RestrictedHooks: fix false positives (method/namespaced calls, first-class callables, attributes) and disregard comments in the hook-name parameter. +- [#853](https://github.com/Automattic/VIP-Coding-Standards/pull/853): Performance/LowExpiryCacheTime: avoid a possible fatal error when a PHP 7.4+ numeric literal or 8.1+ octal literal is used as the cache time. +- [#855](https://github.com/Automattic/VIP-Coding-Standards/pull/855): Performance/FetchingRemoteData: fix flawed function-call detection by extending `AbstractFunctionParameterSniff`. +- [#858](https://github.com/Automattic/VIP-Coding-Standards/pull/858): Classes/DeclarationCompatibility: modernise and fix the sniff. +- [#861](https://github.com/Automattic/VIP-Coding-Standards/pull/861): Security/EscapingVoidReturnFunctions: fix flawed detection of `esc_*()`/`wp_kses*()` calls by extending `AbstractFunctionParameterSniff`; safeguard argument unpacking, attributes and first-class callables. +- [#862](https://github.com/Automattic/VIP-Coding-Standards/pull/862): Functions/DynamicCalls: fix end-of-statement determination and other minor issues. +- [#866](https://github.com/Automattic/VIP-Coding-Standards/pull/866): UserExperience/AdminBarRemoval: several fixes — case-insensitive function-name matching, recognise `add_action()` as an alias, disregard comments in parameters, and no longer treat a CSS file as PHP; plus PHP 8.0+ function-call and 8.1+ first-class-callable support. +- [#867](https://github.com/Automattic/VIP-Coding-Standards/pull/867): Performance/CacheValueOverride: fix false negatives for fully qualified calls and false positives for PHP 8.1+ first-class callables and variables. +- [#869](https://github.com/Automattic/VIP-Coding-Standards/pull/869): Constants/RestrictedConstants: fix a nonsensical comparison and use PHPCSUtils for quote stripping. +- [#872](https://github.com/Automattic/VIP-Coding-Standards/pull/872): Performance/TaxonomyMetaInOptions: extend `AbstractFunctionParameterSniff` and add PHP 8.0+ function-call and nullsafe-operator support. +- [#881](https://github.com/Automattic/VIP-Coding-Standards/pull/881): Hooks/AlwaysReturnInFilter: resolve bugs and adopt PHPCSUtils. + +### Security +- [WordPressCS 3.4.1](https://github.com/WordPress/WordPress-Coding-Standards/security/advisories/GHSA-3pwp-g2mj-5p3v) fixes a vulnerability in the `WordPress.WP.EnqueuedResourceParameters` sniff whereby running it over untrusted code could lead to arbitrary command execution on the scanning host. The VIPCS rulesets do not enable that sniff, so VIPCS' own behaviour is unaffected, but raising the minimum WordPressCS version removes the affected releases from the dependency tree and protects users who additionally run the `WordPress` or `WordPress-Extra` standards. + +### Maintenance +- Composer: + * [#857](https://github.com/Automattic/VIP-Coding-Standards/pull/857): Raise the minimum supported versions of all coding-standard dependencies. This release requires WordPressCS 3.4.1, PHP_CodeSniffer 3.13.5, PHPCSUtils 1.2.3, PHPCSExtra 1.5.1 and VariableAnalysis 2.13.0. + * [#828](https://github.com/Automattic/VIP-Coding-Standards/pull/828): Prevent a `composer.lock` file from being created. + * [#837](https://github.com/Automattic/VIP-Coding-Standards/pull/837): Bump the PHP Parallel Lint requirement. +- [#880](https://github.com/Automattic/VIP-Coding-Standards/pull/880): Bump the minimum PHP version to 7.4, drop PHPUnit 8 support and fix CI. +- Rulesets: + * [#835](https://github.com/Automattic/VIP-Coding-Standards/pull/835): Update the schema URL. + * [#860](https://github.com/Automattic/VIP-Coding-Standards/pull/860): Fix unescaped special characters in the `include`/`exclude` patterns. +- Sniffs: + * [#843](https://github.com/Automattic/VIP-Coding-Standards/pull/843): Fix license information and standardise the file docblocks. + * [#844](https://github.com/Automattic/VIP-Coding-Standards/pull/844): Remove `@since` tags which relate to WordPressCS. + * [#856](https://github.com/Automattic/VIP-Coding-Standards/pull/856): Classes/RestrictedExtendClasses: only listen for the `extends` keyword, significantly improving performance, and add tests. + * [#863](https://github.com/Automattic/VIP-Coding-Standards/pull/863): Functions/RestrictedFunctions: improve the `is_targetted_token()` method. + * [#868](https://github.com/Automattic/VIP-Coding-Standards/pull/868): Security/Underscorejs: start using the PHPCSUtils `FilePath` utility. + * [#870](https://github.com/Automattic/VIP-Coding-Standards/pull/870): Various minor CS fixes. +- Docs: + * [#826](https://github.com/Automattic/VIP-Coding-Standards/pull/826): Improve consistency and specificity. + * [#871](https://github.com/Automattic/VIP-Coding-Standards/pull/871): Various minor fixes. + * [#875](https://github.com/Automattic/VIP-Coding-Standards/pull/875): Performance/WPQueryParams: update a documentation link. + * Update the minimum requirements in the `README`. +- QA: + * [#825](https://github.com/Automattic/VIP-Coding-Standards/pull/825): Fix the PHPStan build. + * [#834](https://github.com/Automattic/VIP-Coding-Standards/pull/834): Silence a deprecation notice during the CS run. + * [#845](https://github.com/Automattic/VIP-Coding-Standards/pull/845): Fix the PHPCompatibility exclusions in the internal ruleset. + * Align the PHPCompatibility `testVersion` with the PHP 7.4 minimum requirement. +- Tests: + * [#833](https://github.com/Automattic/VIP-Coding-Standards/pull/833): Fix `RulesetTest`. + * [#838](https://github.com/Automattic/VIP-Coding-Standards/pull/838): Remove a PHPCS version toggle from `AlwaysReturnInFilterUnitTest`. + * [#890](https://github.com/Automattic/VIP-Coding-Standards/pull/890): Refactor the tests and test runner, and preserve/handle a BOM in the ruleset-test fixtures. +- GitHub Actions: + * [#827](https://github.com/Automattic/VIP-Coding-Standards/pull/827): Always quote variables. + * [#829](https://github.com/Automattic/VIP-Coding-Standards/pull/829): Run against PHP 8.4. + * [#830](https://github.com/Automattic/VIP-Coding-Standards/pull/830): Use the `xmllint-validate` action runner and add extra checks. + * [#884](https://github.com/Automattic/VIP-Coding-Standards/pull/884), [#887](https://github.com/Automattic/VIP-Coding-Standards/pull/887): Pin third-party GitHub Actions to commit SHAs. + * [#877](https://github.com/Automattic/VIP-Coding-Standards/pull/877), [#879](https://github.com/Automattic/VIP-Coding-Standards/pull/879), [#885](https://github.com/Automattic/VIP-Coding-Standards/pull/885), [#888](https://github.com/Automattic/VIP-Coding-Standards/pull/888), [#889](https://github.com/Automattic/VIP-Coding-Standards/pull/889): Dependency updates via Dependabot. + ## [3.0.1] - 2024-05-14 Props: @GaryJones, @jrnfl, @terriann, @rebeccahum @@ -709,6 +802,7 @@ Initial release. Props: david-binda, pkevan. +[3.1.0]: https://github.com/Automattic/VIP-Coding-Standards/compare/3.0.1...3.1.0 [3.0.1]: https://github.com/Automattic/VIP-Coding-Standards/compare/3.0.0...3.0.1 [3.0.0]: https://github.com/Automattic/VIP-Coding-Standards/compare/2.3.4...3.0.0 [2.3.4]: https://github.com/Automattic/VIP-Coding-Standards/compare/2.3.3...2.3.4 diff --git a/LICENSE.md b/LICENSE.md index 5517f00d..79a2790e 100644 --- a/LICENSE.md +++ b/LICENSE.md @@ -63,5 +63,8 @@ Included Files This project includes: -- [WordPress-Coding-Standards](https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards), which is Copyright © 2009 John Godley and contributors. Released under the MIT license https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/blob/develop/LICENSE -- [PHP_CodeSniffer](https://github.com/PHPCSStandards/PHP_CodeSniffer), Copyright © 2012, Squiz Pty Ltd (ABN 77 084 670 600). Released under the following license: https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/master/licence.txt +- [PHP_CodeSniffer](https://github.com/PHPCSStandards/PHP_CodeSniffer), Copyright © 2012, Squiz Pty Ltd (ABN 77 084 670 600); Copyright (c) 2023, PHPCSStandards and contributors. Released under the [BSD 3-Clause "New" or "Revised" License](https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/HEAD/licence.txt) +- [PHPCSUtils](https://github.com/PHPCSStandards/PHPCSUtils), Copyright © 2019, PHPCSUtils Contributors. Released under the [LGPL v3.0 License](https://github.com/PHPCSStandards/PHPCSUtils/blob/develop/LICENSE) +- [PHPCSExtra](https://github.com/PHPCSStandards/PHPCSExtra), Copyright © 2019, PHPCSExtra Contributors. Released under the [LGPL v3.0 License](https://github.com/PHPCSStandards/PHPCSExtra/blob/develop/LICENSE) +- [WordPress-Coding-Standards](https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards), which is Copyright © 2009 John Godley and contributors. Released under the [MIT license](https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/blob/develop/LICENSE). +- [VariableAnalysis](https://github.com/sirbrillig/phpcs-variable-analysis). Released under the [BSD license](https://github.com/sirbrillig/phpcs-variable-analysis/blob/2.x/LICENSE). diff --git a/README.md b/README.md index c0d08b3f..2b6c2e27 100644 --- a/README.md +++ b/README.md @@ -15,12 +15,12 @@ The rulesets use rules from the [WordPress Coding Standards](https://github.com/ ## Minimal requirements -* PHP 5.4+ -* [PHPCS 3.8.0+](https://github.com/PHPCSStandards/PHP_CodeSniffer/releases) -* [PHPCSUtils 1.0.9+](https://github.com/PHPCSStandards/PHPCSUtils) -* [PHPCSExtra 1.2.1+](https://github.com/PHPCSStandards/PHPCSExtra) -* [WPCS 3.0.0+](https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/releases) -* [VariableAnalysis 2.11.17+](https://github.com/sirbrillig/phpcs-variable-analysis/releases) +* PHP 7.4+ +* [PHPCS 3.13.5+](https://github.com/PHPCSStandards/PHP_CodeSniffer/releases) +* [PHPCSUtils 1.2.3+](https://github.com/PHPCSStandards/PHPCSUtils) +* [PHPCSExtra 1.5.1+](https://github.com/PHPCSStandards/PHPCSExtra) +* [WPCS 3.4.1+](https://github.com/WordPress-Coding-Standards/WordPress-Coding-Standards/releases) +* [VariableAnalysis 2.13.0+](https://github.com/sirbrillig/phpcs-variable-analysis/releases) ## Installation diff --git a/WordPress-VIP-Go/ruleset-test-bom.inc b/WordPress-VIP-Go/ruleset-test-bom.inc new file mode 100644 index 00000000..7ecb784f --- /dev/null +++ b/WordPress-VIP-Go/ruleset-test-bom.inc @@ -0,0 +1,3 @@ +My term link'; // Error. -// WordPressVIPMinimum.Functions.DynamicCalls -$my_notokay_func = 'extract'; -$my_notokay_func(); // Error. + + + // WordPressVIPMinimum.Functions.RestrictedFunctions @@ -466,6 +466,9 @@ $args = array( 'nopaging' => true, // Error. ); _query_posts( 'nopaging=true' ); // Error. +$args = array( + 'posts_per_page' => -1, // Error. +); // WordPressVIPMinimum.Performance.OrderByRand $args = array( diff --git a/WordPress-VIP-Go/ruleset-test.php b/WordPress-VIP-Go/ruleset-test.php index d8701db4..6f929fbb 100644 --- a/WordPress-VIP-Go/ruleset-test.php +++ b/WordPress-VIP-Go/ruleset-test.php @@ -36,7 +36,6 @@ 337 => 1, 341 => 1, 342 => 1, - 346 => 1, 350 => 1, 351 => 1, 352 => 1, @@ -88,15 +87,13 @@ 462 => 1, 466 => 1, 468 => 1, - 472 => 1, - 474 => 1, - 480 => 1, - 486 => 1, - 494 => 1, - 507 => 1, - 511 => 1, - 512 => 1, - 513 => 1, + 470 => 1, + 475 => 1, + 477 => 1, + 483 => 1, + 489 => 1, + 497 => 1, + 510 => 1, 514 => 1, 515 => 1, 516 => 1, @@ -105,16 +102,19 @@ 519 => 1, 520 => 1, 521 => 1, - 525 => 1, - 527 => 1, - 545 => 1, - 560 => 1, - 564 => 1, - 565 => 1, - 566 => 1, + 522 => 1, + 523 => 1, + 524 => 1, + 528 => 1, + 530 => 1, + 548 => 1, + 563 => 1, 567 => 1, - 572 => 1, - 574 => 1, + 568 => 1, + 569 => 1, + 570 => 1, + 575 => 1, + 577 => 1, ], 'warnings' => [ 7 => 1, @@ -224,14 +224,14 @@ 454 => 1, 455 => 1, 456 => 1, - 502 => 1, - 503 => 1, - 530 => 1, + 505 => 1, + 506 => 1, 533 => 1, - 540 => 1, - 550 => 1, - 556 => 1, - 579 => 1, + 536 => 1, + 543 => 1, + 553 => 1, + 559 => 1, + 582 => 1, ], 'messages' => [ 7 => [ @@ -324,11 +324,29 @@ ], ]; +// Expected values for the dedicated byte order mark (BOM) fixture. +$bom_expected = [ + 'errors' => [ + 1 => 1, + ], + 'warnings' => [], + 'messages' => [ + 1 => [ + 'File contains UTF-8 byte order mark, which may corrupt your application', + ], + ], +]; + require __DIR__ . '/../tests/RulesetTest.php'; // Run the tests! -$test = new RulesetTest( 'WordPress-VIP-Go', $expected ); -if ( $test->passes() ) { +$test = new RulesetTest( 'WordPress-VIP-Go', $expected ); +$bom_test = new RulesetTest( 'WordPress-VIP-Go', $bom_expected, 'ruleset-test-bom.inc' ); + +// Evaluate both tests before the check so each reports its own discrepancies rather than being short-circuited away. +$test_passes = $test->passes(); +$bom_test_passes = $bom_test->passes(); +if ( $test_passes && $bom_test_passes ) { printf( 'All WordPress-VIP-Go tests passed!' . PHP_EOL ); exit( 0 ); } diff --git a/WordPress-VIP-Go/ruleset.xml b/WordPress-VIP-Go/ruleset.xml index 8cd531c3..c7eddcf5 100644 --- a/WordPress-VIP-Go/ruleset.xml +++ b/WordPress-VIP-Go/ruleset.xml @@ -1,10 +1,13 @@ - + WordPress VIP Go Coding Standards - + + + + @@ -251,5 +254,7 @@ 0 - + + 0 + diff --git a/WordPressVIPMinimum/Sniffs/AbstractVariableRestrictionsSniff.php b/WordPressVIPMinimum/Sniffs/AbstractVariableRestrictionsSniff.php index 06b61bfb..4619807c 100644 --- a/WordPressVIPMinimum/Sniffs/AbstractVariableRestrictionsSniff.php +++ b/WordPressVIPMinimum/Sniffs/AbstractVariableRestrictionsSniff.php @@ -4,6 +4,7 @@ * * @package VIPCS\WordPressVIPMinimum * @link https://github.com/Automattic/VIP-Coding-Standards + * @license https://opensource.org/license/gpl-2-0 GPL-2.0 * @license https://opensource.org/licenses/MIT MIT */ @@ -58,7 +59,7 @@ abstract class AbstractVariableRestrictionsSniff extends Sniff { /** * Returns an array of tokens this test wants to listen for. * - * @return array + * @return array */ public function register() { // Retrieve the groups only once and don't set up a listener if there are no groups. @@ -91,7 +92,7 @@ public function register() { * ) * ) * - * @return array + * @return array>> */ abstract public function getGroups(); @@ -121,8 +122,6 @@ protected function setup_groups() { * @param int $stackPtr The position of the current token in the stack. * @return int|void Integer stack pointer to skip forward or void to continue * normal file processing. - * - * @throws \PHP_CodeSniffer\Exceptions\RuntimeException Exception. */ public function process_token( $stackPtr ) { diff --git a/WordPressVIPMinimum/Sniffs/Classes/DeclarationCompatibilitySniff.php b/WordPressVIPMinimum/Sniffs/Classes/DeclarationCompatibilitySniff.php index a71b4907..10381feb 100644 --- a/WordPressVIPMinimum/Sniffs/Classes/DeclarationCompatibilitySniff.php +++ b/WordPressVIPMinimum/Sniffs/Classes/DeclarationCompatibilitySniff.php @@ -3,40 +3,61 @@ * WordPressVIPMinimum Coding Standard. * * @package VIPCS\WordPressVIPMinimum + * @link https://github.com/Automattic/VIP-Coding-Standards + * @license https://opensource.org/license/gpl-2-0 GPL-2.0 */ namespace WordPressVIPMinimum\Sniffs\Classes; use PHP_CodeSniffer\Files\File; -use PHP_CodeSniffer\Sniffs\AbstractScopeSniff; +use PHP_CodeSniffer\Sniffs\Sniff; use PHPCSUtils\Utils\FunctionDeclarations; use PHPCSUtils\Utils\ObjectDeclarations; /** * Class WordPressVIPMinimum_Sniffs_Classes_DeclarationCompatibilitySniff */ -class DeclarationCompatibilitySniff extends AbstractScopeSniff { +class DeclarationCompatibilitySniff implements Sniff { /** - * The name of the class we are currently checking. + * A list of classes and methods to check. + * + * @deprecated 3.1.0 This should never have been a public property. * - * @var string + * @var array>>> */ - private $currentClass = ''; + public $checkClasses = []; /** - * A list of classes and methods to check. + * List of grouped classes with same methods (as they extend the same parent class). + * + * @deprecated 3.1.0 This should never have been a public property. + * + * @var array + */ + public $checkClassesGroups = []; + + /** + * A list of classes and information on the methods to check for those classes. * - * @var array>> + * @var array>>> */ - public $checkClasses = [ + private $methodSignatures = [ 'WP_Widget' => [ - 'widget' => [ 'args', 'instance' ], - 'update' => [ 'new_instance', 'old_instance' ], - 'form' => [ 'instance' ], + 'widget' => [ + 'args' => [], + 'instance' => [], + ], + 'update' => [ + 'new_instance' => [], + 'old_instance' => [], + ], + 'form' => [ + 'instance' => [], + ], 'WP_Widget' => [ - 'id_base', - 'name', + 'id_base' => [], + 'name' => [], 'widget_options' => [ 'default' => 'array()', ], @@ -44,16 +65,22 @@ class DeclarationCompatibilitySniff extends AbstractScopeSniff { 'default' => 'array()', ], ], - 'get_field_name' => [ 'field_name' ], - 'get_field_id' => [ 'field_name' ], + 'get_field_name' => [ + 'field_name' => [], + ], + 'get_field_id' => [ + 'field_name' => [], + ], '_register' => [], - '_set' => [ 'number' ], + '_set' => [ + 'number' => [], + ], '_get_display_callback' => [], '_get_update_callback' => [], '_get_form_callback' => [], 'is_preview' => [], 'display_callback' => [ - 'args', + 'args' => [], 'widget_args' => [ 'default' => '1', ], @@ -68,14 +95,17 @@ class DeclarationCompatibilitySniff extends AbstractScopeSniff { 'default' => '1', ], ], - 'register_one' => [ + '_register_one' => [ 'number' => [ 'default' => '-1', ], ], - 'save_settings' => [ 'settings' ], + 'save_settings' => [ + 'settings' => [], + ], 'get_settings' => [], ], + 'Walker' => [ 'start_lvl' => [ 'output' => [ @@ -103,7 +133,7 @@ class DeclarationCompatibilitySniff extends AbstractScopeSniff { 'output' => [ 'pass_by_reference' => true, ], - 'data_object', + 'data_object' => [], 'depth' => [ 'default' => '0', ], @@ -115,50 +145,50 @@ class DeclarationCompatibilitySniff extends AbstractScopeSniff { ], ], 'end_el' => [ - 'output' => [ + 'output' => [ 'pass_by_reference' => true, ], - 'data_object', - 'depth' => [ + 'data_object' => [], + 'depth' => [ 'default' => '0', ], - 'args' => [ + 'args' => [ 'default' => 'array()', ], ], 'display_element' => [ - 'element', + 'element' => [], 'children_elements' => [ 'pass_by_reference' => true, ], - 'max_depth', - 'depth', - 'args', + 'max_depth' => [], + 'depth' => [], + 'args' => [], 'output' => [ 'pass_by_reference' => true, ], ], 'walk' => [ - 'elements', - 'max_depth', - 'args' => [ + 'elements' => [], + 'max_depth' => [], + 'args' => [ 'variable_length' => true, ], ], 'paged_walk' => [ - 'elements', - 'max_depth', - 'page_num', - 'per_page', - 'args' => [ + 'elements' => [], + 'max_depth' => [], + 'page_num' => [], + 'per_page' => [], + 'args' => [ 'variable_length' => true, ], ], 'get_number_of_root_elements' => [ - 'elements', + 'elements' => [], ], 'unset_children' => [ - 'element', + 'element' => [], 'children_elements' => [ 'pass_by_reference' => true, ], @@ -167,143 +197,213 @@ class DeclarationCompatibilitySniff extends AbstractScopeSniff { ]; /** - * List of grouped classes with same methods (as they extend the same parent class) + * Classes this sniff checks for being extended. * - * @var array + * @var array Key is the name of a potentially extended class, + * value the canonical name for the method signatures definition. */ - public $checkClassesGroups = [ - 'Walker' => [ - 'Walker_Category_Checklist', - 'Walker_Category', - 'Walker_CategoryDropdown', - 'Walker_PageDropdown', - 'Walker_Nav_Menu', - 'Walker_Page', - 'Walker_Comment', - ], + private $extendedClassToSignatures = [ + 'WP_Widget' => 'WP_Widget', + 'Walker' => 'Walker', + 'Walker_Category_Checklist' => 'Walker', + 'Walker_Category' => 'Walker', + 'Walker_CategoryDropdown' => 'Walker', + 'Walker_PageDropdown' => 'Walker', + 'Walker_Nav_Menu' => 'Walker', + 'Walker_Page' => 'Walker', + 'Walker_Comment' => 'Walker', ]; /** - * Constructs the test with the tokens it wishes to listen for. + * Translate from case-insensitive names to proper case method names. + * + * @var array> Primary key is the class name in proper case. + * Value is an array with method names in lowercase as keys + * and these same method names in proper case as values. */ - public function __construct() { - parent::__construct( [ T_CLASS ], [ T_FUNCTION ], true ); - } + private $methodToProperCase = []; /** - * Processes this test when one of its tokens is encountered. + * Translate from case-insensitive names to proper case class names. * - * @param File $phpcsFile The PHP_CodeSniffer file where the token was found. - * @param int $stackPtr The position of the current token in the stack passed in $tokens. - * @param int $currScope A pointer to the start of the scope. - * - * @return void + * @var array Key is the lowercase name of a class, value the proper case. */ - protected function processTokenWithinScope( File $phpcsFile, $stackPtr, $currScope ) { + private $classToProperCase = []; - $className = ObjectDeclarations::getName( $phpcsFile, $currScope ); - - if ( $className !== $this->currentClass ) { - $this->currentClass = $className; + /** + * Returns the token types that this sniff is interested in. + * + * @return array + */ + public function register() { + // Lowercase all names to allow for correct comparisons, as PHP treats class/function names case-insensitively. + // But also store translation tables to be able to get the proper case. + foreach ( $this->methodSignatures as $key => $value ) { + $methodNames = array_keys( $value ); + $this->methodToProperCase[ $key ] = array_change_key_case( array_combine( $methodNames, $methodNames ), CASE_LOWER ); + + $this->methodSignatures[ $key ] = array_change_key_case( $value, CASE_LOWER ); } - $methodName = FunctionDeclarations::getName( $phpcsFile, $stackPtr ); + $classNames = array_keys( $this->extendedClassToSignatures ); + $this->classToProperCase = array_change_key_case( array_combine( $classNames, $classNames ), CASE_LOWER ); + $this->extendedClassToSignatures = array_change_key_case( $this->extendedClassToSignatures, CASE_LOWER ); - $parentClassName = ObjectDeclarations::findExtendedClassName( $phpcsFile, $currScope ); + return [ + T_CLASS, + T_ANON_CLASS, + ]; + } + + /** + * Processes the tokens that this sniff is interested in. + * + * @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned. + * @param int $stackPtr The position of the current token + * in the stack passed in $tokens. + * + * @return void + */ + public function process( File $phpcsFile, $stackPtr ) { + $parentClassName = ObjectDeclarations::findExtendedClassName( $phpcsFile, $stackPtr ); if ( $parentClassName === false ) { // This class does not extend any other class. return; } - // Need to define the originalParentClassName since we might override the parentClassName due to signature notations grouping. - $originalParentClassName = $parentClassName; - - if ( array_key_exists( $parentClassName, $this->checkClasses ) === false ) { + $parentClassNameLC = ltrim( strtolower( $parentClassName ), '\\' ); // Trim off potential FQN indicator. + if ( isset( $this->extendedClassToSignatures[ $parentClassNameLC ] ) === false ) { // This class does not extend a class we are interested in. - foreach ( $this->checkClassesGroups as $parent => $children ) { - // But it might be one of the grouped classes. - foreach ( $children as $child ) { - if ( $child === $parentClassName ) { - $parentClassName = $parent; - break 2; - } - } - } - if ( array_key_exists( $parentClassName, $this->checkClasses ) === false ) { - // This class really does not extend a class we are interested in. - return; - } + return; } - if ( array_key_exists( $methodName, $this->checkClasses[ $parentClassName ] ) === false && - in_array( $methodName, $this->checkClasses[ $parentClassName ], true ) === false - ) { - // This method is not a one we are interested in. + // Store the originalParentClassName since we might override the parentClassName due to signature notations grouping. + $originalParentClassNamePC = $this->classToProperCase[ $parentClassNameLC ]; + $parentClassName = $this->extendedClassToSignatures[ $parentClassNameLC ]; + + $methods = ObjectDeclarations::getDeclaredMethods( $phpcsFile, $stackPtr ); + if ( empty( $methods ) ) { return; } - $signatureParams = FunctionDeclarations::getParameters( $phpcsFile, $stackPtr ); + foreach ( $methods as $methodName => $functionPtr ) { + $methodNameLC = strtolower( $methodName ); + if ( isset( $this->methodSignatures[ $parentClassName ][ $methodNameLC ] ) === false ) { + // This method is not one we are interested in. + continue; + } + + $methodNamePC = $this->methodToProperCase[ $parentClassName ][ $methodNameLC ]; - $parentSignature = $this->checkClasses[ $parentClassName ][ $methodName ]; + $childParams = FunctionDeclarations::getParameters( $phpcsFile, $functionPtr ); + $childParamCount = count( $childParams ); - if ( count( $signatureParams ) > count( $parentSignature ) ) { - $extra_params = array_slice( $signatureParams, count( $parentSignature ) - count( $signatureParams ) ); - $all_extra_params_have_default = true; - foreach ( $extra_params as $extra_param ) { - if ( array_key_exists( 'default', $extra_param ) === false || $extra_param['default'] !== 'true' ) { - $all_extra_params_have_default = false; + $parentParams = $this->methodSignatures[ $parentClassName ][ $methodNameLC ]; + $parentParamCount = count( $parentParams ); + + /* + * If there are parameters, verify if the last parameter of both the parent and the child are variadic. + * Only the last parameter can be variadic and if the parent has this, the child must also, + * independently of potential extra optional parameters having been inserted before that last parameter. + * + * Also note that a child can make the last parameter variadic, even if the parent parameter was not. + * This will no longer trigger a warning since PHP 8.0. + */ + if ( $childParamCount > 0 && $parentParamCount > 0 ) { + $childLastParam = $childParams[ $childParamCount - 1 ]; + $parentLastParam = $parentParams[ array_keys( $parentParams )[ $parentParamCount - 1 ] ]; + + if ( ( isset( $parentLastParam['variable_length'] ) === true && $parentLastParam['variable_length'] === true ) + && $childLastParam['variable_length'] !== true + ) { + $this->addError( $phpcsFile, $functionPtr, $stackPtr, $originalParentClassNamePC, $methodNamePC, $childParams, $parentParams ); + continue; } } - if ( $all_extra_params_have_default === true ) { - return; // We're good. + + if ( $childParamCount > 0 ) { + // Check that no other parameters in the child signature are declared as variadic. + for ( $i = 0; $i < ( $childParamCount - 1 ); $i++ ) { + if ( $childParams[ $i ]['variable_length'] === true ) { + $this->addError( $phpcsFile, $functionPtr, $stackPtr, $originalParentClassNamePC, $methodNamePC, $childParams, $parentParams ); + continue 2; + } + } } - } - if ( count( $signatureParams ) !== count( $parentSignature ) ) { - $this->addError( $originalParentClassName, $methodName, $signatureParams, $parentSignature, $phpcsFile, $stackPtr ); - return; - } + if ( $childParamCount > $parentParamCount ) { + $extra_params = array_slice( $childParams, $parentParamCount - $childParamCount ); + $all_extra_params_have_default = true; + foreach ( $extra_params as $extra_param ) { + if ( isset( $extra_param['default'] ) === false + && $extra_param['variable_length'] === false + ) { + $all_extra_params_have_default = false; + break; + } + } + + if ( $all_extra_params_have_default === false ) { + $this->addError( $phpcsFile, $functionPtr, $stackPtr, $originalParentClassNamePC, $methodNamePC, $childParams, $parentParams ); + continue; + } + } elseif ( $childParamCount !== $parentParamCount ) { + $this->addError( $phpcsFile, $functionPtr, $stackPtr, $originalParentClassNamePC, $methodNamePC, $childParams, $parentParams ); + continue; + } - $i = 0; - foreach ( $parentSignature as $key => $param ) { - if ( is_array( $param ) === true ) { + $i = 0; + foreach ( $parentParams as $param ) { if ( ( - array_key_exists( 'default', $param ) === true && - array_key_exists( 'default', $signatureParams[ $i ] ) === false + isset( $param['default'] ) === true + && isset( $childParams[ $i ]['default'] ) === false + && $childParams[ $i ]['variable_length'] === false ) || ( - array_key_exists( 'pass_by_reference', $param ) === true && - $param['pass_by_reference'] !== $signatureParams[ $i ]['pass_by_reference'] + // Parameter in parent class has reference, child does not. + isset( $param['pass_by_reference'] ) === true + && $param['pass_by_reference'] !== $childParams[ $i ]['pass_by_reference'] ) || ( - array_key_exists( 'variable_length', $param ) === true && - $param['variable_length'] !== $signatureParams[ $i ]['variable_length'] + // Parameter in parent class does *not* have reference, child does. + ( isset( $param['pass_by_reference'] ) === false + || $param['pass_by_reference'] === false ) + && $childParams[ $i ]['pass_by_reference'] === true ) ) { - $this->addError( $originalParentClassName, $methodName, $signatureParams, $parentSignature, $phpcsFile, $stackPtr ); - return; + $this->addError( $phpcsFile, $functionPtr, $stackPtr, $originalParentClassNamePC, $methodNamePC, $childParams, $parentParams ); + continue 2; } + ++$i; } - ++$i; } } /** - * Generates an error with nice current and parent class method notations + * Generates an error with nice current and parent class method notations. * - * @param string $parentClassName The name of the extended (parent) class. - * @param string $methodName The name of the method currently being examined. - * @param array $currentMethodSignature The list of params and their options of the method which is being examined. - * @param array $parentMethodSignature The list of params and their options of the parent class method. - * @param File $phpcsFile The PHP_CodeSniffer file where the token was found. - * @param int $stackPtr The position of the current token in the stack. + * @param File $phpcsFile The PHP_CodeSniffer file where the token was found. + * @param int $stackPtr The position of the current T_FUNCTION token in the stack. + * @param int $currScope A pointer to the start of the OO scope. + * @param string $parentClassName The name of the extended (parent) class. + * @param string $methodName The name of the method currently being examined. + * @param array> $currentMethodSignature The list of params and their options of the method + * which is being examined. + * @param array> $parentMethodSignature The list of params and their options of the parent class method. * * @return void */ - private function addError( $parentClassName, $methodName, $currentMethodSignature, $parentMethodSignature, $phpcsFile, $stackPtr ) { + private function addError( File $phpcsFile, $stackPtr, $currScope, $parentClassName, $methodName, $currentMethodSignature, $parentMethodSignature ) { + $tokens = $phpcsFile->getTokens(); + $currentClassName = '[AnonymousClass]'; + if ( $tokens[ $currScope ]['code'] !== T_ANON_CLASS ) { + $currentClassName = ObjectDeclarations::getName( $phpcsFile, $currScope ); + } - $currentSignature = sprintf( '%s::%s(%s)', $this->currentClass, $methodName, implode( ', ', $this->generateParamList( $currentMethodSignature ) ) ); + $currentSignature = implode( ', ', $this->generateParamList( $currentMethodSignature ) ); + $currentSignature = sprintf( '%s::%s(%s)', $currentClassName, $methodName, $currentSignature ); - $parentSignature = sprintf( '%s::%s(%s)', $parentClassName, $methodName, implode( ', ', $this->generateParamList( $parentMethodSignature ) ) ); + $parentSignature = implode( ', ', $this->generateParamList( $parentMethodSignature ) ); + $parentSignature = sprintf( '%s::%s(%s)', $parentClassName, $methodName, $parentSignature ); $message = 'Declaration of `%s` should be compatible with `%s`.'; $data = [ $currentSignature, $parentSignature ]; @@ -315,32 +415,32 @@ private function addError( $parentClassName, $methodName, $currentMethodSignatur * * @param array $methodSignature Signature of a method. * - * @return array + * @return array */ private function generateParamList( $methodSignature ) { $paramList = []; foreach ( $methodSignature as $param => $options ) { $paramName = '$'; - if ( is_array( $options ) === false ) { - $paramList[] = '$' . $options; + if ( empty( $options ) === true ) { + $paramList[] = '$' . $param; continue; } - if ( array_key_exists( 'name', $options ) === true ) { + if ( isset( $options['name'] ) === true ) { $paramName = $options['name']; } else { $paramName .= $param; } - if ( array_key_exists( 'variable_length', $options ) === true && $options['variable_length'] === true ) { + if ( isset( $options['variable_length'] ) === true && $options['variable_length'] === true ) { $paramName = '...' . $paramName; } - if ( array_key_exists( 'pass_by_reference', $options ) === true && $options['pass_by_reference'] === true ) { + if ( isset( $options['pass_by_reference'] ) === true && $options['pass_by_reference'] === true ) { $paramName = '&' . $paramName; } - if ( array_key_exists( 'default', $options ) === true && empty( $options['default'] ) === false ) { + if ( isset( $options['default'] ) === true && empty( $options['default'] ) === false ) { $paramName .= ' = ' . trim( $options['default'] ); } @@ -349,12 +449,4 @@ private function generateParamList( $methodSignature ) { return $paramList; } - - /** - * Do nothing outside the scope. Has to be implemented accordingly to parent abstract class. - * - * @param File $phpcsFile PHPCS File. - * @param int $stackPtr Stack position. - */ - public function processTokenOutsideScope( File $phpcsFile, $stackPtr ) {} } diff --git a/WordPressVIPMinimum/Sniffs/Classes/RestrictedExtendClassesSniff.php b/WordPressVIPMinimum/Sniffs/Classes/RestrictedExtendClassesSniff.php index a8b076a2..5120bb9a 100644 --- a/WordPressVIPMinimum/Sniffs/Classes/RestrictedExtendClassesSniff.php +++ b/WordPressVIPMinimum/Sniffs/Classes/RestrictedExtendClassesSniff.php @@ -3,10 +3,13 @@ * WordPressVIPMinimum Coding Standard. * * @package VIPCS\WordPressVIPMinimum + * @link https://github.com/Automattic/VIP-Coding-Standards + * @license https://opensource.org/license/gpl-2-0 GPL-2.0 */ namespace WordPressVIPMinimum\Sniffs\Classes; +use PHPCSUtils\Utils\MessageHelper; use WordPressCS\WordPress\AbstractClassRestrictionsSniff; /** @@ -19,7 +22,7 @@ class RestrictedExtendClassesSniff extends AbstractClassRestrictionsSniff { /** * Groups of classes to restrict. * - * @return array + * @return array>> */ public function getGroups() { return [ @@ -33,6 +36,20 @@ public function getGroups() { ]; } + /** + * Returns an array of tokens this test wants to listen for. + * + * @return array + */ + public function register() { + $targets = parent::register(); + if ( empty( $targets ) ) { + return $targets; + } + + return [ T_EXTENDS ]; + } + /** * Process a matched token. * @@ -44,15 +61,9 @@ public function getGroups() { * @return void */ public function process_matched_token( $stackPtr, $group_name, $matched_content ) { - $tokens = $this->phpcsFile->getTokens(); - - if ( $tokens[ $stackPtr ]['code'] !== T_EXTENDS ) { - // If not extending, bail. - return; - } - foreach ( $this->getGroups() as $group => $group_args ) { - $this->phpcsFile->{ 'add' . $group_args['type'] }( $group_args['message'], $stackPtr, $group ); + $isError = ( $group_args['type'] === 'error' ); + MessageHelper::addMessage( $this->phpcsFile, $group_args['message'], $stackPtr, $isError, $group ); } } } diff --git a/WordPressVIPMinimum/Sniffs/Constants/ConstantStringSniff.php b/WordPressVIPMinimum/Sniffs/Constants/ConstantStringSniff.php index b2a8dafb..c44366a4 100644 --- a/WordPressVIPMinimum/Sniffs/Constants/ConstantStringSniff.php +++ b/WordPressVIPMinimum/Sniffs/Constants/ConstantStringSniff.php @@ -1,60 +1,53 @@ Key is the function name, value irrelevant. + */ + protected $target_functions = [ + 'define' => true, + 'defined' => true, + ]; /** - * Process this test when one of its tokens is encountered. + * Process the parameters of a matched function. * - * @param int $stackPtr The position of the current token in the stack passed in $tokens. + * @param int $stackPtr The position of the current token in the stack. + * @param string $group_name The name of the group which was matched. + * @param string $matched_content The token content (function name) which was matched + * in lowercase. + * @param array $parameters Array with information about the parameters. * * @return void */ - public function process_token( $stackPtr ) { - - if ( in_array( $this->tokens[ $stackPtr ]['content'], [ 'define', 'defined' ], true ) === false ) { - return; - } - - // Find the next non-empty token. - $nextToken = $this->phpcsFile->findNext( Tokens::$emptyTokens, $stackPtr + 1, null, true, null, true ); - - if ( $this->tokens[ $nextToken ]['code'] !== T_OPEN_PARENTHESIS ) { - // Not a function call. - return; - } - - if ( isset( $this->tokens[ $nextToken ]['parenthesis_closer'] ) === false ) { - // Not a function call. - return; - } - - $param = PassedParameters::getParameter( $this->phpcsFile, $stackPtr, 1, 'constant_name' ); + public function process_parameters( $stackPtr, $group_name, $matched_content, $parameters ) { + $param = PassedParameters::getParameterFromStack( $parameters, 1, 'constant_name' ); if ( $param === false ) { // Target parameter not found. return; @@ -71,7 +64,7 @@ public function process_token( $stackPtr ) { $tstring_token = $this->phpcsFile->findNext( T_STRING, $param['start'], $param['end'] + 1 ); - $message = 'Constant name, as a string, should be used along with `%s()`.'; + $message = 'The `%s()` function expects to be passed the constant name as a text string.'; $data = [ $this->tokens[ $stackPtr ]['content'] ]; $this->phpcsFile->addError( $message, $tstring_token, 'NotCheckingConstantName', $data ); } diff --git a/WordPressVIPMinimum/Sniffs/Constants/RestrictedConstantsSniff.php b/WordPressVIPMinimum/Sniffs/Constants/RestrictedConstantsSniff.php index 619c2631..416c947f 100644 --- a/WordPressVIPMinimum/Sniffs/Constants/RestrictedConstantsSniff.php +++ b/WordPressVIPMinimum/Sniffs/Constants/RestrictedConstantsSniff.php @@ -1,14 +1,16 @@ */ public $restrictedConstantNames = [ 'A8C_PROXIED_REQUEST', @@ -28,19 +30,46 @@ class RestrictedConstantsSniff extends Sniff { /** * List of restricted constant declarations. * - * @var array + * @var array */ public $restrictedConstantDeclaration = [ 'JETPACK_DEV_DEBUG', 'WP_CRON_CONTROL_SECRET', ]; + /** + * List of (global) constant names, which should not be referenced in userland code, nor (re-)declared. + * + * {@internal The `public` versions of these properties can't be removed until the next major, + * though a decision is still needed whether they should be removed at all. + * Also see: Automattic/VIP-Coding-Standards#234 for more context and discussion about this.} + * + * @var array Key is the constant name, value is irrelevant. + */ + private $restrictedConstants = []; + + /** + * List of (global) constants, which should not be (re-)declared, but may be referenced. + * + * {@internal The `public` versions of these properties can't be removed until the next major, + * though a decision is still needed whether they should be removed at all. + * Also see: Automattic/VIP-Coding-Standards#234 for more context and discussion about this.} + * + * @var array Key is the constant name, value is irrelevant. + */ + private $restrictedRedeclaration = []; + /** * Returns an array of tokens this test wants to listen for. * - * @return array + * @return array */ public function register() { + // For now, set the `private` properties based on the values of the `public` properties. + // This should be revisited when Automattic/VIP-Coding-Standards#234 gets actioned. + $this->restrictedConstants = array_flip( $this->restrictedConstantNames ); + $this->restrictedRedeclaration = array_flip( $this->restrictedConstantDeclaration ); + return [ T_CONSTANT_ENCAPSED_STRING, T_STRING, @@ -59,15 +88,17 @@ public function process_token( $stackPtr ) { if ( $this->tokens[ $stackPtr ]['code'] === T_STRING ) { $constantName = $this->tokens[ $stackPtr ]['content']; } else { - $constantName = trim( $this->tokens[ $stackPtr ]['content'], "\"'" ); + $constantName = TextStrings::stripQuotes( $this->tokens[ $stackPtr ]['content'] ); } - if ( in_array( $constantName, $this->restrictedConstantNames, true ) === false && in_array( $constantName, $this->restrictedConstantDeclaration, true ) === false ) { + if ( isset( $this->restrictedConstants[ $constantName ] ) === false + && isset( $this->restrictedRedeclaration[ $constantName ] ) === false + ) { // Not the constant we are looking for. return; } - if ( $this->tokens[ $stackPtr ]['code'] === T_STRING && in_array( $constantName, $this->restrictedConstantNames, true ) === true ) { + if ( $this->tokens[ $stackPtr ]['code'] === T_STRING && isset( $this->restrictedConstants[ $constantName ] ) === true ) { $message = 'Code is touching the `%s` constant. Make sure it\'s used appropriately.'; $data = [ $constantName ]; $this->phpcsFile->addWarning( $message, $stackPtr, 'UsingRestrictedConstant', $data ); @@ -96,12 +127,12 @@ public function process_token( $stackPtr ) { return; } - if ( in_array( $this->tokens[ $previous ]['code'], Tokens::$functionNameTokens, true ) === true ) { + if ( $this->tokens[ $previous ]['code'] === T_STRING ) { $data = [ $constantName ]; if ( $this->tokens[ $previous ]['content'] === 'define' ) { $message = 'The definition of `%s` constant is prohibited. Please use a different name.'; $this->phpcsFile->addError( $message, $previous, 'DefiningRestrictedConstant', $data ); - } elseif ( in_array( $constantName, $this->restrictedConstantNames, true ) === true ) { + } elseif ( isset( $this->restrictedConstants[ $constantName ] ) === true ) { $message = 'Code is touching the `%s` constant. Make sure it\'s used appropriately.'; $this->phpcsFile->addWarning( $message, $previous, 'UsingRestrictedConstant', $data ); } diff --git a/WordPressVIPMinimum/Sniffs/Files/IncludingFileSniff.php b/WordPressVIPMinimum/Sniffs/Files/IncludingFileSniff.php index f1bfc2db..ff89682e 100644 --- a/WordPressVIPMinimum/Sniffs/Files/IncludingFileSniff.php +++ b/WordPressVIPMinimum/Sniffs/Files/IncludingFileSniff.php @@ -1,8 +1,10 @@ */ public $getPathFunctions = [ 'dirname', @@ -55,7 +57,7 @@ class IncludingFileSniff extends AbstractFunctionRestrictionsSniff { /** * List of restricted constants. * - * @var array + * @var array */ public $restrictedConstants = [ 'TEMPLATEPATH' => 'get_template_directory', @@ -65,7 +67,7 @@ class IncludingFileSniff extends AbstractFunctionRestrictionsSniff { /** * List of allowed constants. * - * @var array + * @var array */ public $allowedConstants = [ 'ABSPATH', @@ -77,7 +79,7 @@ class IncludingFileSniff extends AbstractFunctionRestrictionsSniff { * List of keywords allowed for use in custom constants. * Note: Customizing this property will overwrite current default values. * - * @var array + * @var array */ public $allowedKeywords = [ 'PATH', @@ -87,7 +89,7 @@ class IncludingFileSniff extends AbstractFunctionRestrictionsSniff { /** * Functions used for modify slashes. * - * @var array + * @var array */ public $slashingFunctions = [ 'trailingslashit', @@ -107,7 +109,7 @@ public function getGroups() { /** * Returns an array of tokens this test wants to listen for. * - * @return array + * @return array */ public function register() { return Tokens::$includeTokens; diff --git a/WordPressVIPMinimum/Sniffs/Files/IncludingNonPHPFileSniff.php b/WordPressVIPMinimum/Sniffs/Files/IncludingNonPHPFileSniff.php index ab32dc9f..d5cd938f 100644 --- a/WordPressVIPMinimum/Sniffs/Files/IncludingNonPHPFileSniff.php +++ b/WordPressVIPMinimum/Sniffs/Files/IncludingNonPHPFileSniff.php @@ -1,8 +1,10 @@ Key is the extension, value is irrelevant. */ private $php_extensions = [ 'php' => true, @@ -34,7 +36,7 @@ class IncludingNonPHPFileSniff extends Sniff { /** * File extensions used for SVG and CSS files. * - * @var array Key is the extension, value is irrelevant. + * @var array Key is the extension, value is irrelevant. */ private $svg_css_extensions = [ 'css' => true, @@ -44,7 +46,7 @@ class IncludingNonPHPFileSniff extends Sniff { /** * Returns an array of tokens this test wants to listen for. * - * @return array + * @return array */ public function register() { return Tokens::$includeTokens; diff --git a/WordPressVIPMinimum/Sniffs/Functions/CheckReturnValueSniff.php b/WordPressVIPMinimum/Sniffs/Functions/CheckReturnValueSniff.php index 5eb40b2f..58a207c3 100644 --- a/WordPressVIPMinimum/Sniffs/Functions/CheckReturnValueSniff.php +++ b/WordPressVIPMinimum/Sniffs/Functions/CheckReturnValueSniff.php @@ -3,6 +3,8 @@ * WordPressVIPMinimum Coding Standard. * * @package VIPCS\WordPressVIPMinimum + * @link https://github.com/Automattic/VIP-Coding-Standards + * @license https://opensource.org/license/gpl-2-0 GPL-2.0 */ namespace WordPressVIPMinimum\Sniffs\Functions; @@ -24,7 +26,7 @@ class CheckReturnValueSniff extends Sniff { /** * Pairs we are about to check. * - * @var array + * @var array> */ public $catch = [ 'esc_url' => [ @@ -48,7 +50,7 @@ class CheckReturnValueSniff extends Sniff { /** * Tokens we are about to examine, which are not functions. * - * @var array + * @var array */ public $notFunctions = [ 'foreach' => T_FOREACH, @@ -57,7 +59,7 @@ class CheckReturnValueSniff extends Sniff { /** * Returns the token types that this sniff is interested in. * - * @return array(int) + * @return array */ public function register() { return [ T_STRING ]; @@ -293,7 +295,7 @@ public function findNonCheckedVariables( $stackPtr ) { * Function used as as callback for the array_reduce call. * * @param string|null $carry The final string. - * @param mixed $item Processed item. + * @param array $item Processed item. * * @return string */ diff --git a/WordPressVIPMinimum/Sniffs/Functions/DynamicCallsSniff.php b/WordPressVIPMinimum/Sniffs/Functions/DynamicCallsSniff.php index ca2f2acc..3f19977a 100644 --- a/WordPressVIPMinimum/Sniffs/Functions/DynamicCallsSniff.php +++ b/WordPressVIPMinimum/Sniffs/Functions/DynamicCallsSniff.php @@ -3,10 +3,13 @@ * WordPressVIPMinimum Coding Standard. * * @package VIPCS\WordPressVIPMinimum + * @link https://github.com/Automattic/VIP-Coding-Standards + * @license https://opensource.org/license/gpl-2-0 GPL-2.0 */ namespace WordPressVIPMinimum\Sniffs\Functions; +use PHP_CodeSniffer\Sniffs\DeprecatedSniff; use PHP_CodeSniffer\Util\Tokens; use PHPCSUtils\Utils\TextStrings; use WordPressVIPMinimum\Sniffs\Sniff; @@ -24,12 +27,12 @@ * * @link http://php.net/manual/en/migration71.incompatible.php */ -class DynamicCallsSniff extends Sniff { +class DynamicCallsSniff extends Sniff implements DeprecatedSniff { /** * Functions that should not be called dynamically. * - * @var array + * @var array */ private $disallowed_functions = [ 'assert' => true, @@ -44,25 +47,34 @@ class DynamicCallsSniff extends Sniff { ]; /** - * Array of variable assignments encountered, along with their values. + * Potential end tokens for which the end pointer has to be set back by one. * - * Populated at run-time. + * {@internal The PHPCS `findEndOfStatement()` method is not completely consistent + * in how it returns the statement end. This is just a simple way to bypass + * the inconsistency for our purposes.} * - * @var array The key is the name of the variable, the value, its assigned value. + * @var array */ - private $variables_arr = []; + private $inclusiveStopPoints = [ + T_COLON => true, + T_COMMA => true, + T_DOUBLE_ARROW => true, + T_SEMICOLON => true, + ]; /** - * The position in the stack where the token was found. + * Array of variable assignments encountered, along with their values. + * + * Populated at run-time. * - * @var int + * @var array The key is the name of the variable, the value, its assigned value. */ - private $stackPtr; + private $variables_arr = []; /** * Returns the token types that this sniff is interested in. * - * @return array(int) + * @return array */ public function register() { return [ T_VARIABLE => T_VARIABLE ]; @@ -76,38 +88,30 @@ public function register() { * @return void */ public function process_token( $stackPtr ) { - $this->stackPtr = $stackPtr; - // First collect all variables encountered and their values. - $this->collect_variables(); + $this->collect_variables( $stackPtr ); // Then find all dynamic calls, and report them. - $this->find_dynamic_calls(); + $this->find_dynamic_calls( $stackPtr ); } /** * Finds any variable-definitions in the file being processed and stores them * internally in a private array. * + * @param int $stackPtr The position in the stack where the token was found. + * * @return void */ - private function collect_variables() { + private function collect_variables( $stackPtr ) { - $current_var_name = $this->tokens[ $this->stackPtr ]['content']; + $current_var_name = $this->tokens[ $stackPtr ]['content']; /* * Find assignments ( $foo = "bar"; ) by finding all non-whitespaces, * and checking if the first one is T_EQUAL. */ - $t_item_key = $this->phpcsFile->findNext( - Tokens::$emptyTokens, - $this->stackPtr + 1, - null, - true, - null, - true - ); - + $t_item_key = $this->phpcsFile->findNext( Tokens::$emptyTokens, $stackPtr + 1, null, true ); if ( $t_item_key === false || $this->tokens[ $t_item_key ]['code'] !== T_EQUAL ) { return; } @@ -115,10 +119,13 @@ private function collect_variables() { /* * Find assignments which only assign a plain text string. */ - $end_of_statement = $this->phpcsFile->findNext( [ T_SEMICOLON, T_CLOSE_TAG ], ( $t_item_key + 1 ) ); - $value_ptr = null; + $end_of_statement = $this->phpcsFile->findEndOfStatement( ( $t_item_key + 1 ) ); + if ( isset( $this->inclusiveStopPoints[ $this->tokens[ $end_of_statement ]['code'] ] ) === true ) { + --$end_of_statement; + } - for ( $i = $t_item_key + 1; $i < $end_of_statement; $i++ ) { + $value_ptr = null; + for ( $i = $t_item_key + 1; $i <= $end_of_statement; $i++ ) { if ( isset( Tokens::$emptyTokens[ $this->tokens[ $i ]['code'] ] ) === true ) { continue; } @@ -158,9 +165,11 @@ private function collect_variables() { * * Report on this when found, using the name of the function in the message. * + * @param int $stackPtr The position in the stack where the token was found. + * * @return void */ - private function find_dynamic_calls() { + private function find_dynamic_calls( $stackPtr ) { // No variables detected; no basis for doing anything. if ( empty( $this->variables_arr ) ) { return; @@ -170,20 +179,47 @@ private function find_dynamic_calls() { * If variable is not found in our registry of variables, do nothing, as we cannot be * sure that the function being called is one of the disallowed ones. */ - if ( ! isset( $this->variables_arr[ $this->tokens[ $this->stackPtr ]['content'] ] ) ) { + if ( ! isset( $this->variables_arr[ $this->tokens[ $stackPtr ]['content'] ] ) ) { return; } /* * Check if we have an '(' next. */ - $next = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $this->stackPtr + 1 ), null, true ); + $next = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $stackPtr + 1 ), null, true ); if ( $next === false || $this->tokens[ $next ]['code'] !== T_OPEN_PARENTHESIS ) { return; } $message = 'Dynamic calling is not recommended in the case of %s().'; - $data = [ $this->variables_arr[ $this->tokens[ $this->stackPtr ]['content'] ] ]; - $this->phpcsFile->addError( $message, $this->stackPtr, 'DynamicCalls', $data ); + $data = [ $this->variables_arr[ $this->tokens[ $stackPtr ]['content'] ] ]; + $this->phpcsFile->addError( $message, $stackPtr, 'DynamicCalls', $data ); + } + + /** + * Provide the version number in which the sniff was deprecated. + * + * @return string + */ + public function getDeprecationVersion() { + return 'VIP-Coding-Standard v3.1.0'; + } + + /** + * Provide the version number in which the sniff will be removed. + * + * @return string + */ + public function getRemovalVersion() { + return 'VIP-Coding-Standard v4.0.0'; + } + + /** + * Provide a custom message to display with the deprecation. + * + * @return string + */ + public function getDeprecationMessage() { + return ''; } } diff --git a/WordPressVIPMinimum/Sniffs/Functions/RestrictedFunctionsSniff.php b/WordPressVIPMinimum/Sniffs/Functions/RestrictedFunctionsSniff.php index 4513140c..bff5ce8d 100644 --- a/WordPressVIPMinimum/Sniffs/Functions/RestrictedFunctionsSniff.php +++ b/WordPressVIPMinimum/Sniffs/Functions/RestrictedFunctionsSniff.php @@ -3,6 +3,8 @@ * WordPressVIPMinimum Coding Standard. * * @package VIPCS\WordPressVIPMinimum + * @link https://github.com/Automattic/VIP-Coding-Standards + * @license https://opensource.org/license/gpl-2-0 GPL-2.0 */ namespace WordPressVIPMinimum\Sniffs\Functions; @@ -18,7 +20,7 @@ class RestrictedFunctionsSniff extends AbstractFunctionRestrictionsSniff { /** * Groups of functions to restrict. * - * @return array + * @return array|array>> */ public function getGroups() { @@ -301,47 +303,27 @@ public function getGroups() { * @return bool */ public function is_targetted_token( $stackPtr ) { - // Exclude function definitions, class methods, and namespaced calls. - if ( $this->tokens[ $stackPtr ]['code'] === \T_STRING && isset( $this->tokens[ $stackPtr - 1 ] ) ) { - // Check if this is really a function. - $next = $this->phpcsFile->findNext( Tokens::$emptyTokens, $stackPtr + 1, null, true ); - if ( $next !== false && $this->tokens[ $next ]['code'] !== T_OPEN_PARENTHESIS ) { - return false; - } + if ( empty( $this->groups[ $this->tokens[ $stackPtr ]['content'] ]['object_var'] ) ) { + return parent::is_targetted_token( $stackPtr ); + } - $prev = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, $stackPtr - 1, null, true ); - if ( $prev !== false ) { + // Start difference to parent class method. + // Check to see if the token is a method call on a specific object variable. + $next = $this->phpcsFile->findNext( Tokens::$emptyTokens, $stackPtr + 1, null, true ); + if ( $next === false || $this->tokens[ $next ]['code'] !== T_OPEN_PARENTHESIS ) { + return false; + } - // Start difference to parent class method. - // Check to see if function is a method on a specific object variable. - if ( ! empty( $this->groups[ $this->tokens[ $stackPtr ]['content'] ]['object_var'] ) ) { - $prevPrev = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, $stackPtr - 2, null, true ); + $prev = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, $stackPtr - 1, null, true ); + if ( $this->tokens[ $prev ]['code'] !== T_OBJECT_OPERATOR + && $this->tokens[ $prev ]['code'] !== T_NULLSAFE_OBJECT_OPERATOR + ) { + return false; + } - return $this->tokens[ $prev ]['code'] === \T_OBJECT_OPERATOR && isset( $this->groups[ $this->tokens[ $stackPtr ]['content'] ]['object_var'][ $this->tokens[ $prevPrev ]['content'] ] ); - } // End difference to parent class method. + $prevPrev = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, $prev - 1, null, true ); - // Skip sniffing if calling a same-named method, or on function definitions. - $skipped = [ - \T_FUNCTION => \T_FUNCTION, - \T_CLASS => \T_CLASS, - \T_AS => \T_AS, // Use declaration alias. - \T_DOUBLE_COLON => \T_DOUBLE_COLON, - \T_OBJECT_OPERATOR => \T_OBJECT_OPERATOR, - \T_NEW => \T_NEW, - ]; - if ( isset( $skipped[ $this->tokens[ $prev ]['code'] ] ) ) { - return false; - } - // Skip namespaced functions, ie: `\foo\bar()` not `\bar()`. - if ( $this->tokens[ $prev ]['code'] === \T_NS_SEPARATOR ) { - $pprev = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, $prev - 1, null, true ); - if ( $pprev !== false && $this->tokens[ $pprev ]['code'] === \T_STRING ) { - return false; - } - } - } - return true; - } - return false; + return $this->tokens[ $prevPrev ]['code'] === T_VARIABLE + && isset( $this->groups[ $this->tokens[ $stackPtr ]['content'] ]['object_var'][ $this->tokens[ $prevPrev ]['content'] ] ); } } diff --git a/WordPressVIPMinimum/Sniffs/Functions/StripTagsSniff.php b/WordPressVIPMinimum/Sniffs/Functions/StripTagsSniff.php index cf29e454..5c1bb62b 100644 --- a/WordPressVIPMinimum/Sniffs/Functions/StripTagsSniff.php +++ b/WordPressVIPMinimum/Sniffs/Functions/StripTagsSniff.php @@ -3,10 +3,13 @@ * WordPressVIPMinimum Coding Standard. * * @package VIPCS\WordPressVIPMinimum + * @link https://github.com/Automattic/VIP-Coding-Standards + * @license https://opensource.org/license/gpl-2-0 GPL-2.0 */ namespace WordPressVIPMinimum\Sniffs\Functions; +use PHPCSUtils\Utils\PassedParameters; use WordPressCS\WordPress\AbstractFunctionParameterSniff; /** @@ -26,10 +29,7 @@ class StripTagsSniff extends AbstractFunctionParameterSniff { /** * Functions this sniff is looking for. * - * @var array The only requirement for this array is that the top level - * array keys are the names of the functions you're looking for. - * Other than that, the array can have arbitrary content - * depending on your needs. + * @var array Key is the function name, value irrelevant. */ protected $target_functions = [ 'strip_tags' => true, @@ -44,16 +44,60 @@ class StripTagsSniff extends AbstractFunctionParameterSniff { * in lowercase. * @param array $parameters Array with information about the parameters. * - * @return int|void Integer stack pointer to skip forward or void to continue - * normal file processing. + * @return void */ public function process_parameters( $stackPtr, $group_name, $matched_content, $parameters ) { - if ( count( $parameters ) === 1 ) { - $message = '`strip_tags()` does not strip CSS and JS in between the script and style tags. Use `wp_strip_all_tags()` to strip all tags.'; - $this->phpcsFile->addWarning( $message, $stackPtr, 'StripTagsOneParameter' ); - } elseif ( isset( $parameters[2] ) ) { + $string_param = PassedParameters::getParameterFromStack( $parameters, 1, 'string' ); + $allowed_tags_param = PassedParameters::getParameterFromStack( $parameters, 2, 'allowed_tags' ); + + if ( $string_param !== false && $allowed_tags_param === false ) { + $this->add_warning( $stackPtr, 'StripTagsOneParameter' ); + } elseif ( $allowed_tags_param !== false ) { $message = '`strip_tags()` does not strip CSS and JS in between the script and style tags. Use `wp_kses()` instead to allow only the HTML you need.'; $this->phpcsFile->addWarning( $message, $stackPtr, 'StripTagsTwoParameters' ); + } else { + $this->add_warning( $stackPtr ); } } + + /** + * Process the function if no parameters were found. + * + * @param int $stackPtr The position of the current token in the stack. + * @param string $group_name The name of the group which was matched. + * @param string $matched_content The token content (function name) which was matched + * in lowercase. + * + * @return void + */ + public function process_no_parameters( $stackPtr, $group_name, $matched_content ) { + $this->add_warning( $stackPtr ); + } + + /** + * Process the function if it is used as a first class callable. + * + * @param int $stackPtr The position of the current token in the stack. + * @param string $group_name The name of the group which was matched. + * @param string $matched_content The token content (function name) which was matched + * in lowercase. + * + * @return void + */ + public function process_first_class_callable( $stackPtr, $group_name, $matched_content ) { + $this->add_warning( $stackPtr ); + } + + /** + * Add a warning if the function is used at all. + * + * @param int $stackPtr The position of the current token in the stack. + * @param string $error_code Error code to use for the warning. + * + * @return void + */ + private function add_warning( $stackPtr, $error_code = 'Used' ) { + $message = '`strip_tags()` does not strip CSS and JS in between the script and style tags. Use `wp_strip_all_tags()` to strip all tags.'; + $this->phpcsFile->addWarning( $message, $stackPtr, $error_code ); + } } diff --git a/WordPressVIPMinimum/Sniffs/Hooks/AlwaysReturnInFilterSniff.php b/WordPressVIPMinimum/Sniffs/Hooks/AlwaysReturnInFilterSniff.php index 7bf70d9a..dea2730b 100644 --- a/WordPressVIPMinimum/Sniffs/Hooks/AlwaysReturnInFilterSniff.php +++ b/WordPressVIPMinimum/Sniffs/Hooks/AlwaysReturnInFilterSniff.php @@ -3,6 +3,8 @@ * WordPressVIPMinimum Coding Standard. * * @package VIPCS\WordPressVIPMinimum + * @link https://github.com/Automattic/VIP-Coding-Standards + * @license https://opensource.org/license/gpl-2-0 GPL-2.0 */ namespace WordPressVIPMinimum\Sniffs\Hooks; @@ -10,6 +12,7 @@ use PHP_CodeSniffer\Util\Tokens; use PHPCSUtils\Utils\Arrays; use PHPCSUtils\Utils\FunctionDeclarations; +use PHPCSUtils\Utils\TextStrings; use WordPressVIPMinimum\Sniffs\Sniff; /** @@ -27,7 +30,7 @@ class AlwaysReturnInFilterSniff extends Sniff { /** * Returns the token types that this sniff is interested in. * - * @return array(int) + * @return array */ public function register() { return [ T_STRING ]; @@ -76,8 +79,11 @@ public function process_token( $stackPtr ) { return; } - if ( $this->tokens[ $callbackPtr ]['code'] === 'PHPCS_T_CLOSURE' ) { + if ( $this->tokens[ $callbackPtr ]['code'] === T_CLOSURE ) { $this->processFunctionBody( $callbackPtr ); + } elseif ( $this->tokens[ $callbackPtr ]['code'] === T_FN ) { + // Arrow functions always return a value implicitly. No check needed. + return; } elseif ( $this->tokens[ $callbackPtr ]['code'] === T_ARRAY || $this->tokens[ $callbackPtr ]['code'] === T_OPEN_SHORT_ARRAY ) { @@ -131,7 +137,7 @@ private function processArray( $stackPtr ) { */ private function processString( $stackPtr, $start = 0, $end = null ) { - $callbackFunctionName = substr( $this->tokens[ $stackPtr ]['content'], 1, -1 ); + $callbackFunctionName = TextStrings::stripQuotes( $this->tokens[ $stackPtr ]['content'] ); $callbackFunctionPtr = $this->phpcsFile->findNext( T_STRING, @@ -163,10 +169,10 @@ private function processFunction( $stackPtr, $start = 0, $end = null ) { $functionName = $this->tokens[ $stackPtr ]['content']; $offset = $start; - while ( $this->phpcsFile->findNext( [ T_FUNCTION ], $offset, $end ) !== false ) { - $functionStackPtr = $this->phpcsFile->findNext( [ T_FUNCTION ], $offset, $end ); - $functionNamePtr = $this->phpcsFile->findNext( Tokens::$emptyTokens, $functionStackPtr + 1, null, true, null, true ); - if ( $this->tokens[ $functionNamePtr ]['code'] === T_STRING && $this->tokens[ $functionNamePtr ]['content'] === $functionName ) { + // phpcs:ignore Generic.CodeAnalysis.AssignmentInCondition.FoundInWhileCondition -- Valid usage. + while ( ( $functionStackPtr = $this->phpcsFile->findNext( T_FUNCTION, $offset, $end ) ) !== false ) { + $declaredName = FunctionDeclarations::getName( $this->phpcsFile, $functionStackPtr ); + if ( $declaredName === $functionName ) { $this->processFunctionBody( $functionStackPtr ); return; } @@ -240,9 +246,15 @@ private function processFunctionBody( $stackPtr ) { } if ( $outsideConditionalReturn === 0 ) { - $message = 'Please, make sure that a callback to `%s` filter is always returning some value.'; - $data = [ $filterName ]; - $this->phpcsFile->addError( $message, $functionBodyScopeStart, 'MissingReturnStatement', $data ); + if ( $this->hasTerminatingStatement( $functionBodyScopeStart, $functionBodyScopeEnd ) ) { + $message = 'The callback for the `%s` filter uses a terminating statement (`exit`, `die`, or `throw`) instead of returning a value. Filter callbacks should always return a value.'; + $data = [ $filterName ]; + $this->phpcsFile->addWarning( $message, $functionBodyScopeStart, 'TerminatingInsteadOfReturn', $data ); + } else { + $message = 'Please, make sure that a callback to `%s` filter is always returning some value.'; + $data = [ $filterName ]; + $this->phpcsFile->addError( $message, $functionBodyScopeStart, 'MissingReturnStatement', $data ); + } } } @@ -262,7 +274,7 @@ private function isInsideIfConditonal( $stackPtr ) { } // Similar case may be a conditional closure. - if ( end( $this->tokens[ $stackPtr ]['conditions'] ) === 'PHPCS_T_CLOSURE' ) { + if ( end( $this->tokens[ $stackPtr ]['conditions'] ) === T_CLOSURE ) { return false; } @@ -282,6 +294,25 @@ private function isInsideIfConditonal( $stackPtr ) { return false; } + /** + * Check whether the function body contains an exit, die, or throw statement. + * + * @param int $scopeStart The scope opener of the function body. + * @param int $scopeEnd The scope closer of the function body. + * + * @return bool + */ + private function hasTerminatingStatement( $scopeStart, $scopeEnd ) { + + $terminatingPtr = $this->phpcsFile->findNext( + [ T_EXIT, T_THROW ], + $scopeStart + 1, + $scopeEnd + ); + + return $terminatingPtr !== false; + } + /** * Is the token returning void * @@ -292,7 +323,7 @@ private function isInsideIfConditonal( $stackPtr ) { private function isReturningVoid( $stackPtr ) { $nextToReturnTokenPtr = $this->phpcsFile->findNext( - [ Tokens::$emptyTokens ], + Tokens::$emptyTokens, $stackPtr + 1, null, true diff --git a/WordPressVIPMinimum/Sniffs/Hooks/PreGetPostsSniff.php b/WordPressVIPMinimum/Sniffs/Hooks/PreGetPostsSniff.php index 88d49e62..3334fc4c 100644 --- a/WordPressVIPMinimum/Sniffs/Hooks/PreGetPostsSniff.php +++ b/WordPressVIPMinimum/Sniffs/Hooks/PreGetPostsSniff.php @@ -3,6 +3,8 @@ * WordPressVIPMinimum Coding Standard. * * @package VIPCS\WordPressVIPMinimum + * @link https://github.com/Automattic/VIP-Coding-Standards + * @license https://opensource.org/license/gpl-2-0 GPL-2.0 */ namespace WordPressVIPMinimum\Sniffs\Hooks; @@ -21,7 +23,7 @@ class PreGetPostsSniff extends Sniff { /** * Returns the token types that this sniff is interested in. * - * @return array(int) + * @return array */ public function register() { return [ T_STRING ]; @@ -76,7 +78,7 @@ public function process_token( $stackPtr ) { return; } - if ( $this->tokens[ $callbackPtr ]['code'] === 'PHPCS_T_CLOSURE' ) { + if ( $this->tokens[ $callbackPtr ]['code'] === T_CLOSURE ) { $this->processClosure( $callbackPtr ); } elseif ( $this->tokens[ $callbackPtr ]['code'] === T_ARRAY || $this->tokens[ $callbackPtr ]['code'] === T_OPEN_SHORT_ARRAY @@ -388,7 +390,7 @@ private function isWPQueryMethodCall( $stackPtr, $method = null ) { true ); - if ( ! $next || $this->tokens[ $next ]['type'] !== 'T_OBJECT_OPERATOR' ) { + if ( ! $next || $this->tokens[ $next ]['code'] !== T_OBJECT_OPERATOR ) { return false; } diff --git a/WordPressVIPMinimum/Sniffs/Hooks/RestrictedHooksSniff.php b/WordPressVIPMinimum/Sniffs/Hooks/RestrictedHooksSniff.php index d15dac7d..2ced308b 100644 --- a/WordPressVIPMinimum/Sniffs/Hooks/RestrictedHooksSniff.php +++ b/WordPressVIPMinimum/Sniffs/Hooks/RestrictedHooksSniff.php @@ -3,10 +3,16 @@ * WordPressVIPMinimum Coding Standard. * * @package VIPCS\WordPressVIPMinimum + * @link https://github.com/Automattic/VIP-Coding-Standards + * @license https://opensource.org/license/gpl-2-0 GPL-2.0 */ namespace WordPressVIPMinimum\Sniffs\Hooks; +use PHP_CodeSniffer\Util\Tokens; +use PHPCSUtils\Utils\MessageHelper; +use PHPCSUtils\Utils\PassedParameters; +use PHPCSUtils\Utils\TextStrings; use WordPressCS\WordPress\AbstractFunctionParameterSniff; /** @@ -26,10 +32,7 @@ class RestrictedHooksSniff extends AbstractFunctionParameterSniff { /** * Functions this sniff is looking for. * - * @var array The only requirement for this array is that the top level - * array keys are the names of the functions you're looking for. - * Other than that, the array can have arbitrary content - * depending on your needs. + * @var array Key is the function name, value irrelevant. */ protected $target_functions = [ 'add_filter' => true, @@ -39,12 +42,12 @@ class RestrictedHooksSniff extends AbstractFunctionParameterSniff { /** * List of restricted filters by groups. * - * @var array + * @var array>> */ private $restricted_hook_groups = [ 'upload_mimes' => [ // TODO: This error message needs a link to the VIP Documentation, see https://github.com/Automattic/VIP-Coding-Standards/issues/235. - 'type' => 'Warning', + 'type' => 'warning', 'msg' => 'Please ensure that the mimes being filtered do not include insecure types (i.e. SVG, SWF, etc.). Manual inspection required.', 'hooks' => [ 'upload_mimes', @@ -52,7 +55,7 @@ class RestrictedHooksSniff extends AbstractFunctionParameterSniff { ], 'http_request' => [ // https://docs.wpvip.com/technical-references/code-quality-and-best-practices/retrieving-remote-data/. - 'type' => 'Warning', + 'type' => 'warning', 'msg' => 'Please ensure that the timeout being filtered is not greater than 3s since remote requests require the user to wait for completion before the rest of the page will load. Manual inspection required.', 'hooks' => [ 'http_request_timeout', @@ -61,7 +64,7 @@ class RestrictedHooksSniff extends AbstractFunctionParameterSniff { ], 'robotstxt' => [ // https://docs.wpvip.com/how-tos/modify-the-robots-txt-file/. - 'type' => 'Warning', + 'type' => 'warning', 'msg' => 'Don\'t forget to flush the robots.txt cache by going to Settings > Reading and toggling the privacy settings.', 'hooks' => [ 'do_robotstxt', @@ -83,11 +86,23 @@ class RestrictedHooksSniff extends AbstractFunctionParameterSniff { * normal file processing. */ public function process_parameters( $stackPtr, $group_name, $matched_content, $parameters ) { + $hook_name_param = PassedParameters::getParameterFromStack( $parameters, 1, 'hook_name' ); + if ( $hook_name_param === false ) { + // Missing required parameter. Nothing to examine. Bow out. + return; + } + + $normalized_hook_name = $this->normalize_hook_name_from_parameter( $hook_name_param ); + if ( $normalized_hook_name === '' ) { + // Dynamic hook name. Cannot reliably determine if it's one of the targets. Bow out. + return; + } + foreach ( $this->restricted_hook_groups as $group => $group_args ) { foreach ( $group_args['hooks'] as $hook ) { - if ( $this->normalize_hook_name_from_parameter( $parameters[1] ) === $hook ) { - $addMethod = 'add' . $group_args['type']; - $this->phpcsFile->{$addMethod}( $group_args['msg'], $stackPtr, $hook ); + if ( $normalized_hook_name === $hook ) { + $isError = ( $group_args['type'] === 'error' ); + MessageHelper::addMessage( $this->phpcsFile, $group_args['msg'], $stackPtr, $isError, $hook ); } } } @@ -98,31 +113,27 @@ public function process_parameters( $stackPtr, $group_name, $matched_content, $p * * @param array $parameter Array with information about a parameter. * - * @return string Normalized hook name. + * @return string Normalized hook name or an empty string if the hook name could not be determined. */ private function normalize_hook_name_from_parameter( $parameter ) { - // If concatenation is found, build hook name. - $concat_ptr = $this->phpcsFile->findNext( - T_STRING_CONCAT, - $parameter['start'], - $parameter['end'], - false, - null, - true - ); + $allowed_tokens = Tokens::$emptyTokens; + $allowed_tokens += [ + T_STRING_CONCAT => T_STRING_CONCAT, + T_CONSTANT_ENCAPSED_STRING => T_CONSTANT_ENCAPSED_STRING, + ]; - if ( $concat_ptr ) { - $hook_name = ''; - for ( $i = $parameter['start'] + 1; $i < $parameter['end']; $i++ ) { - if ( $this->tokens[ $i ]['code'] === T_CONSTANT_ENCAPSED_STRING ) { - $hook_name .= str_replace( [ "'", '"' ], '', $this->tokens[ $i ]['content'] ); - } + $has_disallowed_token = $this->phpcsFile->findNext( $allowed_tokens, $parameter['start'], ( $parameter['end'] + 1 ), true ); + if ( $has_disallowed_token !== false ) { + return ''; + } + + $hook_name = ''; + for ( $i = $parameter['start']; $i <= $parameter['end']; $i++ ) { + if ( $this->tokens[ $i ]['code'] === T_CONSTANT_ENCAPSED_STRING ) { + $hook_name .= TextStrings::stripQuotes( $this->tokens[ $i ]['content'] ); } - } else { - $hook_name = $parameter['raw']; } - // Remove quotes (double and single), and use lowercase. - return strtolower( str_replace( [ "'", '"' ], '', $hook_name ) ); + return $hook_name; } } diff --git a/WordPressVIPMinimum/Sniffs/JS/DangerouslySetInnerHTMLSniff.php b/WordPressVIPMinimum/Sniffs/JS/DangerouslySetInnerHTMLSniff.php index ce68a775..50ba25d8 100644 --- a/WordPressVIPMinimum/Sniffs/JS/DangerouslySetInnerHTMLSniff.php +++ b/WordPressVIPMinimum/Sniffs/JS/DangerouslySetInnerHTMLSniff.php @@ -1,12 +1,15 @@ */ public function register() { return [ @@ -67,4 +70,31 @@ public function process_token( $stackPtr ) { $data = [ $this->tokens[ $stackPtr ]['content'] ]; $this->phpcsFile->addError( $message, $stackPtr, 'Found', $data ); } + + /** + * Provide the version number in which the sniff was deprecated. + * + * @return string + */ + public function getDeprecationVersion() { + return 'VIP-Coding-Standard v3.1.0'; + } + + /** + * Provide the version number in which the sniff will be removed. + * + * @return string + */ + public function getRemovalVersion() { + return 'VIP-Coding-Standard v4.0.0'; + } + + /** + * Provide a custom message to display with the deprecation. + * + * @return string + */ + public function getDeprecationMessage() { + return 'Support for scanning JavaScript files will be removed from PHP_CodeSniffer, so this sniff is no longer viable.'; + } } diff --git a/WordPressVIPMinimum/Sniffs/JS/HTMLExecutingFunctionsSniff.php b/WordPressVIPMinimum/Sniffs/JS/HTMLExecutingFunctionsSniff.php index 0a106f22..cf8739a0 100644 --- a/WordPressVIPMinimum/Sniffs/JS/HTMLExecutingFunctionsSniff.php +++ b/WordPressVIPMinimum/Sniffs/JS/HTMLExecutingFunctionsSniff.php @@ -1,12 +1,15 @@ */ public $HTMLExecutingFunctions = [ 'after' => 'content', // jQuery. @@ -52,7 +55,7 @@ class HTMLExecutingFunctionsSniff extends Sniff { /** * Returns an array of tokens this test wants to listen for. * - * @return array + * @return array */ public function register() { return [ @@ -128,4 +131,31 @@ public function process_token( $stackPtr ) { } } } + + /** + * Provide the version number in which the sniff was deprecated. + * + * @return string + */ + public function getDeprecationVersion() { + return 'VIP-Coding-Standard v3.1.0'; + } + + /** + * Provide the version number in which the sniff will be removed. + * + * @return string + */ + public function getRemovalVersion() { + return 'VIP-Coding-Standard v4.0.0'; + } + + /** + * Provide a custom message to display with the deprecation. + * + * @return string + */ + public function getDeprecationMessage() { + return 'Support for scanning JavaScript files will be removed from PHP_CodeSniffer, so this sniff is no longer viable.'; + } } diff --git a/WordPressVIPMinimum/Sniffs/JS/InnerHTMLSniff.php b/WordPressVIPMinimum/Sniffs/JS/InnerHTMLSniff.php index f35f9894..652feffc 100644 --- a/WordPressVIPMinimum/Sniffs/JS/InnerHTMLSniff.php +++ b/WordPressVIPMinimum/Sniffs/JS/InnerHTMLSniff.php @@ -1,12 +1,15 @@ */ public function register() { return [ @@ -81,4 +84,31 @@ public function process_token( $stackPtr ) { $this->phpcsFile->addWarning( $message, $stackPtr, 'Found', $data ); } } + + /** + * Provide the version number in which the sniff was deprecated. + * + * @return string + */ + public function getDeprecationVersion() { + return 'VIP-Coding-Standard v3.1.0'; + } + + /** + * Provide the version number in which the sniff will be removed. + * + * @return string + */ + public function getRemovalVersion() { + return 'VIP-Coding-Standard v4.0.0'; + } + + /** + * Provide a custom message to display with the deprecation. + * + * @return string + */ + public function getDeprecationMessage() { + return 'Support for scanning JavaScript files will be removed from PHP_CodeSniffer, so this sniff is no longer viable.'; + } } diff --git a/WordPressVIPMinimum/Sniffs/JS/StringConcatSniff.php b/WordPressVIPMinimum/Sniffs/JS/StringConcatSniff.php index 049b35c0..b3d317cb 100644 --- a/WordPressVIPMinimum/Sniffs/JS/StringConcatSniff.php +++ b/WordPressVIPMinimum/Sniffs/JS/StringConcatSniff.php @@ -1,12 +1,15 @@ */ public function register() { return [ @@ -63,8 +66,8 @@ public function process_token( $stackPtr ) { /** * Consolidated violation. * - * @param int $stackPtr The position of the current token in the stack passed in $tokens. - * @param array $data Replacements for the error message. + * @param int $stackPtr The position of the current token in the stack passed in $tokens. + * @param array $data Replacements for the error message. * * @return void */ @@ -72,4 +75,31 @@ private function addFoundError( $stackPtr, array $data ) { $message = 'HTML string concatenation detected, this is a security risk, use DOM node construction or a templating language instead: %s.'; $this->phpcsFile->addError( $message, $stackPtr, 'Found', $data ); } + + /** + * Provide the version number in which the sniff was deprecated. + * + * @return string + */ + public function getDeprecationVersion() { + return 'VIP-Coding-Standard v3.1.0'; + } + + /** + * Provide the version number in which the sniff will be removed. + * + * @return string + */ + public function getRemovalVersion() { + return 'VIP-Coding-Standard v4.0.0'; + } + + /** + * Provide a custom message to display with the deprecation. + * + * @return string + */ + public function getDeprecationMessage() { + return 'Support for scanning JavaScript files will be removed from PHP_CodeSniffer, so this sniff is no longer viable.'; + } } diff --git a/WordPressVIPMinimum/Sniffs/JS/StrippingTagsSniff.php b/WordPressVIPMinimum/Sniffs/JS/StrippingTagsSniff.php index 88fef947..1b05ecff 100644 --- a/WordPressVIPMinimum/Sniffs/JS/StrippingTagsSniff.php +++ b/WordPressVIPMinimum/Sniffs/JS/StrippingTagsSniff.php @@ -1,12 +1,15 @@ */ public function register() { return [ @@ -70,4 +73,31 @@ public function process_token( $stackPtr ) { $this->phpcsFile->addError( $message, $stackPtr, 'VulnerableTagStripping' ); } } + + /** + * Provide the version number in which the sniff was deprecated. + * + * @return string + */ + public function getDeprecationVersion() { + return 'VIP-Coding-Standard v3.1.0'; + } + + /** + * Provide the version number in which the sniff will be removed. + * + * @return string + */ + public function getRemovalVersion() { + return 'VIP-Coding-Standard v4.0.0'; + } + + /** + * Provide a custom message to display with the deprecation. + * + * @return string + */ + public function getDeprecationMessage() { + return 'Support for scanning JavaScript files will be removed from PHP_CodeSniffer, so this sniff is no longer viable.'; + } } diff --git a/WordPressVIPMinimum/Sniffs/JS/WindowSniff.php b/WordPressVIPMinimum/Sniffs/JS/WindowSniff.php index d54d5690..2a9d7946 100644 --- a/WordPressVIPMinimum/Sniffs/JS/WindowSniff.php +++ b/WordPressVIPMinimum/Sniffs/JS/WindowSniff.php @@ -1,12 +1,15 @@ */ public function register() { return [ @@ -38,7 +41,7 @@ public function register() { /** * List of window properties that need to be flagged. * - * @var array + * @var array> */ private $windowProperties = [ 'location' => [ @@ -125,4 +128,31 @@ public function process_token( $stackPtr ) { $message = 'Data from JS global "%s" may contain user-supplied values and should be sanitized before output to prevent XSS.'; $this->phpcsFile->addError( $message, $stackPtr, $nextNextToken, $data ); } + + /** + * Provide the version number in which the sniff was deprecated. + * + * @return string + */ + public function getDeprecationVersion() { + return 'VIP-Coding-Standard v3.1.0'; + } + + /** + * Provide the version number in which the sniff will be removed. + * + * @return string + */ + public function getRemovalVersion() { + return 'VIP-Coding-Standard v4.0.0'; + } + + /** + * Provide a custom message to display with the deprecation. + * + * @return string + */ + public function getDeprecationMessage() { + return 'Support for scanning JavaScript files will be removed from PHP_CodeSniffer, so this sniff is no longer viable.'; + } } diff --git a/WordPressVIPMinimum/Sniffs/Performance/CacheValueOverrideSniff.php b/WordPressVIPMinimum/Sniffs/Performance/CacheValueOverrideSniff.php index 8d39c1a2..4a06a7f1 100644 --- a/WordPressVIPMinimum/Sniffs/Performance/CacheValueOverrideSniff.php +++ b/WordPressVIPMinimum/Sniffs/Performance/CacheValueOverrideSniff.php @@ -3,51 +3,68 @@ * WordPressVIPMinimum Coding Standard. * * @package VIPCS\WordPressVIPMinimum + * @link https://github.com/Automattic/VIP-Coding-Standards + * @license https://opensource.org/license/gpl-2-0 GPL-2.0 */ namespace WordPressVIPMinimum\Sniffs\Performance; use PHP_CodeSniffer\Util\Tokens; -use WordPressVIPMinimum\Sniffs\Sniff; +use PHPCSUtils\Tokens\Collections; +use PHPCSUtils\Utils\Conditions; +use WordPressCS\WordPress\AbstractFunctionRestrictionsSniff; /** * This sniff check whether a cached value is being overridden. */ -class CacheValueOverrideSniff extends Sniff { +class CacheValueOverrideSniff extends AbstractFunctionRestrictionsSniff { /** - * Returns the token types that this sniff is interested in. + * Groups of functions to restrict. * - * @return array(int) + * @return array>> */ - public function register() { - return [ T_STRING ]; + public function getGroups() { + return [ + 'wp_cache_get' => [ + 'functions' => [ 'wp_cache_get' ], + ], + ]; } - /** - * Processes the tokens that this sniff is interested in. + * Process a matched token. * - * @param int $stackPtr The position in the stack where the token was found. + * @param int $stackPtr The position of the current token in the stack. + * @param string $group_name The name of the group which was matched. + * @param string $matched_content The token content (function name) which was matched + * in lowercase. * * @return void */ - public function process_token( $stackPtr ) { - - $functionName = $this->tokens[ $stackPtr ]['content']; + public function process_matched_token( $stackPtr, $group_name, $matched_content ) { + $openBracket = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $stackPtr + 1 ), null, true ); + if ( $openBracket === false || isset( $this->tokens[ $openBracket ]['parenthesis_closer'] ) === false ) { + // Import use statement for function or parse error/live coding. Ignore. + return; + } - if ( $functionName !== 'wp_cache_get' ) { - // Not a function we are looking for. + $closeBracket = $this->tokens[ $openBracket ]['parenthesis_closer']; + $firstNonEmpty = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $openBracket + 1 ), null, true ); + $nextNonEmpty = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $firstNonEmpty + 1 ), null, true ); + if ( $nextNonEmpty === false ) { + // Parse error/live coding. Ignore. return; } - if ( $this->isFunctionCall( $stackPtr ) === false ) { - // Not a function call. + if ( $this->tokens[ $firstNonEmpty ]['code'] === T_ELLIPSIS + && $nextNonEmpty === $closeBracket + ) { + // First class callable. Ignore. return; } $variablePos = $this->isVariableAssignment( $stackPtr ); - if ( $variablePos === false ) { // Not a variable assignment. return; @@ -56,56 +73,50 @@ public function process_token( $stackPtr ) { $variableToken = $this->tokens[ $variablePos ]; $variableName = $variableToken['content']; - // Find the next non-empty token. - $openBracket = $this->phpcsFile->findNext( Tokens::$emptyTokens, $stackPtr + 1, null, true ); + // Figure out the scope we need to search in. + $searchEnd = $this->phpcsFile->numTokens; + $functionPtr = Conditions::getLastCondition( $this->phpcsFile, $stackPtr, [ T_FUNCTION, T_CLOSURE ] ); + if ( $functionPtr !== false && isset( $this->tokens[ $functionPtr ]['scope_closer'] ) ) { + $searchEnd = $this->tokens[ $functionPtr ]['scope_closer']; + } - // Find the closing bracket. - $closeBracket = $this->tokens[ $openBracket ]['parenthesis_closer']; + $nextVariableOccurrence = false; + for ( $i = $closeBracket + 1; $i < $searchEnd; $i++ ) { + if ( $this->tokens[ $i ]['code'] === T_VARIABLE && $this->tokens[ $i ]['content'] === $variableName ) { + $nextVariableOccurrence = $i; + break; + } + + // Skip over any and all closed scopes. + if ( isset( Collections::closedScopes()[ $this->tokens[ $i ]['code'] ] ) ) { + if ( isset( $this->tokens[ $i ]['scope_closer'] ) ) { + $i = $this->tokens[ $i ]['scope_closer']; + } + } + } - $nextVariableOccurrence = $this->phpcsFile->findNext( T_VARIABLE, $closeBracket + 1, null, false, $variableName ); + if ( $nextVariableOccurrence === false ) { + return; + } - $rightAfterNextVariableOccurence = $this->phpcsFile->findNext( Tokens::$emptyTokens, $nextVariableOccurrence + 1, null, true, null, true ); + $rightAfterNextVariableOccurence = $this->phpcsFile->findNext( Tokens::$emptyTokens, $nextVariableOccurrence + 1, $searchEnd, true, null, true ); - if ( $this->tokens[ $rightAfterNextVariableOccurence ]['code'] !== T_EQUAL ) { + if ( $rightAfterNextVariableOccurence === false + || $this->tokens[ $rightAfterNextVariableOccurence ]['code'] !== T_EQUAL + ) { // Not a value override. return; } - $valueAfterEqualSign = $this->phpcsFile->findNext( Tokens::$emptyTokens, $rightAfterNextVariableOccurence + 1, null, true, null, true ); + $valueAfterEqualSign = $this->phpcsFile->findNext( Tokens::$emptyTokens, $rightAfterNextVariableOccurence + 1, $searchEnd, true, null, true ); - if ( $this->tokens[ $valueAfterEqualSign ]['code'] === T_FALSE ) { + if ( $valueAfterEqualSign !== false && $this->tokens[ $valueAfterEqualSign ]['code'] === T_FALSE ) { $message = 'Obtained cached value in `%s` is being overridden. Disabling caching?'; $data = [ $variableName ]; $this->phpcsFile->addError( $message, $nextVariableOccurrence, 'CacheValueOverride', $data ); } } - /** - * Check whether the examined code is a function call. - * - * @param int $stackPtr The position of the current token in the stack. - * - * @return bool - */ - private function isFunctionCall( $stackPtr ) { - - // Find the next non-empty token. - $openBracket = $this->phpcsFile->findNext( Tokens::$emptyTokens, $stackPtr + 1, null, true ); - - if ( $this->tokens[ $openBracket ]['code'] !== T_OPEN_PARENTHESIS ) { - // Not a function call. - return false; - } - - // Find the previous non-empty token. - $search = Tokens::$emptyTokens; - $search[] = T_BITWISE_AND; - $previous = $this->phpcsFile->findPrevious( $search, $stackPtr - 1, null, true ); - - // It's a function definition, not a function call, so return false. - return ! ( $this->tokens[ $previous ]['code'] === T_FUNCTION ); - } - /** * Check whether the examined code is a variable assignment. * @@ -115,9 +126,10 @@ private function isFunctionCall( $stackPtr ) { */ private function isVariableAssignment( $stackPtr ) { - // Find the previous non-empty token. + // Find the previous non-empty token, but allow for FQN function calls. $search = Tokens::$emptyTokens; $search[] = T_BITWISE_AND; + $search[] = T_NS_SEPARATOR; $previous = $this->phpcsFile->findPrevious( $search, $stackPtr - 1, null, true ); if ( $this->tokens[ $previous ]['code'] !== T_EQUAL ) { diff --git a/WordPressVIPMinimum/Sniffs/Performance/FetchingRemoteDataSniff.php b/WordPressVIPMinimum/Sniffs/Performance/FetchingRemoteDataSniff.php index fab5eada..e5b3eb11 100644 --- a/WordPressVIPMinimum/Sniffs/Performance/FetchingRemoteDataSniff.php +++ b/WordPressVIPMinimum/Sniffs/Performance/FetchingRemoteDataSniff.php @@ -4,55 +4,121 @@ * * @package VIPCS\WordPressVIPMinimum * @link https://github.com/Automattic/VIP-Coding-Standards + * @license https://opensource.org/license/gpl-2-0 GPL-2.0 */ namespace WordPressVIPMinimum\Sniffs\Performance; use PHP_CodeSniffer\Util\Tokens; -use WordPressVIPMinimum\Sniffs\Sniff; +use PHPCSUtils\Utils\PassedParameters; +use WordPressCS\WordPress\AbstractFunctionParameterSniff; /** * Restricts usage of file_get_contents(). */ -class FetchingRemoteDataSniff extends Sniff { +class FetchingRemoteDataSniff extends AbstractFunctionParameterSniff { /** - * Returns an array of tokens this test wants to listen for. + * The group name for this group of functions. * - * @return array + * @var string */ - public function register() { - return [ T_STRING ]; - } + protected $group_name = 'file_get_contents'; /** - * Process this test when one of its tokens is encountered. + * Functions this sniff is looking for. * - * @param int $stackPtr The position of the current token in the stack passed in $tokens. + * @var array Key is the function name, value irrelevant. + */ + protected $target_functions = [ + 'file_get_contents' => true, + ]; + + /** + * Process the parameters of a matched function. + * + * @param int $stackPtr The position of the current token in the stack. + * @param string $group_name The name of the group which was matched. + * @param string $matched_content The token content (function name) which was matched + * in lowercase. + * @param array $parameters Array with information about the parameters. * * @return void */ - public function process_token( $stackPtr ) { - - $functionName = $this->tokens[ $stackPtr ]['content']; - if ( $functionName !== 'file_get_contents' ) { + public function process_parameters( $stackPtr, $group_name, $matched_content, $parameters ) { + $filename_param = PassedParameters::getParameterFromStack( $parameters, 1, 'filename' ); + if ( $filename_param === false ) { + // Missing required parameter. Probably live coding, nothing to examine (yet). Bow out. return; } - $data = [ $this->tokens[ $stackPtr ]['content'] ]; + $data = [ $matched_content ]; + $param_start = $filename_param['start']; + $search_end = ( $filename_param['end'] + 1 ); - $fileNameStackPtr = $this->phpcsFile->findNext( Tokens::$stringTokens, $stackPtr + 1, null, false, null, true ); - if ( $fileNameStackPtr === false ) { - $message = '`%s()` is highly discouraged for remote requests, please use `wpcom_vip_file_get_contents()` or `vip_safe_wp_remote_get()` instead. If it\'s for a local file please use WP_Filesystem instead.'; - $this->phpcsFile->addWarning( $message, $stackPtr, 'FileGetContentsUnknown', $data ); + $has_magic_dir = $this->phpcsFile->findNext( T_DIR, $param_start, $search_end ); + if ( $has_magic_dir !== false ) { + // In all likelyhood a local file (disregarding creative code). + return; } - $fileName = $this->tokens[ $fileNameStackPtr ]['content']; + $isRemoteFile = false; + $search_start = $param_start; + // phpcs:ignore Generic.CodeAnalysis.AssignmentInCondition.FoundInWhileCondition -- Valid usage. + while ( ( $has_text_string = $this->phpcsFile->findNext( Tokens::$stringTokens, $search_start, $search_end ) ) !== false ) { + if ( strpos( $this->tokens[ $has_text_string ]['content'], '://' ) !== false ) { + $isRemoteFile = true; + break; + } + + $search_start = ( $has_text_string + 1 ); + } - $isRemoteFile = ( strpos( $fileName, '://' ) !== false ); if ( $isRemoteFile === true ) { $message = '`%s()` is highly discouraged for remote requests, please use `wpcom_vip_file_get_contents()` or `vip_safe_wp_remote_get()` instead.'; $this->phpcsFile->addWarning( $message, $stackPtr, 'FileGetContentsRemoteFile', $data ); + return; } + + /* + * Okay, so we haven't been able to determine for certain this is a remote file. + * Check for tokens which would make the parameter contents dynamic. + */ + $ignore = Tokens::$emptyTokens; + $ignore += Tokens::$stringTokens; + $ignore += [ T_STRING_CONCAT => T_STRING_CONCAT ]; + + $has_non_text_string = $this->phpcsFile->findNext( $ignore, $param_start, $search_end, true ); + if ( $has_non_text_string !== false ) { + $this->add_contents_unknown_warning( $stackPtr, $data ); + } + } + + /** + * Process the function if it is used as a first class callable. + * + * @param int $stackPtr The position of the current token in the stack. + * @param string $group_name The name of the group which was matched. + * @param string $matched_content The token content (function name) which was matched + * in lowercase. + * + * @return void + */ + public function process_first_class_callable( $stackPtr, $group_name, $matched_content ) { + $this->add_contents_unknown_warning( $stackPtr, [ $matched_content ] ); + } + + /** + * Add a warning if the function is used with unknown parameter(s) or with a $filename parameter for which + * it could not be determined if it references a local file or a remote file. + * + * @param int $stackPtr The position of the current token in the stack. + * @param array $data Data to use for string replacement in the error message. + * + * @return void + */ + private function add_contents_unknown_warning( $stackPtr, $data ) { + $message = '`%s()` is highly discouraged for remote requests, please use `wpcom_vip_file_get_contents()` or `vip_safe_wp_remote_get()` instead. If it\'s for a local file please use WP_Filesystem instead.'; + $this->phpcsFile->addWarning( $message, $stackPtr, 'FileGetContentsUnknown', $data ); } } diff --git a/WordPressVIPMinimum/Sniffs/Performance/LowExpiryCacheTimeSniff.php b/WordPressVIPMinimum/Sniffs/Performance/LowExpiryCacheTimeSniff.php index cededb87..340c89a2 100644 --- a/WordPressVIPMinimum/Sniffs/Performance/LowExpiryCacheTimeSniff.php +++ b/WordPressVIPMinimum/Sniffs/Performance/LowExpiryCacheTimeSniff.php @@ -3,11 +3,15 @@ * WordPressVIPMinimum Coding Standard. * * @package VIPCS\WordPressVIPMinimum + * @link https://github.com/Automattic/VIP-Coding-Standards + * @license https://opensource.org/license/gpl-2-0 GPL-2.0 */ namespace WordPressVIPMinimum\Sniffs\Performance; use PHP_CodeSniffer\Util\Tokens; +use PHPCSUtils\Utils\Numbers; +use PHPCSUtils\Utils\PassedParameters; use PHPCSUtils\Utils\TextStrings; use WordPressCS\WordPress\AbstractFunctionParameterSniff; @@ -30,10 +34,7 @@ class LowExpiryCacheTimeSniff extends AbstractFunctionParameterSniff { /** * Functions this sniff is looking for. * - * @var array The only requirement for this array is that the top level - * array keys are the names of the functions you're looking for. - * Other than that, the array can have arbitrary content - * depending on your needs. + * @var array Key is the function name, value irrelevant. */ protected $target_functions = [ 'wp_cache_set' => true, @@ -44,7 +45,7 @@ class LowExpiryCacheTimeSniff extends AbstractFunctionParameterSniff { /** * List of WP time constants, see https://codex.wordpress.org/Easier_Expression_of_Time_Constants. * - * @var array + * @var array */ protected $wp_time_constants = [ 'MINUTE_IN_SECONDS' => 60, @@ -68,21 +69,21 @@ class LowExpiryCacheTimeSniff extends AbstractFunctionParameterSniff { * normal file processing. */ public function process_parameters( $stackPtr, $group_name, $matched_content, $parameters ) { - if ( isset( $parameters[4] ) === false ) { + $expire_param = PassedParameters::getParameterFromStack( $parameters, 4, 'expire' ); + if ( $expire_param === false ) { // If no cache expiry time, bail (i.e. we don't want to flag for something like feeds where it is cached indefinitely until a hook runs). return; } - $param = $parameters[4]; $tokensAsString = ''; $reportPtr = null; $openParens = 0; $message = 'Cache expiry time could not be determined. Please inspect that the fourth parameter passed to %s() evaluates to 300 seconds or more. Found: "%s"'; $error_code = 'CacheTimeUndetermined'; - $data = [ $matched_content, $parameters[4]['raw'] ]; + $data = [ $matched_content, $expire_param['clean'] ]; - for ( $i = $param['start']; $i <= $param['end']; $i++ ) { + for ( $i = $expire_param['start']; $i <= $expire_param['end']; $i++ ) { if ( isset( Tokens::$emptyTokens[ $this->tokens[ $i ]['code'] ] ) === true ) { $tokensAsString .= ' '; continue; @@ -105,8 +106,10 @@ public function process_parameters( $stackPtr, $group_name, $matched_content, $p if ( $this->tokens[ $i ]['code'] === T_LNUMBER || $this->tokens[ $i ]['code'] === T_DNUMBER ) { - // Integer or float. - $tokensAsString .= $this->tokens[ $i ]['content']; + // Make sure that PHP 7.4 numeric literals and PHP 8.1 explicit octals don't cause problems. + $number_info = Numbers::getCompleteNumber( $this->phpcsFile, $i ); + $tokensAsString .= $number_info['decimal']; + $i = $number_info['last_token']; continue; } diff --git a/WordPressVIPMinimum/Sniffs/Performance/NoPagingSniff.php b/WordPressVIPMinimum/Sniffs/Performance/NoPagingSniff.php index c751f021..d3814514 100644 --- a/WordPressVIPMinimum/Sniffs/Performance/NoPagingSniff.php +++ b/WordPressVIPMinimum/Sniffs/Performance/NoPagingSniff.php @@ -4,36 +4,44 @@ * * @package VIPCS\WordPressVIPMinimum * @link https://github.com/Automattic/VIP-Coding-Standards + * @license https://opensource.org/license/gpl-2-0 GPL-2.0 * @license https://opensource.org/licenses/MIT MIT */ namespace WordPressVIPMinimum\Sniffs\Performance; +use PHPCSUtils\Utils\TextStrings; use WordPressCS\WordPress\AbstractArrayAssignmentRestrictionsSniff; /** - * Flag returning high or infinite posts_per_page. + * Flag disabling pagination via `nopaging` or `posts_per_page`/`numberposts` set to `-1`. * * @link https://docs.wpvip.com/technical-references/code-review/#no-limit-queries - * - * @since 0.5.0 */ class NoPagingSniff extends AbstractArrayAssignmentRestrictionsSniff { /** * Groups of variables to restrict. * - * @return array + * @return array>> */ public function getGroups() { return [ 'nopaging' => [ 'type' => 'error', - 'message' => 'Disabling pagination is prohibited in VIP context, do not set `%s` to `%s` ever.', + 'message' => 'Disabling pagination is prohibited in VIP context, do not set `%s` to `%s`.', 'keys' => [ 'nopaging', ], ], + 'posts_per_page' => [ + 'type' => 'error', + 'message' => 'Setting `%s` to `%s` disables pagination and is prohibited in VIP context.', + 'keys' => [ + 'posts_per_page', + 'numberposts', + ], + ], ]; } @@ -50,6 +58,12 @@ public function getGroups() { public function callback( $key, $val, $line, $group ) { $key = strtolower( $key ); - return ( $key === 'nopaging' && ( $val === 'true' || $val === '1' ) ); + if ( $key === 'nopaging' ) { + return ( $val === 'true' || $val === '1' ); + } + + // posts_per_page / numberposts: flag -1 (no limit). + $val = TextStrings::stripQuotes( $val ); + return ( $val === '-1' ); } } diff --git a/WordPressVIPMinimum/Sniffs/Performance/OrderByRandSniff.php b/WordPressVIPMinimum/Sniffs/Performance/OrderByRandSniff.php index 2747b242..962230fb 100644 --- a/WordPressVIPMinimum/Sniffs/Performance/OrderByRandSniff.php +++ b/WordPressVIPMinimum/Sniffs/Performance/OrderByRandSniff.php @@ -4,6 +4,7 @@ * * @package VIPCS\WordPressVIPMinimum * @link https://github.com/Automattic/VIP-Coding-Standards + * @license https://opensource.org/license/gpl-2-0 GPL-2.0 * @license https://opensource.org/licenses/MIT MIT */ @@ -16,15 +17,13 @@ * Flag using orderby => rand. * * @link https://docs.wpvip.com/technical-references/code-review/vip-errors/#h-order-by-rand - * - * @since 0.5.0 */ class OrderByRandSniff extends AbstractArrayAssignmentRestrictionsSniff { /** * Groups of variables to restrict. * - * @return array + * @return array>> */ public function getGroups() { return [ diff --git a/WordPressVIPMinimum/Sniffs/Performance/RegexpCompareSniff.php b/WordPressVIPMinimum/Sniffs/Performance/RegexpCompareSniff.php index 536bd1ee..8873b057 100644 --- a/WordPressVIPMinimum/Sniffs/Performance/RegexpCompareSniff.php +++ b/WordPressVIPMinimum/Sniffs/Performance/RegexpCompareSniff.php @@ -3,6 +3,8 @@ * WordPressVIPMinimum Coding Standard. * * @package VIPCS\WordPressVIPMinimum + * @link https://github.com/Automattic/VIP-Coding-Standards + * @license https://opensource.org/license/gpl-2-0 GPL-2.0 */ namespace WordPressVIPMinimum\Sniffs\Performance; @@ -18,7 +20,7 @@ class RegexpCompareSniff extends AbstractArrayAssignmentRestrictionsSniff { /** * Groups of variables to restrict. * - * @return array + * @return array>> */ public function getGroups() { return [ diff --git a/WordPressVIPMinimum/Sniffs/Performance/RemoteRequestTimeoutSniff.php b/WordPressVIPMinimum/Sniffs/Performance/RemoteRequestTimeoutSniff.php index 072ba5af..f392e7c1 100644 --- a/WordPressVIPMinimum/Sniffs/Performance/RemoteRequestTimeoutSniff.php +++ b/WordPressVIPMinimum/Sniffs/Performance/RemoteRequestTimeoutSniff.php @@ -3,6 +3,8 @@ * WordPressVIPMinimum Coding Standard. * * @package VIPCS\WordPressVIPMinimum + * @link https://github.com/Automattic/VIP-Coding-Standards + * @license https://opensource.org/license/gpl-2-0 GPL-2.0 */ namespace WordPressVIPMinimum\Sniffs\Performance; @@ -17,7 +19,7 @@ class RemoteRequestTimeoutSniff extends AbstractArrayAssignmentRestrictionsSniff /** * Groups of variables to restrict. * - * @return array + * @return array>> */ public function getGroups() { return [ diff --git a/WordPressVIPMinimum/Sniffs/Performance/TaxonomyMetaInOptionsSniff.php b/WordPressVIPMinimum/Sniffs/Performance/TaxonomyMetaInOptionsSniff.php index 07607275..b4a10f33 100644 --- a/WordPressVIPMinimum/Sniffs/Performance/TaxonomyMetaInOptionsSniff.php +++ b/WordPressVIPMinimum/Sniffs/Performance/TaxonomyMetaInOptionsSniff.php @@ -4,22 +4,26 @@ * * @package VIPCS\WordPressVIPMinimum * @link https://github.com/Automattic/VIP-Coding-Standards + * @license https://opensource.org/license/gpl-2-0 GPL-2.0 */ namespace WordPressVIPMinimum\Sniffs\Performance; use PHP_CodeSniffer\Util\Tokens; -use WordPressVIPMinimum\Sniffs\Sniff; +use PHPCSUtils\Utils\PassedParameters; +use WordPressCS\WordPress\AbstractFunctionParameterSniff; /** * Restricts the implementation of taxonomy term meta via options. */ -class TaxonomyMetaInOptionsSniff extends Sniff { +class TaxonomyMetaInOptionsSniff extends AbstractFunctionParameterSniff { /** * List of options_ functions * - * @var array + * @deprecated 3.1.0 This property should never have been public. + * + * @var array */ public $option_functions = [ 'get_option', @@ -31,7 +35,7 @@ class TaxonomyMetaInOptionsSniff extends Sniff { /** * List of possible variable names holding term ID. * - * @var array + * @var array */ public $taxonomy_term_patterns = [ 'category_id', @@ -44,34 +48,47 @@ class TaxonomyMetaInOptionsSniff extends Sniff { ]; /** - * Returns an array of tokens this test wants to listen for. + * The group name for this group of functions. * - * @return array + * @var string */ - public function register() { - return [ T_STRING ]; - } + protected $group_name = 'option_functions'; /** - * Process this test when one of its tokens is encountered + * Functions this sniff is looking for. * - * @param int $stackPtr The position of the current token in the stack passed in $tokens. + * @var array Keys are the target functions, value irrelevant. + */ + protected $target_functions = [ + 'get_option' => true, + 'add_option' => true, + 'update_option' => true, + 'delete_option' => true, + ]; + + + /** + * Process the parameters of a matched function. + * + * @param int $stackPtr The position of the current token in the stack. + * @param string $group_name The name of the group which was matched. + * @param string $matched_content The token content (function name) which was matched + * in lowercase. + * @param array $parameters Array with information about the parameters. * * @return void */ - public function process_token( $stackPtr ) { - - if ( in_array( $this->tokens[ $stackPtr ]['content'], $this->option_functions, true ) === false ) { + public function process_parameters( $stackPtr, $group_name, $matched_content, $parameters ) { + $target_param = PassedParameters::getParameterFromStack( $parameters, 1, 'option' ); + if ( $target_param === false ) { + // Missing (required) target parameter. Probably live coding, nothing to examine (yet). Bow out. return; } - $openBracket = $this->phpcsFile->findNext( Tokens::$emptyTokens, $stackPtr + 1, null, true ); - - if ( $this->tokens[ $openBracket ]['code'] !== T_OPEN_PARENTHESIS ) { - return; - } + $param_start = $target_param['start']; + $param_end = ( $target_param['end'] + 1 ); // Add one to include the last token in the parameter in findNext searches. - $param_ptr = $this->phpcsFile->findNext( Tokens::$emptyTokens, $openBracket + 1, null, true ); + $param_ptr = $this->phpcsFile->findNext( Tokens::$emptyTokens, $param_start, $param_end, true ); if ( $this->tokens[ $param_ptr ]['code'] === T_DOUBLE_QUOTED_STRING ) { foreach ( $this->taxonomy_term_patterns as $taxonomy_term_pattern ) { @@ -100,7 +117,9 @@ public function process_token( $stackPtr ) { } $object_operator = $this->phpcsFile->findNext( Tokens::$emptyTokens, $variable_name + 1, null, true ); - if ( $this->tokens[ $object_operator ]['code'] !== T_OBJECT_OPERATOR ) { + if ( $this->tokens[ $object_operator ]['code'] !== T_OBJECT_OPERATOR + && $this->tokens[ $object_operator ]['code'] !== T_NULLSAFE_OBJECT_OPERATOR + ) { return; } diff --git a/WordPressVIPMinimum/Sniffs/Performance/WPQueryParamsSniff.php b/WordPressVIPMinimum/Sniffs/Performance/WPQueryParamsSniff.php index 9c247f8d..10f09ba1 100644 --- a/WordPressVIPMinimum/Sniffs/Performance/WPQueryParamsSniff.php +++ b/WordPressVIPMinimum/Sniffs/Performance/WPQueryParamsSniff.php @@ -4,6 +4,7 @@ * * @package VIPCS\WordPressVIPMinimum * @link https://github.com/Automattic/VIP-Coding-Standards + * @license https://opensource.org/license/gpl-2-0 GPL-2.0 */ namespace WordPressVIPMinimum\Sniffs\Performance; @@ -28,7 +29,7 @@ class WPQueryParamsSniff extends AbstractArrayAssignmentRestrictionsSniff { /** * Groups of variables to restrict. * - * @return array + * @return array>> */ public function getGroups() { return [ @@ -45,7 +46,7 @@ public function getGroups() { 'PostNotIn' => [ 'name' => 'PostNotIn', 'type' => 'warning', - 'message' => 'Using exclusionary parameters, like %s, in calls to get_posts() should be done with caution, see https://wpvip.com/documentation/performance-improvements-by-removing-usage-of-post__not_in/ for more information.', + 'message' => 'Using exclusionary parameters, like %s, in calls to get_posts() should be done with caution, see https://docs.wpvip.com/databases/optimize-queries/using-post__not_in/ for more information.', 'keys' => [ 'post__not_in', 'exclude', diff --git a/WordPressVIPMinimum/Sniffs/Security/EscapingVoidReturnFunctionsSniff.php b/WordPressVIPMinimum/Sniffs/Security/EscapingVoidReturnFunctionsSniff.php index 6a6ba281..ddbdef9e 100644 --- a/WordPressVIPMinimum/Sniffs/Security/EscapingVoidReturnFunctionsSniff.php +++ b/WordPressVIPMinimum/Sniffs/Security/EscapingVoidReturnFunctionsSniff.php @@ -4,13 +4,15 @@ * * @package VIPCS\WordPressVIPMinimum * @link https://github.com/Automattic/VIP-Coding-Standards + * @license https://opensource.org/license/gpl-2-0 GPL-2.0 */ namespace WordPressVIPMinimum\Sniffs\Security; use PHP_CodeSniffer\Util\Tokens; +use PHPCSUtils\Utils\PassedParameters; +use WordPressCS\WordPress\AbstractFunctionParameterSniff; use WordPressCS\WordPress\Helpers\PrintingFunctionsTrait; -use WordPressVIPMinimum\Sniffs\Sniff; /** * Flag functions that don't return anything, yet are wrapped in an escaping function call. @@ -19,46 +21,131 @@ * * @uses \WordPressCS\WordPress\Helpers\PrintingFunctionsTrait::$customPrintingFunctions */ -class EscapingVoidReturnFunctionsSniff extends Sniff { +class EscapingVoidReturnFunctionsSniff extends AbstractFunctionParameterSniff { use PrintingFunctionsTrait; /** - * Returns an array of tokens this test wants to listen for. + * The group name for this group of functions. * - * @return array + * @var string */ - public function register() { - return [ - T_STRING, - ]; - } + protected $group_name = 'escaping_void'; + + /** + * Functions this sniff is looking for. + * + * @var array Keys are the target functions, + * value, the name and position of the target parameter. + */ + protected $target_functions = [ + 'esc_attr' => [ + 'param_position' => 1, + 'param_name' => 'text', + ], + 'esc_attr__' => [ + 'param_position' => 1, + 'param_name' => 'text', + ], + 'esc_attr_e' => [ + 'param_position' => 1, + 'param_name' => 'text', + ], + 'esc_attr_x' => [ + 'param_position' => 1, + 'param_name' => 'text', + ], + 'esc_html' => [ + 'param_position' => 1, + 'param_name' => 'text', + ], + 'esc_html__' => [ + 'param_position' => 1, + 'param_name' => 'text', + ], + 'esc_html_e' => [ + 'param_position' => 1, + 'param_name' => 'text', + ], + 'esc_html_x' => [ + 'param_position' => 1, + 'param_name' => 'text', + ], + 'esc_js' => [ + 'param_position' => 1, + 'param_name' => 'text', + ], + 'esc_textarea' => [ + 'param_position' => 1, + 'param_name' => 'text', + ], + 'esc_url' => [ + 'param_position' => 1, + 'param_name' => 'url', + ], + 'esc_url_raw' => [ + 'param_position' => 1, + 'param_name' => 'url', + ], + 'esc_xml' => [ + 'param_position' => 1, + 'param_name' => 'text', + ], + 'tag_escape' => [ + 'param_position' => 1, + 'param_name' => 'tag_name', + ], + 'wp_kses' => [ + 'param_position' => 1, + 'param_name' => 'content', + ], + 'wp_kses_data' => [ + 'param_position' => 1, + 'param_name' => 'data', + ], + 'wp_kses_one_attr' => [ + 'param_position' => 1, + 'param_name' => 'attr', + ], + 'wp_kses_post' => [ + 'param_position' => 1, + 'param_name' => 'data', + ], + ]; /** - * Process this test when one of its tokens is encountered + * Process the parameters of a matched function. * - * @param int $stackPtr The position of the current token in the stack passed in $tokens. + * @param int $stackPtr The position of the current token in the stack. + * @param string $group_name The name of the group which was matched. + * @param string $matched_content The token content (function name) which was matched + * in lowercase. + * @param array $parameters Array with information about the parameters. * * @return void */ - public function process_token( $stackPtr ) { + public function process_parameters( $stackPtr, $group_name, $matched_content, $parameters ) { + $param_position = $this->target_functions[ $matched_content ]['param_position']; + $param_name = $this->target_functions[ $matched_content ]['param_name']; - if ( strpos( $this->tokens[ $stackPtr ]['content'], 'esc_' ) !== 0 && strpos( $this->tokens[ $stackPtr ]['content'], 'wp_kses' ) !== 0 ) { - // Not what we are looking for. + $target_param = PassedParameters::getParameterFromStack( $parameters, $param_position, $param_name ); + if ( $target_param === false ) { + // Missing (required) target parameter. Probably live coding, nothing to examine (yet). Bow out. return; } - $next_token = $this->phpcsFile->findNext( Tokens::$emptyTokens, $stackPtr + 1, null, true ); + $ignore = Tokens::$emptyTokens; + $ignore[ T_NS_SEPARATOR ] = T_NS_SEPARATOR; - if ( $this->tokens[ $next_token ]['code'] !== T_OPEN_PARENTHESIS ) { - // Not a function call. + $next_token = $this->phpcsFile->findNext( $ignore, $target_param['start'], ( $target_param['end'] + 1 ), true ); + if ( $next_token === false || $this->tokens[ $next_token ]['code'] !== T_STRING ) { + // Not what we are looking for. return; } - $next_token = $this->phpcsFile->findNext( Tokens::$emptyTokens, $next_token + 1, null, true ); - - if ( $this->tokens[ $next_token ]['code'] !== T_STRING ) { - // Not what we are looking for. + $next_after = $this->phpcsFile->findNext( Tokens::$emptyTokens, $next_token + 1, ( $target_param['end'] + 1 ), true ); + if ( $next_after === false || $this->tokens[ $next_after ]['code'] !== T_OPEN_PARENTHESIS ) { + // Not a function call inside the escaping function. return; } diff --git a/WordPressVIPMinimum/Sniffs/Security/ExitAfterRedirectSniff.php b/WordPressVIPMinimum/Sniffs/Security/ExitAfterRedirectSniff.php index 3c90c315..035cbb76 100644 --- a/WordPressVIPMinimum/Sniffs/Security/ExitAfterRedirectSniff.php +++ b/WordPressVIPMinimum/Sniffs/Security/ExitAfterRedirectSniff.php @@ -4,6 +4,7 @@ * * @package VIPCS\WordPressVIPMinimum * @link https://github.com/Automattic/VIP-Coding-Standards + * @license https://opensource.org/license/gpl-2-0 GPL-2.0 */ namespace WordPressVIPMinimum\Sniffs\Security; @@ -19,7 +20,7 @@ class ExitAfterRedirectSniff extends Sniff { /** * Returns an array of tokens this test wants to listen for. * - * @return array + * @return array */ public function register() { return [ T_STRING ]; diff --git a/WordPressVIPMinimum/Sniffs/Security/MustacheSniff.php b/WordPressVIPMinimum/Sniffs/Security/MustacheSniff.php index b026b443..e430eaa6 100644 --- a/WordPressVIPMinimum/Sniffs/Security/MustacheSniff.php +++ b/WordPressVIPMinimum/Sniffs/Security/MustacheSniff.php @@ -4,10 +4,12 @@ * * @package VIPCS\WordPressVIPMinimum * @link https://github.com/Automattic/VIP-Coding-Standards + * @license https://opensource.org/license/gpl-2-0 GPL-2.0 */ namespace WordPressVIPMinimum\Sniffs\Security; +use PHP_CodeSniffer\Util\Tokens; use WordPressVIPMinimum\Sniffs\Sniff; /** @@ -25,15 +27,13 @@ class MustacheSniff extends Sniff { /** * Returns an array of tokens this test wants to listen for. * - * @return array + * @return array */ public function register() { - return [ - T_CONSTANT_ENCAPSED_STRING, - T_STRING, - T_INLINE_HTML, - T_HEREDOC, - ]; + $targets = Tokens::$textStringTokens; + $targets[ T_STRING ] = T_STRING; + + return $targets; } /** @@ -57,15 +57,22 @@ public function process_token( $stackPtr ) { $this->phpcsFile->addWarning( $message, $stackPtr, 'VariableNotation' ); } - if ( strpos( $this->tokens[ $stackPtr ]['content'], '{{=' ) !== false ) { + $start_delimiter_change = strpos( $this->tokens[ $stackPtr ]['content'], '{{=' ); + if ( $start_delimiter_change !== false ) { // Mustache delimiter change. - $new_delimiter = trim( str_replace( [ '{{=', '=}}' ], '', substr( $this->tokens[ $stackPtr ]['content'], 0, strpos( $this->tokens[ $stackPtr ]['content'], '=}}' ) + 3 ) ) ); - $message = 'Found Mustache delimiter change notation. New delimiter is: %s.'; - $data = [ $new_delimiter ]; - $this->phpcsFile->addWarning( $message, $stackPtr, 'DelimiterChange', $data ); + $end_delimiter_change = strpos( $this->tokens[ $stackPtr ]['content'], '=}}' ); + if ( $end_delimiter_change !== false && $start_delimiter_change < $end_delimiter_change ) { + $start_new_delimiter = $start_delimiter_change + 3; + $new_delimiter_length = $end_delimiter_change - ( $start_delimiter_change + 3 ); + $new_delimiter = substr( $this->tokens[ $stackPtr ]['content'], $start_new_delimiter, $new_delimiter_length ); + + $message = 'Found Mustache delimiter change notation. New delimiter is: %s.'; + $data = [ $new_delimiter ]; + $this->phpcsFile->addWarning( $message, $stackPtr, 'DelimiterChange', $data ); + } } - if ( strpos( $this->tokens[ $stackPtr ]['content'], 'SafeString' ) !== false ) { + if ( strpos( $this->tokens[ $stackPtr ]['content'], '.SafeString' ) !== false ) { // Handlebars.js Handlebars.SafeString does not get escaped. $message = 'Found Handlebars.SafeString call which does not get escaped.'; $this->phpcsFile->addWarning( $message, $stackPtr, 'SafeString' ); diff --git a/WordPressVIPMinimum/Sniffs/Security/PHPFilterFunctionsSniff.php b/WordPressVIPMinimum/Sniffs/Security/PHPFilterFunctionsSniff.php index e2907d9a..32e9f7a9 100644 --- a/WordPressVIPMinimum/Sniffs/Security/PHPFilterFunctionsSniff.php +++ b/WordPressVIPMinimum/Sniffs/Security/PHPFilterFunctionsSniff.php @@ -3,15 +3,23 @@ * WordPressVIPMinimum Coding Standard. * * @package VIPCS\WordPressVIPMinimum + * @link https://github.com/Automattic/VIP-Coding-Standards + * @license https://opensource.org/license/gpl-2-0 GPL-2.0 */ namespace WordPressVIPMinimum\Sniffs\Security; +use PHP_CodeSniffer\Util\Tokens; +use PHPCSUtils\Utils\PassedParameters; use WordPressCS\WordPress\AbstractFunctionParameterSniff; /** * This sniff ensures that proper sanitization is occurring when PHP's filter_* functions are used. * + * {@internal The $options parameter for filter_var_array() and filter_input_array() can take either an + * integer (filter constant) or an array with options, which could include an option setting the filter constant. + * At this time, this sniff does not handle an array with options being passed.} + * * @since 0.4.0 */ class PHPFilterFunctionsSniff extends AbstractFunctionParameterSniff { @@ -24,24 +32,33 @@ class PHPFilterFunctionsSniff extends AbstractFunctionParameterSniff { protected $group_name = 'php_filter_functions'; /** - * Functions this sniff is looking for. + * Functions this sniff is looking for and information on which parameter to check for in those function calls. * - * @var array The only requirement for this array is that the top level - * array keys are the names of the functions you're looking for. - * Other than that, the array can have arbitrary content - * depending on your needs. + * @var array Key is the function name. */ protected $target_functions = [ - 'filter_var' => true, - 'filter_input' => true, - 'filter_var_array' => true, - 'filter_input_array' => true, + 'filter_var' => [ + 'param_position' => 2, + 'param_name' => 'filter', + ], + 'filter_input' => [ + 'param_position' => 3, + 'param_name' => 'filter', + ], + 'filter_var_array' => [ + 'param_position' => 2, + 'param_name' => 'options', + ], + 'filter_input_array' => [ + 'param_position' => 2, + 'param_name' => 'options', + ], ]; /** * List of restricted filter names. * - * @var array + * @var array */ private $restricted_filters = [ 'FILTER_DEFAULT' => true, @@ -61,30 +78,52 @@ class PHPFilterFunctionsSniff extends AbstractFunctionParameterSniff { * normal file processing. */ public function process_parameters( $stackPtr, $group_name, $matched_content, $parameters ) { - if ( $matched_content === 'filter_input' ) { - if ( count( $parameters ) === 2 ) { - $message = 'Missing third parameter for "%s".'; - $data = [ $matched_content ]; - $this->phpcsFile->addWarning( $message, $stackPtr, 'MissingThirdParameter', $data ); - } + $param_position = $this->target_functions[ $matched_content ]['param_position']; + $param_name = $this->target_functions[ $matched_content ]['param_name']; - if ( isset( $parameters[3], $this->restricted_filters[ $parameters[3]['raw'] ] ) ) { - $message = 'Please use an appropriate filter to sanitize, as "%s" does no filtering, see: http://php.net/manual/en/filter.filters.sanitize.php.'; - $data = [ strtoupper( $parameters[3]['raw'] ) ]; - $this->phpcsFile->addWarning( $message, $stackPtr, 'RestrictedFilter', $data ); - } - } else { - if ( count( $parameters ) === 1 ) { - $message = 'Missing second parameter for "%s".'; - $data = [ $matched_content ]; - $this->phpcsFile->addWarning( $message, $stackPtr, 'MissingSecondParameter', $data ); + $target_param = PassedParameters::getParameterFromStack( $parameters, $param_position, $param_name ); + if ( $target_param === false ) { + /* + * Check for PHP 5.6+ argument unpacking. + * + * No need for extensive defensive coding, we already know this is syntactically a valid function call, + * otherwise this method would not have been reached. + */ + $tokens = $this->phpcsFile->getTokens(); + $open_parens = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $stackPtr + 1 ), null, true ); + $has_ellipses = $this->phpcsFile->findNext( T_ELLIPSIS, ( $open_parens + 1 ), $tokens[ $open_parens ]['parenthesis_closer'] ); + + if ( $has_ellipses !== false ) { + $target_nesting_level = 1; + if ( isset( $tokens[ $open_parens ]['nested_parenthesis'] ) ) { + $target_nesting_level = ( count( $tokens[ $open_parens ]['nested_parenthesis'] ) + 1 ); + } + + if ( $target_nesting_level === count( $tokens[ $has_ellipses ]['nested_parenthesis'] ) ) { + // Bow out as undetermined. + return; + } } - if ( isset( $parameters[2], $this->restricted_filters[ $parameters[2]['raw'] ] ) ) { - $message = 'Please use an appropriate filter to sanitize, as "%s" does no filtering, see http://php.net/manual/en/filter.filters.sanitize.php.'; - $data = [ strtoupper( $parameters[2]['raw'] ) ]; - $this->phpcsFile->addWarning( $message, $stackPtr, 'RestrictedFilter', $data ); + $message = 'Missing $%s parameter for "%s()".'; + $data = [ $param_name, $matched_content ]; + + // Error codes should probably be made more descriptive, but that would be a BC-break. + $error_code = 'MissingSecondParameter'; + if ( $param_position === 3 ) { + $error_code = 'MissingThirdParameter'; } + + $this->phpcsFile->addWarning( $message, $stackPtr, $error_code, $data ); + return; + } + + if ( isset( $this->restricted_filters[ $target_param['clean'] ] ) ) { + $first_non_empty = $this->phpcsFile->findNext( Tokens::$emptyTokens, $target_param['start'], ( $target_param['end'] + 1 ), true ); + + $message = 'Please use an appropriate filter to sanitize, as "%s" does no filtering, see: http://php.net/manual/en/filter.filters.sanitize.php.'; + $data = [ $target_param['clean'] ]; + $this->phpcsFile->addWarning( $message, $first_non_empty, 'RestrictedFilter', $data ); } } } diff --git a/WordPressVIPMinimum/Sniffs/Security/ProperEscapingFunctionSniff.php b/WordPressVIPMinimum/Sniffs/Security/ProperEscapingFunctionSniff.php index 12ee05c7..d746aa2c 100644 --- a/WordPressVIPMinimum/Sniffs/Security/ProperEscapingFunctionSniff.php +++ b/WordPressVIPMinimum/Sniffs/Security/ProperEscapingFunctionSniff.php @@ -4,6 +4,7 @@ * * @package VIPCS\WordPressVIPMinimum * @link https://github.com/Automattic/VIP-Coding-Standards + * @license https://opensource.org/license/gpl-2-0 GPL-2.0 */ namespace WordPressVIPMinimum\Sniffs\Security; @@ -28,7 +29,7 @@ class ProperEscapingFunctionSniff extends Sniff { /** * List of escaping functions which are being tested. * - * @var array + * @var array */ protected $escaping_functions = [ 'esc_url' => 'url', @@ -45,7 +46,7 @@ class ProperEscapingFunctionSniff extends Sniff { /** * List of tokens we can skip. * - * @var array + * @var array */ private $echo_or_concat_tokens = [ @@ -63,7 +64,7 @@ class ProperEscapingFunctionSniff extends Sniff { * for public methods which extending sniffs may be * relying on. * - * @var array + * @var array */ private $url_attrs = [ 'href', @@ -79,7 +80,7 @@ class ProperEscapingFunctionSniff extends Sniff { * for public methods which extending sniffs may be * relying on. * - * @var array + * @var array */ private $attr_endings = [ '=', @@ -107,7 +108,7 @@ class ProperEscapingFunctionSniff extends Sniff { /** * Returns an array of tokens this test wants to listen for. * - * @return array + * @return array */ public function register() { $this->echo_or_concat_tokens += Tokens::$emptyTokens; @@ -186,7 +187,7 @@ public function process_token( $stackPtr ) { $html = $this->phpcsFile->findPrevious( $ignore, $stackPtr - 1, null, true ); - // Use $textStringTokens b/c heredoc and nowdoc tokens will never be encountered in this context anyways.. + // Use $textStringTokens b/c heredoc and nowdoc tokens will never be encountered in this context anyways. if ( $html === false || isset( Tokens::$textStringTokens[ $this->tokens[ $html ]['code'] ] ) === false ) { return; } diff --git a/WordPressVIPMinimum/Sniffs/Security/StaticStrreplaceSniff.php b/WordPressVIPMinimum/Sniffs/Security/StaticStrreplaceSniff.php index 58e90e74..017c36a5 100644 --- a/WordPressVIPMinimum/Sniffs/Security/StaticStrreplaceSniff.php +++ b/WordPressVIPMinimum/Sniffs/Security/StaticStrreplaceSniff.php @@ -4,82 +4,141 @@ * * @package VIPCS\WordPressVIPMinimum * @link https://github.com/Automattic/VIP-Coding-Standards + * @license https://opensource.org/license/gpl-2-0 GPL-2.0 */ namespace WordPressVIPMinimum\Sniffs\Security; use PHP_CodeSniffer\Util\Tokens; -use WordPressVIPMinimum\Sniffs\Sniff; +use PHPCSUtils\Tokens\Collections; +use PHPCSUtils\Utils\Arrays; +use PHPCSUtils\Utils\PassedParameters; +use PHPCSUtils\Utils\TextStrings; +use WordPressCS\WordPress\AbstractFunctionParameterSniff; /** * Restricts usage of str_replace with all 3 params being static. */ -class StaticStrreplaceSniff extends Sniff { +class StaticStrreplaceSniff extends AbstractFunctionParameterSniff { /** - * Returns an array of tokens this test wants to listen for. + * The group name for this group of functions. * - * @return array + * @var string */ - public function register() { - return [ T_STRING ]; - } + protected $group_name = 'str_replace'; /** - * Process this test when one of its tokens is encountered + * Functions this sniff is looking for. * - * @param int $stackPtr The position of the current token in the stack passed in $tokens. + * @var array Key is the function name, value irrelevant. + */ + protected $target_functions = [ + 'str_replace' => true, + ]; + + /** + * Process the parameters of a matched function. + * + * @param int $stackPtr The position of the current token in the stack. + * @param string $group_name The name of the group which was matched. + * @param string $matched_content The token content (function name) which was matched + * in lowercase. + * @param array $parameters Array with information about the parameters. * * @return void */ - public function process_token( $stackPtr ) { - - if ( $this->tokens[ $stackPtr ]['content'] !== 'str_replace' ) { + public function process_parameters( $stackPtr, $group_name, $matched_content, $parameters ) { + $search_param = PassedParameters::getParameterFromStack( $parameters, 1, 'search' ); + $replace_param = PassedParameters::getParameterFromStack( $parameters, 2, 'replace' ); + $subject_param = PassedParameters::getParameterFromStack( $parameters, 3, 'subject' ); + + if ( $search_param === false || $replace_param === false || $subject_param === false ) { + /* + * Either an invalid function call (missing PHP required parameter); or function call + * with argument unpacking; or live coding. + * In all these cases, this is not the code pattern this sniff is looking for, so bow out. + */ return; } - $openBracket = $this->phpcsFile->findNext( Tokens::$emptyTokens, $stackPtr + 1, null, true ); - - if ( $this->tokens[ $openBracket ]['code'] !== T_OPEN_PARENTHESIS ) { - return; + foreach ( [ $search_param, $replace_param, $subject_param ] as $param ) { + if ( $this->is_parameter_static_text( $param ) === false ) { + // Non-static text token found. Not what we're looking for. + return; + } } - $next_start_ptr = $openBracket + 1; - for ( $i = 0; $i < 3; $i++ ) { - $param_ptr = $this->phpcsFile->findNext( array_merge( Tokens::$emptyTokens, [ T_COMMA ] ), $next_start_ptr, null, true ); - - if ( $this->tokens[ $param_ptr ]['code'] === T_ARRAY ) { - $openBracket = $this->phpcsFile->findNext( Tokens::$emptyTokens, $param_ptr + 1, null, true ); - if ( $this->tokens[ $openBracket ]['code'] !== T_OPEN_PARENTHESIS ) { - return; - } + $message = 'This code pattern is often used to run a very dangerous shell programs on your server. The code in these files needs to be reviewed, and possibly cleaned.'; + $this->phpcsFile->addError( $message, $stackPtr, 'StaticStrreplace' ); + } - // Find the closing bracket. - $closeBracket = $this->tokens[ $openBracket ]['parenthesis_closer']; + /** + * Check whether the current parameter, or array item, only contains tokens which should be regarded + * as a valid part of a static text string. + * + * @param array $param_info Array with information about a single parameter or array item. + * Must be an array as returned via the PassedParameters class. + * + * @return bool + */ + private function is_parameter_static_text( $param_info ) { + // List of tokens which can be skipped over without further examination. + $static_tokens = [ + T_CONSTANT_ENCAPSED_STRING => T_CONSTANT_ENCAPSED_STRING, + T_PLUS => T_PLUS, + T_STRING_CONCAT => T_STRING_CONCAT, + ]; + $static_tokens += Tokens::$emptyTokens; + + for ( $i = $param_info['start']; $i <= $param_info['end']; $i++ ) { + $next_to_examine = $this->phpcsFile->findNext( $static_tokens, $i, ( $param_info['end'] + 1 ), true ); + if ( $next_to_examine === false ) { + // The parameter contained only tokens which could be considered static text. + return true; + } - $array_item_ptr = $this->phpcsFile->findNext( array_merge( Tokens::$emptyTokens, [ T_COMMA ] ), $openBracket + 1, $closeBracket, true ); - while ( $array_item_ptr !== false ) { + if ( isset( Collections::arrayOpenTokensBC()[ $this->tokens[ $next_to_examine ]['code'] ] ) ) { + $arrayOpenClose = Arrays::getOpenClose( $this->phpcsFile, $next_to_examine ); + if ( $arrayOpenClose === false ) { + // Short list, parse error or live coding, bow out. + return false; + } - if ( $this->tokens[ $array_item_ptr ]['code'] !== T_CONSTANT_ENCAPSED_STRING ) { - return; + $array_items = PassedParameters::getParameters( $this->phpcsFile, $next_to_examine ); + foreach ( $array_items as $array_item ) { + if ( $this->is_parameter_static_text( $array_item ) === false ) { + return false; } - $array_item_ptr = $this->phpcsFile->findNext( array_merge( Tokens::$emptyTokens, [ T_COMMA ] ), $array_item_ptr + 1, $closeBracket, true ); } - $next_start_ptr = $closeBracket + 1; + // The array only contained items with tokens which could be considered static text. + $i = $arrayOpenClose['closer']; continue; - } - if ( $this->tokens[ $param_ptr ]['code'] !== T_CONSTANT_ENCAPSED_STRING ) { - return; + if ( $this->tokens[ $next_to_examine ]['code'] === T_START_HEREDOC ) { + $heredoc_text = TextStrings::getCompleteTextString( $this->phpcsFile, $next_to_examine ); + $stripped_text = TextStrings::stripEmbeds( $heredoc_text ); + if ( $heredoc_text !== $stripped_text ) { + // Heredoc with interpolated expression(s). Not a static text. + return false; + } } - $next_start_ptr = $param_ptr + 1; + if ( ( $this->tokens[ $next_to_examine ]['code'] === T_START_HEREDOC + || $this->tokens[ $next_to_examine ]['code'] === T_START_NOWDOC ) + && isset( $this->tokens[ $next_to_examine ]['scope_closer'] ) + ) { + // No interpolation. Skip to end of a heredoc/nowdoc. + $i = $this->tokens[ $next_to_examine ]['scope_closer']; + continue; + } + // Any other token means this parameter should be regarded as non-static text. Not what we're looking for. + return false; } - $message = 'This code pattern is often used to run a very dangerous shell programs on your server. The code in these files needs to be reviewed, and possibly cleaned.'; - $this->phpcsFile->addError( $message, $stackPtr, 'StaticStrreplace' ); + return true; } } diff --git a/WordPressVIPMinimum/Sniffs/Security/TwigSniff.php b/WordPressVIPMinimum/Sniffs/Security/TwigSniff.php deleted file mode 100644 index fdc8f643..00000000 --- a/WordPressVIPMinimum/Sniffs/Security/TwigSniff.php +++ /dev/null @@ -1,59 +0,0 @@ -tokens[ $stackPtr ]['content'] ) === 1 ) { - // Twig autoescape disabled. - $message = 'Found Twig autoescape disabling notation.'; - $this->phpcsFile->addWarning( $message, $stackPtr, 'AutoescapeFalse' ); - } - - if ( preg_match( '/\|\s*raw/', $this->tokens[ $stackPtr ]['content'] ) === 1 ) { - // Twig default unescape filter. - $message = 'Found Twig default unescape filter: "|raw".'; - $this->phpcsFile->addWarning( $message, $stackPtr, 'RawFound' ); - } - } -} diff --git a/WordPressVIPMinimum/Sniffs/Security/UnderscorejsSniff.php b/WordPressVIPMinimum/Sniffs/Security/UnderscorejsSniff.php index ecb40db2..33339362 100644 --- a/WordPressVIPMinimum/Sniffs/Security/UnderscorejsSniff.php +++ b/WordPressVIPMinimum/Sniffs/Security/UnderscorejsSniff.php @@ -4,11 +4,13 @@ * * @package VIPCS\WordPressVIPMinimum * @link https://github.com/Automattic/VIP-Coding-Standards + * @license https://opensource.org/license/gpl-2-0 GPL-2.0 */ namespace WordPressVIPMinimum\Sniffs\Security; use PHP_CodeSniffer\Util\Tokens; +use PHPCSUtils\Utils\FilePath; use PHPCSUtils\Utils\TextStrings; use WordPressVIPMinimum\Sniffs\Sniff; @@ -50,7 +52,7 @@ class UnderscorejsSniff extends Sniff { /** * Returns an array of tokens this test wants to listen for. * - * @return array + * @return array */ public function register() { $targets = Tokens::$textStringTokens; @@ -71,7 +73,7 @@ public function process_token( $stackPtr ) { /* * Ignore Gruntfile.js files as they are configuration, not code. */ - $file_name = TextStrings::stripQuotes( $this->phpcsFile->getFileName() ); + $file_name = FilePath::getName( $this->phpcsFile ); $file_name = strtolower( basename( $file_name ) ); if ( $file_name === 'gruntfile.js' ) { diff --git a/WordPressVIPMinimum/Sniffs/Security/VuejsSniff.php b/WordPressVIPMinimum/Sniffs/Security/VuejsSniff.php index 7f9d9e73..bf7de67a 100644 --- a/WordPressVIPMinimum/Sniffs/Security/VuejsSniff.php +++ b/WordPressVIPMinimum/Sniffs/Security/VuejsSniff.php @@ -25,7 +25,7 @@ class VuejsSniff extends Sniff { /** * Returns an array of tokens this test wants to listen for. * - * @return array + * @return array */ public function register() { return [ diff --git a/WordPressVIPMinimum/Sniffs/Sniff.php b/WordPressVIPMinimum/Sniffs/Sniff.php index 8a53228f..66a987d0 100644 --- a/WordPressVIPMinimum/Sniffs/Sniff.php +++ b/WordPressVIPMinimum/Sniffs/Sniff.php @@ -4,6 +4,7 @@ * * @package VIPCS\WordPressVIPMinimum * @link https://github.com/Automattic/VIP-Coding-Standards + * @license https://opensource.org/license/gpl-2-0 GPL-2.0 * @license https://opensource.org/licenses/MIT MIT */ diff --git a/WordPressVIPMinimum/Sniffs/UserExperience/AdminBarRemovalSniff.php b/WordPressVIPMinimum/Sniffs/UserExperience/AdminBarRemovalSniff.php index 3db0aea6..198051d7 100644 --- a/WordPressVIPMinimum/Sniffs/UserExperience/AdminBarRemovalSniff.php +++ b/WordPressVIPMinimum/Sniffs/UserExperience/AdminBarRemovalSniff.php @@ -4,13 +4,16 @@ * * @package VIPCS\WordPressVIPMinimum * @link https://github.com/Automattic/VIP-Coding-Standards + * @license https://opensource.org/license/gpl-2-0 GPL-2.0 * @license https://opensource.org/licenses/MIT MIT */ namespace WordPressVIPMinimum\Sniffs\UserExperience; use PHP_CodeSniffer\Util\Tokens; +use PHPCSUtils\Utils\FilePath; use PHPCSUtils\Utils\GetTokensAsString; +use PHPCSUtils\Utils\PassedParameters; use PHPCSUtils\Utils\TextStrings; use WordPressCS\WordPress\AbstractFunctionParameterSniff; @@ -18,15 +21,13 @@ * Discourages removal of the admin bar. * * @link https://docs.wpvip.com/technical-references/code-review/vip-warnings/#h-removing-the-admin-bar - * - * @since 0.5.0 */ class AdminBarRemovalSniff extends AbstractFunctionParameterSniff { /** * A list of tokenizers this sniff supports. * - * @var array + * @var array */ public $supportedTokenizers = [ 'PHP', @@ -48,17 +49,18 @@ class AdminBarRemovalSniff extends AbstractFunctionParameterSniff { /** * Functions this sniff is looking for. * - * @var array + * @var array Key is the function name, value irrelevant. */ protected $target_functions = [ 'show_admin_bar' => true, 'add_filter' => true, + 'add_action' => true, // Alias of add_filter(). ]; /** * CSS properties this sniff is looking for. * - * @var array + * @var array> */ protected $target_css_properties = [ 'visibility' => [ @@ -78,7 +80,7 @@ class AdminBarRemovalSniff extends AbstractFunctionParameterSniff { /** * CSS selectors this sniff is looking for. * - * @var array + * @var array */ protected $target_css_selectors = [ '.show-admin-bar', @@ -86,11 +88,11 @@ class AdminBarRemovalSniff extends AbstractFunctionParameterSniff { ]; /** - * String tokens within PHP files we want to deal with. + * Text string tokens within PHP files we want to deal with. * * Set from the register() method. * - * @var array + * @var array */ private $string_tokens = []; @@ -118,10 +120,10 @@ class AdminBarRemovalSniff extends AbstractFunctionParameterSniff { /** * Returns an array of tokens this test wants to listen for. * - * @return array + * @return array */ public function register() { - // Set up all string targets. + // Set up all text string targets. $this->string_tokens = Tokens::$textStringTokens; $targets = $this->string_tokens; @@ -157,8 +159,8 @@ public function register() { */ public function process_token( $stackPtr ) { - $file_name = $this->phpcsFile->getFilename(); - $file_extension = substr( strrchr( $file_name, '.' ), 1 ); + $file_name = FilePath::getName( $this->phpcsFile ); + $file_extension = pathinfo( $file_name, \PATHINFO_EXTENSION ); if ( $file_extension === 'css' ) { if ( $this->tokens[ $stackPtr ]['code'] === \T_STYLE ) { @@ -201,21 +203,42 @@ public function process_parameters( $stackPtr, $group_name, $matched_content, $p $error = false; switch ( $matched_content ) { case 'show_admin_bar': + $show_param = PassedParameters::getParameterFromStack( $parameters, 1, 'show' ); + if ( $show_param === false ) { + break; + } + $error = true; - if ( $this->remove_only === true && $parameters[1]['raw'] === 'true' ) { + if ( $this->remove_only === true && $show_param['clean'] === 'true' ) { $error = false; } break; + case 'add_action': case 'add_filter': - $filter_name = TextStrings::stripQuotes( $parameters[1]['raw'] ); + $hook_name_param = PassedParameters::getParameterFromStack( $parameters, 1, 'hook_name' ); + if ( $hook_name_param === false ) { + break; + } + + $filter_name = TextStrings::stripQuotes( $hook_name_param['clean'] ); if ( $filter_name !== 'show_admin_bar' ) { break; } - $error = true; - if ( $this->remove_only === true && isset( $parameters[2]['raw'] ) && TextStrings::stripQuotes( $parameters[2]['raw'] ) === '__return_true' ) { - $error = false; + $callback_param = PassedParameters::getParameterFromStack( $parameters, 2, 'callback' ); + $error = true; + if ( $this->remove_only === true && $callback_param !== false ) { + $clean_param = strtolower( TextStrings::stripQuotes( $callback_param['clean'] ) ); + + $expected = Tokens::$emptyTokens + Tokens::$stringTokens; + $has_non_textstring = $this->phpcsFile->findNext( $expected, $callback_param['start'], ( $callback_param['end'] + 1 ), true ); + if ( ( $has_non_textstring === false && $clean_param === '__return_true' ) + || ( $has_non_textstring !== false + && preg_match( '`^\\\\?__return_true\s*\(\s*\.\.\.\s*\)$`', $clean_param ) === 1 ) + ) { + $error = false; + } } break; } diff --git a/WordPressVIPMinimum/Sniffs/Variables/RestrictedVariablesSniff.php b/WordPressVIPMinimum/Sniffs/Variables/RestrictedVariablesSniff.php index 65687642..033bc585 100644 --- a/WordPressVIPMinimum/Sniffs/Variables/RestrictedVariablesSniff.php +++ b/WordPressVIPMinimum/Sniffs/Variables/RestrictedVariablesSniff.php @@ -4,6 +4,7 @@ * * @package VIPCS\WordPressVIPMinimum * @link https://github.com/Automattic/VIP-Coding-Standards + * @license https://opensource.org/license/gpl-2-0 GPL-2.0 * @license https://opensource.org/licenses/MIT MIT */ @@ -13,8 +14,6 @@ /** * Restricts usage of some variables in VIP context. - * - * @since 0.5.0 */ class RestrictedVariablesSniff extends AbstractVariableRestrictionsSniff { @@ -31,7 +30,7 @@ class RestrictedVariablesSniff extends AbstractVariableRestrictionsSniff { * ) * ) * - * @return array + * @return array>> */ public function getGroups() { return [ diff --git a/WordPressVIPMinimum/Sniffs/Variables/ServerVariablesSniff.php b/WordPressVIPMinimum/Sniffs/Variables/ServerVariablesSniff.php index 09f00bf9..f3769380 100644 --- a/WordPressVIPMinimum/Sniffs/Variables/ServerVariablesSniff.php +++ b/WordPressVIPMinimum/Sniffs/Variables/ServerVariablesSniff.php @@ -1,13 +1,16 @@ > */ public $restrictedVariables = [ 'authVariables' => [ @@ -35,7 +38,7 @@ class ServerVariablesSniff extends Sniff { /** * Returns an array of tokens this test wants to listen for. * - * @return array + * @return array */ public function register() { return [ @@ -52,21 +55,94 @@ public function register() { */ public function process_token( $stackPtr ) { - if ( $this->tokens[ $stackPtr ]['content'] !== '$_SERVER' ) { - // Not the variable we are looking for. + if ( $this->tokens[ $stackPtr ]['content'] !== '$_SERVER' + && $this->tokens[ $stackPtr ]['content'] !== '$GLOBALS' + ) { + // Not a variable we are looking for. return; } - $variableNamePtr = $this->phpcsFile->findNext( [ T_CONSTANT_ENCAPSED_STRING ], $stackPtr + 1, null, false, null, true ); - $variableName = str_replace( [ "'", '"' ], '', $this->tokens[ $variableNamePtr ]['content'] ); + $searchStart = $stackPtr; + if ( $this->tokens[ $stackPtr ]['content'] === '$GLOBALS' ) { + $globalsIndexPtr = $this->get_array_access_key( $stackPtr ); + if ( $globalsIndexPtr === false ) { + // Couldn't find an array index token usable for the purposes of this sniff. Bow out. + return; + } - if ( isset( $this->restrictedVariables['authVariables'][ $variableName ] ) ) { + $globalsIndexName = TextStrings::stripQuotes( $this->tokens[ $globalsIndexPtr ]['content'] ); + if ( $globalsIndexName !== '_SERVER' ) { + // Not access to `$GLOBALS['_SERVER']`. + return; + } + + // Set the start point for the next array access key search to the close bracket of this array index. + // No need for defensive coding as we already know there will be a valid close bracket next. + $searchStart = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $globalsIndexPtr + 1 ), null, true ); + } + + $prevNonEmpty = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $stackPtr - 1 ), null, true ); + if ( $this->tokens[ $prevNonEmpty ]['code'] === T_DOUBLE_COLON ) { + // Access to OO property mirroring the name of the superglobal. Not our concern. + return; + } + + $indexPtr = $this->get_array_access_key( $searchStart ); + if ( $indexPtr === false ) { + // Couldn't find an array index token usable for the purposes of this sniff. Bow out as undetermined. + return; + } + + $indexName = TextStrings::stripQuotes( $this->tokens[ $indexPtr ]['content'] ); + + if ( isset( $this->restrictedVariables['authVariables'][ $indexName ] ) ) { $message = 'Basic authentication should not be handled via PHP code.'; $this->phpcsFile->addError( $message, $stackPtr, 'BasicAuthentication' ); - } elseif ( isset( $this->restrictedVariables['userControlledVariables'][ $variableName ] ) ) { + } elseif ( isset( $this->restrictedVariables['userControlledVariables'][ $indexName ] ) ) { $message = 'Header "%s" is user-controlled and should be properly validated before use.'; - $data = [ $variableName ]; + $data = [ $indexName ]; $this->phpcsFile->addError( $message, $stackPtr, 'UserControlledHeaders', $data ); } } + + /** + * Get the array access key. + * + * Find the array access key and check if it is: + * - comprised of a single functional token. + * - that token is a T_CONSTANT_ENCAPSED_STRING. + * + * @param int $stackPtr The position of either a variable or the close bracket of a previous array access. + * + * @return int|false Stack pointer to the index token; or FALSE for + * live coding, non-indexed array assignment, or non plain text array keys. + */ + private function get_array_access_key( $stackPtr ) { + $openBracket = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $stackPtr + 1 ), null, true ); + if ( $openBracket === false + || $this->tokens[ $openBracket ]['code'] !== T_OPEN_SQUARE_BRACKET + || isset( $this->tokens[ $openBracket ]['bracket_closer'] ) === false + ) { + // If it isn't an open bracket, this isn't array access. And without closer, it is a parse error/live coding. + return false; + } + + $closeBracket = $this->tokens[ $openBracket ]['bracket_closer']; + + $indexPtr = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $openBracket + 1 ), $closeBracket, true ); + if ( $indexPtr === false + || $this->tokens[ $indexPtr ]['code'] !== T_CONSTANT_ENCAPSED_STRING + ) { + // No array access (like for array assignment without key) or key is not plain text. + return false; + } + + $hasOtherTokens = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $indexPtr + 1 ), $closeBracket, true ); + if ( $hasOtherTokens !== false ) { + // The array index is comprised of multiple tokens. Bow out as undetermined. + return false; + } + + return $indexPtr; + } } diff --git a/WordPressVIPMinimum/Tests/Classes/DeclarationCompatibilityUnitTest.1.inc b/WordPressVIPMinimum/Tests/Classes/DeclarationCompatibilityUnitTest.1.inc new file mode 100644 index 00000000..8ae13ced --- /dev/null +++ b/WordPressVIPMinimum/Tests/Classes/DeclarationCompatibilityUnitTest.1.inc @@ -0,0 +1,189 @@ + Key is the line number, value is the number of expected errors. */ - public function getErrorList() { - return [ - 4 => 1, - 7 => 1, - 10 => 1, - 13 => 1, - 16 => 1, - 19 => 1, - 25 => 1, - 40 => 1, - 43 => 1, - 46 => 1, - 49 => 1, - 52 => 1, - 61 => 1, - 67 => 1, - 70 => 1, - 76 => 1, - 79 => 1, - 88 => 1, - 106 => 1, - 112 => 1, - 119 => 1, - 128 => 1, - 134 => 1, - 137 => 1, - 140 => 1, - ]; + public function getErrorList( $testFile = '' ) { + switch ( $testFile ) { + case 'DeclarationCompatibilityUnitTest.1.inc': + return [ + 53 => 1, + 54 => 1, + 55 => 1, + 56 => 1, + 57 => 1, + 58 => 1, + 59 => 1, + 60 => 1, + 61 => 1, + 62 => 1, + 63 => 1, + 64 => 1, + 68 => 1, + 69 => 1, + 70 => 1, + 71 => 1, + 75 => 1, + 76 => 1, + 77 => 1, + 78 => 1, + 79 => 1, + 80 => 1, + 81 => 1, + 82 => 1, + 83 => 1, + 84 => 1, + 85 => 1, + 86 => 1, + 87 => 1, + 88 => 1, + 89 => 1, + 90 => 1, + 91 => 1, + 92 => 1, + 96 => 1, + 97 => 1, + 98 => 1, + 99 => 1, + 100 => 1, + 104 => 1, + 105 => 1, + 106 => 1, + 107 => 1, + 108 => 1, + 109 => 1, + 110 => 1, + 111 => 1, + 112 => 1, + 113 => 1, + 114 => 1, + 115 => 1, + 156 => 1, + 157 => 1, + 158 => 1, + 159 => 1, + 160 => 1, + 161 => 1, + 162 => 1, + 163 => 1, + 164 => 1, + 165 => 1, + 166 => 1, + 171 => 1, + 172 => 1, + 178 => 1, + 186 => 1, + ]; + + case 'DeclarationCompatibilityUnitTest.2.inc': + return [ + 43 => 1, + 44 => 1, + 45 => 1, + 46 => 1, + 47 => 1, + 48 => 1, + 49 => 1, + 50 => 1, + 51 => 1, + 55 => 1, + 56 => 1, + 57 => 1, + 58 => 1, + 59 => 1, + 60 => 1, + 61 => 1, + 62 => 1, + 66 => 1, + 67 => 1, + 68 => 1, + 72 => 1, + 73 => 1, + 74 => 1, + 75 => 1, + 79 => 1, + 80 => 1, + 81 => 1, + 82 => 1, + 83 => 1, + 84 => 1, + 88 => 1, + 92 => 1, + 93 => 1, + 97 => 1, + 98 => 1, + 99 => 1, + 100 => 1, + 101 => 1, + 102 => 1, + 103 => 1, + 104 => 1, + 105 => 1, + 132 => 1, + 133 => 1, + 134 => 1, + 135 => 1, + 136 => 1, + 137 => 1, + 138 => 1, + 143 => 1, + 144 => 1, + ]; + + default: + return []; + } } /** diff --git a/WordPressVIPMinimum/Tests/Classes/RestrictedExtendClassesUnitTest.inc b/WordPressVIPMinimum/Tests/Classes/RestrictedExtendClassesUnitTest.inc index 49b4aff1..dc302c63 100644 --- a/WordPressVIPMinimum/Tests/Classes/RestrictedExtendClassesUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Classes/RestrictedExtendClassesUnitTest.inc @@ -2,11 +2,21 @@ class OkTestClass extends WPCOM_VIP_CLI_Command { } // Ok - correct class. class AnotherOkTestClass extends WPC__CLI_Command { } // Ok - spacing + similar class. -class Ok_TestClass extends wpcom_vip_cli_command { } // Ok - lowercase correct class. -class Ok_Test_Class extends WpCoM_VIP_cli_command { } // Ok - mixed case correct class. +class Ok_TestClass extends \wpcom_vip_cli_command { } // Ok - lowercase correct class. +$ok = new class extends WpCoM_VIP_cli_command { }; // Ok - mixed case correct class. class WP_CLI_Command { } // Ok - not extending. class BadTestClass extends WP_CLI_Command { } // Warning - wrong class. -class AnotherBadTestClass extends wp_CLI_comMand { } // Warning - mixed case wrong class. -class AnotherBad_TestClass extends WP_CLI_comMand { } // Warning - spacing + mixed case wrong class. +class AnotherBadTestClass extends \wp_CLI_comMand { } // Warning - mixed case wrong class. +$bad = new class() extends WP_CLI_comMand { }; // Warning - spacing + mixed case wrong class. class Bad_Test_Class extends wp_cli_command { } // Warning - lowercase wrong class. + +// phpcs:set WordPressVIPMinimum.Classes.RestrictedExtendClasses exclude[] wp_cli +class IgnoredClass extends WP_CLI_Command { } // OK, group is excluded. + +// Reset to the default value. +// phpcs:set WordPressVIPMinimum.Classes.RestrictedExtendClasses exclude[] + +class Ok_NotTheTargetFQN extends \Fully\Qualified\WP_CLI_Command { } // Ok - not the right namespace. +class Ok_NotTheTargetPQN extends Partially\Qualified\WP_CLI_Command { } // Ok - not the right namespace. +class Bad_NamespaceRelative extends namespace\WP_CLI_Command { } // Warning - this is not a namespaced file, so "namespace" resolves to the global namespace. diff --git a/WordPressVIPMinimum/Tests/Classes/RestrictedExtendClassesUnitTest.php b/WordPressVIPMinimum/Tests/Classes/RestrictedExtendClassesUnitTest.php index 51c1cf00..f5605082 100644 --- a/WordPressVIPMinimum/Tests/Classes/RestrictedExtendClassesUnitTest.php +++ b/WordPressVIPMinimum/Tests/Classes/RestrictedExtendClassesUnitTest.php @@ -36,6 +36,7 @@ public function getWarningList() { 10 => 1, 11 => 1, 12 => 1, + 22 => 1, ]; } } diff --git a/WordPressVIPMinimum/Tests/Constants/ConstantStringUnitTest.inc b/WordPressVIPMinimum/Tests/Constants/ConstantStringUnitTest.inc index ec4ab2f4..ffd04cd6 100644 --- a/WordPressVIPMinimum/Tests/Constants/ConstantStringUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Constants/ConstantStringUnitTest.inc @@ -31,3 +31,38 @@ if ( ! defined($generator->get()) { // OK. $defined = defined(); // OK, ignore. $defined = defined( /*comment*/ ); // OK, ignore. + +// Safeguard that calls to namespaced functions or methods are not confused with the global functions. +if ( ! Not\defined( WPCOM_VIP ) ) { // Okay. + $obj->define( WPCOM_VIP, true ); // Okay. +} + +if ( ! \Fully\Qualified\defined( WPCOM_VIP ) ) { // Okay. + namespace\define( WPCOM_VIP, true ); // Okay. +} + +// Safeguard that fully qualified function calls are handled correctly. +if ( ! \defined( 'WPCOM_VIP' ) ) { // Okay. + \define( 'WPCOM_VIP', true, ); // Okay. +} + +if ( ! \defined( WPCOM_VIP, ) ) { // Error. + \define( WPCOM_VIP, true ); // Error. +} + +// Safeguard correct handling of case-insensitivity of function names. +if ( ! DEFINED( WPCOM_VIP ) ) { // Error. + Define( WPCOM_VIP, true ); // Error. +} + +// Safeguard correct handling of function calls using PHP 8.0+ named parameters. +\defined( constant_nam: WPCOM_VIP ); // Okay, well not really, typo in param name, but that's not the concern of the sniff. +define(value: true, constant_name: 'WPCOM_VIP', ); // Okay. +define(case_insensitive: true, constant_name: WPCOM_VIP ); // Error. + +// Looks like a function call, but is a PHP 8.0+ class instantiation via an attribute. +#[Define('name', 'value')] +function foo() {} + +define(...$params); // PHP 5.6 argument unpacking will be ignored. +add_action('my_action', define(...)); // PHP 8.1 first class callable will be ignored. diff --git a/WordPressVIPMinimum/Tests/Constants/ConstantStringUnitTest.php b/WordPressVIPMinimum/Tests/Constants/ConstantStringUnitTest.php index f2cfe47d..73efa934 100644 --- a/WordPressVIPMinimum/Tests/Constants/ConstantStringUnitTest.php +++ b/WordPressVIPMinimum/Tests/Constants/ConstantStringUnitTest.php @@ -23,8 +23,13 @@ class ConstantStringUnitTest extends AbstractSniffUnitTest { */ public function getErrorList() { return [ - 7 => 1, - 8 => 1, + 7 => 1, + 8 => 1, + 49 => 1, + 50 => 1, + 54 => 1, + 55 => 1, + 61 => 1, ]; } diff --git a/WordPressVIPMinimum/Tests/Constants/RestrictedConstantsUnitTest.inc b/WordPressVIPMinimum/Tests/Constants/RestrictedConstantsUnitTest.inc index 3fd9cd23..ff0c678f 100644 --- a/WordPressVIPMinimum/Tests/Constants/RestrictedConstantsUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Constants/RestrictedConstantsUnitTest.inc @@ -20,4 +20,6 @@ if ( defined( 'JETPACK_DEV_DEBUG' ) ) { // Okay. Can touch. if ( constant( 'WP_CRON_CONTROL_SECRET' ) ) { // Okay. Can touch. -} \ No newline at end of file +} + +define( '"A8C_PROXIED_REQUEST"', false ); // Okay, well not really as the name is not a valid PHP constant name, but that's not our concern. diff --git a/WordPressVIPMinimum/Tests/Constants/RestrictedConstantsUnitTest.php b/WordPressVIPMinimum/Tests/Constants/RestrictedConstantsUnitTest.php index 3c718f6b..4429b324 100644 --- a/WordPressVIPMinimum/Tests/Constants/RestrictedConstantsUnitTest.php +++ b/WordPressVIPMinimum/Tests/Constants/RestrictedConstantsUnitTest.php @@ -10,7 +10,7 @@ use PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest; /** - * Unit test class for the ConstantRestrictions sniff. + * Unit test class for the RestrictedConstants sniff. * * @covers \WordPressVIPMinimum\Sniffs\Constants\RestrictedConstantsSniff */ diff --git a/WordPressVIPMinimum/Tests/Files/IncludingFileUnitTest.php b/WordPressVIPMinimum/Tests/Files/IncludingFileUnitTest.php index 4c4043b1..202bd4ae 100644 --- a/WordPressVIPMinimum/Tests/Files/IncludingFileUnitTest.php +++ b/WordPressVIPMinimum/Tests/Files/IncludingFileUnitTest.php @@ -8,6 +8,7 @@ namespace WordPressVIPMinimum\Tests\Files; use PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest; + /** * Unit test class for the IncludingFile sniff. * diff --git a/WordPressVIPMinimum/Tests/Functions/DynamicCallsUnitTest.inc b/WordPressVIPMinimum/Tests/Functions/DynamicCallsUnitTest.1.inc similarity index 71% rename from WordPressVIPMinimum/Tests/Functions/DynamicCallsUnitTest.inc rename to WordPressVIPMinimum/Tests/Functions/DynamicCallsUnitTest.1.inc index fc307b2f..976c4c00 100644 --- a/WordPressVIPMinimum/Tests/Functions/DynamicCallsUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Functions/DynamicCallsUnitTest.1.inc @@ -3,7 +3,7 @@ function my_test() { echo esc_html( "foo" ); } - +$irrelevant = 10; $my_notokay_func = 'extract'; $my_notokay_func(); // Bad. @@ -34,5 +34,15 @@ $ensure_no_notices_are_thrown_on_parse_error = /*comment*/ ; $test_double_quoted_string = "assert"; $test_double_quoted_string(); // Bad. -// Intentional parse error. This has to be the last test in the file. -$my_notokay_func +function hasStaticVars() { + static $staticvar_foo = 'irrelevant', $staticvar_bar = 'func_num_args', $staticvar_baz = 'nothing'; + $staticvar_foo(); // OK. + $staticvar_bar(); // Bad. + $staticvar_baz(); // OK. +} + +function functionParams( $param_foo = 'func_get_arg', $param_bar = 'nothing', $param_baz = 'func_num_args') { + $param_foo(); // Bad. + $param_bar(); // OK. + $param_baz(); // Bad. +} diff --git a/WordPressVIPMinimum/Tests/Functions/DynamicCallsUnitTest.2.inc b/WordPressVIPMinimum/Tests/Functions/DynamicCallsUnitTest.2.inc new file mode 100644 index 00000000..fca40064 --- /dev/null +++ b/WordPressVIPMinimum/Tests/Functions/DynamicCallsUnitTest.2.inc @@ -0,0 +1,4 @@ + Key is the line number, value is the number of expected errors. */ - public function getErrorList() { - return [ - 9 => 1, - 15 => 1, - 35 => 1, - ]; + public function getErrorList( $testFile = '' ) { + + switch ( $testFile ) { + case 'DynamicCallsUnitTest.1.inc': + return [ + 9 => 1, + 15 => 1, + 35 => 1, + 40 => 1, + 45 => 1, + 47 => 1, + ]; + + default: + return []; + } } /** diff --git a/WordPressVIPMinimum/Tests/Functions/RestrictedFunctionsUnitTest.inc b/WordPressVIPMinimum/Tests/Functions/RestrictedFunctionsUnitTest.inc index 19cba6b3..4a3f57b4 100644 --- a/WordPressVIPMinimum/Tests/Functions/RestrictedFunctionsUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Functions/RestrictedFunctionsUnitTest.inc @@ -230,3 +230,19 @@ $popular = custom_stats_get_csv( 'postviews', [ 'days' => 2, 'limit' => 20 ] $foo = new Link; // OK, class, not function. $foo = new Mail(); // OK, class, not function. + +class ReturnByRef { + function &chmod($a) {} // OK, method declaration, not function call. +} + +// Class instantiations in PHP 8.0+ attributes are not function calls. +class AttributesShouldBeIgnored { + #[WP_Is_Mobile()] // OK. + public function foo() {} +} + +$wp_rewrite->flush_rules; // OK, property access, not function call. +$wp_rewrite /*comment*/ -> /*comment*/ flush_rules(); // Error. +$wp_rewrite?->flush_rules(); // Error. + +array_walk($roles, add_role(...)); // Error. PHP 8.1 first class callable. diff --git a/WordPressVIPMinimum/Tests/Functions/RestrictedFunctionsUnitTest.php b/WordPressVIPMinimum/Tests/Functions/RestrictedFunctionsUnitTest.php index 0b4fcc5c..1c727fac 100644 --- a/WordPressVIPMinimum/Tests/Functions/RestrictedFunctionsUnitTest.php +++ b/WordPressVIPMinimum/Tests/Functions/RestrictedFunctionsUnitTest.php @@ -97,6 +97,9 @@ public function getErrorList() { 199 => 1, 200 => 1, 228 => 1, + 245 => 1, + 246 => 1, + 248 => 1, ]; } diff --git a/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.inc b/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.inc index 1e625c4e..2636026c 100644 --- a/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.inc @@ -1,13 +1,48 @@ haxx0red'; -$html = '
'; +/* + * Not the sniff target. + */ +use strip_tags; +use MyNs\{ + function strip_tags, +}; + +my\ns\strip_tags($a, $b); +$this->strip_tags($a, $b); +$this?->strip_tags($a, $b); +MyClass::strip_tags($a, $b); +echo STRIP_TAGS; +namespace\strip_tags($a, $b); + +// Looks like a function call, but is a PHP 8.0+ class instantiation via an attribute. +#[Strip_tags('text')] +function foo() {} strip_tag( 'Test', $html ); // Ok - similarly-named function. wp_strip_all_tags( $string ); // Ok. + +/* + * These should all be flagged with a warning. + */ strip_tags( 'Testing' ); // Warning. -strip_tags( 'Test', $html ); // Warning. -strip_tags( 'Test' . ', ' . 'HTML' ); // Warning - concatenation on first parameter. -strip_tags( 'Test, String', $html ); // Warning - comma in first parameter. -strip_tags( $string ); // Warning. +STRIP_TAGS( 'Test', $html ); // Warning. +\strip_tags( 'Test' . ', ' . 'HTML', ); // Warning. +strip_tags( 'Test, String', $html, ); // Warning. +Strip_Tags( $string ); // Warning. + +// The function should always be flagged, even during live coding (missing required parameter). +strip_tags(); + +strip_tags(...$params); // PHP 5.6 argument unpacking. + +// Safeguard correct handling of function calls using PHP 8.0+ named parameters. +strip_tags(allowed_tags: $allowed, string: $html ); // Warning. +strip_tags(string: $html); // Warning. +strip_tags(allowed_tags: $allowed); // Warning. Invalid function call, but that's not the concern of this sniff. + +\strip_tags(string: $html, allowed_tag: $allowed); // Warning (mind: deliberate typo in param name). +strip_tags(html: $html); // Warning (mind: deliberately using incorrect param name). + +add_action('my_action', strip_tags(...)); // PHP 8.1 first class callable. diff --git a/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.php b/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.php index 3dfe3703..cd5ebf6c 100644 --- a/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.php +++ b/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.php @@ -32,11 +32,19 @@ public function getErrorList() { */ public function getWarningList() { return [ - 9 => 1, - 10 => 1, - 11 => 1, - 12 => 1, - 13 => 1, + 29 => 1, + 30 => 1, + 31 => 1, + 32 => 1, + 33 => 1, + 36 => 1, + 38 => 1, + 41 => 1, + 42 => 1, + 43 => 1, + 45 => 1, + 46 => 1, + 48 => 1, ]; } } diff --git a/WordPressVIPMinimum/Tests/Hooks/AlwaysReturnInFilterUnitTest.inc b/WordPressVIPMinimum/Tests/Hooks/AlwaysReturnInFilterUnitTest.inc index 77930dd5..086b0eab 100644 --- a/WordPressVIPMinimum/Tests/Hooks/AlwaysReturnInFilterUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Hooks/AlwaysReturnInFilterUnitTest.inc @@ -185,9 +185,63 @@ class tokenizer_bug_test { add_filter( 'tokenizer_bug', [ $this, 'tokenizer_bug' ] ); } - public function tokenizer_bug( $param ): namespace\Foo {} // Ok (tokenizer bug PHPCS < 3.5.7) / Error in PHPCS >= 3.5.7. + public function tokenizer_bug( $param ): namespace\Foo {} // Error. } +// Arrow function callbacks always return implicitly. Ok. +add_filter( 'good_arrow_function', fn( $x ) => $x . ' suffix' ); + +add_filter( 'good_arrow_function_null', fn( $x ) => null ); // Ok, still returns a value (null). + +add_filter( 'bad_void_return_with_space', function() { // Error. + return ; +} ); + +// === exit/die/throw in filter callbacks (issue #719) === + +// Exit used as unconditional termination path — warning, not error. +function exit_in_filter( $redirect_url ) { // Warning. + if ( some_condition() ) { + return $redirect_url; + } + wp_redirect( $redirect_url ); + exit; +} +add_filter( 'redirect_canonical', 'exit_in_filter' ); + +// Die used as unconditional termination path — warning, not error. +add_filter( 'die_in_filter', function( $url ) { // Warning. + if ( some_condition() ) { + return $url; + } + die( 'Terminated' ); +} ); + +// Throw used as unconditional termination path — warning, not error. +class throw_in_filter_class { + public function __construct() { + add_filter( 'throw_in_filter', [ $this, 'validate' ] ); + } + + public function validate( $value ) { // Warning. + if ( is_valid( $value ) ) { + return $value; + } + throw new \RuntimeException( 'Invalid value' ); + } +} + +// Exit with no return at all — warning, not error. +add_filter( 'exit_no_return', function( $input ) { // Warning. + do_something( $input ); + exit; +} ); + +// No return and no exit/die/throw — remains an error. +add_filter( 'still_missing_return', function( $input ) { // Error. + do_something( $input ); +} ); + // Intentional parse error. This has to be the last test in the file! class parse_error_test { public function __construct() { diff --git a/WordPressVIPMinimum/Tests/Hooks/AlwaysReturnInFilterUnitTest.php b/WordPressVIPMinimum/Tests/Hooks/AlwaysReturnInFilterUnitTest.php index 8576297e..9201985f 100644 --- a/WordPressVIPMinimum/Tests/Hooks/AlwaysReturnInFilterUnitTest.php +++ b/WordPressVIPMinimum/Tests/Hooks/AlwaysReturnInFilterUnitTest.php @@ -7,11 +7,10 @@ namespace WordPressVIPMinimum\Tests\Hooks; -use PHP_CodeSniffer\Config; use PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest; /** - * Unit test class for the Hooks/AlwaysReturn sniff. + * Unit test class for the AlwaysReturnInFilter sniff. * * @covers \WordPressVIPMinimum\Sniffs\Hooks\AlwaysReturnInFilterSniff */ @@ -31,7 +30,9 @@ public function getErrorList() { 105 => 1, 129 => 1, 163 => 1, - 188 => version_compare( Config::VERSION, '3.5.7', '>=' ) ? 1 : 0, + 188 => 1, + 196 => 1, + 241 => 1, ]; } @@ -43,6 +44,10 @@ public function getErrorList() { public function getWarningList() { return [ 180 => 1, + 203 => 1, + 213 => 1, + 226 => 1, + 235 => 1, ]; } } diff --git a/WordPressVIPMinimum/Tests/Hooks/RestrictedHooksUnitTest.inc b/WordPressVIPMinimum/Tests/Hooks/RestrictedHooksUnitTest.inc index 9236f8ee..6eab3fdd 100644 --- a/WordPressVIPMinimum/Tests/Hooks/RestrictedHooksUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Hooks/RestrictedHooksUnitTest.inc @@ -1,24 +1,84 @@ add_action($a, $b); +$this?->add_filter($a, $b); +MyClass::add_action($a, $b); +echo ADD_FILTER; +namespace\add_action($a, $b); + + +/* + * These should all be okay. + */ +add_filter( 'not_target_hook', 'good_example_function' ); +\add_filter( 'upload_mimesX' , 'good_example_function' ); + +add_action(...$params); // PHP 5.6 argument unpacking. + +// Looks like a function call, but is a PHP 8.0+ class instantiation via an attribute. +#[ADD_FILTER('text')] +function foo() {} + +// PHP 8.1 first class callable. +// As we have no knowledge about what parameters will be passed, we shouldn't flag this. +array_walk($filters, add_filter(...)); + +// Ignore as undetermined. +Add_Filter( $hook_name, 'undetermined' ); +\add_action( $obj->get_filterName(), 'undetermined' ); +add_filter( MyClass::FILTER_NAME, 'undetermined', ); +\add_filter( "upload_$mimes", 'undetermined' ); + +// Incomplete function call, should be ignored by the sniff. +$incorrect_but_ok = add_filter(); +$incorrect_but_ok = add_action(); + + +/* + * These should all be flagged with a warning. + */ +add_filter( 'do_robotstxt', 'bad_example_function' ); // Simple string. +add_action('upload_mimes' , [$obj, 'method']); // Incorrect spacing. +add_filter( 'robots_txt','bad_example_function'); // Incorrect spacing. +\add_filter( "http_request_timeout" , fn($param) => $param * 10); // Double quotes. + +ADD_FILTER( 'upload_' . 'mimes','bad_example_function'); // Single concatenation. +add_filter( 'upl' . 'oad_' . 'mimes','bad_example_function'); // Multiple concatenation. +add_filter( "upload_" . 'mimes' , bad_example_function(...)); // Single concatenation with double and single quotes. +add_filter( 'upl' . "oad_" . "mimes",'bad_example_function'); // Multiple concatenation with double and single quotes. +\add_action( 'http_request_args', function() { // Anonymous callback. // Do stuff. }); add_action( 'upload_mimes', 'bad_example_function' ); // Check `add_action()`, which is an alias for `add_filter()`. add_filter( 'http_request_timeout', 'bad_example_function' ); // Simple string. add_filter('http_request_args', 'bad_example_function' ); // Simple string + incorrect spacing. -add_action( 'do_robotstxt', 'my_do_robotstxt'); // Simple string. +add_action( /*comment*/ 'do_robotstxt', 'my_do_robotstxt'); // Simple string. add_filter( 'robots_txt', function() { // Anonymous callback. } ); + +// Safeguard correct handling of function calls using PHP 8.0+ named parameters. +add_action(callback: 'invalid', priority: 10); // OK, well, not really, missing required $hook_name param, but that's not the concern of this sniff. +add_action(callback: 'do_robotstxt', hook_name: 'not_our_target'); // OK. +add_action(hookName: 'not_our_target', callback: 'do_robotstxt',); // OK, well, not really, typo in param name, but that's not the concern of the sniff. + +add_filter(priority: 10, hook_name: 'robots_txt', callback: some_function(...) ); // Warning. + +// Hook names are case-sensitive. +add_filter( 'upLoad_mimeS' , $callback); // OK, not our target. + +// Bug fix - spacing vs concatenation. +add_filter('do_' . 'robots' . 'txt', 'bad_example_function'); // Warning. + +// Ignore partially dynamic hook names. +add_filter( 'robots_' . $something . 'txt' , $callback); // OK, ignored as undetermined. +add_filter( 'http_request_timeout' . $something, $callback); // OK, ignored as undetermined. + +// Ensure quote stripping is done correctly. +add_filter( 'upload"_mimes', 'bad_example_function' ); // OK, not a filter we're looking for. +add_filter( "upload_'mimes", 'bad_example_function' ); // OK, not a filter we're looking for. diff --git a/WordPressVIPMinimum/Tests/Hooks/RestrictedHooksUnitTest.php b/WordPressVIPMinimum/Tests/Hooks/RestrictedHooksUnitTest.php index 2f6172be..022e5e1a 100644 --- a/WordPressVIPMinimum/Tests/Hooks/RestrictedHooksUnitTest.php +++ b/WordPressVIPMinimum/Tests/Hooks/RestrictedHooksUnitTest.php @@ -8,8 +8,9 @@ namespace WordPressVIPMinimum\Tests\Hooks; use PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest; + /** - * Unit test class for the Filters/RestrictedHooks sniff. + * Unit test class for the RestrictedHooks sniff. * * @since 0.4.0 * @@ -33,21 +34,22 @@ public function getErrorList() { */ public function getWarningList() { return [ - 7 => 1, - 8 => 1, - 9 => 1, - 10 => 1, - 11 => 1, - 12 => 1, - 13 => 1, - 14 => 1, - 15 => 1, - 16 => 1, - 19 => 1, - 20 => 1, - 21 => 1, - 22 => 1, - 23 => 1, + 46 => 1, + 47 => 1, + 48 => 1, + 49 => 1, + 51 => 1, + 52 => 1, + 53 => 1, + 54 => 1, + 55 => 1, + 58 => 1, + 59 => 1, + 60 => 1, + 61 => 1, + 62 => 1, + 70 => 1, + 76 => 1, ]; } } diff --git a/WordPressVIPMinimum/Tests/JS/DangerouslySetInnerHTMLUnitTest.php b/WordPressVIPMinimum/Tests/JS/DangerouslySetInnerHTMLUnitTest.php index 8f2ca838..68e85bb1 100644 --- a/WordPressVIPMinimum/Tests/JS/DangerouslySetInnerHTMLUnitTest.php +++ b/WordPressVIPMinimum/Tests/JS/DangerouslySetInnerHTMLUnitTest.php @@ -10,7 +10,7 @@ use PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest; /** - * Unit test class for the HTML String concatenation in JS sniff. + * Unit test class for the DangerouslySetInnerHTML sniff. * * @covers \WordPressVIPMinimum\Sniffs\JS\DangerouslySetInnerHTMLSniff */ diff --git a/WordPressVIPMinimum/Tests/JS/InnerHTMLUnitTest.php b/WordPressVIPMinimum/Tests/JS/InnerHTMLUnitTest.php index 20c01177..3c9e5de3 100644 --- a/WordPressVIPMinimum/Tests/JS/InnerHTMLUnitTest.php +++ b/WordPressVIPMinimum/Tests/JS/InnerHTMLUnitTest.php @@ -10,7 +10,7 @@ use PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest; /** - * Unit test class for the HTML String concatenation in JS sniff. + * Unit test class for the InnerHTML sniff. * * @covers \WordPressVIPMinimum\Sniffs\JS\InnerHTMLSniff */ diff --git a/WordPressVIPMinimum/Tests/JS/WindowUnitTest.php b/WordPressVIPMinimum/Tests/JS/WindowUnitTest.php index 32ed5cae..0de812d0 100644 --- a/WordPressVIPMinimum/Tests/JS/WindowUnitTest.php +++ b/WordPressVIPMinimum/Tests/JS/WindowUnitTest.php @@ -10,7 +10,7 @@ use PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest; /** - * Unit test class for the HTML String concatenation in JS sniff. + * Unit test class for the JS/Window sniff. * * @covers \WordPressVIPMinimum\Sniffs\JS\WindowSniff */ diff --git a/WordPressVIPMinimum/Tests/Performance/CacheValueOverrideUnitTest.1.inc b/WordPressVIPMinimum/Tests/Performance/CacheValueOverrideUnitTest.1.inc new file mode 100644 index 00000000..75874857 --- /dev/null +++ b/WordPressVIPMinimum/Tests/Performance/CacheValueOverrideUnitTest.1.inc @@ -0,0 +1,127 @@ +wp_cache_get(); $methodCall = false; +$nullsafeMethodCall = $this?->wp_cache_get(); $nullsafeMethodCall = false; +$staticMethodCall = MyClass::wp_cache_get(); $staticMethodCall = false; +echo WP_CACHE_GET; +$namespacedFunction = namespace\wp_cache_get(); $namespacedFunction = false; + +function &wp_cache_get() {} + +// Looks like a function call, but is a PHP 8.0+ class instantiation via an attribute. +#[WP_Cache_Get('text')] +function foo() {} + + +/* + * These should all be okay. + */ +$good_wp_users = wp_cache_get( md5( self::CACHE_KEY . '_wp_users'), self::CACHE_GROUP ); + +if ( empty( $good_wp_users ) ) { + $good_wp_users = false; + $good_wp_users = get_users(); +} + +function NoAssignment() { + $unrelated_var = false; + \wp_cache_get( $key, $group ) === false && do_something_to_create_new_cache(); + $unrelated_var = false; +} + +function functionCallsAreNotCaseSensitiveGood() { + $retrieved = WP_Cache_Get( ...$params ); + if ( empty( $retrieved ) ) { + $retrieved = false; + } +} + +function fqnFunctionCallGood() { + $no_params_not_our_concern = \wp_cache_get(); + if ( empty( $no_params_not_our_concern ) ) { + $no_params_not_our_concern = false; + } +} + +function cacheOverrideDoesntAssignFalse() { + $not_false = wp_cache_get( ...$params ); + $not_false = true; +} + + +function cacheOverrideAssignsUnknownValue( $unknown ) { + $not_false = wp_cache_get( ...$params ); + $not_false = $unknown; +} + + +/* + * These should all be flagged. + */ +$bad_wp_users = wp_cache_get( md5( self::CACHE_KEY . '_wp_users'), self::CACHE_GROUP ); +$bad_wp_users = false; // Bad. + +if ( empty( $bad_wp_users ) ) { + $bad_wp_users = get_users(); +} + +function functionCallsAreNotCaseSensitiveBad() { + $cache_retrieved = WP_Cache_Get( ...$params ); + // Comment. + do_something_else(); + $cache_retrieved = false; // Bad. +} + +function fqnFunctionCallBad() { + $shouldBeCaught = \wp_cache_get(); + $shouldBeCaught = false; // Bad. +} + +// Ignore PHP 8.1 first class callable. +// Ignore as the assignment is not for the return value of the function, but for the callable. +function firstClassCallable() { + $callable = wp_cache_get(...); + $callable = false; // OK. +} + +// Bug fix - the variable re-assignment must be in the same scope. +function controlStructuresAreNotClosedScopes() { + if (false === ($cache = \wp_cache_get( $key, $group ))) { + // Create new cache. + } + $cache = false; // Bad. +} + + +function closedScopeA() { + $myCache = \wp_cache_get(); +} + +function closedScopeB($myCache = false) {} // OK. + +function closedScopeC() { + $yourCache = wp_cache_get(); + + $closure = function () { + $yourCache = false; // OK. + }; +} + +$globalScope = wp_cache_get(); +function closedScopeD() { + $globalScope = false; // OK. +} + +$globalScope = wp_cache_get(); +function closedScopeE($globalScope = false;) {} // OK. + +$globalScope = wp_cache_get(); +class FooBar { + function method($globalScope = false) {} // OK. +} diff --git a/WordPressVIPMinimum/Tests/Performance/CacheValueOverrideUnitTest.2.inc b/WordPressVIPMinimum/Tests/Performance/CacheValueOverrideUnitTest.2.inc new file mode 100644 index 00000000..37435fd1 --- /dev/null +++ b/WordPressVIPMinimum/Tests/Performance/CacheValueOverrideUnitTest.2.inc @@ -0,0 +1,5 @@ + Key is the line number, value is the number of expected errors. */ - public function getErrorList() { - return [ - 5 => 1, - ]; + public function getErrorList( $testFile = '' ) { + + switch ( $testFile ) { + case 'CacheValueOverrideUnitTest.1.inc': + return [ + 68 => 1, + 78 => 1, + 83 => 1, + 98 => 1, + ]; + + default: + return []; + } } /** diff --git a/WordPressVIPMinimum/Tests/Performance/FetchingRemoteDataUnitTest.inc b/WordPressVIPMinimum/Tests/Performance/FetchingRemoteDataUnitTest.inc index 9edbd772..40e48b7f 100644 --- a/WordPressVIPMinimum/Tests/Performance/FetchingRemoteDataUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Performance/FetchingRemoteDataUnitTest.inc @@ -1,7 +1,88 @@ file_get_contents($url); +$this?->file_get_contents($file); +MyClass::file_get_contents($file); +echo FILE_GET_CONTENTS; +namespace\file_get_contents($url); -$external_resource = file_get_contents( 'https://example.com' ); // NOK. \ No newline at end of file + +/* + * These should all be okay. + */ +$file_content = file_get_contents( 'my-file.css' ); +$file_content = \File_Get_Contents( /*comment*/ 'my-file.svg', ); +$file_content = file_get_contents( __DIR__ . "/filename.css" ); + +// Looks like a function call, but is a PHP 8.0+ class instantiation via an attribute. +#[File_Get_Contents('text')] +function foo() {} + +// Incomplete function call, should be ignored by the sniff. +$live_coding = file_get_contents(); + + +/* + * These should all be flagged with a warning. + */ +// Warning FileGetContentsRemoteFile. +$external_resource = file_get_contents( 'https://example.com', ); +\file_get_contents( "http://$url" ); +$external_resource = FILE_GET_CONTENTS( + // Sort of protocol relative URL (the ":" is superfluous). + '://example.com' +); +$external_resource = file_get_contents( 'HTTP://example.com', ); + +// Warning FileGetContentsUnknown. +file_get_contents( $url, true, $context, $offset, $length ); +\File_GET_Contents( $obj->get_fileName() ); +file_get_contents( MyClass::FILE_NAME, ); + +file_get_contents(...$params); // PHP 5.6 argument unpacking. Warning FileGetContentsUnknown. + +// Safeguard correct handling of function calls using PHP 8.0+ named parameters. +file_get_contents(context: $context, ); // OK, well, not really, missing required $filename param, but that's not the concern of this sniff. +file_get_contents(file: 'https://example.com',); // OK, well, not really, typo in param name, but that's not the concern of the sniff. +file_get_contents( + context: stream_context_create( + options: [ + $proxy => 'tcp://proxy.example.com:5100', + ], + ), + filename: 'my-file.svg', +); // OK. + +file_get_contents( + context: stream_context_create( + options: [ + $cafile => __DIR__ . '/cacert.pem', + ], + ), + filename: 'https://example.com' +); // Warning FileGetContentsRemoteFile. +file_get_contents( + context: stream_context_create( + options: [ + $cafile => __DIR__ . '/cacert.pem', + ], + ), + filename: $fileName, +); // Warning FileGetContentsUnknown. + +array_walk($files, file_get_contents(...)); // PHP 8.1 first class callable. Warning FileGetContentsUnknown. + +// Bug: don't throw a warning when __DIR__ is used as this indicates use with a local file. +\file_get_contents( __DIR__ . $filename ); // OK. + +// Bug: check all text string tokens in the parameter. +$result = file_get_contents(( is_ssl() ? 'http' : 'https' ) . '://example.com'); // Warning FileGetContentsRemoteFile. + +// Bug: don't presume if the parameter contains a text string token, that will be the only token. +\file_get_contents( $url . '/filename.css' ); // Warning FileGetContentsUnknown. diff --git a/WordPressVIPMinimum/Tests/Performance/FetchingRemoteDataUnitTest.php b/WordPressVIPMinimum/Tests/Performance/FetchingRemoteDataUnitTest.php index 05c27d4d..9f9ede3a 100644 --- a/WordPressVIPMinimum/Tests/Performance/FetchingRemoteDataUnitTest.php +++ b/WordPressVIPMinimum/Tests/Performance/FetchingRemoteDataUnitTest.php @@ -10,7 +10,7 @@ use PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest; /** - * Unit test class for the ExitAfterRedirect sniff. + * Unit test class for the FetchingRemoteData sniff. * * @covers \WordPressVIPMinimum\Sniffs\Performance\FetchingRemoteDataSniff */ @@ -32,7 +32,19 @@ public function getErrorList() { */ public function getWarningList() { return [ - 7 => 1, + 35 => 1, + 36 => 1, + 37 => 1, + 41 => 1, + 44 => 1, + 45 => 1, + 46 => 1, + 48 => 1, + 62 => 1, + 70 => 1, + 79 => 1, + 85 => 1, + 88 => 1, ]; } } diff --git a/WordPressVIPMinimum/Tests/Performance/LowExpiryCacheTimeUnitTest.inc b/WordPressVIPMinimum/Tests/Performance/LowExpiryCacheTimeUnitTest.1.inc similarity index 60% rename from WordPressVIPMinimum/Tests/Performance/LowExpiryCacheTimeUnitTest.inc rename to WordPressVIPMinimum/Tests/Performance/LowExpiryCacheTimeUnitTest.1.inc index a6572d6d..603881ad 100644 --- a/WordPressVIPMinimum/Tests/Performance/LowExpiryCacheTimeUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Performance/LowExpiryCacheTimeUnitTest.1.inc @@ -10,12 +10,12 @@ wp_cache_set( $testing, $data, 'test_group', 5*MINUTE_IN_SECONDS ); wp_cache_set( 123, $data, 'test_group', 5 * MINUTE_IN_SECONDS ); wp_cache_set( 1234, $data, '', 425 ); wp_cache_set( $testing, $data, null, 350 ); -wp_cache_set( $testing, $data ); +\wp_cache_set( $testing, $data ); wp_cache_set( 'test', $data, $group ); wp_cache_add( 'test', $data, $group, 300 ); wp_cache_add( $testing, $data, 'test_group', 6*MINUTE_IN_SECONDS ); -wp_cache_add( 1234, $data, '', 425 ); +WP_CACHE_ADD( 1234, $data, '', 425 ); wp_cache_add( $testing, $data, null, 350 ); wp_cache_replace( 'test', $data, $group, 300 ); @@ -30,13 +30,13 @@ wp_cache_set( 123, $data, null, 1.5 * MINUTE_IN_SECONDS ); // Lower than 300. wp_cache_set( $testing, $data, '', 1.5 * MINUTE_IN_SECONDS ); // Lower than 300. wp_cache_add( 'test', $data, $group, 100 ); // Lower than 300. -wp_cache_add( 'test', $data, $group, 2*MINUTE_IN_SECONDS ); // Lower than 300. +\wp_cache_add( 'test', $data, $group, 2*MINUTE_IN_SECONDS ); // Lower than 300. wp_cache_add( 123, $data, null, 1.5 * MINUTE_IN_SECONDS ); // Lower than 300. wp_cache_add( $testing, $data, '', 1.5 * MINUTE_IN_SECONDS ); // Lower than 300. wp_cache_replace( 'test', $data, $group, 100 ); // Lower than 300. wp_cache_replace( 'test', $data, $group, 2*MINUTE_IN_SECONDS ); // Lower than 300. -wp_cache_replace( 123, $data, null, 1.5 * MINUTE_IN_SECONDS ); // Lower than 300. +WP_CACHE_REPLACE( 123, $data, null, 1.5 * MINUTE_IN_SECONDS ); // Lower than 300. wp_cache_replace( $testing, $data, '', 1.5 * MINUTE_IN_SECONDS ); // Lower than 300. // Test error being reported on the line containing the parameter. @@ -49,7 +49,7 @@ wp_cache_replace( // Test calculations with floats. wp_cache_replace( $testing, $data, '', 7.5 * MINUTE_IN_SECONDS ); // OK. -wp_cache_replace( $testing, $data, '', 500 * 0.1 ); // Bad. +wp_cache_replace( $testing, $data, '', 500 * 0.1, ); // Bad. // Test comment handling. wp_cache_add( 'test', $data, $group, /* Deliberately left empty */ ); // OK. @@ -71,11 +71,11 @@ wp_cache_add( ); // OK. // Test variable/constant with or without calculation being passed. -wp_cache_set( $key, $data, '', $time ); // Manual inspection warning. +WP_Cache_Set( $key, $data, '', $time ); // Manual inspection warning. wp_cache_set( $key, $data, '', PREFIX_FIVE_MINUTES ); // Manual inspection warning. -wp_cache_set( $key, $data, '', 20 * $time ); // Manual inspection warning. -wp_cache_set( $key, $data, '', $base + $extra ); // Manual inspection warning. -wp_cache_set( $key, $data, '', 300 + $extra ); // Manual inspection warning. +wp_cache_set( $key, $data, '', 20 * $time /*comment*/ ); // Manual inspection warning. +wp_cache_set( $key, $data, '', $base + $extra, ); // Manual inspection warning. +\wp_cache_set( $key, $data, '', 300 + $extra ); // Manual inspection warning. wp_cache_set( $key, $data, '', PREFIX_CUSTOM_TIME * 5); // Manual inspection warning. // Test calculations with additional aritmetic operators. @@ -84,9 +84,9 @@ wp_cache_add( 'test', $data, $group, WEEK_IN_SECONDS / 3 + HOUR_IN_SECONDS ); / // Test calculations grouped with parentheses. wp_cache_set( $key, $data, '', (24 * 60 * 60) ); // OK. -wp_cache_set( $key, $data, '', (-(2 * 60) + 600) ); // OK. +wp_cache_set( $key, $data, '', (-(2 * 60) + 600), ); // OK. wp_cache_set( $key, $data, '', (2 * 60) ); // Bad. -wp_cache_set( $key, $data, '', (-(2 * 60) + 600 ); // OK - includes parse error, close parenthesis missing. + // Test handling of numbers passed as strings. wp_cache_set( 'test', $data, $group, '300' ); // OK - type cast to integer within the function. @@ -110,10 +110,10 @@ wp_cache_set( 'test', $data, $group, \MONTH_IN_SECONDS ); // OK. // Test passing something which may look like one of the time constants, but isn't. wp_cache_set( 'test', $data, $group, month_in_seconds ); // Bad - constants are case-sensitive. -wp_cache_set( 'test', $data, $group, HOUR_IN_SECONDS::methodName() ); // Bad - not a constant. +wp_cache_set( 'test', $data, $group, /*comment*/ HOUR_IN_SECONDS::methodName() ); // Bad - not a constant. wp_cache_set( 'test', $data, $group, $obj->MONTH_IN_SECONDS ); // Bad - not a constant. wp_cache_set( 'test', $data, $group, $obj::MONTH_IN_SECONDS ); // Bad - not the WP constant. -wp_cache_set( 'test', $data, $group, PluginNamespace\SubLevel\DAY_IN_SECONDS ); // Bad - not the WP constant. +WP_Cache_Set( 'test', $data, $group, PluginNamespace\SubLevel\DAY_IN_SECONDS, ); // Bad - not the WP constant. // Test passing negative number as cache time. wp_cache_set( 'test', $data, $group, -300 ); // Bad. @@ -121,3 +121,41 @@ wp_cache_add( $testing, $data, 'test_group', -6 * MINUTE_IN_SECONDS ); // Bad. // Test more complex logic in the parameter. wp_cache_add( $key, $data, '', ($toggle ? 200 : 400) ); // Manual inspection warning. + +// Test handling of non-numeric data in text string. +wp_cache_set( 'test', $data, $group, '' ); // Manual inspection warning. +wp_cache_set( 'test', $data, $group, '300 Mulberry Street' ); // Manual inspection warning. + +// Test handling of edge case/parse error. +wp_cache_set( 'test', $data, $group,\); // OK, ignore. + +// Test handling of some modern PHP syntaxes. +wp_cache_add('test', $data, $group, ...$params); // PHP 5.6 argument unpacking. Manual inspection warning. +wp_cache_add( $key, $data, '', $toggle ?? 400) ); // PHP 7.0 null coalesce. Manual inspection warning. +add_action('my_action', wp_cache_set(...)); // PHP 8.1 first class callable. OK, ignore. + +// Looks like a function call, but is a PHP 8.0+ class instantiation via an attribute. +#[WP_Cache_Replace('text', 'data', 'group', 50)] +function foo() {} + +// Alternative numeric base. +wp_cache_set( $key, $data, '', 0620 ); // Octal number. OK (=400). +wp_cache_set( $key, $data, '', 0x190 ); // Hexidecimal number. OK (=400). +wp_cache_set( $key, $data, '', 0b110010000 ); // PHP 5.4 binary number. OK (=400). +wp_cache_set( $key, $data, '', 1_000 ); // PHP 7.4 numeric literal with underscore. OK. +wp_cache_set( $key, $data, '', 0o620 ); // PHP 8.1 octal literal. OK (=400). + +wp_cache_set( $key, $data, '', 0226 ); // Octal number. Bad (=150). +wp_cache_set( $key, $data, '', 0x96 ); // Hexidecimal number. Bad (=150). +wp_cache_set( $key, $data, '', 0b10010110 ); // PHP 5.4 binary number. Bad (=150). +wp_cache_set( $key, $data, '', 1_50 ); // PHP 7.4 numeric literal with underscore. Bad. +wp_cache_set( $key, $data, '', 0o226 ); // PHP 8.1 octal literal. Bad (=150). + +// Safeguard handling of function calls using PHP 8.0+ named parameters. +wp_cache_add(data: $data, group: $group); // OK, well, not really, missing required $key param, but that's not the concern of this sniff. +wp_cache_replace(data: $data, expire: 400, group: $group); // OK. +wp_cache_add($key, group: $group, data: $data, expires: 100,); // OK, well, not really, typo in param name, but that's not the concern of the sniff. +wp_cache_replace($key, expire: 400, group: $group, data: 100,); // OK. + +wp_cache_replace($key, $data, expire: 100 ); // Bad. +wp_cache_replace(expire: 100, data: $data, key: $group); // Bad. diff --git a/WordPressVIPMinimum/Tests/Performance/LowExpiryCacheTimeUnitTest.2.inc b/WordPressVIPMinimum/Tests/Performance/LowExpiryCacheTimeUnitTest.2.inc new file mode 100644 index 00000000..dc33e31f --- /dev/null +++ b/WordPressVIPMinimum/Tests/Performance/LowExpiryCacheTimeUnitTest.2.inc @@ -0,0 +1,5 @@ + Key is the line number, value is the number of expected warnings. */ - public function getWarningList() { - return [ - 27 => 1, - 28 => 1, - 29 => 1, - 30 => 1, - 32 => 1, - 33 => 1, - 34 => 1, - 35 => 1, - 37 => 1, - 38 => 1, - 39 => 1, - 40 => 1, - 47 => 1, - 52 => 1, - 56 => 1, - 74 => 1, - 75 => 1, - 76 => 1, - 77 => 1, - 78 => 1, - 79 => 1, - 82 => ( PHP_VERSION_ID > 50600 ) ? 0 : 1, - 88 => 1, - 94 => 1, - 95 => 1, - 105 => 1, - 112 => 1, - 113 => 1, - 114 => 1, - 115 => 1, - 116 => 1, - 119 => 1, - 120 => 1, - 123 => 1, - ]; + public function getWarningList( $testFile = '' ) { + switch ( $testFile ) { + case 'LowExpiryCacheTimeUnitTest.1.inc': + return [ + 27 => 1, + 28 => 1, + 29 => 1, + 30 => 1, + 32 => 1, + 33 => 1, + 34 => 1, + 35 => 1, + 37 => 1, + 38 => 1, + 39 => 1, + 40 => 1, + 47 => 1, + 52 => 1, + 56 => 1, + 74 => 1, + 75 => 1, + 76 => 1, + 77 => 1, + 78 => 1, + 79 => 1, + 82 => ( PHP_VERSION_ID > 50600 ) ? 0 : 1, + 88 => 1, + 94 => 1, + 95 => 1, + 105 => 1, + 112 => 1, + 113 => 1, + 114 => 1, + 115 => 1, + 116 => 1, + 119 => 1, + 120 => 1, + 123 => 1, + 126 => 1, + 127 => 1, + 133 => 1, + 134 => 1, + 148 => 1, + 149 => 1, + 150 => 1, + 151 => 1, + 152 => 1, + 160 => 1, + 161 => 1, + ]; + + default: + return []; + } } } diff --git a/WordPressVIPMinimum/Tests/Performance/NoPagingUnitTest.inc b/WordPressVIPMinimum/Tests/Performance/NoPagingUnitTest.inc index 9af35243..8272e8e2 100644 --- a/WordPressVIPMinimum/Tests/Performance/NoPagingUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Performance/NoPagingUnitTest.inc @@ -1,15 +1,85 @@ true, // Bad. ); _query_posts( 'nopaging=true' ); // Bad. -$query_args['my_posts_per_page'] = -1; // OK. +$query_args['my_posts_per_page'] = -1; // OK - not a recognized key. // Verify handling with no trailing comma at end of array. $args = array( 'nopaging' => true // Bad. ); $args = [ 'nopaging' => true ]; // Bad. + +// nopaging with integer 1. +$args = [ 'nopaging' => 1 ]; // Bad. + +// nopaging with false - should not trigger. +$args = [ 'nopaging' => false ]; // OK. + +// nopaging with integer 0 - should not trigger. +$args = [ 'nopaging' => 0 ]; // OK. + +// nopaging via variable assignment. +$args['nopaging'] = true; // Bad. + +// nopaging via variable assignment with false. +$args['nopaging'] = false; // OK. + +// === posts_per_page tests === + +// posts_per_page set to -1 in long array syntax. +$args = array( + 'posts_per_page' => -1, // Bad. +); + +// posts_per_page set to -1 in short array syntax. +$args = [ 'posts_per_page' => -1 ]; // Bad. + +// posts_per_page set to -1 as string. +$args = [ 'posts_per_page' => '-1' ]; // Bad. + +// posts_per_page set to a normal value - should not trigger. +$args = [ 'posts_per_page' => 50 ]; // OK. + +// posts_per_page set to 1 - should not trigger. +$args = [ 'posts_per_page' => 1 ]; // OK. + +// numberposts set to -1. +$args = [ 'numberposts' => -1 ]; // Bad. + +// numberposts set to a normal value - should not trigger. +$args = [ 'numberposts' => 10 ]; // OK. + +// posts_per_page via variable assignment. +$args['posts_per_page'] = -1; // Bad. + +// posts_per_page via variable assignment with normal value. +$args['posts_per_page'] = 10; // OK. + +// Query string with posts_per_page=-1. +_query_posts( 'posts_per_page=-1&orderby=date' ); // Bad. + +// Query string with posts_per_page set to a normal value. +_query_posts( 'posts_per_page=50' ); // OK. + +// Query string with numberposts=-1. +_query_posts( 'numberposts=-1' ); // Bad. + +// === Ignoring with phpcs:ignore comments === + +// phpcs:ignore WordPressVIPMinimum.Performance.NoPaging.posts_per_page_posts_per_page -- Intentional: fetching all items for export. +$args = [ 'posts_per_page' => -1 ]; // OK - ignored. + +$args = [ + // phpcs:ignore WordPressVIPMinimum.Performance.NoPaging.posts_per_page_numberposts -- Intentional: legacy API requires all results. + 'numberposts' => -1, // OK - ignored. +]; + +// phpcs:ignore WordPressVIPMinimum.Performance.NoPaging.nopaging_nopaging -- Intentional: sitemap generation. +$args = [ 'nopaging' => true ]; // OK - ignored. diff --git a/WordPressVIPMinimum/Tests/Performance/NoPagingUnitTest.php b/WordPressVIPMinimum/Tests/Performance/NoPagingUnitTest.php index c5586e4c..89e9e267 100644 --- a/WordPressVIPMinimum/Tests/Performance/NoPagingUnitTest.php +++ b/WordPressVIPMinimum/Tests/Performance/NoPagingUnitTest.php @@ -12,8 +12,6 @@ /** * Unit test class for the NoPaging sniff. * - * @since 0.5.0 - * * @covers \WordPressVIPMinimum\Sniffs\Performance\NoPagingSniff */ class NoPagingUnitTest extends AbstractSniffUnitTest { @@ -25,10 +23,19 @@ class NoPagingUnitTest extends AbstractSniffUnitTest { */ public function getErrorList() { return [ - 4 => 1, - 7 => 1, - 13 => 1, + 6 => 1, + 9 => 1, 15 => 1, + 17 => 1, + 20 => 1, + 29 => 1, + 38 => 1, + 42 => 1, + 45 => 1, + 54 => 1, + 60 => 1, + 66 => 1, + 72 => 1, ]; } diff --git a/WordPressVIPMinimum/Tests/Performance/OrderByRandUnitTest.php b/WordPressVIPMinimum/Tests/Performance/OrderByRandUnitTest.php index a39fb4d8..b79bbc1c 100644 --- a/WordPressVIPMinimum/Tests/Performance/OrderByRandUnitTest.php +++ b/WordPressVIPMinimum/Tests/Performance/OrderByRandUnitTest.php @@ -12,8 +12,6 @@ /** * Unit test class for the OrderByRand sniff. * - * @since 0.5.0 - * * @covers \WordPressVIPMinimum\Sniffs\Performance\OrderByRandSniff */ class OrderByRandUnitTest extends AbstractSniffUnitTest { diff --git a/WordPressVIPMinimum/Tests/Performance/TaxonomyMetaInOptionsUnitTest.inc b/WordPressVIPMinimum/Tests/Performance/TaxonomyMetaInOptionsUnitTest.inc index 6e67c82b..a89e6ec0 100644 --- a/WordPressVIPMinimum/Tests/Performance/TaxonomyMetaInOptionsUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Performance/TaxonomyMetaInOptionsUnitTest.inc @@ -1,8 +1,66 @@ term_id" ); // NOK. -delete_option( "taxonomy_rating_{$obj->term_id}" ); // NOK. -get_option( "taxonomy_rating_$tag_id" ); // NOK. -update_option( 'taxonomy_rating_' . $category_id ); // NOK. -get_option( "taxonomy_rating_{$cat_id}" ); // NOK. -add_option( 'taxonomy_rating_' . $obj->term_id ); // NOK. \ No newline at end of file +/* + * Not the sniff target. + */ +use function add_option; + +my\ns\get_option( "taxonomy_rating_$tag_id" ); +$this->delete_option( "taxonomy_rating_$tag_id" ); +$this?->update_option( "taxonomy_rating_$tag_id" ); +MyClass::get_option( "taxonomy_rating_$tag_id", ); +echo DELETE_OPTION; +namespace\add_option( "taxonomy_rating_$tag_id" ); + +function &update_option() {} + +/* + * These should all be okay. + */ +// Incomplete function call, should be ignored by the sniff. +$incorrect_but_ok = get_option(); +$incorrect_but_ok = update_option(); + +// Ignore as undetermined. +Get_Option( $option ); +\add_option( $obj->get_optionname() ); +update_option( OPTION_NAME, ); +\DELETE_OPTION( MyClass::$option_name ); + +get_option(...$params); // PHP 5.6 argument unpacking, parameter value unknown. + +// Looks like a function call, but is a PHP 8.0+ class instantiation via an attribute. +#[Get_Option('taxonomy_rating_')] +function foo() {} + +array_walk($options, \update_option(...),); // PHP 8.1 first class callable. + +// Ignore as not touching taxonomy term patterns. +\get_option( 'option_name' ); +add_option( 'option_' . $post_id, ); +update_Option( "prefix_{$setting}" ); +add_option( 'option_' . MY_CONSTANT, ); +add_option( 'foo_' . $obj->$propname ); +update_option( 'foo_' . $obj->page_id ); // NOK. + + +/* + * These should all be flagged. + */ +get_option( "taxonomy_rating_$obj->term_id", ); // NOK. +\delete_option( "taxonomy_rating_{$obj->term_id}" ); // NOK. +get_option( /*comment*/ "taxonomy_rating_$tag_id" ); // NOK. +UpDate_OPTION( 'taxonomy_rating_' . $category_id /*comment*/ , ); // NOK. +\Get_Option( "taxonomy_rating_{$cat_id}" ); // NOK. +add_option( 'taxonomy_rating_' . $obj->term_id ); // NOK. + + +// Safeguard handling of function calls using PHP 8.0+ named parameters. +get_option(default_value: false); // OK, well, not really, missing required $option param, but that's not the concern of this sniff. +add_option(value: "$tag_id", option: 'option_name'); // OK. +update_option(autoload: true, optin: "$tag_id", value: 10,); // OK, well, not really, typo in param name, but that's not the concern of the sniff. + +add_option(value: $value, option: "$tag_id" ); // Bad. + +// Safeguard handling of PHP 8.0+ nullsafe object operator. +get_option( 'taxonomy_rating_' . $obj?->term_id ); // NOK. diff --git a/WordPressVIPMinimum/Tests/Performance/TaxonomyMetaInOptionsUnitTest.php b/WordPressVIPMinimum/Tests/Performance/TaxonomyMetaInOptionsUnitTest.php index dff27d68..cc57df21 100644 --- a/WordPressVIPMinimum/Tests/Performance/TaxonomyMetaInOptionsUnitTest.php +++ b/WordPressVIPMinimum/Tests/Performance/TaxonomyMetaInOptionsUnitTest.php @@ -32,12 +32,14 @@ public function getErrorList() { */ public function getWarningList() { return [ - 3 => 1, - 4 => 1, - 5 => 1, - 6 => 1, - 7 => 1, - 8 => 1, + 50 => 1, + 51 => 1, + 52 => 1, + 53 => 1, + 54 => 1, + 55 => 1, + 63 => 1, + 66 => 1, ]; } } diff --git a/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.inc b/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.inc index 9a9de966..3ab7ba59 100644 --- a/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.inc @@ -1,4 +1,100 @@ esc_js( _deprecated_file() ); +$this?->esc_textarea( _e( $something ) ); +MyClass::esc_url( wp_die(), ); +echo WP_KSES; +namespace\wp_kses_post( user_error( $something ) ); + +function esc_js( _E $param ) {} // Class "_E" as type declaration. + +/* + * These should all be okay. + */ +// Incomplete function call, should be ignored by the sniff. +$incorrect_but_ok = esc_html(); +$incorrect_but_ok = wp_kses(); + +// Parameter does not contain a function call. +esc_html_x( $hook_name ); +\wp_kses_data( $obj->_deprecated_function() ); +esc_attr__( CONSTANT_NAME, ); +\ESC_ATTR_X( "do_$something" ); + +esc_attr(...$params); // PHP 5.6 argument unpacking. + +#[Esc_Textarea('text')] // PHP 8.0+ class instantiation via an attribute. Can't contain a nested function call anyway. +function foo() {} + +array_walk($text_strings, \esc_html__(...),); // PHP 8.1 first class callable. + +// Parameter contains a function call but not calling one of the printing functions. +esc_html( __( $something ) ); +Esc_URL_Raw( \get( $url ) ); +\esc_xml( deprecated_argument() ); // Note: missing "_" prefix for printing function. +wp_kses_one_attr ( + /* comment */ + get_attr(), +); + + +/* + * These should all be flagged. + */ +esc_html( _e( $something ) ); +esc_attr__( \_deprecated_argument( $a ) ); +ESC_ATTR_E( _Deprecated_Constructor($a), ); +\esc_attr_x( _deprecated_file(), ); +esc_attr( _deprecated_function() ); +esc_HTML__( _deprecated_hook() ); +esc_html_e( _Doing_It_Wrong( $a ) ); +esc_html_X( \_e( $foo ), ); +\esc_html( /*comment*/ _ex( $foo ) ); +esc_js( printf( $foo ) ); +Esc_textarea( trigger_error( $foo ) ); +\esc_URL_raw( \user_Error( $foo, '' ), ); +esc_url( vprintf( $foo, ) /*comment*/ ); +WP_Kses_Data( WP_DIE( $foo ), ); +wp_kses_one_attr /*comment*/ ( wp_dropdown_pages( $pages ) ); +wp_kses_post( + \_deprecated_function( $fn ) +); +wp_kses( _ex() ); + +// Adding custom printing functions is supported. +// phpcs:set WordPressVIPMinimum.Security.EscapingVoidReturnFunctions customPrintingFunctions[] to_screen,my_print +esc_attr( to_screen( $var1 ) ); +\wp_kses_post( my_print() ); +// phpcs:set WordPressVIPMinimum.Security.EscapingVoidReturnFunctions customPrintingFunctions[] + +tag_escape( $tag ); // OK. +tag_escape( _e() ); // Bad. + +// Bug: these are not function calls inside. +esc_attr__( User_Error::CONSTANT_NAME, ); // OK. +esc_js( _doing_it_wrong::class ); // OK, PHP 5.5 ::class resolution. + +// Safeguard handling of function calls using PHP 8.0+ named parameters. +esc_attr_x( context: get_context(), domain: _e(),); // OK, well, not really, missing required $text param, but that's not the concern of this sniff. +esc_url_raw(protocols: $protocols, url: $url); // OK. +wp_kses_one_attr(element: $element, att: trigger_error( $foo ) ); // OK, well, not really, typo in param name, but that's not the concern of the sniff. +wp_kses( content: \not_deprecated_function( $fn ) ); // OK. +wp_kses( content: /*comment*/ ); // OK, well, not really, invalid function call, but that's not the concern of the sniff. + +esc_html_x(context: $c, text: _doing_it_wrong() ); // Bad. +wp_kses(allowed_html: $allowed_html, content: printf() ); // Bad. +esc_url(protocols: $protocols, url: vprintf()); // Bad. + +// These are no longer flagged as they are not "escaping" functions for the purpose of this sniff. +esc_sql( trigger_error( $foo ) ); // OK. +wp_kses_allowed_html( _deprecated_function() ); // OK. + +// These are no longer flagged as these are not the WP native escaping functions, so we don't know what parameter to check. +esc_something( trigger_error( $foo ) ); // OK. +wp_kses_page( _deprecated_function() ); // OK. diff --git a/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.php b/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.php index 52734c3f..954e2bb2 100644 --- a/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.php +++ b/WordPressVIPMinimum/Tests/Security/EscapingVoidReturnFunctionsUnitTest.php @@ -23,7 +23,29 @@ class EscapingVoidReturnFunctionsUnitTest extends AbstractSniffUnitTest { */ public function getErrorList() { return [ - 3 => 1, + 50 => 1, + 51 => 1, + 52 => 1, + 53 => 1, + 54 => 1, + 55 => 1, + 56 => 1, + 57 => 1, + 58 => 1, + 59 => 1, + 60 => 1, + 61 => 1, + 62 => 1, + 63 => 1, + 64 => 1, + 65 => 1, + 68 => 1, + 72 => 1, + 73 => 1, + 77 => 1, + 90 => 1, + 91 => 1, + 92 => 1, ]; } diff --git a/WordPressVIPMinimum/Tests/Security/MustacheUnitTest.inc b/WordPressVIPMinimum/Tests/Security/MustacheUnitTest.inc index 12e2cb37..67d808d2 100644 --- a/WordPressVIPMinimum/Tests/Security/MustacheUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Security/MustacheUnitTest.inc @@ -21,3 +21,34 @@ echo '{{&data}}'; // NOK: data. // Issue 541#issuecomment-1692323177: don't flag GB syntax.
+ +{$baz}}'>{{{data}}}
"; // NOK: data. + +echo << +{{{data}}} +{{&data}} +HERE; +echo <<<"HERE" +{{=<% %>=}} + +HERE; + +echo <<<'NOW' + +NOW; + +// Don't throw false positives on incorrect/incomplete mustache delimiter change. +$m = '{{=<%'; // Missing end of delimiter change. +$m = '%>=}} {{=<%'; // Incorrect order, not a delimiter change. + +// Correctly recognize mid-line delimiter change. +$m = '{{default_tags}} {{=<% %>=}} <% erb_style_tags %> <%={{ }}=%> {{ default_tags_again }}'; // NOK: delimiter change. + +// Prevent false positives on SafeString being a partial name. +echo 'MySafeString'; diff --git a/WordPressVIPMinimum/Tests/Security/MustacheUnitTest.php b/WordPressVIPMinimum/Tests/Security/MustacheUnitTest.php index 2c441539..1e192a74 100644 --- a/WordPressVIPMinimum/Tests/Security/MustacheUnitTest.php +++ b/WordPressVIPMinimum/Tests/Security/MustacheUnitTest.php @@ -38,6 +38,14 @@ public function getWarningList() { 7 => 1, 8 => 1, 18 => 1, + 26 => 1, + 30 => 1, + 31 => 1, + 34 => 1, + 40 => 1, + 41 => 1, + 42 => 1, + 51 => 1, ]; } } diff --git a/WordPressVIPMinimum/Tests/Security/PHPFilterFunctionsUnitTest.inc b/WordPressVIPMinimum/Tests/Security/PHPFilterFunctionsUnitTest.inc index 9960f4ed..0159aac5 100644 --- a/WordPressVIPMinimum/Tests/Security/PHPFilterFunctionsUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Security/PHPFilterFunctionsUnitTest.inc @@ -1,29 +1,100 @@ filter_var_array($a, $b); +$this?->filter_input_array($a, $b); +MyClass::filter_var($a, $b); +echo FILTER_INPUT; +namespace\filter_var_array($a, $b); + +// Looks like a function call, but is a PHP 8.0+ class instantiation via an attribute. +#[Filter_Input('text')] +function foo() {} + +// PHP 8.1 first class callable. +// As we have no knowledge about what parameters will be passed, we shouldn't flag this. +add_filter('my_filter', filter_var(...)); + + +/* + * These should all be okay. + */ filter_var( $url, FILTER_SANITIZE_URL ); -filter_var( 'test', FILTER_SANITIZE_STRING ); -filter_var( "test", FILTER_SANITIZE_STRING ); -filter_input( INPUT_GET, 'foo', FILTER_SANITIZE_STRING ); +\filter_var( 'test', FILTER_SANITIZE_STRING ); +FILTER_INPUT( INPUT_GET, 'foo', FILTER_SANITIZE_STRING, ); filter_input( INPUT_GET, "foo" , FILTER_SANITIZE_STRING ); -filter_var_array( $array, FILTER_SANITIZE_STRING ); -filter_input_array( $array, FILTER_SANITIZE_STRING ); -filter_input_array( $array,FILTER_SANITIZE_STRING ); - -// Bad. -filter_input( INPUT_GET, 'foo' ); // Missing third parameter. -filter_input( INPUT_GET, 'foo', FILTER_DEFAULT ); // This filter ID does nothing. -filter_input( INPUT_GET, "foo", FILTER_UNSAFE_RAW ); // This filter ID does nothing. -filter_var( $url ); // Missing second parameter. -filter_var( $url, FILTER_DEFAULT ); // This filter ID does nothing. +filter_input_array( $array, filter_default ); // Constants are case-sensitive, so this is not the FILTER_DEFAULT constant. + +// Ignore as undetermined. +filter_var( "test", get_filter() ); +\Filter_Var_Array( $array, $filterName ); +filter_input_array( $array,$obj->get_filter() , ); + +// Incomplete function call, should be ignored by the sniff. +$incorrect_but_ok = filter_input(); + +/* + * These should all be flagged with a warning. + */ +filter_input( INPUT_GET, 'foo' ); // Missing $filter parameter. +\filter_input( INPUT_GET, 'foo', FILTER_DEFAULT ); // This filter ID does nothing. +filter_input( INPUT_GET, "foo", FILTER_UNSAFE_RAW /* comment */ ,); // This filter ID does nothing. + +filter_var( $url ); // Missing $filter parameter. +Filter_Var( $url, FILTER_DEFAULT ); // This filter ID does nothing. filter_var( 'https://google.com', FILTER_UNSAFE_RAW ); // This filter ID does nothing. -filter_var_array( $array ); // Missing second parameter. + +filter_var_array( $array, ); // Missing $options parameter. filter_var_array( $array, FILTER_DEFAULT ); // This filter ID does nothing. filter_var_array( $array, FILTER_UNSAFE_RAW ); // This filter ID does nothing. -filter_input_array( $array ); // Missing second parameter. -filter_input_array( $array, FILTER_DEFAULT ); // This filter ID does nothing. -filter_input_array( $array, FILTER_UNSAFE_RAW ); // This filter ID does nothing. + +filter_input_array( $array ); // Missing $options parameter. +\FILTER_INPUT_ARRAY( $array, FILTER_DEFAULT ); // This filter ID does nothing. +filter_input_array( $array, FILTER_UNSAFE_RAW, ); // This filter ID does nothing. + +// Safeguard handling of function calls using PHP 8.0+ named parameters. +filter_input(var_name: $var_name, filter: FILTER_SANITIZE_STRING, type: FILTER_DEFAULT); // OK, invalid input value for $type, but that's not our concern. +filter_input(var_name: $var_name, filter: $filter, type: $type); // Ignore, undetermined. +filter_input( + var_name: $var_name, + filter: FILTER_DEFAULT, // This filter ID does nothing. + type: $type, +); + +filter_var(filter: FILTER_SANITIZE_URL); // OK, well not really, missing required parameter, but that's not our concern. +filter_var($value, options: FILTER_NULL_ON_FAILURE); // Missing $filter parameter. +filter_var(value: $value, filters: FILTER_SANITIZE_URL); // Typo in parameter name, report as missing $filter parameter. + +filter_var_array(options: FILTER_UNSAFE_RAW, array: $array); // This filter ID does nothing. + +filter_input_array($type, add_empty: false, options: FILTER_DEFAULT,); // This filter ID does nothing. + +// Ignore function calls using PHP 5.6 argument unpacking as we don't know what parameters were passed. +filter_input(INPUT_GET, ...$params); +trim(filter_input(INPUT_GET, ...$params)); +// ... but only ignore unpacking if done at the correct nesting level. +filter_input(INPUT_GET, $obj->getVarname(...$params)); // Missing $filter parameter. + +// False negatives: $options arrays are currently not (yet) supported by this sniff. +// See: https://www.php.net/manual/en/function.filter-var-array.php and https://www.php.net/manual/en/function.filter-input-array.php +filter_var_array( + $array, + array('keyA' => FILTER_DEFAULT), // This filter ID does nothing. +); +filter_input_array( + $array, + [ + 'keyA' => [ + 'filter' => FILTER_UNSAFE_RAW, // This filter ID does nothing. + 'flags' => FILTER_FORCE_ARRAY, + ], + 'keyB' => [ + 'filter' => FILTER_SANITIZE_ENCODED, + ], + ] +); diff --git a/WordPressVIPMinimum/Tests/Security/PHPFilterFunctionsUnitTest.php b/WordPressVIPMinimum/Tests/Security/PHPFilterFunctionsUnitTest.php index dae998cc..1faa0728 100644 --- a/WordPressVIPMinimum/Tests/Security/PHPFilterFunctionsUnitTest.php +++ b/WordPressVIPMinimum/Tests/Security/PHPFilterFunctionsUnitTest.php @@ -10,7 +10,7 @@ use PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest; /** - * Unit test class for the WP_Query params sniff. + * Unit test class for the PHPFilterFunctions sniff. * * @covers \WordPressVIPMinimum\Sniffs\Security\PHPFilterFunctionsSniff */ @@ -32,18 +32,24 @@ public function getErrorList() { */ public function getWarningList() { return [ - 18 => 1, - 19 => 1, - 20 => 1, - 21 => 1, - 22 => 1, - 23 => 1, - 24 => 1, - 25 => 1, - 26 => 1, - 27 => 1, - 28 => 1, - 29 => 1, + 44 => 1, + 45 => 1, + 46 => 1, + 48 => 1, + 49 => 1, + 50 => 1, + 52 => 1, + 53 => 1, + 54 => 1, + 56 => 1, + 57 => 1, + 58 => 1, + 65 => 1, + 70 => 1, + 71 => 1, + 73 => 1, + 75 => 1, + 81 => 1, ]; } } diff --git a/WordPressVIPMinimum/Tests/Security/StaticStrreplaceUnitTest.inc b/WordPressVIPMinimum/Tests/Security/StaticStrreplaceUnitTest.inc index d5dd8a1f..4b7af5dc 100644 --- a/WordPressVIPMinimum/Tests/Security/StaticStrreplaceUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Security/StaticStrreplaceUnitTest.inc @@ -1,13 +1,101 @@ str_replace('foo', 'bar', 'foobar'); +$this?->str_replace('foo', 'bar', 'foobar'); +MyClass::str_replace('foo', 'bar', 'foobar'); +echo STR_REPLACE; +namespace\str_replace('foo', 'bar', 'foobar', $count); + +// Looks like a function call, but is a PHP 8.0+ class instantiation via an attribute. +#[Str_Replace('foo', 'bar', 'foobar')] +function foo() {} + +// Ignore PHP 5.6 argument unpacking. +str_replace('foo', 'bar', ...$params); + +// Ignore PHP 8.1 first class callable. +add_filter('my_filter', str_replace(...)); + +// Incomplete function calls, should be ignored by the sniff. +str_replace(); +str_replace('foo', 'bar'); + + +/* + * These should all be okay. + */ +str_replace( $foo->getSearch(), 'bar', 'foobar' ); +\str_replace( 'foo', $bar, 'foobar' ); +str_replace( 'foo', 'bar', $foobar, ); +STR_REPLACE( $foo, $bar, 'foobar' ); +str_replace( 'foo', get_replacements(), $foobar, $count ); + +str_replace( array( 'foo', "$bar" ), $bar, 'foobar' ); + +str_replace( array( $foo, $bar ), array( 'bar', 'foo' ), array( 'foobar', 'foobar' ) ); +\Str_Replace( [ 'foo', 'bar' ], array( $foo, SOME_CONSTANT ), 'foobar' ); +str_replace( array( 'foo', "bar" ), [ 'bar', 'foo' ], $foobar, ); +str_replace( [ $foo, $bar->prop ], /*comment*/ [ 'bar', 'foo' ], $foobar ); + + +/* + * These should all be flagged with a warning. + */ str_replace( 'foo', 'bar', 'foobar' ); // Bad. +Str_replace( + 'foo', // Comment. + 'bar', /* comment */ + 'foobar' +); // Bad. +str_replace( array( 'foo', 'bar' ), array( 'bar', 'foo' ), array( 'foobar', 'foobar' ) ); // Bad. +str_replace( 'foo', 'bar', 'foobar', $count ); // Bad. + +// Handle PHP 5.4+ short array syntax correctly. +\str_replace( [ 'foo', 'bar' ], [ 'bar', 'foo' ], [ 'foobar', 'foobar' ], ); // Bad. + +// Don't confuse PHP 7.1+ short list with short array. +str_replace( [ $a, $b ] = $search, [ 'bar', 'foo' ], 'foobar', ); // OK. + +// Safeguard support for PHP 8.0+ named parameters. +str_replace( search: [ 'foo', 'bar' ], replace: 'baz', count: $count ); // OK, well not really, missing required $subject param, but not our concern. +str_replace( search: 'foo', replace: 'baz', subjects: [ 'foobar', 'foobar' ] ); // OK, well not really, typo in $subject param name, not our concern. +str_replace( subject: $subject, count: $count, replace: 'baz', search: 'foo', ); // OK, different parameter order. -str_replace( 'foo', 'bar', $foobar ); // Ok. +str_replace( count: $count, subject: [ 'foobar', 'foobar' ], search: 'foo', replace: 'baz', ); // Bad with different parameter order. +str_replace( [ 'foo', 'bar' ], 'baz', count: $count, subject: 'foobar' ); // Bad, with mixed named and unnamed params. -str_replace( array( 'foo', 'bar' ), array( 'bar', 'foo' ), 'foobar' ); // Bad. +// Make sure some variations of how plain strings can be passed are handled. +str_replace( 'fo' . 'o', 'bar', 'foobar' ); // Bad. +str_replace( array( 'foo', 'bar' ) + array( 'baz', 'fop' ), array( 'bar', 'foo' ), 'foobar' ); // Bad. -str_replace( array( 'foo', 'bar' ), array( 'bar', 'foo' ), $foobar ); // Ok. +// Safeguard correct handling of PHP 5.3+ nowdocs. +str_replace( + 'foo', + 'bar', + <<<'EOD' +foobar +EOD +); // Bad. -str_replace( array( 'foo', 'bar' ), array( $foo, $bar ), $foobar ); // Ok. +// Safeguard correct handling of heredocs without interpolation. +str_replace( array(<< 1, - 7 => 1, + 50 => 1, + 51 => 1, + 56 => 1, + 57 => 1, + 60 => 1, + 70 => 1, + 71 => 1, + 74 => 1, + 75 => 1, + 78 => 1, + 87 => 1, ]; } diff --git a/WordPressVIPMinimum/Tests/Security/TwigUnitTest.inc b/WordPressVIPMinimum/Tests/Security/TwigUnitTest.inc deleted file mode 100644 index d0423e0c..00000000 --- a/WordPressVIPMinimum/Tests/Security/TwigUnitTest.inc +++ /dev/null @@ -1,12 +0,0 @@ - - \ No newline at end of file diff --git a/WordPressVIPMinimum/Tests/Security/TwigUnitTest.php b/WordPressVIPMinimum/Tests/Security/TwigUnitTest.php deleted file mode 100644 index 0c390843..00000000 --- a/WordPressVIPMinimum/Tests/Security/TwigUnitTest.php +++ /dev/null @@ -1,39 +0,0 @@ - Key is the line number, value is the number of expected errors. - */ - public function getErrorList() { - return []; - } - - /** - * Returns the lines where warnings should occur. - * - * @return array Key is the line number, value is the number of expected warnings. - */ - public function getWarningList() { - return [ - 5 => 1, - 10 => 1, - ]; - } -} diff --git a/WordPressVIPMinimum/Tests/UserExperience/AdminBarRemovalUnitTest.inc b/WordPressVIPMinimum/Tests/UserExperience/AdminBarRemovalUnitTest.inc index f452ae83..abc0a248 100644 --- a/WordPressVIPMinimum/Tests/UserExperience/AdminBarRemovalUnitTest.inc +++ b/WordPressVIPMinimum/Tests/UserExperience/AdminBarRemovalUnitTest.inc @@ -107,3 +107,77 @@ EOT; show_admin_bar( false ); // OK. +$this?->show_admin_bar( false ); // OK. +MyClass::add_filter( 'show_admin_bar', '__return_false' ); // OK. +echo ADD_FILTER; // OK. +namespace\add_filter( 'show_admin_bar', '__return_false' ); // OK. + +#[Show_Admin_Bar(false)] // OK. PHP 8.0+ class instantiation via an attribute. Can't contain a nested function call anyway. +function foo() {} + +array_walk($filters, \add_filter(...),); // OK. PHP 8.1 first class callable. + +// Incomplete function calls, should be ignored by the sniff. +$incorrect_but_ok = show_admin_bar(); // OK. +$incorrect_but_ok = add_filter(); // OK. + +// Safeguard that the sniff only flags the "show_admin_bar" filter. +add_filter( 'not_show_admin_bar', '__return_false', ); // OK. + +// Document that dynamic values will be flagged. +show_admin_bar( $unknown ); // Bad. +add_filter( 'show_admin_bar', $callable, ); // Bad. +add_filter('show_admin_bar', ...$params); // Bad. PHP 5.6 argument unpacking. + +// Document that fully qualified function calls and functions in unconventional case will correctly be recognized. +\add_filter( "show_admin_bar", '__return_true' ); // OK. +\Add_Filter( 'show_admin_bar', "__return_false", ); // Bad. + +\show_Admin_bar( true, ); // OK. +\show_admin_bar( false ); // Bad. +\SHOW_ADMIN_BAR( false, ); // Bad. + +// Comments in parameters should be ignored. +show_admin_bar( true /* turn it on */ ); // OK. +add_filter( + // Admin bar gives access to admin for users with the right permissions. + 'show_admin_bar', + '__return_false' +); // Bad. +add_filter( + 'show_admin_bar', + // Turn it on. + '__return_true' +); // OK. + +// Safeguard handling of function calls using PHP 8.0+ named parameters. +show_admin_bar( shown: false ); // OK, well not really, typo in param name, but that's not the concern of the sniff. +\show_admin_bar( show: true ); // OK. +show_admin_bar( show: $toggle ); // Bad. + +add_filter(callback: '__return_false', priority: 10); // OK, well, not really, missing required $hook_name param, but that's not the concern of this sniff. +\add_filter(callback: '__return_false', hook_name: 'not_our_target'); // OK. +add_filter(hookName: 'show_admin_bar', callback: '__return_false',); // OK, well, not really, typo in param name, but that's not the concern of the sniff. +add_filter( callback: '__return_true', hook_name: 'show_admin_bar', ); // Ok. +\add_filter( callback: '__return_false', hook_name: 'show_admin_bar', ); // Bad. + +// Bug: add_action() is an alias of add_filter. +add_action( 'show_admin_bar', $callable, ); // Bad. + +// Safeguard handling of filter callback being passed as PHP 8.1+ first class callable. +add_filter( 'show_admin_bar', __return_true(...) ); // OK. +add_filter( 'show_admin_bar', \__return_true( ... ) ); // OK. +add_action( 'show_admin_bar', __return_false ( ... ) , ); // Bad. +add_filter( 'show_admin_bar', \__return_false(...) ); // Bad. +add_filter( 'show_admin_bar', "__return_true(...)" ); // Bad. Invalid callback. + +// Bug fix: function names are case-insensitive. +\add_filter( "show_admin_bar", '__Return_TRUE' ); // OK. +add_filter( 'show_admin_bar', __Return_True(...) ); // OK. diff --git a/WordPressVIPMinimum/Tests/UserExperience/AdminBarRemovalUnitTest.php b/WordPressVIPMinimum/Tests/UserExperience/AdminBarRemovalUnitTest.php index 6fd137c6..88f7e823 100644 --- a/WordPressVIPMinimum/Tests/UserExperience/AdminBarRemovalUnitTest.php +++ b/WordPressVIPMinimum/Tests/UserExperience/AdminBarRemovalUnitTest.php @@ -12,8 +12,6 @@ /** * Unit test class for the AdminBarRemoval sniff. * - * @since 0.5.0 - * * @covers \WordPressVIPMinimum\Sniffs\UserExperience\AdminBarRemovalSniff */ class AdminBarRemovalUnitTest extends AbstractSniffUnitTest { @@ -53,6 +51,19 @@ public function getErrorList( $testFile = '' ) { 103 => 1, 104 => 1, 105 => 1, + 135 => 1, + 136 => 1, + 137 => 1, + 141 => 1, + 144 => 1, + 145 => 1, + 149 => 1, + 163 => 1, + 169 => 1, + 172 => 1, + 177 => 1, + 178 => 1, + 179 => 1, ]; case 'AdminBarRemovalUnitTest.css': @@ -87,15 +98,6 @@ public function getErrorList( $testFile = '' ) { * @return array Key is the line number, value is the number of expected warnings. */ public function getWarningList( $testFile = '' ) { - switch ( $testFile ) { - case 'AdminBarRemovalUnitTest.css': - return []; - - case 'AdminBarRemovalUnitTest.inc': - return []; - - default: - return []; - } + return []; } } diff --git a/WordPressVIPMinimum/Tests/Variables/RestrictedVariablesUnitTest.php b/WordPressVIPMinimum/Tests/Variables/RestrictedVariablesUnitTest.php index 36b833b2..e5b7ecf2 100644 --- a/WordPressVIPMinimum/Tests/Variables/RestrictedVariablesUnitTest.php +++ b/WordPressVIPMinimum/Tests/Variables/RestrictedVariablesUnitTest.php @@ -10,10 +10,7 @@ use PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest; /** - * Unit test class for the VIP_RestrictedVariables sniff. - * - * @since 0.3.0 - * @since 0.13.0 Class name changed: this class is now namespaced. + * Unit test class for the RestrictedVariables sniff. * * @covers \WordPressVIPMinimum\Sniffs\Variables\RestrictedVariablesSniff */ diff --git a/WordPressVIPMinimum/Tests/Variables/ServerVariablesUnitTest.inc b/WordPressVIPMinimum/Tests/Variables/ServerVariablesUnitTest.inc index a22bae0c..28de0071 100644 --- a/WordPressVIPMinimum/Tests/Variables/ServerVariablesUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Variables/ServerVariablesUnitTest.inc @@ -1,9 +1,75 @@ 1, - 5 => 1, - 6 => 1, - 7 => 1, + 31 => 1, + 32 => 1, + 33 => 1, + 36 => 1, + 37 => 1, + 71 => 1, + 72 => 1, + 73 => 1, + 74 => 1, + 75 => 1, ]; } diff --git a/WordPressVIPMinimum/ruleset-test-bom.inc b/WordPressVIPMinimum/ruleset-test-bom.inc new file mode 100644 index 00000000..7ecb784f --- /dev/null +++ b/WordPressVIPMinimum/ruleset-test-bom.inc @@ -0,0 +1,3 @@ +My term link'; // Error. -// WordPressVIPMinimum.Functions.DynamicCalls -$my_notokay_func = 'extract'; -$my_notokay_func(); // Error. + + + // WordPressVIPMinimum.Functions.RestrictedFunctions @@ -463,6 +463,9 @@ $args = array( // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.Un 'nopaging' => true, // Error. ); _query_posts( 'nopaging=true' ); // Error. +$args = array( // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable + 'posts_per_page' => -1, // Error. +); // WordPressVIPMinimum.Performance.OrderByRand $args = array( // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable diff --git a/WordPressVIPMinimum/ruleset-test.php b/WordPressVIPMinimum/ruleset-test.php index e78ae6a7..8fd42f54 100644 --- a/WordPressVIPMinimum/ruleset-test.php +++ b/WordPressVIPMinimum/ruleset-test.php @@ -81,7 +81,6 @@ 299 => 1, 303 => 1, 304 => 1, - 308 => 1, 312 => 1, 313 => 1, 314 => 1, @@ -156,15 +155,13 @@ 451 => 1, 463 => 1, 465 => 1, - 469 => 1, - 471 => 1, - 477 => 1, - 483 => 1, - 491 => 1, - 505 => 1, - 509 => 1, - 510 => 1, - 511 => 1, + 467 => 1, + 472 => 1, + 474 => 1, + 480 => 1, + 486 => 1, + 494 => 1, + 508 => 1, 512 => 1, 513 => 1, 514 => 1, @@ -173,29 +170,32 @@ 517 => 1, 518 => 1, 519 => 1, - 523 => 1, - 525 => 1, - 550 => 1, - 551 => 1, + 520 => 1, + 521 => 1, + 522 => 1, + 526 => 1, + 528 => 1, + 553 => 1, 554 => 1, - 569 => 1, - 570 => 1, + 557 => 1, + 572 => 1, 573 => 1, - 574 => 1, - 575 => 1, + 576 => 1, + 577 => 1, 578 => 1, 581 => 1, - 582 => 1, - 583 => 1, - 588 => 1, - 590 => 1, - 594 => 1, - 595 => 1, - 596 => 1, + 584 => 1, + 585 => 1, + 586 => 1, + 591 => 1, + 593 => 1, 597 => 1, - 612 => 1, - 614 => 1, - 621 => 1, + 598 => 1, + 599 => 1, + 600 => 1, + 615 => 1, + 617 => 1, + 624 => 1, ], 'warnings' => [ 32 => 1, @@ -274,21 +274,21 @@ 457 => 1, 458 => 1, 459 => 1, - 499 => 1, - 500 => 1, - 504 => 1, - 528 => 1, - 529 => 1, - 530 => 1, + 502 => 1, + 503 => 1, + 507 => 1, 531 => 1, 532 => 1, + 533 => 1, + 534 => 1, 535 => 1, 538 => 1, - 545 => 1, - 559 => 1, - 565 => 1, - 589 => 1, - 618 => 1, + 541 => 1, + 548 => 1, + 562 => 1, + 568 => 1, + 592 => 1, + 621 => 1, ], 'messages' => [ 130 => [ @@ -309,11 +309,29 @@ ], ]; +// Expected values for the dedicated byte order mark (BOM) fixture. +$bom_expected = [ + 'errors' => [ + 1 => 1, + ], + 'warnings' => [], + 'messages' => [ + 1 => [ + 'File contains UTF-8 byte order mark, which may corrupt your application', + ], + ], +]; + require __DIR__ . '/../tests/RulesetTest.php'; // Run the tests! -$test = new RulesetTest( 'WordPressVIPMinimum', $expected ); -if ( $test->passes() ) { +$test = new RulesetTest( 'WordPressVIPMinimum', $expected ); +$bom_test = new RulesetTest( 'WordPressVIPMinimum', $bom_expected, 'ruleset-test-bom.inc' ); + +// Evaluate both tests before the check so each reports its own discrepancies rather than being short-circuited away. +$test_passes = $test->passes(); +$bom_test_passes = $bom_test->passes(); +if ( $test_passes && $bom_test_passes ) { printf( 'All WordPressVIPMinimum tests passed!' . PHP_EOL ); exit( 0 ); } diff --git a/WordPressVIPMinimum/ruleset.xml b/WordPressVIPMinimum/ruleset.xml index 6a3f10c1..c0f66990 100644 --- a/WordPressVIPMinimum/ruleset.xml +++ b/WordPressVIPMinimum/ruleset.xml @@ -1,5 +1,5 @@ - + WordPress VIP Minimum Coding Standards - + + + + + + + @@ -18,10 +32,6 @@ - - *.twig - - @@ -50,6 +60,7 @@ + @@ -58,10 +69,10 @@ - *.php - *.inc - *.js - *.css + *\.php + *\.inc + *\.js + *\.css diff --git a/bin/xml-lint b/bin/xml-lint deleted file mode 100755 index 6ab4e113..00000000 --- a/bin/xml-lint +++ /dev/null @@ -1,17 +0,0 @@ -#!/usr/bin/env bash -# -# Check XML files. -# -# @link http://xmlsoft.org/xmllint.html -# -# EXAMPLE TO RUN LOCALLY: -# -# ./bin/xml-lint - -# Validate the ruleset XML files. -xmllint --noout --schema ./vendor/squizlabs/php_codesniffer/phpcs.xsd ./*/ruleset.xml - -# Check the code-style consistency of the XML files. -export XMLLINT_INDENT=" " # This is a tab character. -diff -B --tabsize=4 ./WordPressVIPMinimum/ruleset.xml <(xmllint --format "./WordPressVIPMinimum/ruleset.xml") -diff -B --tabsize=4 ./WordPress-VIP-Go/ruleset.xml <(xmllint --format "./WordPress-VIP-Go/ruleset.xml") diff --git a/composer.json b/composer.json index c65c23bb..30ca3ab7 100644 --- a/composer.json +++ b/composer.json @@ -16,30 +16,30 @@ } ], "require": { - "php": ">=5.4", - "phpcsstandards/phpcsextra": "^1.2.1", - "phpcsstandards/phpcsutils": "^1.0.11", - "sirbrillig/phpcs-variable-analysis": "^2.11.18", - "squizlabs/php_codesniffer": "^3.9.2", - "wp-coding-standards/wpcs": "^3.1.0" + "php": ">=7.4", + "phpcsstandards/phpcsextra": "^1.5.1", + "phpcsstandards/phpcsutils": "^1.2.3", + "sirbrillig/phpcs-variable-analysis": "^2.13.0", + "squizlabs/php_codesniffer": "^3.13.5", + "wp-coding-standards/wpcs": "^3.4.1" }, "require-dev": { - "php-parallel-lint/php-parallel-lint": "^1.3.2", + "php-parallel-lint/php-parallel-lint": "^1.4.0", "php-parallel-lint/php-console-highlighter": "^1.0.0", "phpcompatibility/php-compatibility": "^9", - "phpcsstandards/phpcsdevtools": "^1.0", - "phpunit/phpunit": "^4 || ^5 || ^6 || ^7 || ^8 || ^9" + "phpcsstandards/phpcsdevtools": "^1.2.3", + "phpunit/phpunit": "^9" }, "config": { "allow-plugins": { "dealerdirect/phpcodesniffer-composer-installer": true - } + }, + "lock": false }, "scripts": { "test-ruleset": "bin/ruleset-tests", "lint": [ - "bin/php-lint", - "bin/xml-lint" + "bin/php-lint" ], "cs": "@php ./vendor/squizlabs/php_codesniffer/bin/phpcs", "test": "bin/unit-tests", @@ -56,7 +56,7 @@ ] }, "scripts-descriptions": { - "lint": "VIPCS: Lint PHP and XML files in against parse errors.", + "lint": "VIPCS: Lint PHP files against parse errors.", "cs": "VIPCS: Check the code style and code quality of the codebase via PHPCS.", "test": "VIPCS: Run the unit tests for the VIPCS sniffs.", "test-coverage": "VIPCS: Run the unit tests for the VIPCS sniffs with coverage enabled.", diff --git a/tests/RulesetTest.php b/tests/RulesetTest.php index f6fed8fe..dc03e3ef 100644 --- a/tests/RulesetTest.php +++ b/tests/RulesetTest.php @@ -28,7 +28,7 @@ class RulesetTest { * * This is the giant array in the ruleset-test.php files. * - * @var array + * @var array>> */ public $expected = []; @@ -74,6 +74,13 @@ class RulesetTest { */ private $phpcs_bin = 'phpcs'; + /** + * Name of the fixture file (relative to the ruleset directory) to check. + * + * @var string + */ + private $fixture = 'ruleset-test.inc'; + /** * String returned by PHP_CodeSniffer report for an Error. */ @@ -82,12 +89,14 @@ class RulesetTest { /** * Init the object by processing the test file. * - * @param string $ruleset Name of the ruleset e.g. WordPressVIPMinimum or WordPress-VIP-Go. - * @param array $expected The array of expected errors, warnings and messages. + * @param string $ruleset Name of the ruleset e.g. WordPressVIPMinimum or WordPress-VIP-Go. + * @param array>> $expected The array of expected errors, warnings and messages. + * @param string $fixture Name of the fixture file within the ruleset directory to check. Defaults to `ruleset-test.inc`. */ - public function __construct( $ruleset, $expected = [] ) { + public function __construct( $ruleset, $expected = [], $fixture = 'ruleset-test.inc' ) { $this->ruleset = $ruleset; $this->expected = $expected; + $this->fixture = $fixture; // Travis and Windows support. $phpcs_bin = getenv( 'PHPCS_BIN' ); @@ -99,7 +108,7 @@ public function __construct( $ruleset, $expected = [] ) { } // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped - printf( 'Testing the ' . $this->ruleset . ' ruleset.' . PHP_EOL ); + printf( 'Testing the ' . $this->ruleset . ' ruleset against ' . $this->fixture . '.' . PHP_EOL ); $output = $this->collect_phpcs_result(); @@ -143,19 +152,29 @@ private function run() { */ private function collect_phpcs_result() { $php = ''; - if ( \PHP_BINARY && in_array( \PHP_SAPI, [ 'cgi-fcgi', 'cli', 'cli-server', 'phpdbg' ], true ) ) { + if ( defined( 'PHP_BINARY' ) && in_array( \PHP_SAPI, [ 'cgi-fcgi', 'cli', 'cli-server', 'phpdbg' ], true ) ) { $php = \PHP_BINARY . ' '; } - $shell = sprintf( - '%1$s%2$s --severity=1 --standard=%3$s --report=json ./%3$s/ruleset-test.inc', + $report_file = dirname( __DIR__ ) . '/ruleset-tests-report.json'; + $shell = sprintf( + '%1$s%2$s --severity=1 --standard=%3$s --report-json=%4$s ./%3$s/%5$s', $php, // Current PHP executable if available. $this->phpcs_bin, - $this->ruleset + $this->ruleset, + $report_file, + $this->fixture ); // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.system_calls_shell_exec -- This is test code, not production. - $output = shell_exec( $shell ); + shell_exec( $shell ); + + // phpcs:ignore WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents -- This code is not run in the context of WP. + $output = file_get_contents( $report_file ); + + // Delete the report as we no longer need it. + // phpcs:ignore WordPress.PHP.NoSilencedErrors.Discouraged,WordPress.WP.AlternativeFunctions.unlink_unlink + @unlink( $report_file ); return json_decode( $output ); }