diff --git a/.gitattributes b/.gitattributes index 0a102227..7707243d 100644 --- a/.gitattributes +++ b/.gitattributes @@ -5,17 +5,14 @@ # https://www.reddit.com/r/PHP/comments/2jzp6k/i_dont_need_your_tests_in_my_production # https://blog.madewithlove.be/post/gitattributes/ # -/.gitattributes export-ignore -/.gitignore export-ignore -/.phpcs.xml export-ignore -/.phpcs.xml.dist export-ignore -/phpcs.xml export-ignore -/phpcs.xml.dist export-ignore -/phpunit.xml export-ignore -/phpunit.xml.dist export-ignore -/.cache export-ignore -/.github export-ignore -/Yoast/Tests export-ignore +/.cache export-ignore +/.github export-ignore +/Yoast/Tests export-ignore +.gitattributes export-ignore +.gitignore export-ignore +.phpcs.xml.dist export-ignore +phpstan.neon.dist export-ignore +phpunit.xml.dist export-ignore # # Auto detect text files and perform LF normalization diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index cd7c7191..2b206d5e 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -92,7 +92,7 @@ Clean up: make sure to reset all the file changes made during testing! #### Public properties Some sniffs will behave differently based on the value of the sniff's `public` properties. -These properties can be set [from a custom ruleset](https://github.com/squizlabs/PHP_CodeSniffer/wiki/Annotated-Ruleset) or in a test situation, in-line using `// phpcs:set Yoast.Category.SniffName propertyName propertyValue`. +These properties can be set [from a custom ruleset](https://github.com/PHPCSStandards/PHP_CodeSniffer/wiki/Annotated-Ruleset) or in a test situation, in-line using `// phpcs:set Yoast.Category.SniffName propertyName propertyValue`. If the results you are getting when testing are different from what you expected, first thing to do is to check whether the sniff has `public` properties, what those properties are set to (in your custom ruleset or in a test file in-line) and whether that setting is interferring with the results. diff --git a/.github/PULL_REQUEST_TEMPLATE/dependency_change.md b/.github/PULL_REQUEST_TEMPLATE/dependency_change.md deleted file mode 100644 index d7848654..00000000 --- a/.github/PULL_REQUEST_TEMPLATE/dependency_change.md +++ /dev/null @@ -1,19 +0,0 @@ ---- -name: Dependency change -about: Update one or more of the YoastCS dependencies -labels: "yoast cs/qa" ---- - -## Description - -* - -Refs: - -* - - diff --git a/.github/PULL_REQUEST_TEMPLATE/generic.md b/.github/PULL_REQUEST_TEMPLATE/generic.md deleted file mode 100644 index ab92da69..00000000 --- a/.github/PULL_REQUEST_TEMPLATE/generic.md +++ /dev/null @@ -1,6 +0,0 @@ ---- -name: General -about: Change which doesn't fit any of the other categories -labels: "yoast cs/qa" ---- - diff --git a/.github/PULL_REQUEST_TEMPLATE/sniff_change.md b/.github/PULL_REQUEST_TEMPLATE/sniff_change.md deleted file mode 100644 index add6a4e6..00000000 --- a/.github/PULL_REQUEST_TEMPLATE/sniff_change.md +++ /dev/null @@ -1,41 +0,0 @@ ---- -name: Sniff change -about: Introduce a new sniff or update an existing sniff -labels: "yoast cs/qa" ---- - -## Description - -* - - -Refs: - -* - - -## Test instructions - -This PR can be tested by following these steps: - -* - - -## Sniff feature completeness - -* [ ] **Documentation**: I have added/updated the sniff `Standard.xml` documentation to match this change. - * [ ] Not applicable. -* [ ] **Functionality**: This change adds auto-fixer(s). - * [ ] Not applicable. -* [ ] **Unit tests**: I have added unit tests to verify the code works as intended. -* [ ] **End-to-end tests**: I have run the new/updated sniff against one or more of the Yoast plugin repositories to find false positives/negatives. - -Fixes # diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 72db5c9b..5af18898 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -15,6 +15,8 @@ updates: prefix: "GH Actions:" labels: - "yoast cs/qa" + reviewers: + - "jrfnl" # Maintain dependencies for Composer. - package-ecosystem: "composer" @@ -31,3 +33,5 @@ updates: prefix: "Composer:" labels: - "yoast cs/qa" + reviewers: + - "jrfnl" diff --git a/.github/workflows/basics.yml b/.github/workflows/basics.yml index 5e0d5f81..50de3085 100644 --- a/.github/workflows/basics.yml +++ b/.github/workflows/basics.yml @@ -6,6 +6,8 @@ on: push: paths-ignore: - '**.md' + branches-ignore: + - 'main' pull_request: # Allow manually triggering the workflow. workflow_dispatch: @@ -26,12 +28,12 @@ jobs: steps: - name: Checkout code - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Install PHP uses: shivammathur/setup-php@v2 with: - php-version: '7.4' + php-version: 'latest' coverage: none tools: cs2pr @@ -40,17 +42,20 @@ jobs: - name: Validate Composer installation run: composer validate --no-check-all --strict - - name: 'Composer: adjust dependencies' - run: | - # The sniff stage doesn't run the unit tests, so no need for PHPUnit. - composer remove --no-update --dev phpunit/phpunit --no-scripts --no-interaction - # Using PHPCS `master` as an early detection system for bugs upstream. - composer require --no-update --no-scripts squizlabs/php_codesniffer:"dev-master" --no-interaction - # Using WPCS `master` (=stable). This can be changed back to `dev-develop` after the WPCS 3.0.0 release. - composer require --no-update --no-scripts wp-coding-standards/wpcs:"dev-master" --no-interaction + # Use the WIP/develop branches of all CS dependencies as an early detection system for bugs upstream. + - name: 'Composer: adjust dependencies - use dev versions of CS dependencies' + run: > + composer require --no-update --no-scripts --no-interaction + squizlabs/php_codesniffer:"dev-master" + phpcsstandards/phpcsutils:"dev-develop" + phpcsstandards/phpcsextra:"dev-develop" + wp-coding-standards/wpcs:"dev-develop as 3.99" # Alias needed to prevent composer conflict with VIPCS. + slevomat/coding-standard:"dev-master" + sirbrillig/phpcs-variable-analysis:"2.x" + automattic/vipwpcs:"dev-develop" # Install dependencies and handle caching in one go. - # @link https://github.com/marketplace/actions/install-composer-dependencies + # @link https://github.com/marketplace/actions/install-php-dependencies-with-composer - name: Install Composer dependencies uses: ramsey/composer-install@v2 with: @@ -64,7 +69,8 @@ jobs: # Show XML violations inline in the file diff. # @link https://github.com/marketplace/actions/xmllint-problem-matcher - - uses: korelstar/xmllint-problem-matcher@v1 + - name: Enable showing XML issues inline + uses: korelstar/xmllint-problem-matcher@v1 # Validate the ruleset XML file. # @link http://xmlsoft.org/xmllint.html @@ -73,7 +79,6 @@ jobs: # Validate the Docs XML files. # @link http://xmlsoft.org/xmllint.html - # For the build to properly error when validating against a scheme, these each have to be in their own condition. - name: Validate the XML sniff docs against schema run: xmllint --noout --schema vendor/phpcsstandards/phpcsdevtools/DocsXsd/phpcsdocs.xsd ./Yoast/Docs/*/*Standard.xml @@ -95,3 +100,31 @@ jobs: # Check that the sniffs available are feature complete. - name: Check sniff feature completeness run: composer check-complete + + phpstan: + name: "PHPStan" + + runs-on: "ubuntu-latest" + + steps: + - name: Checkout code + uses: actions/checkout@v3 + + - name: Install PHP + uses: shivammathur/setup-php@v2 + with: + php-version: 'latest' + coverage: none + tools: phpstan + + # Install dependencies and handle caching in one go. + # 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@v2" + with: + # Bust the cache at least once a month - output format: YYYY-MM. + custom-cache-suffix: $(date -u "+%Y-%m") + + - name: Run PHPStan + run: phpstan analyse diff --git a/.github/workflows/manage-labels.yml b/.github/workflows/manage-labels.yml new file mode 100644 index 00000000..faa0f7f9 --- /dev/null +++ b/.github/workflows/manage-labels.yml @@ -0,0 +1,68 @@ +name: Remove outdated labels + +on: + # https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#pull_request_target + pull_request_target: + types: + - closed + issues: + types: + - closed + +jobs: + on-pr-merge: + runs-on: ubuntu-latest + if: github.repository_owner == 'Yoast' && github.event.pull_request.merged == true + + name: Clean up labels on PR merge + + steps: + - uses: mondeja/remove-labels-gh-action@v1 + with: + token: ${{ secrets.GITHUB_TOKEN }} + labels: | + Status: awaiting feedback + Status: blocked + Status: needs investigation + Status: wait for upstream/PHPCSExtra + Status: wait for upstream/Slevomat + Status: wait for upstream/VIPCS + Status: wait for upstream/WordPressCS + + on-pr-close: + runs-on: ubuntu-latest + if: github.repository_owner == 'Yoast' && github.event_name == 'pull_request_target' && github.event.pull_request.merged == false + + name: Clean up labels on PR close + + steps: + - uses: mondeja/remove-labels-gh-action@v1 + with: + token: ${{ secrets.GITHUB_TOKEN }} + labels: | + Status: awaiting feedback + Status: blocked + Status: needs investigation + Status: wait for upstream/PHPCSExtra + Status: wait for upstream/Slevomat + Status: wait for upstream/VIPCS + Status: wait for upstream/WordPressCS + + on-issue-close: + runs-on: ubuntu-latest + if: github.repository_owner == 'Yoast' && github.event.issue.state == 'closed' + + name: Clean up labels on issue close + + steps: + - uses: mondeja/remove-labels-gh-action@v1 + with: + token: ${{ secrets.GITHUB_TOKEN }} + labels: | + Status: awaiting feedback + Status: blocked + Status: needs investigation + Status: wait for upstream/PHPCSExtra + Status: wait for upstream/Slevomat + Status: wait for upstream/VIPCS + Status: wait for upstream/WordPressCS diff --git a/.github/workflows/quicktest.yml b/.github/workflows/quicktest.yml index f1fcc14e..24e925a5 100644 --- a/.github/workflows/quicktest.yml +++ b/.github/workflows/quicktest.yml @@ -19,38 +19,31 @@ concurrency: jobs: #### QUICK TEST STAGE #### # This is a much quicker test which only runs the unit tests and linting against low/high - # supported PHP/PHPCS/WPCS combinations. + # of the supported PHP/CS dependencies combinations. quicktest: runs-on: ubuntu-latest strategy: matrix: + php_version: ['7.2', 'latest'] + cs_dependencies: ['lowest', 'stable'] + include: - php_version: 'latest' - phpcs_version: 'dev-master' - wpcs_version: 'dev-master' - - php_version: 'latest' - phpcs_version: '3.7.2' - wpcs_version: '2.3.0' - - php_version: '5.4' - phpcs_version: 'dev-master' - wpcs_version: '2.3.0' - - php_version: '5.4' - phpcs_version: '3.7.2' - wpcs_version: 'dev-master' + cs_dependencies: 'dev' - name: "QTest${{ matrix.phpcs_version == 'dev-master' && ' + Lint' || '' }}: PHP ${{ matrix.php_version }} - PHPCS ${{ matrix.phpcs_version }} - WPCS ${{ matrix.wpcs_version }}" + name: "QTest${{ matrix.cs_dependencies == 'stable' && ' + Lint' || '' }}: PHP ${{ matrix.php_version }} - CS deps ${{ matrix.cs_dependencies }}" steps: - name: Checkout code - uses: actions/checkout@v3 + uses: actions/checkout@v4 - # On stable PHPCS versions, allow for PHP deprecation notices. + # 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. - name: Setup ini config id: set_ini run: | - if [ "${{ matrix.phpcs_version }}" != "dev-master" ]; then + if [ "${{ matrix.cs_dependencies }}" != "dev" ]; then 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 @@ -63,43 +56,36 @@ jobs: ini-values: ${{ steps.set_ini.outputs.PHP_INI }} coverage: none - - name: 'Composer: adjust dependencies' - run: | - # Set the PHPCS version to test against. - composer require --no-update --no-scripts squizlabs/php_codesniffer:"${{ matrix.phpcs_version }}" --no-interaction - # Set the WPCS version to test against. - composer require --no-update --no-scripts wp-coding-standards/wpcs:"${{ matrix.wpcs_version }}" --no-interaction + - name: "Composer: set PHPCS dependencies for tests (dev)" + if: ${{ matrix.cs_dependencies == 'dev' }} + run: > + composer require --no-update --no-scripts --no-interaction + squizlabs/php_codesniffer:"dev-master" + phpcsstandards/phpcsutils:"dev-develop" + wp-coding-standards/wpcs:"dev-develop as 3.99" # Alias needed to prevent composer conflict with VIPCS. # Install dependencies and handle caching in one go. - # @link https://github.com/marketplace/actions/install-composer-dependencies - - name: Install Composer dependencies - normal - if: ${{ startsWith( matrix.php_version, '8' ) == false && matrix.php_version != 'latest' }} + # @link https://github.com/marketplace/actions/install-php-dependencies-with-composer + - name: Install Composer dependencies uses: ramsey/composer-install@v2 with: # Bust the cache at least once a month - output format: YYYY-MM. custom-cache-suffix: $(date -u "+%Y-%m") - # For the PHP 8.0 and higher, we need to install with ignore platform reqs as not all dependencies allow it. - - name: Install Composer dependencies - with ignore platform - if: ${{ startsWith( matrix.php_version, '8' ) || matrix.php_version == 'latest' }} - uses: ramsey/composer-install@v2 - with: - composer-options: --ignore-platform-req=php+ - custom-cache-suffix: $(date -u "+%Y-%m") + - name: "Composer: downgrade PHPCS dependencies for tests (lowest) (with ignore platform)" + if: ${{ matrix.cs_dependencies == 'lowest' }} + run: > + composer update --prefer-lowest --no-scripts --no-interaction + squizlabs/php_codesniffer + phpcsstandards/phpcsutils + wp-coding-standards/wpcs - name: Verify installed standards run: vendor/bin/phpcs -i - name: Lint against parse errors - if: matrix.phpcs_version == 'dev-master' + if: matrix.cs_dependencies == 'stable' run: composer lint - - name: Run the unit tests - PHP 5.4 - 8.0 - if: ${{ matrix.php_version < '8.1' && matrix.php_version != 'latest' }} + - name: Run the unit tests run: composer test - - - name: Run the unit tests - PHP 8.1+ - if: ${{ matrix.php_version >= '8.1' || matrix.php_version == 'latest'}} - run: composer test -- --no-configuration --dont-report-useless-tests - env: - PHPCS_IGNORE_TESTS: 'PHPCompatibility,WordPress' diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 8a8567ba..b7f05df4 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -5,6 +5,7 @@ on: push: branches: - main + - develop paths-ignore: - '**.md' pull_request: @@ -17,62 +18,60 @@ concurrency: group: ${{ github.workflow }}-${{ github.ref }} cancel-in-progress: true +env: + PHPCS_HIGHEST: 'dev-master' + UTILS_HIGHEST: 'dev-develop' + WPCS_HIGHEST: 'dev-develop as 3.99' # Alias needed to prevent composer conflict with VIPCS. + jobs: #### TEST STAGE #### test: + if: ${{ github.ref != 'refs/heads/develop' }} runs-on: ubuntu-latest strategy: - # Keys: - # - experimental: Whether the build is "allowed to fail". matrix: # The GHA matrix works different from Travis. # You can define jobs here and then augment them with extra variables in `include`, # as well as add extra jobs in `include`. # @link https://docs.github.com/en/free-pro-team@latest/actions/reference/workflow-syntax-for-github-actions#jobsjob_idstrategymatrix # - # Note: while WPCS 3.0.0 is under development, the matrix will use `dev-master`. - # Once it has been released and YoastCS has been made compatible, the matrix should switch (back) - # WPCS `dev-master` to `dev-develop`. - php_version: ['5.4', '5.5', '5.6', '7.0', '7.1', '7.2', '7.3', '7.4', '8.0', '8.1', '8.2'] - phpcs_version: ['3.7.2', 'dev-master'] - wpcs_version: ['2.3.0', 'dev-master'] - experimental: [false] + # The matrix is set up so as not to duplicate the builds which are run for code coverage. + php_version: ['7.3', '7.4', '8.0', '8.1', '8.2'] + cs_dependencies: ['lowest', 'stable'] include: - # Experimental builds. These are allowed to fail. - - # PHP nightly + # Make the matrix complete (when combined with the code coverage builds). + - php_version: '7.2' + cs_dependencies: 'stable' - php_version: '8.3' - phpcs_version: 'dev-master' - wpcs_version: 'dev-master' - experimental: true - # Test against WPCS unstable. Re-enable when WPCS is not in dev for the next major. - #- php_version: '8.0' - # phpcs_version: 'dev-master' - # wpcs_version: 'dev-develop' - # experimental: true + cs_dependencies: 'stable' + + # Test against dev versions of all CS dependencies with select PHP versions for early detection of issues. + - php_version: '7.4' + cs_dependencies: 'dev' + - php_version: '8.1' + cs_dependencies: 'dev' - # Test against the next major of PHPCS. Temporarily disabled due to upstream bugs. - #- php_version: '7.4' - # phpcs_version: '4.0.x-dev' - # wpcs_version: 'dev-develop' - # experimental: true + # Experimental build(s). These are allowed to fail. + # PHP nightly + - php_version: '8.4' + cs_dependencies: 'dev' - name: "Test${{ matrix.phpcs_version == 'dev-master' && matrix.wpcs_version == 'dev-master' && ' + Lint' || '' }}: PHP ${{ matrix.php_version }} - PHPCS ${{ matrix.phpcs_version }} - WPCS ${{ matrix.wpcs_version }}" + name: "Test${{ matrix.cs_dependencies == 'stable' && ' + Lint' || '' }}: PHP ${{ matrix.php_version }} - CS Deps ${{ matrix.cs_dependencies }}" - continue-on-error: ${{ matrix.experimental }} + continue-on-error: ${{ matrix.php == '8.4' }} steps: - name: Checkout code - uses: actions/checkout@v3 + uses: actions/checkout@v4 - # On stable PHPCS versions, allow for PHP deprecation notices. + # 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. - name: Setup ini config id: set_ini run: | - if [[ "${{ matrix.phpcs_version }}" != "dev-master" && "${{ matrix.phpcs_version }}" != "4.0.x-dev" ]]; then + if [ "${{ matrix.cs_dependencies }}" != "dev" ]; then 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 @@ -85,61 +84,144 @@ jobs: ini-values: ${{ steps.set_ini.outputs.PHP_INI }} coverage: none tools: cs2pr - env: - # Token is needed for the PHPCS 4.x run against source. - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - - # Set Composer up to download only PHPCS from source for PHPCS 4.x. - # The source is needed to get the base testcase from PHPCS. - # All other jobs can use `auto`, which is Composer's default value. - - name: 'Composer: conditionally prefer source for PHPCS' - if: ${{ startsWith( matrix.phpcs_version, '4' ) }} - run: composer config preferred-install.squizlabs/php_codesniffer source - - - name: 'Composer: adjust dependencies' - run: | - # Set the PHPCS version to test against. - composer require --no-update --no-scripts squizlabs/php_codesniffer:"${{ matrix.phpcs_version }}" --no-interaction - # Set the WPCS version to test against. - composer require --no-update --no-scripts wp-coding-standards/wpcs:"${{ matrix.wpcs_version }}" --no-interaction - - name: 'Composer: conditionally remove PHPCSDevtools' - if: ${{ startsWith( matrix.phpcs_version, '4' ) }} - # Remove devtools as it will not (yet) install on PHPCS 4.x. - run: composer remove --no-update --dev phpcsstandards/phpcsdevtools --no-interaction + - name: "Composer: set PHPCS dependencies for tests (dev)" + if: ${{ matrix.cs_dependencies == 'dev' }} + run: > + composer require --no-update --no-scripts --no-interaction + squizlabs/php_codesniffer:"${{ env.PHPCS_HIGHEST }}" + phpcsstandards/phpcsutils:"${{ env.UTILS_HIGHEST }}" + wp-coding-standards/wpcs:"${{ env.WPCS_HIGHEST }}" # Install dependencies and handle caching in one go. - # @link https://github.com/marketplace/actions/install-composer-dependencies + # @link https://github.com/marketplace/actions/install-php-dependencies-with-composer - name: Install Composer dependencies - normal - if: ${{ startsWith( matrix.php_version, '8' ) == false }} uses: ramsey/composer-install@v2 with: # Bust the cache at least once a month - output format: YYYY-MM. custom-cache-suffix: $(date -u "+%Y-%m") - # For the PHP 8/"nightly", we need to install with ignore platform reqs as we're still using PHPUnit 7. - - name: Install Composer dependencies - with ignore platform - if: ${{ startsWith( matrix.php_version, '8' ) }} - uses: ramsey/composer-install@v2 + - name: "Composer: downgrade PHPCS dependencies for tests (lowest)" + if: ${{ matrix.cs_dependencies == 'lowest' }} + run: > + composer update --prefer-lowest --no-scripts --no-interaction + squizlabs/php_codesniffer + phpcsstandards/phpcsutils + wp-coding-standards/wpcs + + - name: Verify installed versions + run: composer info --no-dev + + - name: Verify installed standards + run: vendor/bin/phpcs -i + + # The results of the linting will be shown inline in the PR via the CS2PR tool. + # @link https://github.com/staabm/annotate-pull-request-from-checkstyle/ + - name: Lint against parse errors + if: ${{ matrix.cs_dependencies == 'stable' }} + run: composer lint -- --checkstyle | cs2pr + + - name: Run the unit tests + run: composer test + + #### CODE COVERAGE STAGE #### + # N.B.: Coverage is only checked on the lowest and highest stable PHP versions + # and a low/high of each major for PHPCS. + # These builds are left out off the "test" stage so as not to duplicate test runs. + coverage: + # No use running the coverage builds if there are failing test builds. + needs: test + # The default condition is success(), but this is false when one of the previous jobs is skipped + if: always() && (needs.test.result == 'success' || needs.test.result == 'skipped') + + runs-on: ubuntu-latest + + strategy: + matrix: + php_version: ['7.2', '8.3'] + cs_dependencies: ['lowest', 'dev'] + + name: "Coverage${{ matrix.cs_dependencies == 'stable' && ' + Lint' || '' }}: PHP ${{ matrix.php_version }} - CS Deps ${{ matrix.cs_dependencies }}" + + steps: + - name: Checkout code + uses: actions/checkout@v4 + + # 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. + - name: Setup ini config + id: set_ini + run: | + if [ "${{ matrix.cs_dependencies }}" != "dev" ]; then + 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 + fi + + - name: Install PHP + uses: shivammathur/setup-php@v2 with: - composer-options: --ignore-platform-req=php+ + php-version: ${{ matrix.php_version }} + ini-values: ${{ steps.set_ini.outputs.PHP_INI }} + coverage: xdebug + tools: cs2pr + + - name: "Composer: set PHPCS dependencies for tests (dev)" + if: ${{ matrix.cs_dependencies == 'dev' }} + run: > + composer require --no-update --no-scripts --no-interaction + squizlabs/php_codesniffer:"${{ env.PHPCS_HIGHEST }}" + phpcsstandards/phpcsutils:"${{ env.UTILS_HIGHEST }}" + wp-coding-standards/wpcs:"${{ env.WPCS_HIGHEST }}" + + # 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@v2" + with: + # Bust the cache at least once a month - output format: YYYY-MM. custom-cache-suffix: $(date -u "+%Y-%m") + - name: "Composer: downgrade PHPCS dependencies for tests (lowest)" + if: ${{ matrix.cs_dependencies == 'lowest' }} + run: > + composer update --prefer-lowest --no-scripts --no-interaction + squizlabs/php_codesniffer + phpcsstandards/phpcsutils + wp-coding-standards/wpcs + + - name: Verify installed versions + run: composer info --no-dev + - name: Verify installed standards run: vendor/bin/phpcs -i # The results of the linting will be shown inline in the PR via the CS2PR tool. # @link https://github.com/staabm/annotate-pull-request-from-checkstyle/ - name: Lint against parse errors - if: ${{ matrix.phpcs_version == 'dev-master' && matrix.wpcs_version == 'dev-master' }} + if: ${{ matrix.cs_dependencies == 'dev' }} run: composer lint -- --checkstyle | cs2pr - - name: Run the unit tests - PHP 5.4 - 8.0 - if: ${{ matrix.php_version < '8.1' }} - run: composer test + - name: Run the unit tests with code coverage + run: composer coverage - - name: Run the unit tests - PHP 8.1+ - if: ${{ matrix.php_version >= '8.1' }} - run: composer test -- --no-configuration --dont-report-useless-tests - env: - PHPCS_IGNORE_TESTS: 'PHPCompatibility,WordPress' + - name: Upload coverage results to Coveralls + if: ${{ success() }} + uses: coverallsapp/github-action@v2 + with: + format: clover + file: build/logs/clover.xml + flag-name: php-${{ matrix.php_version }}-cs-${{ matrix.cs_dependencies }} + parallel: true + + coveralls-finish: + needs: coverage + if: always() && needs.coverage.result == 'success' + + runs-on: ubuntu-latest + + steps: + - name: Coveralls Finished + uses: coverallsapp/github-action@v2 + with: + parallel-finished: true diff --git a/.gitignore b/.gitignore index 5d83e0c3..ad33565e 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,9 @@ vendor/ composer.lock -.phpcs.xml .cache/phpcs.cache +.phpcs.xml phpcs.xml phpunit.xml +.phpunit.result.cache +build/ +phpstan.neon diff --git a/.phpcs.xml.dist b/.phpcs.xml.dist index 7f70a828..03b2fb86 100644 --- a/.phpcs.xml.dist +++ b/.phpcs.xml.dist @@ -8,7 +8,7 @@ @@ -37,14 +37,8 @@ ############################################################################# --> - - - - - - @@ -68,7 +62,16 @@ - + + + *\.php$ + + + + + + *\.php$ + - + - - + + - - + + + + + + + diff --git a/CHANGELOG.md b/CHANGELOG.md index 4bf6d903..2765c03a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,134 @@ All notable changes to this project will be documented in this file. This project adheres to [Semantic Versioning](https://semver.org/) and [Keep a CHANGELOG](https://keepachangelog.com/). +### [3.0.0] - 2023-12-14 + +#### Added +* Composer/PHPCS: Dependencies on the following external PHPCS standards packages: [PHPCSUtils], [PHPCSExtra], [SlevomatCodingStandard], [VariableAnalysis] and [WordPressVIP Coding Standards]. +* PHPCS: A best effort has been made to add support for new PHP syntaxes/features to all YoastCS native sniffs and utility functions (or to verify/improve existing support). + YoastCS native sniffs and utilities have received fixes for the following syntaxes: + * PHP 5.6 + - Parameter unpacking in function calls. + * PHP 8.0 + - Named arguments in function calls. + * PHP 8.1 + - Enumerations. + - First class callables. + * PHP 8.2 + - Readonly classes. +* PHPCS: The `Yoast.Commenting.CoversTag` sniff includes a new warning for the use of `ClassName<*>` type `@covers` annotations, as these have been deprecated as of PHPUnit 9.0. +* PHPCS: The `Yoast.Files.FileName` sniff now has the (optional) ability to check whether file names comply with PSR-4. + To enable this ability, add the custom `psr4_paths` property to your ruleset. The `psr4_paths` property is an array property and mirrors the `psr4` setting in the Composer `autoload` directive. It expects a namespace prefix as the array key and a comma separated list of relative paths as the array value. Multiple PSR-4 paths can be passed (array elements). + For files containing OO structures in a "PSR4 path", the `oo_prefixes` and the `excluded_files_strict_check` properties will be ignored. +* PHPCS: The `Yoast.NamingConventions.NamespaceName` sniff will now throw a `MissingPrefix` error if a prefix is expected, but the namespace name does not include a prefix. +* PHPCS: The `Yoast.NamingConventions.NamespaceName` sniff will now throw a `DirectoryInvalid` error if a file is in a directory which would not result in a valid namespace name. +* PHPCS: The `Yoast.NamingConventions.NamespaceName` sniff now has the (optional) ability to check whether namespace names comply with PSR-4. + To enable this ability, add the custom `psr4_paths` property to your ruleset. The `psr4_paths` property is an array property and mirrors the `psr4` setting in the Composer `autoload` directive. It expects a namespace prefix as the array key and a comma separated list of relative paths as the array value. Multiple PSR-4 paths can be passed (array elements). + A `psr4_paths` property will take precedence over the, potentially set, `src_directory` and `prefixes` properties. +* PHPCS: The following sniffs/error codes have been added to/enabled in the YoastCS ruleset (with appropriate configuration): + - All new sniffs which were added/included in [WordPressCS 3.0.0](https://github.com/WordPress/WordPress-Coding-Standards/releases/tag/3.0.0). + - `PSR1.Classes.ClassDeclaration` (for tests only) + - `PSR12.Properties.ConstantVisibility` + - `SlevomatCodingStandard.Arrays.DisallowImplicitArrayCreation` + - `SlevomatCodingStandard.Classes.ClassStructure` + - `SlevomatCodingStandard.Classes.ModernClassNameReference` + - `SlevomatCodingStandard.Functions.StaticClosure` + - `SlevomatCodingStandard.Namespaces.AlphabeticallySortedUses` + - `SlevomatCodingStandard.Namespaces.FullyQualifiedGlobalConstants` + - `SlevomatCodingStandard.Namespaces.FullyQualifiedGlobalFunctions` + - `SlevomatCodingStandard.Namespaces.ReferenceUsedNamesOnly` + - `SlevomatCodingStandard.Namespaces.UnusedUses` + - `SlevomatCodingStandard.Namespaces.UseFromSameNamespace` + - `SlevomatCodingStandard.TypeHints.DisallowMixedTypeHint` (tests excluded) + - `SlevomatCodingStandard.TypeHints.LongTypeHints` + - `SlevomatCodingStandard.TypeHints.NullableTypeForNullDefaultValue` + - `SlevomatCodingStandard.TypeHints.NullTypeHintOnLastPosition` + - `SlevomatCodingStandard.TypeHints.ParameterTypeHint` + - `SlevomatCodingStandard.TypeHints.PropertyTypeHint` + - `SlevomatCodingStandard.TypeHints.ReturnTypeHint` + - `Squiz.Commenting.FunctionComment.InvalidReturnNotVoid` + - `Squiz.Commenting.FunctionComment.MissingReturn` + - `Squiz.Commenting.FunctionComment.ParamCommentNotCapital` + - `Squiz.Commenting.FunctionComment.SpacingAfterParamName` + - `Squiz.WhiteSpace.SuperfluousWhitespace.EmptyLines` + - `Universal.Classes.RequireFinalClass` (for tests only, doubles/mocks excluded) + - `Universal.CodeAnalysis.NoDoubleNegative` + - `Universal.ControlStructures.DisallowAlternativeSyntax` + - `Universal.ControlStructures.IfElseDeclaration` + - `Universal.FunctionDeclarations.NoLongClosures` + - `Universal.Operators.ConcatPosition` + - `Universal.Operators.DisallowLogicalAndOr` + - `Universal.PHP.LowercasePHPTag` + - `Universal.UseStatements.DisallowUseConst` + - `Universal.UseStatements.DisallowUseFunction` + - `VariableAnalysis.CodeAnalysis.VariableAnalysis` + - `WordPressVIPMinimum.Classes.DeclarationCompatibility` + - `WordPressVIPMinimum.Hooks.AlwaysReturnInFilter` + - `WordPressVIPMinimum.Security.EscapingVoidReturnFunctions` + - `WordPressVIPMinimum.Security.ProperEscapingFunction` +* PHPCS: New `PathHelper`, `PathValidationHelper` and `PSR4PathsTraits` classes/traits for use by the sniffs. +* Readme: section on the YoastCS `Threshold` report. + +#### Changed +* :warning: The minimum supported PHP version for this package is now PHP 7.2 (was 5.4). +* PHPCS: all sniffs are now runtime compatible with PHP 7.2 - 8.3. +* :warning: PHPCS: All non-abstract classes in YoastCS are now `final` and all non-`public` methods and properties are now `private`. + Additionally, all non-private methods in traits have also been made `final`. +* Composer: Supported version of [PHP_CodeSniffer] has been changed from `^3.7.1` to `^3.8.0`. +* :warning: Composer: Supported version of [WordPressCS] has been changed from `^2.3.0` to `^3.0.1`. + YoastCS is now fully compatible with WordPressCS 3.0. + Note: WordPressCS 3.0.0 contains breaking changes. Please read the [WordPressCS release announcement](https://make.wordpress.org/core/2023/08/21/wordpresscs-3-0-0-is-now-available/) and follow the [WordPressCS upgrade guides](https://github.com/WordPress/WordPress-Coding-Standards/wiki/). +* PHPCS: The default value for the `minimum_wp_version` (previously `minimum_supported_wp_version`) property which is used by various WPCS sniffs has been updated to WP `6.2` (was `6.0`). +* PHPCS: Files in a `wp-content/plugins/` subdirectory will now always be ignored for PHPCS scans. +* PHPCS: The ruleset included value for the `doubles_path` property in the `Yoast.Files.TestDoubles` sniff has been updated to include the typical Yoast `test/Unit/Doubles` and `test/WP/Doubles` directories as per the restructured tests. +* PHPCS: The `Yoast.Commenting.CodeCoverageIgnoreDeprecated` sniff will now also examine class docblocks. +* PHPCS: The `Yoast.Commenting.FileComment` sniff will no longer flag a file docblock in a namespaced file which doesn't contain an OO structure as redundant. +* PHPCS: The `Yoast.Files.FileName` sniff will now also examine the file name of PHP files using only the PHP short open tag (`` annotations as an invalid format when combined with a `@coversDefaultClass` tag. +* PHPCS: The `Yoast.Commenting.TestHaveCoversTag` sniff will no longer examine global functions. +* PHPCS: The `Yoast.Files.FileName` sniff will now handle the values for the `excluded_files_strict_check` property in a case-sensitive manner (as file names are case-sensitive on most operating systems). +* PHPCS: The `Yoast.Files.TestDoubles` sniff will now handle the values for the `doubles_path` property in a case-sensitive manner (as directory names are case-sensitive on most operating systems). +* PHPCS: The `Yoast.NamingConventions.NamespaceName` sniff will now bow out earlier if the namespace name is invalid (parse error). +* PHPCS: The `Yoast.NamingConventions.NamespaceName` sniff will now handle "directory to namespace name" translations more accurately and will no longer throw an error if the directory name contains an underscore. +* PHPCS: The `Yoast.NamingConventions.ObjectNameDepth` sniff now has a more accurate object name depth calculation for OO structures with a name in `CamelCaps`. + This should prevent various false positives for test classes/test doubles. +* PHPCS: The `Yoast.NamingConventions.ObjectNameDepth` sniff will no longer check if a class extends a known "TestCase" to determine whether to allow for extra object name depth, it will just base itself on the name of the object under examination, which should prevent some false positives. +* PHPCS: The fixer in the `Yoast.Yoast.JsonEncodeAlternative` sniff (previously `Yoast.Yoast.AlternativeFunctions`) will no longer inadvertently create a parse error when fixing fully qualified function calls. +* PHPCS: The `Yoast.Yoast.JsonEncodeAlternative` sniff (previously `Yoast.Yoast.AlternativeFunctions`) will now no longer try to auto-fix when it encounters PHP 5.6+ parameter unpacking. +* PHPCS: The `Yoast.Yoast.JsonEncodeAlternative` sniff (previously `Yoast.Yoast.AlternativeFunctions`) will now no longer try to auto-fix when it encounters a PHP 8.1+ first class callable. + ### [2.3.1] - 2023-03-09 #### Changed @@ -468,15 +596,21 @@ This project adheres to [Semantic Versioning](https://semver.org/) and [Keep a C Initial public release as a stand-alone package. -[PHP_CodeSniffer]: https://github.com/squizlabs/PHP_CodeSniffer/releases -[WordPressCS]: https://github.com/WordPress/WordPress-Coding-Standards/blob/develop/CHANGELOG.md -[PHPCompatibilityWP]: https://github.com/PHPCompatibility/PHPCompatibilityWP#changelog -[PHPCompatibility]: https://github.com/PHPCompatibility/PHPCompatibility/blob/master/CHANGELOG.md -[PHP Mess Detector]: https://github.com/phpmd/phpmd/blob/master/CHANGELOG -[Composer PHPCS plugin]: https://github.com/PHPCSStandards/composer-installer/releases -[PHP Parallel Lint]: https://github.com/php-parallel-lint/PHP-Parallel-Lint/releases -[PHP Console Highlighter]: https://github.com/php-parallel-lint/PHP-Console-Highlighter/releases - +[PHP_CodeSniffer]: https://github.com/PHPCSStandards/PHP_CodeSniffer/releases +[Composer PHPCS plugin]: https://github.com/PHPCSStandards/composer-installer/releases +[PHPCompatibilityWP]: https://github.com/PHPCompatibility/PHPCompatibilityWP#changelog +[PHPCompatibility]: https://github.com/PHPCompatibility/PHPCompatibility/blob/master/CHANGELOG.md +[PHPCSExtra]: https://github.com/PHPCSStandards/PHPCSExtra/releases +[SlevomatCodingStandard]: https://github.com/slevomat/coding-standard/releases +[VariableAnalysis]: https://github.com/sirbrillig/phpcs-variable-analysis/releases +[WordPressCS]: https://github.com/WordPress/WordPress-Coding-Standards/blob/develop/CHANGELOG.md +[WordPressVIP Coding Standards]: https://github.com/Automattic/VIP-Coding-Standards/releases +[PHP Mess Detector]: https://github.com/phpmd/phpmd/blob/master/CHANGELOG +[PHP Parallel Lint]: https://github.com/php-parallel-lint/PHP-Parallel-Lint/releases +[PHP Console Highlighter]: https://github.com/php-parallel-lint/PHP-Console-Highlighter/releases + +[3.0.0]: https://github.com/Yoast/yoastcs/compare/2.3.1...3.0.0 +[2.3.1]: https://github.com/Yoast/yoastcs/compare/2.3.0...2.3.1 [2.3.0]: https://github.com/Yoast/yoastcs/compare/2.2.1...2.3.0 [2.2.1]: https://github.com/Yoast/yoastcs/compare/2.2.0...2.2.1 [2.2.0]: https://github.com/Yoast/yoastcs/compare/2.1.0...2.2.0 diff --git a/README.md b/README.md index b6ea3245..91ae9f00 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,7 @@ # Yoast Coding Standards +[![Coverage Status](https://coveralls.io/repos/github/Yoast/yoastcs/badge.svg?branch=develop)](https://coveralls.io/github/Yoast/yoastcs?branch=develop) + Yoast Coding Standards (YoastCS) is a project with rulesets for code style and quality tools to be used in Yoast projects. ## Installation @@ -9,21 +11,20 @@ Yoast Coding Standards (YoastCS) is a project with rulesets for code style and q Standards are provided as a [Composer](https://getcomposer.org/) package and can be installed with: ```bash -composer create-project yoast/yoastcs:"dev-main" +composer global config allow-plugins.dealerdirect/phpcodesniffer-composer-installer true +composer global require --dev yoast/yoastcs:"^3.0" ``` -Composer will automatically install dependencies, register standards paths, and set default PHP Code Sniffer standard to `Yoast`. - ### As dependency To include standards as part of a project require them as development dependencies: ```bash composer config allow-plugins.dealerdirect/phpcodesniffer-composer-installer true -composer require --dev yoast/yoastcs:"^2.0" +composer require --dev yoast/yoastcs:"^3.0" ``` -Composer will automatically install dependencies and register the YoastCS and other external standards with PHP_CodeSniffer. +Composer will automatically install dependencies and register YoastCS and other external standards with PHP_CodeSniffer. ## Tools provided via YoastCS @@ -51,7 +52,7 @@ Typically, (a variation on) the following snippet would be added to the `compose ## PHP Code Sniffer -Set of [PHP_CodeSniffer](https://github.com/squizlabs/PHP_CodeSniffer) rules. +Set of [PHP_CodeSniffer](https://github.com/PHPCSStandards/PHP_CodeSniffer) rules. Severity levels: @@ -61,9 +62,13 @@ Severity levels: ### The YoastCS Standard The `Yoast` standard for PHP_CodeSniffer is comprised of the following: -* The `WordPress` ruleset from the [WordPress Coding Standards](https://github.com/WordPress/WordPress-Coding-Standards) implementing the official [WordPress PHP Coding Standards](https://make.wordpress.org/core/handbook/coding-standards/php/), with some [select exclusions](https://github.com/Yoast/yoastcs/blob/develop/Yoast/ruleset.xml#L29-L75). +* The `WordPress` ruleset from the [WordPress Coding Standards](https://github.com/WordPress/WordPress-Coding-Standards) implementing the official [WordPress PHP Coding Standards](https://developer.wordpress.org/coding-standards/wordpress-coding-standards/php/), with some [select exclusions](https://github.com/Yoast/yoastcs/blob/develop/Yoast/ruleset.xml#L29-L75). * The [`PHPCompatibilityWP`](https://github.com/PHPCompatibility/PHPCompatibilityWP) ruleset which checks code for PHP cross-version compatibility while preventing false positives for functionality polyfilled within WordPress. -* Select additional sniffs taken from [`PHP_CodeSniffer`](https://github.com/squizlabs/PHP_CodeSniffer). +* The [`VariableAnalysis`](https://github.com/sirbrillig/phpcs-variable-analysis/) ruleset. +* Select additional sniffs taken from [`PHP_CodeSniffer`](https://github.com/PHPCSStandards/PHP_CodeSniffer). +* Select additional sniffs taken from [`PHPCSExtra`](https://github.com/PHPCSStandards/PHPCSExtra). +* Select additional sniffs taken from [`SlevomatCodingStandard`](https://github.com/slevomat/coding-standard). +* Select additional sniffs taken from [WordPress VIP Coding Standards](https://github.com/Automattic/VIP-Coding-Standards/). * A number of custom Yoast specific sniffs. Files within version management and dependency related directories, such as the Composer `vendor` directory, are excluded from the scans by default. @@ -90,7 +95,7 @@ Not all sniffs have documentation available about what they sniff for, but for t "vendor/bin/phpcs" --extensions=php /path/to/folder/ ``` -For more command-line options, please have a read through the [PHP_CodeSniffer documentation](https://github.com/squizlabs/PHP_CodeSniffer/wiki/Usage). +For more command-line options, please have a read through the [PHP_CodeSniffer documentation](https://github.com/PHPCSStandards/PHP_CodeSniffer/wiki/Usage). #### Yoast plugin repositories @@ -98,7 +103,7 @@ All Yoast plugin repositories contain a `[.]phpcs.xml.dist` file which contains From the root of these repositories, you can run PHPCS by using: ```bash -composer check-cs +composer check-cs-warnings ``` #### PhpStorm @@ -107,6 +112,37 @@ Refer to [Using PHP Code Sniffer Tool](https://www.jetbrains.com/phpstorm/help/u After installation, the `Yoast` standard will be available as a choice in PHP Code Sniffer Validation inspection. +### The YoastCS "Threshold" report + +The YoastCS package includes a custom `YoastCS\Yoast\Reports\Threshold` report for PHP_CodeSniffer to compare the current PHPCS run results with predefined "threshold" settings. + +The report will look in the runtime environment for the following two environment variables and will take the values of those as the thresholds to compare the PHPCS run results against: +* `YOASTCS_THRESHOLD_ERRORS` +* `YOASTCS_THRESHOLD_WARNINGS` + +If the environment variables are not set, they will default to 0 for both, i.e. no errors or warnings allowed. + +The report will not print any details about the issues found, it just shows a summary based on the thresholds: +``` +PHP CODE SNIFFER THRESHOLD COMPARISON +------------------------------------------------------------------------------------------------------------------------ +Coding standards ERRORS: 148/130. +Coding standards WARNINGS: 539/539. + +Please fix any errors introduced in your code and run PHPCS again to verify. +Please fix any warnings introduced in your code and run PHPCS again to verify. +``` + +After the report has run, a global `YOASTCS_ABOVE_THRESHOLD` constant (boolean) will be available which can be used in calling scripts. + +To use this report, run PHPCS with the following command-line argument: `--report=YoastCS\Yoast\Reports\Threshold`. +_Note: depending on the OS the command is run on, the backslashes in the report name may need to be escaped (doubled)._ + +For those Yoast plugin repositories which use thresholds, the status can be checked locally by running: +```bash +composer check-cs-thresholds +``` + ## Changelog The changelog for this package can be found in the [CHANGELOG.md](https://github.com/Yoast/yoastcs/blob/develop/CHANGELOG.md) file. diff --git a/Yoast/Docs/Commenting/CodeCoverageIgnoreDeprecatedStandard.xml b/Yoast/Docs/Commenting/CodeCoverageIgnoreDeprecatedStandard.xml index 83ffbe39..75ff7494 100644 --- a/Yoast/Docs/Commenting/CodeCoverageIgnoreDeprecatedStandard.xml +++ b/Yoast/Docs/Commenting/CodeCoverageIgnoreDeprecatedStandard.xml @@ -5,7 +5,7 @@ > diff --git a/Yoast/Docs/Commenting/CoversTagStandard.xml b/Yoast/Docs/Commenting/CoversTagStandard.xml index 659041d5..b898bf4b 100644 --- a/Yoast/Docs/Commenting/CoversTagStandard.xml +++ b/Yoast/Docs/Commenting/CoversTagStandard.xml @@ -5,10 +5,11 @@ > @@ -24,7 +25,7 @@ class Some_Test extends TestCase { } ]]> - + @@ -106,6 +107,41 @@ class Some_Test extends TestCase { * @covers ::globalFunction */ function test_something() {} +} + ]]> + + + + tags is deprecated since PHPUnit 9.0 and support has been removed in PHPUnit 10.0. + These type of annotations should not be used. + ]]> + + + + ::globalFunction + * @covers \ClassName::methodName + */ + function test_something() {} +} + ]]> + + + \ClassName:: + * @covers \ClassName:: + * @covers \ClassName + */ + function test_something() {} } ]]> diff --git a/Yoast/Docs/Commenting/FileCommentStandard.xml b/Yoast/Docs/Commenting/FileCommentStandard.xml index 41da79db..ddbae88a 100644 --- a/Yoast/Docs/Commenting/FileCommentStandard.xml +++ b/Yoast/Docs/Commenting/FileCommentStandard.xml @@ -5,7 +5,7 @@ > diff --git a/Yoast/Docs/ControlStructures/IfElseDeclarationStandard.xml b/Yoast/Docs/ControlStructures/IfElseDeclarationStandard.xml deleted file mode 100644 index c2f9b211..00000000 --- a/Yoast/Docs/ControlStructures/IfElseDeclarationStandard.xml +++ /dev/null @@ -1,59 +0,0 @@ - - - - - - - - -elseif ($bar) { - $var = 2; -} - ]]> - - - elseif ($bar) { - $var = 2; -} - ]]> - - - - - - - - elseif ($bar) { - $var = 2; -} - ]]> - - - else { - $var = 2; - } - ]]> - - - diff --git a/Yoast/Docs/Files/FileNameStandard.xml b/Yoast/Docs/Files/FileNameStandard.xml index 21c25aa2..957a4c34 100644 --- a/Yoast/Docs/Files/FileNameStandard.xml +++ b/Yoast/Docs/Files/FileNameStandard.xml @@ -44,7 +44,7 @@ class WPSEO_Utils {} @@ -65,6 +65,33 @@ class WPSEO_Utils {} + + + + Yoast_Output_Thing.php --> +Yoast_Output_Thing {} + ]]> + + + outline-something.php --> +Yoast_Outline_Something {} + ]]> + + + + diff --git a/Yoast/Docs/Files/TestDoublesStandard.xml b/Yoast/Docs/Files/TestDoublesStandard.xml index 9dc05853..c6b1b9fe 100644 --- a/Yoast/Docs/Files/TestDoublesStandard.xml +++ b/Yoast/Docs/Files/TestDoublesStandard.xml @@ -5,40 +5,11 @@ > - - - - - - - - - - - - - - - - - - ; - ]]> - - - { - // Code. -} - ]]> - - - - - - - - - Yoast\Sub; - ]]> - - - ; - ]]> - - - - - - - - - - - - namespace Yoast\Sub\B { -} - ]]> - - - diff --git a/Yoast/Docs/NamingConventions/NamespaceNameStandard.xml b/Yoast/Docs/NamingConventions/NamespaceNameStandard.xml index 826f530f..be5f3bf0 100644 --- a/Yoast/Docs/NamingConventions/NamespaceNameStandard.xml +++ b/Yoast/Docs/NamingConventions/NamespaceNameStandard.xml @@ -1,15 +1,34 @@ + + + + Yoast\WP\Plugin\Admin\Forms; + ]]> + + + Admin\Forms; + ]]> + + + + @@ -31,8 +50,18 @@ namespace Yoast\WP\Plugin\Tests\Foo\Bar\Flo\Sub; @@ -51,4 +80,20 @@ namespace Yoast\WP\Plugin\Unrelated; ]]> + + + User_Forms/file.php --> +User_Forms; + ]]> + + + User_forms/file.php --> +user_Forms; + ]]> + + diff --git a/Yoast/Docs/NamingConventions/ObjectNameDepthStandard.xml b/Yoast/Docs/NamingConventions/ObjectNameDepthStandard.xml index ce3a5b75..5d3fd7fe 100644 --- a/Yoast/Docs/NamingConventions/ObjectNameDepthStandard.xml +++ b/Yoast/Docs/NamingConventions/ObjectNameDepthStandard.xml @@ -5,9 +5,9 @@ > diff --git a/Yoast/Docs/Yoast/AlternativeFunctionsStandard.xml b/Yoast/Docs/Yoast/JsonEncodeAlternativeStandard.xml similarity index 81% rename from Yoast/Docs/Yoast/AlternativeFunctionsStandard.xml rename to Yoast/Docs/Yoast/JsonEncodeAlternativeStandard.xml index 4487f80b..4e44e2cf 100644 --- a/Yoast/Docs/Yoast/AlternativeFunctionsStandard.xml +++ b/Yoast/Docs/Yoast/JsonEncodeAlternativeStandard.xml @@ -1,11 +1,11 @@ diff --git a/Yoast/Reports/Threshold.php b/Yoast/Reports/Threshold.php index 024359a3..6894cd67 100644 --- a/Yoast/Reports/Threshold.php +++ b/Yoast/Reports/Threshold.php @@ -19,43 +19,46 @@ * available which can be used in calling scripts. * * @since 2.2.0 + * + * @phpcs:disable VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable -- Flags unused params which are required via the interface. Invalid. + * @phpcs:disable SlevomatCodingStandard.TypeHints.DisallowMixedTypeHint.DisallowedMixedTypeHint -- Type is too complex to document properly. */ -class Threshold implements Report { +final class Threshold implements Report { /** * Escape sequence for making text white on the command-line. * * @var string */ - const WHITE = "\033[1m"; + private const WHITE = "\033[1m"; /** * Escape sequence for making text red on the command-line. * * @var string */ - const RED = "\033[31m"; + private const RED = "\033[31m"; /** * Escape sequence for making text green on the command-line. * * @var string */ - const GREEN = "\033[32m"; + private const GREEN = "\033[32m"; /** * Escape sequence for making text orange/yellow on the command-line. * * @var string */ - const YELLOW = "\033[33m"; + private const YELLOW = "\033[33m"; /** * Escape sequence for resetting the text colour. * * @var string */ - const RESET = "\033[0m"; + private const RESET = "\033[0m"; /** * Generate a partial report for a single processed file. @@ -64,10 +67,10 @@ class Threshold implements Report { * and FALSE if it ignored the file. Returning TRUE indicates that the file and * its data should be counted in the grand totals. * - * @param array $report Prepared report data. - * @param File $phpcsFile The file being reported on. - * @param bool $showSources Whether to show the source codes. - * @param int $width Maximum allowed line width. + * @param array>> $report Prepared report data. + * @param File $phpcsFile The file being reported on. + * @param bool $showSources Whether to show the source codes. + * @param int $width Maximum allowed line width. * * @return bool */ @@ -155,6 +158,9 @@ public function generate( } // Make the threshold comparison outcome available to the calling script. - \define( 'YOASTCS_ABOVE_THRESHOLD', $above_threshold ); + // The conditional define is only so as to make the method testable. + if ( \defined( 'YOASTCS_ABOVE_THRESHOLD' ) === false ) { + \define( 'YOASTCS_ABOVE_THRESHOLD', $above_threshold ); + } } } diff --git a/Yoast/Sniffs/Commenting/CodeCoverageIgnoreDeprecatedSniff.php b/Yoast/Sniffs/Commenting/CodeCoverageIgnoreDeprecatedSniff.php index 026045a2..ab2ef68f 100644 --- a/Yoast/Sniffs/Commenting/CodeCoverageIgnoreDeprecatedSniff.php +++ b/Yoast/Sniffs/Commenting/CodeCoverageIgnoreDeprecatedSniff.php @@ -5,27 +5,30 @@ use PHP_CodeSniffer\Files\File; use PHP_CodeSniffer\Sniffs\Sniff; use PHP_CodeSniffer\Util\Tokens; +use PHPCSUtils\Tokens\Collections; /** - * Verifies functions which are marked as `deprecated` have a `codeCoverageIgnore` tag + * Verifies functions/OO structures which are marked as `deprecated` have a `codeCoverageIgnore` tag * in their docblock. * - * @package Yoast\YoastCS - * @author Juliette Reinders Folmer - * - * @since 1.1.0 + * @since 1.1.0 + * @since 3.0.0 Not just checks function docblocks, but also class/OO docblocks. */ -class CodeCoverageIgnoreDeprecatedSniff implements Sniff { +final class CodeCoverageIgnoreDeprecatedSniff implements Sniff { /** * Returns an array of tokens this test wants to listen for. * - * @return (int|string)[] + * @return array */ public function register() { - return [ - \T_FUNCTION, - ]; + $targets = Tokens::$ooScopeTokens; + // Ignore interfaces as they can't contain code. Ignore anon classes as they are normally nested in another construct. + unset( $targets[ \T_ANON_CLASS ], $targets[ \T_INTERFACE ] ); + + $targets[ \T_FUNCTION ] = \T_FUNCTION; + + return $targets; } /** @@ -40,7 +43,12 @@ public function process( File $phpcsFile, $stackPtr ) { $tokens = $phpcsFile->getTokens(); - $ignore = Tokens::$methodPrefixes; + if ( $tokens[ $stackPtr ]['code'] === \T_FUNCTION ) { + $ignore = Tokens::$methodPrefixes; + } + else { + $ignore = Collections::classModifierKeywords(); + } $ignore[ \T_WHITESPACE ] = \T_WHITESPACE; for ( $commentEnd = ( $stackPtr - 1 ); $commentEnd >= 0; $commentEnd-- ) { @@ -48,9 +56,7 @@ public function process( File $phpcsFile, $stackPtr ) { continue; } - if ( $tokens[ $commentEnd ]['code'] === \T_ATTRIBUTE_END - && isset( $tokens[ $commentEnd ]['attribute_opener'] ) === true - ) { + if ( isset( $tokens[ $commentEnd ]['attribute_opener'] ) === true ) { $commentEnd = $tokens[ $commentEnd ]['attribute_opener']; continue; } @@ -58,7 +64,6 @@ public function process( File $phpcsFile, $stackPtr ) { break; } - if ( $tokens[ $commentEnd ]['code'] !== \T_DOC_COMMENT_CLOSE_TAG ) { // Function without (proper) docblock. Not our concern. return; @@ -75,7 +80,7 @@ public function process( File $phpcsFile, $stackPtr ) { } if ( $deprecated === false ) { - // Not a deprecated function. + // Not a deprecated function/OO structure. return; } @@ -95,7 +100,7 @@ public function process( File $phpcsFile, $stackPtr ) { $hasTagAsString = $phpcsFile->findNext( \T_DOC_COMMENT_STRING, ( $commentStart + 1 ), $commentEnd, false, 'codeCoverageIgnore' ); if ( $hasTagAsString !== false ) { $prev = $phpcsFile->findPrevious( \T_DOC_COMMENT_WHITESPACE, ( $hasTagAsString - 1 ), $commentStart, true ); - if ( $prev !== false && $tokens[ $prev ]['code'] === \T_DOC_COMMENT_STAR ) { + if ( $tokens[ $prev ]['code'] === \T_DOC_COMMENT_STAR ) { $fix = $phpcsFile->addFixableError( 'The `codeCoverageIgnore` annotation in the function docblock needs to be prefixed with an `@`.', $hasTagAsString, diff --git a/Yoast/Sniffs/Commenting/CoversTagSniff.php b/Yoast/Sniffs/Commenting/CoversTagSniff.php index dfdff6c0..5f6e8bc3 100644 --- a/Yoast/Sniffs/Commenting/CoversTagSniff.php +++ b/Yoast/Sniffs/Commenting/CoversTagSniff.php @@ -4,6 +4,7 @@ use PHP_CodeSniffer\Files\File; use PHP_CodeSniffer\Sniffs\Sniff; +use PHPCSUtils\Utils\GetTokensAsString; /** * Verifies that a @covers tag annotation follows a format supported by PHPUnit. @@ -12,21 +13,39 @@ * - each @covers tag has an annotation; * - there are no duplicate @covers tags; * - there are no duplicate @coversNothing tags; - * - a method does not have both a @covers as well as a @coversNothing tag. + * - a method does not have both a @covers as well as a @coversNothing tag; + * - deprecated @covers tag formats are flagged. (since 3.0.0) * - * @package Yoast\YoastCS - * @author Juliette Reinders Folmer - * - * @since 1.3.0 + * @since 1.3.0 */ -class CoversTagSniff implements Sniff { +final class CoversTagSniff implements Sniff { /** * Regex to check for valid content of a @covers tags. * * @var string */ - const VALID_CONTENT_REGEX = '(?:\\\\?(?:(?[a-zA-Z_\x80-\xff][a-zA-Z0-9_\x80-\xff]*)\\\\)*(?P>OOName)(?:|::<[!]?(?:public|protected|private)>|::(?(?!public$|protected$|private$)(?P>OOName)))?|::(?P>functionName)|\\\\?(?:(?P>OOName)\\\\)+(?P>functionName))'; + private const VALID_CONTENT_REGEX = '(?:\\\\?(?:(?[a-zA-Z_\x80-\xff][a-zA-Z0-9_\x80-\xff]*)\\\\)*(?P>OOName)(?:|::<[!]?(?:public|protected|private)>|::(?(?!public$|protected$|private$)(?P>OOName)))?|::(?P>functionName)|::<[!]?(?:public|protected|private)>|\\\\?(?:(?P>OOName)\\\\)+(?P>functionName))'; + + /** + * Regex to check for deprecated `@covers ClassName` tag format. + * + * @link https://github.com/sebastianbergmann/phpunit/issues/3630 PHPUnit 9.0 deprecation. + * @link https://github.com/sebastianbergmann/phpunit/issues/3631 PHPUnit 10.0 removal. + * + * @var string + */ + private const DEPRECATED_FORMAT_EXTENDED = '`^(?:\\\\)?(?:(?[a-zA-Z_\x80-\xff][a-zA-Z0-9_\x80-\xff]*)\\\\)*(?P>OOName)$`'; + + /** + * Regex to check for deprecated `@covers *::<[!]public|protected|private>` tag format. + * + * @link https://github.com/sebastianbergmann/phpunit/issues/3630 PHPUnit 9.0 deprecation. + * @link https://github.com/sebastianbergmann/phpunit/issues/3631 PHPUnit 10.0 removal. + * + * @var string + */ + private const DEPRECATED_FORMAT_VISIBILITY = '`::<[!]?(?:public|protected|private)>$`'; /** * Base error message. @@ -35,12 +54,12 @@ class CoversTagSniff implements Sniff { * * @var string */ - const ERROR_MSG = 'Invalid @covers annotation found.'; + private const ERROR_MSG = 'Invalid @covers annotation found.'; /** * Returns an array of tokens this test wants to listen for. * - * @return (int|string)[] + * @return array */ public function register() { return [ @@ -59,6 +78,9 @@ public function register() { public function process( File $phpcsFile, $stackPtr ) { $tokens = $phpcsFile->getTokens(); + /* + * Find all relevant tags and check for common mistakes in the tag format. + */ $firstCoversTag = false; $coversTags = []; $coversNothingTags = []; @@ -78,7 +100,8 @@ public function process( File $phpcsFile, $stackPtr ) { // Found a @covers tag. $next = $phpcsFile->findNext( \T_DOC_COMMENT_WHITESPACE, ( $tag + 1 ), null, true ); - if ( $tokens[ $next ]['code'] !== \T_DOC_COMMENT_STRING + if ( $next === false // Shouldn't be possible. + || $tokens[ $next ]['code'] !== \T_DOC_COMMENT_STRING || $tokens[ $next ]['line'] !== $tokens[ $tag ]['line'] ) { $phpcsFile->addError( @@ -93,6 +116,16 @@ public function process( File $phpcsFile, $stackPtr ) { $annotation = $tokens[ $next ]['content']; $coversTags[ "$tag-$next" ] = $annotation; + // Check for deprecated/removed @covers formats. + if ( \preg_match( self::DEPRECATED_FORMAT_EXTENDED, $annotation ) === 1 + || \preg_match( self::DEPRECATED_FORMAT_VISIBILITY, $annotation ) === 1 + ) { + $warning = 'Use of the "ClassName<*>" type values for @covers annotations has been deprecated in PHPUnit 9.0'; + $warning .= ' and support has been removed in PHPUnit 10.0. Found: %s'; + $data = [ $annotation ]; + $phpcsFile->addWarning( $warning, $next, 'RemovedFormat', $data ); + } + if ( \preg_match( '`^' . self::VALID_CONTENT_REGEX . '$`', $annotation ) === 1 ) { continue; } @@ -148,6 +181,9 @@ public function process( File $phpcsFile, $stackPtr ) { $phpcsFile->addError( $error, $next, 'Invalid', $data ); } + /* + * Check that a docblock doesn't contain both `@covers` tags as well as `@coversNothing` tag(s). + */ $coversNothingCount = \count( $coversNothingTags ); if ( $firstCoversTag !== false && $coversNothingCount > 0 ) { $error = 'A test can\'t both cover something as well as cover nothing. First @coversNothing tag encountered on line %d; first @covers tag encountered on line %d'; @@ -159,6 +195,9 @@ public function process( File $phpcsFile, $stackPtr ) { $phpcsFile->addError( $error, $tokens[ $stackPtr ]['comment_closer'], 'Contradictory', $data ); } + /* + * Check for duplicate `@coversNothing` tags. + */ if ( $coversNothingCount > 1 ) { $error = 'Only one @coversNothing tag allowed per test'; $code = 'DuplicateCoversNothing'; @@ -179,7 +218,6 @@ public function process( File $phpcsFile, $stackPtr ) { $phpcsFile->addError( $error, $tokens[ $stackPtr ]['comment_closer'], $code ); } else { - $fix = $phpcsFile->addFixableError( $error, $tokens[ $stackPtr ]['comment_closer'], $code ); if ( $fix === true ) { $skipFirst = ( $coversNothingCount === $removalCount ); @@ -207,6 +245,9 @@ public function process( File $phpcsFile, $stackPtr ) { } } + /* + * Check for duplicate `@covers ...` tags. + */ $coversCount = \count( $coversTags ); if ( $coversCount > 1 ) { $unique = \array_unique( $coversTags ); @@ -220,6 +261,7 @@ public function process( File $phpcsFile, $stackPtr ) { } $first = null; + $data = []; foreach ( $coversTags as $ptrs => $annot ) { if ( $annotation !== $annot ) { continue; @@ -233,13 +275,12 @@ public function process( File $phpcsFile, $stackPtr ) { $ptrs = \explode( '-', $ptrs ); - $fix = $phpcsFile->addFixableError( $error, $ptrs[0], $code, $data ); + $fix = $phpcsFile->addFixableError( $error, (int) $ptrs[0], $code, $data ); if ( $fix === true ) { - $phpcsFile->fixer->beginChangeset(); // Remove the whole line. - for ( $i = ( $ptrs[1] ); $i >= 0; $i-- ) { + for ( $i = (int) $ptrs[1]; $i >= 0; $i-- ) { if ( $tokens[ $i ]['line'] !== $tokens[ $ptrs[1] ]['line'] ) { if ( $tokens[ $i ]['code'] === \T_DOC_COMMENT_WHITESPACE && $tokens[ $i ]['content'] === $phpcsFile->eolChar @@ -271,7 +312,7 @@ public function process( File $phpcsFile, $stackPtr ) { * * @return bool Whether an error has been thrown or not. */ - protected function fixSimpleError( File $phpcsFile, $stackPtr, $expected, $errorCode ) { + private function fixSimpleError( File $phpcsFile, $stackPtr, $expected, $errorCode ) { $tokens = $phpcsFile->getTokens(); $annotation = $tokens[ $stackPtr ]['content']; @@ -305,7 +346,7 @@ protected function fixSimpleError( File $phpcsFile, $stackPtr, $expected, $error * * @return bool Whether to skip the rest of the annotation examination or not. */ - protected function fixAnnotationToSplit( File $phpcsFile, $stackPtr, $errorCode, $separator ) { + private function fixAnnotationToSplit( File $phpcsFile, $stackPtr, $errorCode, $separator ) { $fix = $phpcsFile->addFixableError( 'Each @covers annotation should reference only one covered structure', $stackPtr, @@ -330,7 +371,7 @@ protected function fixAnnotationToSplit( File $phpcsFile, $stackPtr, $errorCode, $phpcsFile->fixer->replaceToken( $i, '' ); } - $stub = $phpcsFile->getTokensAsString( $i, ( $stackPtr - $i ), true ); + $stub = GetTokensAsString::origContent( $phpcsFile, $i, ( $stackPtr - 1 ) ); $replacement = ''; foreach ( $annotations as $annotation ) { $replacement .= $stub . $annotation; diff --git a/Yoast/Sniffs/Commenting/FileCommentSniff.php b/Yoast/Sniffs/Commenting/FileCommentSniff.php index fc0607d0..b7f42de8 100644 --- a/Yoast/Sniffs/Commenting/FileCommentSniff.php +++ b/Yoast/Sniffs/Commenting/FileCommentSniff.php @@ -7,7 +7,10 @@ use PHP_CodeSniffer\Util\Tokens; /** - * Namespaced files do not need a file docblock in YoastCS. + * Namespaced files containing an OO structure do not need a file docblock in YoastCS. + * + * For namespaced files NOT containing an OO structure, a file docblock is permitted, + * though not required. * * Note: Files without a namespace declaration do still need a file docblock. * This includes files which have a non-named namespace declaration which @@ -22,12 +25,11 @@ * see if it's still relevant to have this sniff and if so, if the sniff needs * adjustments.}} * - * @package Yoast\YoastCS - * @author Juliette Reinders Folmer - * - * @since 1.2.0 + * @since 1.2.0 + * @since 3.0.0 Behaviour for files containing an OO structure vs without has been updated + * to allow a file docblock when there is no OO structure. */ -class FileCommentSniff extends Squiz_FileCommentSniff { +final class FileCommentSniff extends Squiz_FileCommentSniff { /** * Processes this test, when one of its tokens is encountered. @@ -41,6 +43,9 @@ public function process( File $phpcsFile, $stackPtr ) { $tokens = $phpcsFile->getTokens(); + /* + * Check if the file is namespaced. If not, fall through to the parent sniff. + */ $namespace_token = $stackPtr; do { $namespace_token = $phpcsFile->findNext( Tokens::$emptyTokens, ( $namespace_token + 1 ), null, true ); @@ -80,39 +85,57 @@ public function process( File $phpcsFile, $stackPtr ) { return parent::process( $phpcsFile, $stackPtr ); } + /* + * As of here, we know we are in a namespaced file. + */ + $comment_start = $phpcsFile->findNext( \T_WHITESPACE, ( $stackPtr + 1 ), $namespace_token, true ); - if ( $comment_start !== false && $tokens[ $comment_start ]['code'] === \T_DOC_COMMENT_OPEN_TAG ) { - - // Respect phpcs:disable comments in the file docblock. - $ignore = false; - if ( $phpcsFile->config->annotations === true && isset( $tokens[ $comment_start ]['comment_closer'] ) ) { - for ( $i = ( $comment_start + 1 ); $i < $tokens[ $comment_start ]['comment_closer']; $i++ ) { - if ( $tokens[ $i ]['code'] !== \T_PHPCS_DISABLE ) { - continue; - } - - if ( empty( $tokens[ $i ]['sniffCodes'] ) === true - || isset( $tokens[ $i ]['sniffCodes']['Yoast'] ) === true - || isset( $tokens[ $i ]['sniffCodes']['Yoast.Commenting'] ) === true - || isset( $tokens[ $i ]['sniffCodes']['Yoast.Commenting.FileComment'] ) === true - || isset( $tokens[ $i ]['sniffCodes']['Yoast.Commenting.FileComment.Unnecessary'] ) === true - ) { - $ignore = true; - break; - } + if ( $comment_start === false || $tokens[ $comment_start ]['code'] !== \T_DOC_COMMENT_OPEN_TAG ) { + // No file comment found, we're good. + return ( $phpcsFile->numTokens + 1 ); + } + + // Respect phpcs:disable comments in the file docblock. + if ( $phpcsFile->config->annotations === true && isset( $tokens[ $comment_start ]['comment_closer'] ) ) { + for ( $i = ( $comment_start + 1 ); $i < $tokens[ $comment_start ]['comment_closer']; $i++ ) { + if ( $tokens[ $i ]['code'] !== \T_PHPCS_DISABLE ) { + continue; } - } - if ( $ignore === false ) { - $phpcsFile->addWarning( - 'A file containing a (named) namespace declaration does not need a file docblock', - $comment_start, - 'Unnecessary' - ); + if ( empty( $tokens[ $i ]['sniffCodes'] ) === true + || isset( $tokens[ $i ]['sniffCodes']['Yoast'] ) === true + || isset( $tokens[ $i ]['sniffCodes']['Yoast.Commenting'] ) === true + || isset( $tokens[ $i ]['sniffCodes']['Yoast.Commenting.FileComment'] ) === true + || isset( $tokens[ $i ]['sniffCodes']['Yoast.Commenting.FileComment.Unnecessary'] ) === true + ) { + // Applicable disable annotation found. + return ( $phpcsFile->numTokens + 1 ); + } } } + /* + * Okay, so we have a file docblock in a namespaced file, now check if there is a named + * OO structure declaration. + */ + $find = Tokens::$ooScopeTokens; + unset( $find[ \T_ANON_CLASS ] ); + $hasOODeclaration = $phpcsFile->findNext( $find, $next_non_empty ); + if ( $hasOODeclaration === false ) { + /* + * This is a file which doesn't contain (just) an OO declaration. + * If there is a file docblock, allow it and check it like any other file docblock. + */ + return parent::process( $phpcsFile, $stackPtr ); + } + + $phpcsFile->addWarning( + 'A file containing a (named) namespace declaration does not need a file docblock', + $comment_start, + 'Unnecessary' + ); + return ( $phpcsFile->numTokens + 1 ); } } diff --git a/Yoast/Sniffs/Commenting/TestsHaveCoversTagSniff.php b/Yoast/Sniffs/Commenting/TestsHaveCoversTagSniff.php index b3b102e8..8355a5d7 100644 --- a/Yoast/Sniffs/Commenting/TestsHaveCoversTagSniff.php +++ b/Yoast/Sniffs/Commenting/TestsHaveCoversTagSniff.php @@ -5,21 +5,22 @@ use PHP_CodeSniffer\Files\File; use PHP_CodeSniffer\Sniffs\Sniff; use PHP_CodeSniffer\Util\Tokens; +use PHPCSUtils\Tokens\Collections; +use PHPCSUtils\Utils\FunctionDeclarations; +use PHPCSUtils\Utils\ObjectDeclarations; +use PHPCSUtils\Utils\Scopes; /** * Verifies that all test functions have at least one @covers tag. * - * @package Yoast\YoastCS - * @author Juliette Reinders Folmer - * - * @since 1.3.0 + * @since 1.3.0 */ -class TestsHaveCoversTagSniff implements Sniff { +final class TestsHaveCoversTagSniff implements Sniff { /** * Returns an array of tokens this test wants to listen for. * - * @return (int|string)[] + * @return array */ public function register() { return [ @@ -46,9 +47,8 @@ public function process( File $phpcsFile, $stackPtr ) { return $this->process_class( $phpcsFile, $stackPtr ); } - if ( $tokens[ $stackPtr ]['code'] === \T_FUNCTION ) { - return $this->process_function( $phpcsFile, $stackPtr ); - } + // This must be a T_FUNCTION token. + $this->process_function( $phpcsFile, $stackPtr ); } /** @@ -60,13 +60,14 @@ public function process( File $phpcsFile, $stackPtr ) { * @return void|int If covers annotations were found (or this is not a test class), * will return the stack pointer to the end of the class. */ - protected function process_class( File $phpcsFile, $stackPtr ) { + private function process_class( File $phpcsFile, $stackPtr ) { $tokens = $phpcsFile->getTokens(); - $name = $phpcsFile->getDeclarationName( $stackPtr ); + $name = ObjectDeclarations::getName( $phpcsFile, $stackPtr ); - if ( \substr( $name, -4 ) !== 'Test' - && \substr( $name, -8 ) !== 'TestCase' - && \substr( $name, 0, 4 ) !== 'Test' + if ( empty( $name ) + || ( \substr( $name, -4 ) !== 'Test' + && \substr( $name, -8 ) !== 'TestCase' + && \substr( $name, 0, 4 ) !== 'Test' ) ) { // Not a test class. if ( isset( $tokens[ $stackPtr ]['scope_closer'] ) ) { @@ -77,12 +78,9 @@ protected function process_class( File $phpcsFile, $stackPtr ) { return; } - // @todo: Once PHPCSUtils is out, replace with call to new findCommentAboveOOStructure() method. - $ignore = [ - \T_WHITESPACE => \T_WHITESPACE, - \T_ABSTRACT => \T_ABSTRACT, - \T_FINAL => \T_FINAL, - ]; + // @todo: Once PHPCSUtils 1.2.0 (?) is out, replace with call to new findCommentAboveOOStructure() method. + $ignore = Collections::classModifierKeywords(); + $ignore[ \T_WHITESPACE ] = \T_WHITESPACE; $commentEnd = $stackPtr; for ( $commentEnd = ( $stackPtr - 1 ); $commentEnd >= 0; $commentEnd-- ) { @@ -112,10 +110,12 @@ protected function process_class( File $phpcsFile, $stackPtr ) { foreach ( $tokens[ $commentStart ]['comment_tags'] as $tag ) { if ( $tokens[ $tag ]['content'] === '@covers' ) { $foundCovers = true; + break; } if ( $tokens[ $tag ]['content'] === '@coversNothing' ) { $foundCoversNothing = true; + break; } } @@ -135,10 +135,15 @@ protected function process_class( File $phpcsFile, $stackPtr ) { * * @return void */ - protected function process_function( File $phpcsFile, $stackPtr ) { + private function process_function( File $phpcsFile, $stackPtr ) { $tokens = $phpcsFile->getTokens(); - // @todo: Once PHPCSUtils is out, replace with call to new findCommentAboveFunction() method. + if ( Scopes::isOOMethod( $phpcsFile, $stackPtr ) === false ) { + // This is a global function, not a method in a test class. + return; + } + + // @todo: Once PHPCSUtils 1.2.0 (?) is out, replace with call to new findCommentAboveFunction() method. $ignore = Tokens::$methodPrefixes; $ignore[ \T_WHITESPACE ] = \T_WHITESPACE; @@ -183,13 +188,18 @@ protected function process_function( File $phpcsFile, $stackPtr ) { } } - $name = $phpcsFile->getDeclarationName( $stackPtr ); + $name = FunctionDeclarations::getName( $phpcsFile, $stackPtr ); + if ( empty( $name ) ) { + // Parse error. Ignore this method as it will never be run as a test. + return; + } + if ( \stripos( $name, 'test' ) !== 0 && $foundTest === false ) { // Not a test method. return; } - $method_props = $phpcsFile->getMethodProperties( $stackPtr ); + $method_props = FunctionDeclarations::getProperties( $phpcsFile, $stackPtr ); if ( $method_props['is_abstract'] === true ) { // Abstract test method, not implemented. return; diff --git a/Yoast/Sniffs/ControlStructures/IfElseDeclarationSniff.php b/Yoast/Sniffs/ControlStructures/IfElseDeclarationSniff.php deleted file mode 100644 index 78476b86..00000000 --- a/Yoast/Sniffs/ControlStructures/IfElseDeclarationSniff.php +++ /dev/null @@ -1,89 +0,0 @@ -getTokens(); - - if ( isset( $tokens[ $stackPtr ]['scope_opener'] ) ) { - $scope_open = $tokens[ $stackPtr ]['scope_opener']; - } - else { - // Deal with "else if". - $next = $phpcsFile->findNext( Tokens::$emptyTokens, ( $stackPtr + 1 ), null, true ); - if ( $tokens[ $next ]['code'] === \T_IF && isset( $tokens[ $next ]['scope_opener'] ) ) { - $scope_open = $tokens[ $next ]['scope_opener']; - } - } - - if ( ! isset( $scope_open ) || $tokens[ $scope_open ]['code'] === \T_COLON ) { - // No scope opener found or alternative syntax (not our concern). - return; - } - - $previous_scope_closer = $phpcsFile->findPrevious( \T_CLOSE_CURLY_BRACKET, ( $stackPtr - 1 ) ); - - if ( $tokens[ $previous_scope_closer ]['line'] === $tokens[ $stackPtr ]['line'] ) { - $phpcsFile->addError( - '%s statement must be on a new line', - $stackPtr, - 'NewLine', - [ \ucfirst( $tokens[ $stackPtr ]['content'] ) ] - ); - } - elseif ( $tokens[ $previous_scope_closer ]['column'] !== $tokens[ $stackPtr ]['column'] ) { - $phpcsFile->addError( - '%s statement not aligned with previous part of the control structure', - $stackPtr, - 'Alignment', - [ \ucfirst( $tokens[ $stackPtr ]['content'] ) ] - ); - } - - $previous_non_empty = $phpcsFile->findPrevious( Tokens::$emptyTokens, ( $stackPtr - 1 ), null, true ); - - if ( $previous_scope_closer !== $previous_non_empty ) { - $error = 'Nothing but whitespace and comments allowed between closing bracket and %s statement, found "%s"'; - $data = [ - $tokens[ $stackPtr ]['content'], - \trim( $phpcsFile->getTokensAsString( ( $previous_scope_closer + 1 ), ( $stackPtr - ( $previous_scope_closer + 1 ) ) ) ), - ]; - $phpcsFile->addError( $error, $stackPtr, 'StatementFound', $data ); - } - } -} diff --git a/Yoast/Sniffs/Files/FileNameSniff.php b/Yoast/Sniffs/Files/FileNameSniff.php index 8fa26823..6192c2e1 100644 --- a/Yoast/Sniffs/Files/FileNameSniff.php +++ b/Yoast/Sniffs/Files/FileNameSniff.php @@ -4,7 +4,12 @@ use PHP_CodeSniffer\Files\File; use PHP_CodeSniffer\Sniffs\Sniff; -use PHP_CodeSniffer\Util\Common; +use PHPCSUtils\Tokens\Collections; +use PHPCSUtils\Utils\ObjectDeclarations; +use PHPCSUtils\Utils\TextStrings; +use YoastCS\Yoast\Utils\PathHelper; +use YoastCS\Yoast\Utils\PathValidationHelper; +use YoastCS\Yoast\Utils\PSR4PathsTrait; /** * Ensures files comply with the Yoast file name rules. @@ -13,17 +18,33 @@ * - (WP) Filenames should be lowercase and words should be separated by dashes (not underscores). * - All class files should only contain one class (enforced by another sniff) and the file name * should reflect the class name without the plugin specific prefix. - * - All interface and trait files should only contain one interface/trait (enforced by another sniff) - * and the file name should reflect the interface/trait name without the plugin specific prefix - * and with an "-interface" or "-trait" suffix. + * - All interface, trait and enum files should only contain one interface/trait/enum (enforced by another sniff) + * and the file name should reflect the interface/trait/enum name without the plugin specific prefix + * and with an "-interface", "-trait" or "-enum" suffix. * - Files which don't contain an object structure, but do contain function declarations should * have a "-functions" suffix. * - * @package Yoast\YoastCS + * @since 0.5 + * @since 3.0.0 - The sniff will now also be enforced for files only using the PHP short open tag. + * - The sniff now also has the ability to check for PSR-4 compliant file names. * - * @since 0.5 + * @uses \YoastCS\Yoast\Utils\PSR4PathsTrait::$psr4_paths */ -class FileNameSniff implements Sniff { +final class FileNameSniff implements Sniff { + + use PSR4PathsTrait; + + /** + * Object tokens to search for in a file. + * + * @var array + */ + private const NAMED_OO_TOKENS = [ + \T_CLASS, + \T_ENUM, + \T_INTERFACE, + \T_TRAIT, + ]; /** * List of prefixes for object structures. @@ -35,7 +56,7 @@ class FileNameSniff implements Sniff { * - When several overlapping prefixes match, the longest matching prefix * will be removed. * - * @var string[] + * @var array */ public $oo_prefixes = []; @@ -55,28 +76,58 @@ class FileNameSniff implements Sniff { * from the root of the repository - , the PHPCS `--basepath` config variable * needs to be set. If it is not, a warning will be issued. * - * @var string[] + * @var array */ public $excluded_files_strict_check = []; /** - * Object tokens to search for in a file. + * Cache of previously set OO prefixes. + * + * Prevents having to do the same prefix validation over and over again. * - * @var (int|string)[] + * @var array */ - private $oo_tokens = [ - \T_CLASS, - \T_INTERFACE, - \T_TRAIT, - ]; + private $previous_oo_prefixes = []; + + /** + * Validated & cleaned up OO set prefixes. + * + * @var array + */ + private $clean_oo_prefixes = []; + + /** + * Cache of previously set list of excluded files. + * + * Prevents having to do the same file validation over and over again. + * + * @var array + */ + private $previous_excluded_files = []; + + /** + * Validated & cleaned up list of absolute paths to the excluded files. + * + * @var array Both the key and the value will be the same absolute path. + */ + private $validated_excluded_files = []; + + /** + * Track if the "missing basepath" warning has been thrown. + * + * This prevents this warning potentially being thrown for every single file in a PHPCS run. + * + * @var bool + */ + private $basepath_warning_thrown = false; /** * Returns an array of tokens this test wants to listen for. * - * @return (int|string)[] + * @return array */ public function register() { - return [ \T_OPEN_TAG ]; + return Collections::phpOpenTags(); } /** @@ -90,10 +141,35 @@ public function register() { */ public function process( File $phpcsFile, $stackPtr ) { // Stripping potential quotes to ensure `stdin_path` passed by IDEs does not include quotes. - $file = \preg_replace( '`^([\'"])(.*)\1$`Ds', '$2', $phpcsFile->getFileName() ); + $file = TextStrings::stripQuotes( $phpcsFile->getFileName() ); if ( $file === 'STDIN' ) { - return; + return ( $phpcsFile->numTokens + 1 ); // @codeCoverageIgnore + } + + // Respect phpcs:disable comments as long as they are not accompanied by an enable. + $tokens = $phpcsFile->getTokens(); + $i = -1; + while ( $i = $phpcsFile->findNext( \T_PHPCS_DISABLE, ( $i + 1 ) ) ) { + if ( empty( $tokens[ $i ]['sniffCodes'] ) + || isset( $tokens[ $i ]['sniffCodes']['Yoast'] ) + || isset( $tokens[ $i ]['sniffCodes']['Yoast.Files'] ) + || isset( $tokens[ $i ]['sniffCodes']['Yoast.Files.FileName'] ) + ) { + do { + $i = $phpcsFile->findNext( \T_PHPCS_ENABLE, ( $i + 1 ) ); + } while ( $i !== false + && ! empty( $tokens[ $i ]['sniffCodes'] ) + && ! isset( $tokens[ $i ]['sniffCodes']['Yoast'] ) + && ! isset( $tokens[ $i ]['sniffCodes']['Yoast.Files'] ) + && ! isset( $tokens[ $i ]['sniffCodes']['Yoast.Files.FileName'] ) + ); + + if ( $i === false ) { + // The entire (rest of the) file is disabled. + return ( $phpcsFile->numTokens + 1 ); + } + } } $path_info = \pathinfo( $file ); @@ -114,68 +190,76 @@ public function process( File $phpcsFile, $stackPtr ) { $extension = $path_info['extension']; } - $error = 'Filenames should be all lowercase with hyphens as word separators. Expected %s, but found %s.'; + $error = 'Filenames should be all lowercase with hyphens as word separators.'; $error_code = 'NotHyphenatedLowercase'; - $expected = \strtolower( \str_replace( '_', '-', $file_name ) ); - - if ( $this->is_file_excluded( $phpcsFile, $file ) === false ) { - $oo_structure = $phpcsFile->findNext( $this->oo_tokens, $stackPtr ); - if ( $oo_structure !== false ) { - - $tokens = $phpcsFile->getTokens(); - $name = $phpcsFile->getDeclarationName( $oo_structure ); - - $prefixes = $this->clean_custom_array_property( $this->oo_prefixes ); - if ( ! empty( $prefixes ) ) { - // Use reverse natural sorting to get the longest of overlapping prefixes first. - \rsort( $prefixes, ( \SORT_NATURAL | \SORT_FLAG_CASE ) ); - foreach ( $prefixes as $prefix ) { - if ( $name !== $prefix && \stripos( $name, $prefix ) === 0 ) { - $name = \substr( $name, \strlen( $prefix ) ); - $name = \ltrim( $name, '_-' ); - break; - } - } - } + $expected = \strtolower( \preg_replace( '`[[:punct:]]`', '-', $file_name ) ); - $expected = \strtolower( \str_replace( '_', '-', $name ) ); + if ( ! isset( $phpcsFile->config->basepath ) ) { + $this->add_missing_basepath_warning( $phpcsFile ); + } - switch ( $tokens[ $oo_structure ]['code'] ) { - case \T_CLASS: - $error = 'Class file names should be based on the class name without the plugin prefix. Expected %s, but found %s.'; - $error_code = 'InvalidClassFileName'; - break; + $oo_structure = $phpcsFile->findNext( self::NAMED_OO_TOKENS, $stackPtr ); + if ( $oo_structure !== false ) { - case \T_INTERFACE: - $error = 'Interface file names should be based on the interface name without the plugin prefix and should have "-interface" as a suffix. Expected "%s", but found "%s".'; - $error_code = 'InvalidInterfaceFileName'; + $oo_name = ObjectDeclarations::getName( $phpcsFile, $oo_structure ); - // Don't duplicate "interface" in the filename. - if ( \substr( $expected, -10 ) !== '-interface' ) { - $expected .= '-interface'; + if ( ! empty( $oo_name ) ) { + + if ( $this->is_in_psr4_path( $phpcsFile, $file ) ) { + $error = 'Directory marked as a PSR-4 path. File names should 100%% match the name of the OO structure contained in the file for PSR-4 compliance.'; + $error_code = 'InvalidPSR4FileName'; + $expected = $oo_name; + } + elseif ( $this->is_file_excluded( $phpcsFile, $file ) === false ) { + $this->validate_oo_prefixes(); + if ( ! empty( $this->clean_oo_prefixes ) ) { + foreach ( $this->clean_oo_prefixes as $prefix ) { + if ( $oo_name !== $prefix && \stripos( $oo_name, $prefix ) === 0 ) { + $oo_name = \substr( $oo_name, \strlen( $prefix ) ); + $oo_name = \ltrim( $oo_name, '_-' ); + break; + } } - break; + } - case \T_TRAIT: - $error = 'Trait file names should be based on the trait name without the plugin prefix and should have "-trait" as a suffix. Expected "%s", but found "%s".'; - $error_code = 'InvalidTraitFileName'; + $expected = \strtolower( \str_replace( '_', '-', $oo_name ) ); - // Don't duplicate "trait" in the filename. - if ( \substr( $expected, -6 ) !== '-trait' ) { - $expected .= '-trait'; - } - break; + switch ( $tokens[ $oo_structure ]['code'] ) { + case \T_CLASS: + $error = 'Class file names should be based on the class name without the plugin prefix.'; + $error_code = 'InvalidClassFileName'; + break; + + // Interfaces, traits, enums. + default: + $oo_type = \strtolower( $tokens[ $oo_structure ]['content'] ); + $oo_type_ucfirst = \ucfirst( $oo_type ); + + $error = \sprintf( + '%1$s file names should be based on the %2$s name without the plugin prefix and should have "-%2$s" as a suffix.', + $oo_type_ucfirst, + $oo_type + ); + $error_code = \sprintf( 'Invalid%sFileName', $oo_type_ucfirst ); + + // Don't duplicate "interface/trait/enum" in the filename. + $expected_suffix = '-' . $oo_type; + if ( \substr( $expected, -\strlen( $expected_suffix ) ) !== $expected_suffix ) { + $expected .= $expected_suffix; + } + break; + } } } - else { - $has_function = $phpcsFile->findNext( \T_FUNCTION, $stackPtr ); - if ( $has_function !== false && $file_name !== 'functions' ) { - $error = 'Files containing function declarations should have "-functions" as a suffix. Expected "%s", but found "%s".'; - $error_code = 'InvalidFunctionsFileName'; - - if ( \substr( $expected, -10 ) !== '-functions' ) { - $expected .= '-functions'; - } + } + elseif ( $this->is_file_excluded( $phpcsFile, $file ) === false ) { + $has_function = $phpcsFile->findNext( \T_FUNCTION, $stackPtr ); + if ( $has_function !== false && $file_name !== 'functions' ) { + $error = 'Files containing function declarations should have "-functions" as a suffix.'; + $error_code = 'InvalidFunctionsFileName'; + + if ( \substr( $expected, -10 ) !== '-functions' ) { + $expected .= '-functions'; } } } @@ -183,7 +267,7 @@ public function process( File $phpcsFile, $stackPtr ) { // Throw the error. if ( $expected !== '' && $file_name !== $expected ) { $phpcsFile->addError( - $error, + $error . ' Expected "%s", but found "%s".', 0, $error_code, [ @@ -205,35 +289,15 @@ public function process( File $phpcsFile, $stackPtr ) { * * @return bool */ - protected function is_file_excluded( File $phpcsFile, $path_to_file ) { - $exclude = $this->clean_custom_array_property( $this->excluded_files_strict_check, true, true ); - - if ( ! empty( $exclude ) ) { - $exclude = \array_map( [ $this, 'normalize_directory_separators' ], $exclude ); - $path_to_file = $this->normalize_directory_separators( $path_to_file ); - - if ( ! isset( $phpcsFile->config->basepath ) ) { - $phpcsFile->addWarning( - 'For the exclude property to work with relative file path files, the --basepath needs to be set.', - 0, - 'MissingBasePath' - ); - } - else { - $base_path = $this->normalize_directory_separators( $phpcsFile->config->basepath ); - $path_to_file = Common::stripBasepath( $path_to_file, $base_path ); - } - - // Lowercase the filename to not interfere with the lowercase/dashes rule. - $path_to_file = \strtolower( \ltrim( $path_to_file, '/' ) ); - - if ( isset( $exclude[ $path_to_file ] ) ) { - // Filename is on the exclude list. - return true; - } + private function is_file_excluded( File $phpcsFile, $path_to_file ) { + $this->validate_excluded_files( $phpcsFile ); + if ( empty( $this->validated_excluded_files ) ) { + return false; } - return false; + $path_to_file = PathHelper::normalize_absolute_path( $path_to_file ); + + return isset( $this->validated_excluded_files[ $path_to_file ] ); } /** @@ -243,36 +307,90 @@ protected function is_file_excluded( File $phpcsFile, $path_to_file ) { * - Remove whitespace surrounding values. * - Remove empty array entries. * - * Optionally flips the array to allow for using `isset` instead of `in_array`. + * @param array $property The current property value. * - * @param mixed $property The current property value. - * @param bool $flip Whether to flip the array values to keys. - * @param bool $to_lower Whether to lowercase the array values. + * @return array + */ + private function clean_custom_array_property( $property ) { + return \array_filter( \array_map( 'trim', $property ) ); + } + + /** + * Validate and sort the OO prefixes passed from a custom ruleset. * - * @return (string|bool)[] + * This will only need to be done once in a normal PHPCS run, though for + * tests the function may be called multiple times. + * + * @return void */ - protected function clean_custom_array_property( $property, $flip = false, $to_lower = false ) { - $property = \array_filter( \array_map( 'trim', $property ) ); + private function validate_oo_prefixes() { + if ( $this->previous_oo_prefixes === $this->oo_prefixes ) { + return; + } + + // Set the cache *before* validation so as to not break the above compare. + $this->previous_oo_prefixes = $this->oo_prefixes; + + $this->clean_oo_prefixes = $this->clean_custom_array_property( $this->oo_prefixes ); + + if ( ! empty( $this->clean_oo_prefixes ) ) { + // Use reverse natural sorting to get the longest of overlapping prefixes first. + \rsort( $this->clean_oo_prefixes, ( \SORT_NATURAL | \SORT_FLAG_CASE ) ); + } + } + + /** + * Validate the list of excluded files passed from a custom ruleset. + * + * This will only need to be done once in a normal PHPCS run, though for + * tests the function may be called multiple times. + * + * @param File $phpcsFile The file being scanned. + * + * @return void + */ + private function validate_excluded_files( $phpcsFile ) { + // The basepath check needs to be done first as otherwise the previous/current comparison would be broken. + if ( ! isset( $phpcsFile->config->basepath ) ) { + // Only relevant for the tests: make sure previously set validated paths are cleared out. + $this->validated_excluded_files = []; - if ( $to_lower === true ) { - $property = \array_map( 'strtolower', $property ); + // No use continuing as we can't turn relative paths into absolute paths. + return; } - if ( $flip === true ) { - $property = \array_fill_keys( $property, false ); + if ( $this->previous_excluded_files === $this->excluded_files_strict_check ) { + return; } - return $property; + // Set the cache *before* validation so as to not break the above compare. + $this->previous_excluded_files = $this->excluded_files_strict_check; + + $absolute_paths = PathValidationHelper::relative_to_absolute( $phpcsFile, $this->excluded_files_strict_check ); + $absolute_paths = \array_unique( $absolute_paths ); + $absolute_paths = \array_values( $absolute_paths ); + + $this->validated_excluded_files = \array_combine( $absolute_paths, $absolute_paths ); } /** - * Normalize all directory separators to be a forward slash and remove prefixed slash. + * Throw a warning if the basepath is missing (and this warning hasn't been thrown before). * - * @param string $path Path to normalize. + * @param File $phpcsFile The file being scanned. * - * @return string + * @return void */ - private function normalize_directory_separators( $path ) { - return \ltrim( \strtr( $path, '\\', '/' ), '/' ); + private function add_missing_basepath_warning( File $phpcsFile ) { + if ( $this->basepath_warning_thrown === true ) { + return; + } + + $phpcsFile->addWarning( + 'For the excluded files and the psr4 paths properties to work with relative file paths, the --basepath needs to be set.', + 0, + 'MissingBasePath' + ); + + $this->basepath_warning_thrown = true; } } diff --git a/Yoast/Sniffs/Files/TestDoublesSniff.php b/Yoast/Sniffs/Files/TestDoublesSniff.php index 7b923cd5..1cd18f34 100644 --- a/Yoast/Sniffs/Files/TestDoublesSniff.php +++ b/Yoast/Sniffs/Files/TestDoublesSniff.php @@ -4,18 +4,22 @@ use PHP_CodeSniffer\Files\File; use PHP_CodeSniffer\Sniffs\Sniff; +use PHP_CodeSniffer\Util\Tokens; +use PHPCSUtils\Utils\ObjectDeclarations; +use PHPCSUtils\Utils\TextStrings; +use YoastCS\Yoast\Utils\PathHelper; +use YoastCS\Yoast\Utils\PathValidationHelper; /** - * Check that all mock/doubles classes are in their own file and in a `doubles` directory. + * Check that all mock/doubles OO structures are in a dedicated directory for test fixtures. * - * Additionally, checks that all classes in the `doubles` directory/directories - * have `Double` or `Mock` in the class name. + * Additionally, checks that all OO structures in this fixtures directory/directories + * have `Double` or `Mock` in their name. * - * @package Yoast\YoastCS - * - * @since 1.0.0 + * @since 1.0.0 + * @since 3.0.0 This sniff will no longer check for multiple OO object declarations within one file. */ -class TestDoublesSniff implements Sniff { +final class TestDoublesSniff implements Sniff { /** * Relative paths to the directories where the test doubles/mocks are allowed to be placed. @@ -23,42 +27,34 @@ class TestDoublesSniff implements Sniff { * The paths should be relative to the root/basepath of the project and can be * customized from within a custom ruleset. * - * Preferably only one path is provided per project, but in exceptional circumstances - * multiple paths can be allowed. - * - * The new PHPCS 3.4.0 array `extend` feature can be used to add to this list. - * To overrule the list, just set the property. - * {@link https://github.com/squizlabs/PHP_CodeSniffer/pull/2154} - * * @since 1.0.0 * @since 1.1.0 The property type has changed from string to array. * Use of this property with a string value has been deprecated. + * @since 3.0.0 The default value has changed to an empty array. + * This property will now always need to be set from within a ruleset. * - * @var string[] + * @var array */ - public $doubles_path = [ - '/tests/doubles', - ]; + public $doubles_path = []; /** - * Validated absolute target paths for test double/mock classes or an empty array + * Validated absolute target paths for test fixture directories or an empty array * if the intended target directory/directories don't exist. * - * @var string[] + * @var array */ - protected $target_paths; + private $target_paths; /** * Returns an array of tokens this test wants to listen for. * - * @return (int|string)[] + * @return array */ public function register() { - return [ - \T_CLASS, - \T_INTERFACE, - \T_TRAIT, - ]; + $targets = Tokens::$ooScopeTokens; + unset( $targets[ \T_ANON_CLASS ] ); + + return $targets; } /** @@ -72,10 +68,10 @@ public function register() { */ public function process( File $phpcsFile, $stackPtr ) { // Stripping potential quotes to ensure `stdin_path` passed by IDEs does not include quotes. - $file = \preg_replace( '`^([\'"])(.*)\1$`Ds', '$2', $phpcsFile->getFileName() ); + $file = TextStrings::stripQuotes( $phpcsFile->getFileName() ); if ( $file === 'STDIN' ) { - return; + return; // @codeCoverageIgnore } if ( ! isset( $phpcsFile->config->basepath ) ) { @@ -99,33 +95,13 @@ public function process( File $phpcsFile, $stackPtr ) { return ( $phpcsFile->numTokens + 1 ); } - /* - * BC-compatibility for when the property was still a string. - * - * {@internal This should be removed in YoastCS 2.0.0.}} - */ - if ( \is_string( $this->doubles_path ) ) { - $this->doubles_path = (array) $this->doubles_path; - } - - $tokens = $phpcsFile->getTokens(); - $base_path = $this->normalize_directory_separators( $phpcsFile->config->basepath ); - $base_path = \rtrim( $base_path, '/' ) . '/'; // Make sure the base_path ends in a single slash. - if ( ! isset( $this->target_paths ) || \defined( 'PHP_CODESNIFFER_IN_TESTS' ) ) { - $this->target_paths = []; - - foreach ( $this->doubles_path as $doubles_path ) { - $target_path = $base_path; - $target_path .= \trim( $this->normalize_directory_separators( $doubles_path ), '/' ) . '/'; - - if ( \file_exists( $target_path ) && \is_dir( $target_path ) ) { - $this->target_paths[] = \strtolower( $target_path ); - } - } + $this->target_paths = PathValidationHelper::relative_to_absolute( $phpcsFile, $this->doubles_path ); + $this->target_paths = \array_filter( $this->target_paths, 'file_exists' ); + $this->target_paths = \array_filter( $this->target_paths, 'is_dir' ); } - $object_name = $phpcsFile->getDeclarationName( $stackPtr ); + $object_name = ObjectDeclarations::getName( $phpcsFile, $stackPtr ); if ( empty( $object_name ) ) { return; } @@ -135,100 +111,72 @@ public function process( File $phpcsFile, $stackPtr ) { $name_contains_double_or_mock = true; } - + $tokens = $phpcsFile->getTokens(); if ( empty( $this->target_paths ) === true ) { - if ( $name_contains_double_or_mock === true ) { - // No valid target paths found. - $data = [ - $phpcsFile->config->basepath, - ]; - - if ( \count( $this->doubles_path ) === 1 ) { - $data[] = 'directory'; - $data[] = \implode( '', $this->doubles_path ); - } - else { - $all_paths = \implode( '", "', $this->doubles_path ); - $all_paths = \substr_replace( $all_paths, ' and', \strrpos( $all_paths, ',' ), 1 ); - - $data[] = 'directories'; - $data[] = $all_paths; - } - - $phpcsFile->addError( - 'Double/Mock test helper class detected, but no test doubles sub-%2$s found in "%1$s". Expected: "%3$s". Please create the sub-%2$s.', - $stackPtr, - 'NoDoublesDirectory', - $data - ); - } - } - else { - $path_to_file = $this->normalize_directory_separators( $file ); - $is_double_dir = false; - - foreach ( $this->target_paths as $target_path ) { - if ( \stripos( $path_to_file, $target_path ) !== false ) { - $is_double_dir = true; - break; - } + if ( $name_contains_double_or_mock === false ) { + return; } + // Mock/Double class found, but no valid target paths found. $data = [ $tokens[ $stackPtr ]['content'], - $object_name, + $phpcsFile->config->basepath, ]; - if ( $name_contains_double_or_mock === true && $is_double_dir === false ) { - $phpcsFile->addError( - 'Double/Mock test helper classes should be placed in a dedicated test doubles sub-directory. Found %s: %s', - $stackPtr, - 'WrongDirectory', - $data - ); + if ( \count( $this->doubles_path ) === 1 ) { + $data[] = 'directory'; + $data[] = \implode( '', $this->doubles_path ); } - elseif ( $name_contains_double_or_mock === false && $is_double_dir === true ) { - $phpcsFile->addError( - 'Double/Mock test helper classes should contain "Double" or "Mock" in the class name. Found %s: %s', - $stackPtr, - 'InvalidClassName', - $data - ); + else { + $all_paths = \implode( '", "', $this->doubles_path ); + $all_paths = \substr_replace( $all_paths, ' and', \strrpos( $all_paths, ',' ), 1 ); + + $data[] = 'directories'; + $data[] = $all_paths; } + + $phpcsFile->addError( + 'Double/Mock test helper %1$s detected, but no test fixtures sub-%3$s found in "%2$s". Expected: "%4$s". Please create the sub-%3$s.', + $stackPtr, + 'NoDoublesDirectory', + $data + ); + + return; } - if ( $name_contains_double_or_mock === true ) { - $more_objects_in_file = $phpcsFile->findNext( $this->register(), ( $stackPtr + 1 ) ); - if ( $more_objects_in_file === false ) { - $more_objects_in_file = $phpcsFile->findPrevious( $this->register(), ( $stackPtr - 1 ) ); - } + $path_to_file = PathHelper::normalize_absolute_path( $file ); + $is_double_dir = false; - if ( $more_objects_in_file !== false ) { - $data = [ - $tokens[ $stackPtr ]['content'], - $object_name, - $tokens[ $more_objects_in_file ]['content'], - $phpcsFile->getDeclarationName( $more_objects_in_file ), - ]; - - $phpcsFile->addError( - 'Double/Mock test helper classes should be in their own file. Found %1$s: %2$s and %3$s: %4$s', - $stackPtr, - 'OneObjectPerFile', - $data - ); + foreach ( $this->target_paths as $target_path ) { + if ( PathHelper::path_starts_with( $path_to_file, $target_path ) === true ) { + $is_double_dir = true; + break; } } - } - /** - * Normalize all directory separators to be a forward slash. - * - * @param string $path Path to normalize. - * - * @return string - */ - private function normalize_directory_separators( $path ) { - return \strtr( $path, '\\', '/' ); + $data = [ + $tokens[ $stackPtr ]['content'], + $object_name, + ]; + + if ( $name_contains_double_or_mock === true && $is_double_dir === false ) { + $phpcsFile->addError( + 'Double/Mock test helpers should be placed in a dedicated test fixtures sub-directory. Found %s: %s', + $stackPtr, + 'WrongDirectory', + $data + ); + return; + } + + if ( $name_contains_double_or_mock === false && $is_double_dir === true ) { + $phpcsFile->addError( + 'Double/Mock test helpers should contain "Double" or "Mock" in the class name. Found %s: %s', + $stackPtr, + 'InvalidClassName', + $data + ); + } } } diff --git a/Yoast/Sniffs/Namespaces/NamespaceDeclarationSniff.php b/Yoast/Sniffs/Namespaces/NamespaceDeclarationSniff.php deleted file mode 100644 index 1d9179a3..00000000 --- a/Yoast/Sniffs/Namespaces/NamespaceDeclarationSniff.php +++ /dev/null @@ -1,110 +0,0 @@ -getTokens(); - - $statements = []; - - while ( ( $stackPtr = $phpcsFile->findNext( \T_NAMESPACE, ( $stackPtr + 1 ) ) ) !== false ) { - - $next_non_empty = $phpcsFile->findNext( Tokens::$emptyTokens, ( $stackPtr + 1 ), null, true ); - if ( $next_non_empty === false ) { - // Live coding or parse error. - break; - } - - if ( $tokens[ $next_non_empty ]['code'] === \T_NS_SEPARATOR ) { - // Not a namespace declaration, but the use of the namespace keyword as operator. - continue; - } - - // OK, found a namespace declaration. - $statements[] = $stackPtr; - - if ( isset( $tokens[ $stackPtr ]['scope_condition'] ) - && $tokens[ $stackPtr ]['scope_condition'] === $stackPtr - ) { - // Scoped namespace declaration. - $phpcsFile->addError( - 'Scoped namespace declarations are not allowed.', - $stackPtr, - 'ScopedForbidden' - ); - } - - if ( $tokens[ $next_non_empty ]['code'] === \T_SEMICOLON - || $tokens[ $next_non_empty ]['code'] === \T_OPEN_CURLY_BRACKET - ) { - // Namespace declaration without namespace name (= global namespace). - $phpcsFile->addError( - 'Namespace declarations without a namespace name are not allowed.', - $stackPtr, - 'NoNameForbidden' - ); - } - } - - $count = \count( $statements ); - if ( $count > 1 ) { - $data = [ - $count, - $tokens[ $statements[0] ]['line'], - ]; - - for ( $i = 1; $i < $count; $i++ ) { - $phpcsFile->addError( - 'There should be only one namespace declaration per file. Found %d namespace declarations. The first declaration was found on line %d', - $statements[ $i ], - 'MultipleFound', - $data - ); - } - } - - return ( $phpcsFile->numTokens + 1 ); - } -} diff --git a/Yoast/Sniffs/NamingConventions/NamespaceNameSniff.php b/Yoast/Sniffs/NamingConventions/NamespaceNameSniff.php index cd21b3d8..293aff39 100644 --- a/Yoast/Sniffs/NamingConventions/NamespaceNameSniff.php +++ b/Yoast/Sniffs/NamingConventions/NamespaceNameSniff.php @@ -4,9 +4,13 @@ use PHP_CodeSniffer\Files\File; use PHP_CodeSniffer\Sniffs\Sniff; -use PHP_CodeSniffer\Util\Common; -use PHP_CodeSniffer\Util\Tokens; +use PHPCSUtils\Utils\Namespaces; +use PHPCSUtils\Utils\NamingConventions; +use PHPCSUtils\Utils\TextStrings; use YoastCS\Yoast\Utils\CustomPrefixesTrait; +use YoastCS\Yoast\Utils\PathHelper; +use YoastCS\Yoast\Utils\PathValidationHelper; +use YoastCS\Yoast\Utils\PSR4PathsTrait; /** * Check namespace name declarations. @@ -19,14 +23,28 @@ * as well as that the levels directly translate to the (sub-)directory a file is * placed in. * - * @package Yoast\YoastCS - * @author Juliette Reinders Folmer + * @since 2.0.0 + * @since 3.0.0 - Added new check to verify a prefix is used. + * - The sniff now also has the ability to check for PSR-4 compliant namespace names. * - * @since 2.0.0 + * @uses \YoastCS\Yoast\Utils\CustomPrefixesTrait::$prefixes + * @uses \YoastCS\Yoast\Utils\PSR4PathsTrait::$psr4_paths */ -class NamespaceNameSniff implements Sniff { +final class NamespaceNameSniff implements Sniff { use CustomPrefixesTrait; + use PSR4PathsTrait; + + /** + * Double/Mock/Fixture directories to allow for. + * + * @var array Key is the subdirectory name, value the length of that name. + */ + private const DOUBLE_DIRS = [ + '\Doubles\\' => 9, + '\Mocks\\' => 7, + '\Fixtures\\' => 10, + ]; /** * Project root(s). @@ -36,7 +54,7 @@ class NamespaceNameSniff implements Sniff { * one or more sub-directories of the project root can be indicated * as starting points for the translation. * - * @var string[] + * @var array */ public $src_directory = []; @@ -67,28 +85,28 @@ class NamespaceNameSniff implements Sniff { /** * Project roots after validation. * - * Validated src_directories will look like `src/`, i.e.: - * - have linux slashes; - * - not be prefixed with a slash; - * - have a trailing slash. + * Validated src_directories will look like "$basepath/src/", i.e.: + * - absolute paths; + * - with linux slashes; + * - and a trailing slash. * - * @var string[] + * @var array */ - private $validated_src_directory = []; + private $validated_src_directory; /** * Cache of previously set project roots. * * Prevents having to do the same validation over and over again. * - * @var string[] + * @var array */ - private $previous_src_directory = []; + private $previous_src_directory; /** * Returns an array of tokens this test wants to listen for. * - * @return (int|string)[] + * @return array */ public function register() { return [ \T_NAMESPACE ]; @@ -97,9 +115,9 @@ public function register() { /** * Filter out all prefixes which don't have namespace separators. * - * @param string[] $prefixes The unvalidated prefixes. + * @param array $prefixes The unvalidated prefixes. * - * @return string[] + * @return array */ protected function filter_prefixes( $prefixes ) { return $this->filter_allow_only_namespace_prefixes( $prefixes ); @@ -115,48 +133,49 @@ protected function filter_prefixes( $prefixes ) { */ public function process( File $phpcsFile, $stackPtr ) { - $tokens = $phpcsFile->getTokens(); - - if ( empty( $tokens[ $stackPtr ]['conditions'] ) === false ) { - // Not a namespace declaration. + $namespace_name = Namespaces::getDeclaredName( $phpcsFile, $stackPtr ); + if ( empty( $namespace_name ) ) { + // Either not a namespace declaration or global namespace. return; } - $next_non_empty = $phpcsFile->findNext( Tokens::$emptyTokens, ( $stackPtr + 1 ), null, true ); - if ( $tokens[ $next_non_empty ]['code'] === \T_NS_SEPARATOR ) { - // Not a namespace declaration. - return; + // Stripping potential quotes to ensure `stdin_path` passed by IDEs does not include quotes. + $file = TextStrings::stripQuotes( $phpcsFile->getFileName() ); + if ( $file === 'STDIN' ) { + $file = ''; // @codeCoverageIgnore + } + else { + $file = PathHelper::normalize_absolute_path( $file ); } - // Get the complete namespace name. - $namespace_name = $tokens[ $next_non_empty ]['content']; - for ( $i = ( $next_non_empty + 1 ); $i < $phpcsFile->numTokens; $i++ ) { - if ( isset( Tokens::$emptyTokens[ $tokens[ $i ]['code'] ] ) ) { - continue; - } - - if ( $tokens[ $i ]['code'] !== \T_STRING && $tokens[ $i ]['code'] !== \T_NS_SEPARATOR ) { - // Reached end of the namespace declaration. - break; - } - - $namespace_name .= $tokens[ $i ]['content']; + $valid_prefixes = []; + $psr4_info = false; + if ( $file !== '' ) { + $psr4_info = $this->get_psr4_info( $phpcsFile, $file ); } - if ( $i === $phpcsFile->numTokens ) { - // Live coding. - return; + if ( \is_array( $psr4_info ) ) { + // If a PSR4 path matched, there will only ever be one valid prefix for the matched path. + $valid_prefixes = [ $psr4_info['prefix'] . '\\' ]; } + else { + // Safeguard that the PSR-4 prefixes are always included. + // Makes sure level depth check still happens even if there is no basepath or path doesn't match PSR-4 path. + if ( empty( $this->prefixes ) && ! empty( $this->psr4_paths ) ) { + $this->prefixes = \array_keys( $this->psr4_paths ); + } - $this->validate_prefixes(); + $this->validate_prefixes(); + $valid_prefixes = $this->validated_prefixes; + } - // Strip off the plugin prefix. + // Strip off the (longest) plugin prefix. $namespace_name_no_prefix = $namespace_name; $found_prefix = ''; - if ( ! empty( $this->validated_prefixes ) ) { + if ( ! empty( $valid_prefixes ) ) { $name = $namespace_name . '\\'; // Validated prefixes always have a \ at the end. - foreach ( $this->validated_prefixes as $prefix ) { - if ( \strpos( $name . '\\', $prefix ) === 0 ) { + foreach ( $valid_prefixes as $prefix ) { + if ( \strpos( $name, $prefix ) === 0 ) { $namespace_name_no_prefix = \rtrim( \substr( $name, \strlen( $prefix ) ), '\\' ); $found_prefix = \rtrim( $prefix, '\\' ); break; @@ -165,24 +184,77 @@ public function process( File $phpcsFile, $stackPtr ) { unset( $prefix, $name ); } + // Check if a prefix is used. + if ( ! empty( $valid_prefixes ) && $found_prefix === '' ) { + $prefixes = $valid_prefixes; + + if ( $psr4_info !== false ) { + $error = 'PSR-4 namespace name for this path is required to start with the "%1$s" prefix.'; + $errorcode = 'MissingPSR4Prefix'; + } + else { + $error = 'A namespace name is required to start with one of the following prefixes: "%s"'; + $errorcode = 'MissingPrefix'; + + $prefixes = \array_merge( $prefixes, \array_keys( $this->psr4_paths ) ); + $prefixes = \array_unique( $prefixes ); + + if ( \count( $prefixes ) === 1 ) { + $error = 'A namespace name is required to start with the "%s" prefix.'; + } + else { + \natcasesort( $prefixes ); + } + } + + $data = [ \implode( '", "', $prefixes ) ]; + + $phpcsFile->addError( $error, $stackPtr, $errorcode, $data ); + } + /* * Check the namespace level depth. */ if ( $namespace_name_no_prefix !== '' ) { $namespace_for_level_check = $namespace_name_no_prefix; - // Allow for `Tests\` and `Tests\Doubles\` after the prefix. - $starts_with_tests = ( \strpos( $namespace_for_level_check, 'Tests\\' ) === 0 ); - if ( $starts_with_tests === true ) { - $namespace_for_level_check = \substr( $namespace_for_level_check, 6 ); - } + // Allow for a variation of `Tests\` and `Tests\*\Doubles\` after the prefix. + $starts_with_tests = ( \strpos( $namespace_for_level_check, 'Tests\\' ) === 0 ); + $prefix_ends_with_tests = ( \substr( $found_prefix, -6 ) === '\Tests' ); + if ( $starts_with_tests === true || $prefix_ends_with_tests === true ) { + $stripped = false; + foreach ( self::DOUBLE_DIRS as $dir => $length ) { + if ( \strpos( $namespace_for_level_check, $dir ) !== false ) { + $namespace_for_level_check = \substr( $namespace_for_level_check, ( \strpos( $namespace_for_level_check, $dir ) + $length ) ); + $stripped = true; + break; + } + } - if ( ( $starts_with_tests === true - // Allow for non-conventional test directory layout, like in YoastSEO Free. - || \strpos( $found_prefix, '\\Tests\\' ) !== false ) - && \strpos( $namespace_for_level_check, 'Doubles\\' ) === 0 - ) { - $namespace_for_level_check = \substr( $namespace_for_level_check, 8 ); + if ( $stripped === false ) { + if ( $starts_with_tests === true ) { + // No double dir found, now check/strip typical test dirs. + if ( \strpos( $namespace_for_level_check, 'Tests\WP\\' ) === 0 ) { + $namespace_for_level_check = \substr( $namespace_for_level_check, 9 ); + } + elseif ( \strpos( $namespace_for_level_check, 'Tests\Unit\\' ) === 0 ) { + $namespace_for_level_check = \substr( $namespace_for_level_check, 11 ); + } + else { + // Okay, so this only has the `Tests` prefix, just strip it. + $namespace_for_level_check = \substr( $namespace_for_level_check, 6 ); + } + } + elseif ( $prefix_ends_with_tests === true ) { + // Prefix which already includes `Tests`. + if ( \strpos( $namespace_for_level_check, 'WP\\' ) === 0 ) { + $namespace_for_level_check = \substr( $namespace_for_level_check, 3 ); + } + elseif ( \strpos( $namespace_for_level_check, 'Unit\\' ) === 0 ) { + $namespace_for_level_check = \substr( $namespace_for_level_check, 5 ); + } + } + } } $parts = \explode( '\\', $namespace_for_level_check ); @@ -221,92 +293,126 @@ public function process( File $phpcsFile, $stackPtr ) { return; } - $base_path = $this->normalize_directory_separators( $phpcsFile->config->basepath ); - - // Stripping potential quotes to ensure `stdin_path` passed by IDEs does not include quotes. - $file = \preg_replace( '`^([\'"])(.*)\1$`Ds', '$2', $phpcsFile->getFileName() ); - - if ( $file === 'STDIN' ) { - return; + if ( $file === '' ) { + // STDIN. + return; // @codeCoverageIgnore } - $directory = $this->normalize_directory_separators( \dirname( $file ) ); - $relative_directory = Common::stripBasepath( $directory, $base_path ); - if ( $relative_directory === '.' ) { - $relative_directory = ''; + $relative_directory = ''; + if ( \is_array( $psr4_info ) ) { + $relative_directory = $psr4_info['relative']; } else { - if ( $relative_directory[0] !== '/' ) { - /* - * Basepath stripping appears to work differently depending on OS. - * On Windows there still is a slash at the start, on Unix/Mac there isn't. - * Normalize to allow comparison. - */ - $relative_directory = '/' . $relative_directory; - } + $directory = PathHelper::normalize_absolute_path( \dirname( $file ) ); - // Add trailing slash to prevent matching '/sub' to '/sub-directory'. - $relative_directory .= '/'; - } + $this->validate_src_directory( $phpcsFile ); - $this->validate_src_directory(); + if ( empty( $this->validated_src_directory ) === false ) { + foreach ( $this->validated_src_directory as $absolute_src_path ) { + if ( PathHelper::path_starts_with( $directory, $absolute_src_path ) === false ) { + continue; + } - if ( empty( $this->validated_src_directory ) === false ) { - foreach ( $this->validated_src_directory as $subdirectory ) { - if ( \strpos( $relative_directory, $subdirectory ) !== 0 ) { - continue; + $relative_directory = PathHelper::strip_basepath( $directory, $absolute_src_path ); + break; } - - $relative_directory = \substr( $relative_directory, \strlen( $subdirectory ) ); - break; } } - // Now any potential src directory has been stripped, remove the slashes again. - $relative_directory = \trim( $relative_directory, '/' ); - - $namespace_name_for_translation = \str_replace( - [ '_', '\\' ], // Find. - [ '-', '/' ], // Replace with. - $namespace_name_no_prefix - ); - - if ( \strcasecmp( $relative_directory, $namespace_name_for_translation ) === 0 ) { - return; + if ( $relative_directory === '.' ) { + $relative_directory = ''; } + // Now any potential src directory has been stripped, remove surrounding slashes. + $relative_directory = \trim( $relative_directory, '/' ); + + // Directory to namespace translation. $expected = '[Plugin\Prefix]'; if ( $found_prefix !== '' ) { $expected = $found_prefix; } + // Namespace name doesn't have the correct prefix, but we do know what the prefix should be. + elseif ( \is_array( $psr4_info ) ) { + $expected = $psr4_info['prefix']; + } + elseif ( \count( $valid_prefixes ) === 1 ) { + $expected = \rtrim( $valid_prefixes[0], '\\' ); + } + + $clean = []; + $name_for_compare = ''; if ( $relative_directory !== '' ) { $levels = \explode( '/', $relative_directory ); - $levels = \array_filter( $levels ); // Remove empties. + $levels = \array_filter( $levels ); // Remove empties, just in case. + foreach ( $levels as $level ) { - $words = \explode( '-', $level ); - $words = \array_map( 'ucfirst', $words ); - $expected .= '\\' . \implode( '_', $words ); + $cleaned_level = $level; + if ( $psr4_info === false ) { + $cleaned_level = \preg_replace( '`[[:punct:]]`', '_', $cleaned_level ); + $words = \explode( '_', $cleaned_level ); + $words = \array_map( 'ucfirst', $words ); + $cleaned_level = \implode( '_', $words ); + } + + if ( NamingConventions::isValidIdentifierName( $cleaned_level ) === false ) { + $phpcsFile->addError( + 'Translating the directory name to a namespace name would not yield a valid namespace name. Rename the "%s" directory.', + 0, + 'DirectoryInvalid', + [ $level ] + ); + + // Continuing would be useless as the name would be invalid anyway. + return; + } + + $clean[] = $cleaned_level; + } + + $name_for_compare = \implode( '\\', $clean ); + } + + // Check whether the namespace name complies with the rules. + if ( $psr4_info !== false ) { + // Check for PSR-4 compliant namespace. + if ( \strcmp( $name_for_compare, $namespace_name_no_prefix ) === 0 ) { + return; } + + $error = 'Directory marked as a PSR-4 path. The namespace name should match the path exactly in a case-sensitive manner. Expected namespace name: "%s"; found: "%s"'; + $code = 'NotPSR4Compliant'; + } + else { + // Check for "old-style" namespace. + if ( \strcasecmp( $name_for_compare, $namespace_name_no_prefix ) === 0 ) { + return; + } + + $error = 'The namespace (sub)level(s) should reflect the directory path to the file. Expected: "%s"; Found: "%s"'; + $code = 'Invalid'; } - $phpcsFile->addError( - 'The namespace (sub)level(s) should reflect the directory path to the file. Expected: "%s"; Found: "%s"', - $stackPtr, - 'Invalid', - [ - $expected, - $namespace_name, - ] - ); + if ( $name_for_compare !== '' ) { + $expected .= '\\' . $name_for_compare; + } + + $data = [ + $expected, + $namespace_name, + ]; + + $phpcsFile->addError( $error, $stackPtr, $code, $data ); } /** * Validate a $src_directory property when set in a custom ruleset. * + * @param File $phpcsFile The file being scanned. + * * @return void */ - protected function validate_src_directory() { + private function validate_src_directory( File $phpcsFile ) { if ( $this->previous_src_directory === $this->src_directory ) { return; } @@ -314,54 +420,23 @@ protected function validate_src_directory() { // Set the cache *before* validation so as to not break the above compare. $this->previous_src_directory = $this->src_directory; - $src_directory = (array) $this->src_directory; - $src_directory = \array_filter( \array_map( 'trim', $src_directory ) ); - - if ( empty( $src_directory ) ) { - $this->validated_src_directory = []; - return; - } - - $validated = []; - foreach ( $src_directory as $directory ) { - if ( \strpos( $directory, '..' ) !== false ) { - // Do not allow walking up the directory hierarchy. - continue; - } + // Clear out previously validated src directories. + $this->validated_src_directory = []; - $directory = $this->normalize_directory_separators( $directory ); + // Note: the check whether a basepath is available is done in the main `process()` routine. + $base_path = PathHelper::normalize_absolute_path( $phpcsFile->config->basepath ); - if ( $directory === '.' ) { - // The basepath/root directory is the default, so ignore. - continue; - } + // Add any src directories. + $absolute_paths = PathValidationHelper::relative_to_absolute( $phpcsFile, $this->src_directory ); - if ( \strpos( $directory, './' ) === 0 ) { - $directory = \substr( $directory, 2 ); - } - - if ( $directory === '' ) { - continue; - } - - $validated[] = '/' . $directory . '/'; + // The base path is always a valid src directory. + if ( isset( $absolute_paths['.'] ) === false ) { + $absolute_paths['.'] = $base_path; } - // Use reverse natural sorting to get the longest directory first. - \rsort( $validated, ( \SORT_NATURAL | \SORT_FLAG_CASE ) ); - - // Set the validated prefixes cache. - $this->validated_src_directory = $validated; - } + $this->validated_src_directory = \array_unique( $absolute_paths ); - /** - * Normalize all directory separators to be a forward slash and remove prefixed and suffixed slashes. - * - * @param string $path Path to normalize. - * - * @return string - */ - private function normalize_directory_separators( $path ) { - return \trim( \strtr( $path, '\\', '/' ), '/' ); + // Use reverse natural sorting to get the longest directory first. + \rsort( $this->validated_src_directory, ( \SORT_NATURAL | \SORT_FLAG_CASE ) ); } } diff --git a/Yoast/Sniffs/NamingConventions/ObjectNameDepthSniff.php b/Yoast/Sniffs/NamingConventions/ObjectNameDepthSniff.php index 872ff12b..155f51db 100644 --- a/Yoast/Sniffs/NamingConventions/ObjectNameDepthSniff.php +++ b/Yoast/Sniffs/NamingConventions/ObjectNameDepthSniff.php @@ -2,17 +2,37 @@ namespace YoastCS\Yoast\Sniffs\NamingConventions; -use WordPressCS\WordPress\Sniff as WPCS_Sniff; +use PHP_CodeSniffer\Files\File; +use PHP_CodeSniffer\Sniffs\Sniff; +use PHP_CodeSniffer\Util\Tokens; +use PHPCSUtils\Tokens\Collections; +use PHPCSUtils\Utils\Namespaces; +use PHPCSUtils\Utils\ObjectDeclarations; +use WordPressCS\WordPress\Helpers\SnakeCaseHelper; /** * Check the number of words in object names declared within a namespace. * - * @package Yoast\YoastCS - * @author Juliette Reinders Folmer - * - * @since 2.0.0 + * @since 2.0.0 + * @since 3.0.0 This sniff no longer extends the WPCS abstract Sniff class. */ -class ObjectNameDepthSniff extends WPCS_Sniff { +final class ObjectNameDepthSniff implements Sniff { + + /** + * Suffixes commonly used for classes in the test suites. + * + * The key is the suffix in lowercase. The value indicates whether this suffix is + * only allowed when the class extends a known test class. + * The value is currently unused. + * + * @var array + */ + private const TEST_SUFFIXES = [ + 'test' => true, + 'testcase' => true, + 'mock' => false, + 'double' => false, + ]; /** * Maximum number of words. @@ -38,49 +58,34 @@ class ObjectNameDepthSniff extends WPCS_Sniff { */ public $recommended_max_words = 3; - /** - * Suffixes commonly used for classes in the test suites. - * - * The key is the suffix. The value indicates whether this suffix is - * only allowed when the class extends a known test class. - * - * @var bool[] - */ - private $test_suffixes = [ - 'Test' => true, - 'Mock' => false, - 'Double' => false, - ]; - /** * Returns an array of tokens this test wants to listen for. * - * @return (int|string)[] + * @return array */ public function register() { - return [ - \T_CLASS, - \T_INTERFACE, - \T_TRAIT, - ]; + $targets = Tokens::$ooScopeTokens; + unset( $targets[ \T_ANON_CLASS ] ); + + return $targets; } /** * Processes this test, when one of its tokens is encountered. * - * @param int $stackPtr The position of the current token - * in the stack passed in $tokens. + * @param 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_token( $stackPtr ) { + public function process( File $phpcsFile, $stackPtr ) { // Check whether we are in a namespace or not. - if ( $this->determine_namespace( $stackPtr ) === '' ) { + if ( Namespaces::determineNamespace( $phpcsFile, $stackPtr ) === '' ) { return; } - $object_name = $this->phpcsFile->getDeclarationName( $stackPtr ); + $object_name = ObjectDeclarations::getName( $phpcsFile, $stackPtr ); if ( empty( $object_name ) ) { return; } @@ -89,22 +94,30 @@ public function process_token( $stackPtr ) { // Handle names which are potentially in CamelCaps. if ( \strpos( $snakecase_object_name, '_' ) === false ) { - $snakecase_object_name = self::get_snake_case_name_suggestion( $snakecase_object_name ); + $snakecase_object_name = SnakeCaseHelper::get_suggestion( $snakecase_object_name ); + + // Always treat "TestCase" as one word. + if ( \substr( $snakecase_object_name, -9 ) === 'test_case' ) { + $snakecase_object_name = \substr( $snakecase_object_name, 0, -9 ) . 'testcase'; + } } $parts = \explode( '_', $snakecase_object_name ); $part_count = \count( $parts ); /* - * Allow the class name to be one part longer for confirmed test/mock/double classes. + * Allow the OO name to be one part longer for confirmed test/mock/double OO structures. */ - $last = \array_pop( $parts ); - if ( isset( $this->test_suffixes[ $last ] ) ) { - if ( $this->test_suffixes[ $last ] === true && $this->is_test_class( $stackPtr ) ) { + $tokens = $phpcsFile->getTokens(); + + $last = \strtolower( \array_pop( $parts ) ); + if ( isset( self::TEST_SUFFIXES[ $last ] ) ) { + if ( $tokens[ $stackPtr ]['code'] === \T_ENUM ) { + // Enums cannot extend, but a mock/double without direct link to the parent could be needed. --$part_count; } else { - $extends = $this->phpcsFile->findExtendedClassName( $stackPtr ); + $extends = ObjectDeclarations::findExtendedClassName( $phpcsFile, $stackPtr ); if ( \is_string( $extends ) ) { --$part_count; } @@ -112,50 +125,47 @@ public function process_token( $stackPtr ) { } if ( $part_count <= $this->recommended_max_words && $part_count <= $this->max_words ) { - $this->phpcsFile->recordMetric( $stackPtr, 'Nr of words in object name', $part_count ); + $phpcsFile->recordMetric( $stackPtr, 'Nr of words in object name', $part_count ); return; } - // Check if the class is deprecated. - $ignore = [ - \T_ABSTRACT => \T_ABSTRACT, - \T_FINAL => \T_FINAL, - \T_WHITESPACE => \T_WHITESPACE, - ]; + // Check if the OO structure is deprecated. + $ignore = Collections::classModifierKeywords(); + $ignore[ \T_WHITESPACE ] = \T_WHITESPACE; $comment_end = $stackPtr; for ( $comment_end = ( $stackPtr - 1 ); $comment_end >= 0; $comment_end-- ) { - if ( isset( $ignore[ $this->tokens[ $comment_end ]['code'] ] ) === true ) { + if ( isset( $ignore[ $tokens[ $comment_end ]['code'] ] ) === true ) { continue; } - if ( $this->tokens[ $comment_end ]['code'] === \T_ATTRIBUTE_END - && isset( $this->tokens[ $comment_end ]['attribute_opener'] ) === true + if ( $tokens[ $comment_end ]['code'] === \T_ATTRIBUTE_END + && isset( $tokens[ $comment_end ]['attribute_opener'] ) === true ) { - $comment_end = $this->tokens[ $comment_end ]['attribute_opener']; + $comment_end = $tokens[ $comment_end ]['attribute_opener']; continue; } break; } - if ( $this->tokens[ $comment_end ]['code'] === \T_DOC_COMMENT_CLOSE_TAG ) { - // Only check if the class has a docblock. - $comment_start = $this->tokens[ $comment_end ]['comment_opener']; - foreach ( $this->tokens[ $comment_start ]['comment_tags'] as $tag ) { - if ( $this->tokens[ $tag ]['content'] === '@deprecated' ) { - // Deprecated class, ignore. + if ( $tokens[ $comment_end ]['code'] === \T_DOC_COMMENT_CLOSE_TAG ) { + // Only check if the OO structure has a docblock. + $comment_start = $tokens[ $comment_end ]['comment_opener']; + foreach ( $tokens[ $comment_start ]['comment_tags'] as $tag ) { + if ( $tokens[ $tag ]['content'] === '@deprecated' ) { + // Deprecated OO structure, ignore. return; } } } - $this->phpcsFile->recordMetric( $stackPtr, 'Nr of words in object name', $part_count ); + $phpcsFile->recordMetric( $stackPtr, 'Nr of words in object name', $part_count ); - // Active class. - $object_type = 'a ' . $this->tokens[ $stackPtr ]['content']; - if ( $this->tokens[ $stackPtr ]['code'] === \T_INTERFACE ) { - $object_type = 'an ' . $this->tokens[ $stackPtr ]['content']; + // Not a deprecated OO structure, this OO structure should comply with the rules. + $object_type = 'a ' . $tokens[ $stackPtr ]['content']; + if ( $tokens[ $stackPtr ]['code'] === \T_INTERFACE || $tokens[ $stackPtr ]['code'] === \T_ENUM ) { + $object_type = 'an ' . $tokens[ $stackPtr ]['content']; } if ( $part_count > $this->max_words ) { @@ -167,7 +177,7 @@ public function process_token( $stackPtr ) { $object_name, ]; - $this->phpcsFile->addError( $error, $stackPtr, 'MaxExceeded', $data ); + $phpcsFile->addError( $error, $stackPtr, 'MaxExceeded', $data ); } elseif ( $part_count > $this->recommended_max_words ) { $error = 'The name of %s should not consist of more than %d words. Words found: %d in %s'; @@ -178,7 +188,7 @@ public function process_token( $stackPtr ) { $object_name, ]; - $this->phpcsFile->addWarning( $error, $stackPtr, 'TooLong', $data ); + $phpcsFile->addWarning( $error, $stackPtr, 'TooLong', $data ); } } } diff --git a/Yoast/Sniffs/NamingConventions/ValidHookNameSniff.php b/Yoast/Sniffs/NamingConventions/ValidHookNameSniff.php index 7f1f5e77..ea0274b6 100644 --- a/Yoast/Sniffs/NamingConventions/ValidHookNameSniff.php +++ b/Yoast/Sniffs/NamingConventions/ValidHookNameSniff.php @@ -3,6 +3,8 @@ namespace YoastCS\Yoast\Sniffs\NamingConventions; use PHP_CodeSniffer\Util\Tokens; +use PHPCSUtils\Utils\TextStrings; +use WordPressCS\WordPress\Helpers\WPHookHelper; use WordPressCS\WordPress\Sniffs\NamingConventions\ValidHookNameSniff as WPCS_ValidHookNameSniff; use YoastCS\Yoast\Utils\CustomPrefixesTrait; @@ -21,18 +23,17 @@ * Check the number of words in hook names and the use of the correct prefix-type, * but only when plugin specific prefixes have been passed. * - * {@internal For now allows for an array of both old-style as well as new-style + * {@internal For now, allows for an array of both old-style as well as new-style * prefixes during the transition period. * Once all plugins have been transitioned over to use the new-style * namespace-like prefix for hooks, the `WrongPrefix` warning should be * changed to an error and only namespace-like prefixes should be allowed.} * - * @package Yoast\YoastCS - * @author Juliette Reinders Folmer + * @since 2.0.0 * - * @since 2.0.0 + * @uses \YoastCS\Yoast\Utils\CustomPrefixesTrait::$prefixes */ -class ValidHookNameSniff extends WPCS_ValidHookNameSniff { +final class ValidHookNameSniff extends WPCS_ValidHookNameSniff { use CustomPrefixesTrait; @@ -71,7 +72,7 @@ class ValidHookNameSniff extends WPCS_ValidHookNameSniff { * Keep track of the content of first text string which was passed to the `transform()` * method as it may be repeatedly called for the same token. * - * @var bool + * @var string */ private $first_string = ''; @@ -89,14 +90,21 @@ class ValidHookNameSniff extends WPCS_ValidHookNameSniff { /** * 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. - * @param array $parameters Array with information about the parameters. + * @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_parameters( $stackPtr, $group_name, $matched_content, $parameters ) { + $hook_name_param = WPHookHelper::get_hook_name_param( $matched_content, $parameters ); + if ( $hook_name_param === false ) { + // If we can't find the hook name parameter, there's nothing to do, so bow out. + return; + } + /* * The custom prefix should be in the first text passed to `transform()` for each * matched function call. @@ -113,13 +121,12 @@ public function process_parameters( $stackPtr, $group_name, $matched_content, $p * If any prefixes were passed, check if this is a hook belonging to the plugin being checked. */ if ( empty( $this->validated_prefixes ) === false ) { - $param = $parameters[1]; - $first_non_empty = $this->phpcsFile->findNext( Tokens::$emptyTokens, $param['start'], ( $param['end'] + 1 ), true ); + $first_non_empty = $this->phpcsFile->findNext( Tokens::$emptyTokens, $hook_name_param['start'], ( $hook_name_param['end'] + 1 ), true ); $found_prefix = ''; if ( isset( Tokens::$stringTokens[ $this->tokens[ $first_non_empty ]['code'] ] ) ) { $this->prefix_quote_style = $this->tokens[ $first_non_empty ]['content'][0]; - $content = \trim( $this->strip_quotes( $this->tokens[ $first_non_empty ]['content'] ) ); + $content = \trim( TextStrings::stripQuotes( $this->tokens[ $first_non_empty ]['content'] ) ); foreach ( $this->validated_prefixes as $prefix ) { if ( \strpos( $prefix, '\\' ) === false @@ -157,7 +164,7 @@ public function process_parameters( $stackPtr, $group_name, $matched_content, $p } // Do the YoastCS specific hook name length and prefix check. - $this->verify_yoast_hook_name( $stackPtr, $parameters ); + $this->verify_yoast_hook_name( $stackPtr, $hook_name_param ); } /** @@ -167,47 +174,50 @@ public function process_parameters( $stackPtr, $group_name, $matched_content, $p * plugin prefix for hook names and remembers whether a prefix was found to allow * checking whether it was the correct one. * - * @param string $string The target string. + * @param string $text_string The target string. * @param string $regex The punctuation regular expression to use. * @param string $transform_type Whether to do a partial or complete transform. * Valid values are: 'full', 'case', 'punctuation'. * @return string */ - protected function transform( $string, $regex, $transform_type = 'full' ) { + protected function transform( $text_string, $regex, $transform_type = 'full' ) { if ( empty( $this->validated_prefixes ) ) { - return parent::transform( $string, $regex, $transform_type ); + return parent::transform( $text_string, $regex, $transform_type ); } if ( $this->first_string === '' ) { - $this->first_string = $string; + $this->first_string = $text_string; } // Not the first text string. - if ( $string !== $this->first_string ) { - return parent::transform( $string, $regex, $transform_type ); + if ( $text_string !== $this->first_string ) { + return parent::transform( $text_string, $regex, $transform_type ); } // Repeated call for the first text string. if ( $this->found_prefix !== '' ) { - $string = \substr( $string, \strlen( $this->found_prefix ) ); + $text_string = \substr( $text_string, \strlen( $this->found_prefix ) ); } - return $this->found_prefix . parent::transform( $string, $regex, $transform_type ); + return $this->found_prefix . parent::transform( $text_string, $regex, $transform_type ); } /** * Additional YoastCS specific hook name checks. * - * @param int $stackPtr The position of the current token in the stack. - * @param array $parameters Array with information about the parameters. + * @param int $stackPtr The position of the current token in the stack. + * @param array $hook_name_param Array with information about the hook name parameter. * * @return void */ - public function verify_yoast_hook_name( $stackPtr, $parameters ) { + private function verify_yoast_hook_name( $stackPtr, $hook_name_param ) { - $param = $parameters[1]; - $first_non_empty = $this->phpcsFile->findNext( Tokens::$emptyTokens, $param['start'], ( $param['end'] + 1 ), true ); + $first_non_empty = $this->phpcsFile->findNext( Tokens::$emptyTokens, $hook_name_param['start'], ( $hook_name_param['end'] + 1 ), true ); + if ( $first_non_empty === false ) { + // Shouldn't be possible as we've checked this before. + return; // @codeCoverageIgnore + } /* * Check that the namespace-like prefix is used for hooks. @@ -270,7 +280,7 @@ public function verify_yoast_hook_name( $stackPtr, $parameters ) { $allow = [ \T_CONSTANT_ENCAPSED_STRING ]; $allow += Tokens::$emptyTokens; - $has_non_string = $this->phpcsFile->findNext( $allow, $param['start'], ( $param['end'] + 1 ), true ); + $has_non_string = $this->phpcsFile->findNext( $allow, $hook_name_param['start'], ( $hook_name_param['end'] + 1 ), true ); if ( $has_non_string !== false ) { /* * Double quoted string or a hook name concatenated together, checking the word count for the @@ -281,10 +291,13 @@ public function verify_yoast_hook_name( $stackPtr, $parameters ) { * be thrown when PHPCS is explicitly requested to check with a lower severity. */ $this->phpcsFile->addWarning( - 'Hook name could not reliably be examined for maximum word count. Please verify this hook name manually. Found: %s', + 'Hook name could not reliably be examined for maximum word count (max is %d words). Please verify this hook name manually. Found: %s', $first_non_empty, 'NonString', - [ $param['raw'] ], + [ + $this->max_words, + $hook_name_param['raw'], + ], 3 ); @@ -297,7 +310,7 @@ public function verify_yoast_hook_name( $stackPtr, $parameters ) { * Check the hook name depth. */ $hook_ptr = $first_non_empty; // If no other tokens were found, the first non empty will be the hook name. - $hook_name = $this->strip_quotes( $this->tokens[ $hook_ptr ]['content'] ); + $hook_name = TextStrings::stripQuotes( $this->tokens[ $hook_ptr ]['content'] ); $hook_name = \substr( $hook_name, \strlen( $this->found_prefix ) ); $parts = \explode( '_', $hook_name ); @@ -305,10 +318,6 @@ public function verify_yoast_hook_name( $stackPtr, $parameters ) { $this->phpcsFile->recordMetric( $stackPtr, 'Nr of words in hook name', $part_count ); - if ( $part_count <= $this->recommended_max_words && $part_count <= $this->max_words ) { - return; - } - if ( $part_count > $this->max_words ) { $error = 'A hook name is not allowed to consist of more than %d words after the plugin prefix. Words found: %d in %s'; $data = [ diff --git a/Yoast/Sniffs/Tools/BrainMonkeyRaceConditionSniff.php b/Yoast/Sniffs/Tools/BrainMonkeyRaceConditionSniff.php index 75cc8ff5..f72a1b83 100644 --- a/Yoast/Sniffs/Tools/BrainMonkeyRaceConditionSniff.php +++ b/Yoast/Sniffs/Tools/BrainMonkeyRaceConditionSniff.php @@ -2,8 +2,15 @@ namespace YoastCS\Yoast\Sniffs\Tools; +use PHP_CodeSniffer\Files\File; +use PHP_CodeSniffer\Sniffs\Sniff; use PHP_CodeSniffer\Util\Tokens; -use WordPressCS\WordPress\Sniff; +use PHPCSUtils\Tokens\Collections; +use PHPCSUtils\Utils\Conditions; +use PHPCSUtils\Utils\FunctionDeclarations; +use PHPCSUtils\Utils\PassedParameters; +use PHPCSUtils\Utils\Scopes; +use PHPCSUtils\Utils\TextStrings; /** * Sniff to detect a particular nasty race condition which can occur in tests using the BrainMonkey utilities. @@ -11,103 +18,82 @@ * @link https://github.com/Yoast/yoastcs/issues/264 * * @since 2.3.0 - * - * @package Yoast\YoastCS - * @author Juliette Reinders Folmer + * @since 3.0.0 This sniff no longer extends the WPCS abstract Sniff class. */ -class BrainMonkeyRaceConditionSniff extends Sniff { +final class BrainMonkeyRaceConditionSniff implements Sniff { /** * Returns an array of tokens this test wants to listen for. * - * @return array + * @return array */ public function register() { return [ \T_STRING ]; } /** - * Processes a sniff when one of its tokens is encountered. + * Processes this test, when one of its tokens is encountered. * - * @param int $stackPtr The position of the current token in the stack. + * @param 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_token( $stackPtr ) { - if ( \strtolower( $this->tokens[ $stackPtr ]['content'] ) !== 'expect' ) { + public function process( File $phpcsFile, $stackPtr ) { + $tokens = $phpcsFile->getTokens(); + + if ( \strtolower( $tokens[ $stackPtr ]['content'] ) !== 'expect' ) { return; } - $nextNonEmpty = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $stackPtr + 1 ), null, true ); - if ( $nextNonEmpty === false || $this->tokens[ $nextNonEmpty ]['code'] !== \T_OPEN_PARENTHESIS ) { + $nextNonEmpty = $phpcsFile->findNext( Tokens::$emptyTokens, ( $stackPtr + 1 ), null, true ); + if ( $nextNonEmpty === false || $tokens[ $nextNonEmpty ]['code'] !== \T_OPEN_PARENTHESIS ) { // Definitely not a function call. return; } - $prevNonEmpty = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $stackPtr - 1 ), null, true ); + $prevNonEmpty = $phpcsFile->findPrevious( Tokens::$emptyTokens, ( $stackPtr - 1 ), null, true ); if ( $prevNonEmpty === false - || $this->tokens[ $prevNonEmpty ]['code'] === \T_DOUBLE_COLON - || $this->tokens[ $prevNonEmpty ]['code'] === \T_OBJECT_OPERATOR - || $this->tokens[ $prevNonEmpty ]['type'] === 'T_NULLSAFE_OBJECT_OPERATOR' - || $this->tokens[ $prevNonEmpty ]['code'] === \T_FUNCTION + || isset( Collections::objectOperators()[ $tokens[ $prevNonEmpty ]['code'] ] ) + || $tokens[ $prevNonEmpty ]['code'] === \T_FUNCTION ) { // Method call or function declaration, not a function call. return; } - // Make sure the token is in a class method. - if ( empty( $this->tokens[ $stackPtr ]['conditions'] ) ) { + $functionToken = Conditions::getLastCondition( $phpcsFile, $stackPtr, [ \T_FUNCTION ] ); + if ( $functionToken === false ) { return; } - $conditions = $this->tokens[ $stackPtr ]['conditions']; - $conditions = \array_reverse( $conditions, true ); - - $seenOO = false; - $seenFn = false; - $functionToken = false; - - foreach ( $conditions as $token => $condition ) { - if ( $seenFn === true ) { - // First condition after the function condition MUST be OO, otherwise it's not a method. - $seenOO = isset( Tokens::$ooScopeTokens[ $condition ] ); - break; - } - - if ( $condition === \T_FUNCTION ) { - $functionToken = $token; - $seenFn = true; - } - } - - if ( $seenFn === false || $seenOO === false || $functionToken === false ) { + if ( Scopes::isOOMethod( $phpcsFile, $functionToken ) === false ) { return; } // Check that this is an expect() for one of the hook functions. - $params = $this->get_function_call_parameters( $stackPtr ); - if ( empty( $params ) ) { + $param = PassedParameters::getParameter( $phpcsFile, $stackPtr, 1, 'function_name' ); + if ( empty( $param ) ) { return; } $expected = Tokens::$emptyTokens; $expected[] = \T_CONSTANT_ENCAPSED_STRING; - $hasUnexpected = $this->phpcsFile->findNext( $expected, $params[1]['start'], ( $params[1]['end'] + 1 ), true ); + $hasUnexpected = $phpcsFile->findNext( $expected, $param['start'], ( $param['end'] + 1 ), true ); if ( $hasUnexpected !== false ) { return; } - $text = $this->phpcsFile->findNext( Tokens::$emptyTokens, $params[1]['start'], ( $params[1]['end'] + 1 ), true ); - $textContent = $this->strip_quotes( $this->tokens[ $text ]['content'] ); + $text = $phpcsFile->findNext( Tokens::$emptyTokens, $param['start'], ( $param['end'] + 1 ), true ); + $textContent = TextStrings::stripQuotes( $tokens[ $text ]['content'] ); if ( $textContent !== 'apply_filters' && $textContent !== 'do_action' ) { return; } // Now walk the contents of the function declaration to see if we can find the other function call. - if ( isset( $this->tokens[ $functionToken ]['scope_opener'], $this->tokens[ $functionToken ]['scope_closer'] ) === false ) { - // We don't know the start or end of the function. - return; + if ( isset( $tokens[ $functionToken ]['scope_opener'], $tokens[ $functionToken ]['scope_closer'] ) === false ) { + // We don't know the start or end of the function. Edge case which can't happen under normal circumstances. + return; // @codeCoverageIgnore } $targetContent = 'expectdone'; @@ -115,33 +101,33 @@ public function process_token( $stackPtr ) { $targetContent = 'expectapplied'; } - $start = $this->tokens[ $functionToken ]['scope_opener']; - $end = $this->tokens[ $functionToken ]['scope_closer']; + $start = $tokens[ $functionToken ]['scope_opener']; + $end = $tokens[ $functionToken ]['scope_closer']; for ( $i = $start; $i < $end; $i++ ) { - if ( $this->tokens[ $i ]['code'] !== \T_STRING ) { + if ( $tokens[ $i ]['code'] !== \T_STRING ) { continue; } - if ( \strtolower( $this->tokens[ $i ]['content'] ) !== $targetContent ) { + if ( \strtolower( $tokens[ $i ]['content'] ) !== $targetContent ) { continue; } // Make sure it is a function call. - $next = $this->phpcsFile->findNext( Tokens::$emptyTokens, ( $i + 1 ), null, true ); - if ( $next === false || $this->tokens[ $next ]['code'] !== \T_OPEN_PARENTHESIS ) { + $next = $phpcsFile->findNext( Tokens::$emptyTokens, ( $i + 1 ), null, true ); + if ( $next === false || $tokens[ $next ]['code'] !== \T_OPEN_PARENTHESIS ) { continue; } // Okay, we have found the race condition. Throw error. $message = 'The %s() test method contains both a call to Monkey\Functions\expect( %s ), as well as a call to %s(). This causes a race condition which will cause the tests to fail. Only use one of these in a test.'; $data = [ - $this->phpcsFile->getDeclarationName( $functionToken ), - $this->tokens[ $text ]['content'], + FunctionDeclarations::getName( $phpcsFile, $functionToken ), + $tokens[ $text ]['content'], ( $targetContent === 'expectdone' ) ? 'Monkey\Actions\expectDone' : 'Monkey\Filters\expectApplied', ]; - $this->phpcsFile->addError( $message, $functionToken, 'Found', $data ); + $phpcsFile->addError( $message, $functionToken, 'Found', $data ); break; } } diff --git a/Yoast/Sniffs/WhiteSpace/FunctionSpacingSniff.php b/Yoast/Sniffs/WhiteSpace/FunctionSpacingSniff.php index ecae05cc..e594e4e9 100644 --- a/Yoast/Sniffs/WhiteSpace/FunctionSpacingSniff.php +++ b/Yoast/Sniffs/WhiteSpace/FunctionSpacingSniff.php @@ -5,6 +5,7 @@ use PHP_CodeSniffer\Files\File; use PHP_CodeSniffer\Standards\Squiz\Sniffs\WhiteSpace\FunctionSpacingSniff as Squiz_FunctionSpacingSniff; use PHP_CodeSniffer\Util\Tokens; +use PHPCSUtils\Utils\Conditions; /** * Verifies the space between methods. @@ -13,12 +14,9 @@ * in the global namespace as those are often wrapped in an if clause which causes * a fixer conflict. * - * @package Yoast\YoastCS - * @author Juliette Reinders Folmer - * - * @since 1.0.0 + * @since 1.0.0 */ -class FunctionSpacingSniff extends Squiz_FunctionSpacingSniff { +final class FunctionSpacingSniff extends Squiz_FunctionSpacingSniff { /** * The number of blank lines between functions. @@ -53,14 +51,14 @@ class FunctionSpacingSniff extends Squiz_FunctionSpacingSniff { * @param File $phpcsFile The file being scanned. * @param int $stackPtr The position of the current token in the stack passed in $tokens. * - * @return void|int Optionally returns stack pointer to skip to. + * @return void */ public function process( File $phpcsFile, $stackPtr ) { - // Check that the function is nested in an OO structure (class, trait, interface). - if ( $phpcsFile->hasCondition( $stackPtr, Tokens::$ooScopeTokens ) === false ) { + // Check that the function is nested in an OO structure (class, trait, interface, enum). + if ( Conditions::hasCondition( $phpcsFile, $stackPtr, Tokens::$ooScopeTokens ) === false ) { return; } - return parent::process( $phpcsFile, $stackPtr ); + parent::process( $phpcsFile, $stackPtr ); } } diff --git a/Yoast/Sniffs/Yoast/AlternativeFunctionsSniff.php b/Yoast/Sniffs/Yoast/AlternativeFunctionsSniff.php deleted file mode 100644 index ae9c198b..00000000 --- a/Yoast/Sniffs/Yoast/AlternativeFunctionsSniff.php +++ /dev/null @@ -1,96 +0,0 @@ - [ - 'type' => 'error', - 'message' => 'Detected a call to %s(). Use %s() instead.', - 'functions' => [ - 'json_encode', - 'wp_json_encode', - ], - 'replacement' => 'WPSEO_Utils::format_json_encode', - ], - ]; - } - - /** - * Process a matched token. - * - * @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. - * - * @return void - */ - public function process_matched_token( $stackPtr, $group_name, $matched_content ) { - - $replacement = ''; - if ( isset( $this->groups[ $group_name ]['replacement'] ) ) { - $replacement = $this->groups[ $group_name ]['replacement']; - } - - $fixable = true; - $message = $this->groups[ $group_name ]['message']; - $is_error = ( $this->groups[ $group_name ]['type'] === 'error' ); - $error_code = $this->string_to_errorcode( $group_name . '_' . $matched_content ); - $data = [ - $matched_content, - $replacement, - ]; - - /* - * Deal with specific situations. - */ - switch ( $matched_content ) { - case 'json_encode': - case 'wp_json_encode': - /* - * The function `WPSEO_Utils:format_json_encode()` is only a valid alternative - * when only the first parameter is passed. - */ - if ( $this->get_function_call_parameter_count( $stackPtr ) !== 1 ) { - $fixable = false; - $error_code .= 'WithAdditionalParams'; - } - - break; - } - - if ( $fixable === false ) { - $this->addMessage( $message, $stackPtr, $is_error, $error_code, $data ); - return; - } - - $fix = $this->addFixableMessage( $message, $stackPtr, $is_error, $error_code, $data ); - if ( $fix === true ) { - $namespaced = $this->determine_namespace( $stackPtr ); - - if ( empty( $namespaced ) || empty( $replacement ) ) { - $this->phpcsFile->fixer->replaceToken( $stackPtr, $replacement ); - } - else { - $this->phpcsFile->fixer->replaceToken( $stackPtr, '\\' . $replacement ); - } - } - } -} diff --git a/Yoast/Sniffs/Yoast/JsonEncodeAlternativeSniff.php b/Yoast/Sniffs/Yoast/JsonEncodeAlternativeSniff.php new file mode 100644 index 00000000..05aa48fe --- /dev/null +++ b/Yoast/Sniffs/Yoast/JsonEncodeAlternativeSniff.php @@ -0,0 +1,179 @@ + Function names as the keys and the name of the first declared parameter + * as the value. + * There can be multiple parameter names if the parameter + * was renamed over time. + */ + private const PARAM_INFO = [ + 'json_encode' => 'value', + + /* + * The current parameter name is `$data`, but this is expected to be changed to `$value` in WP 6.5. + * See: https://core.trac.wordpress.org/ticket/59630 + */ + 'wp_json_encode' => [ 'data', 'value' ], + ]; + + /** + * Groups of functions to restrict. + * + * @return array> + */ + public function getGroups() { + return [ + 'json_encode' => [ + 'functions' => [ + 'json_encode', + 'wp_json_encode', + ], + ], + ]; + } + + /** + * Process a matched token. + * + * @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_matched_token( $stackPtr, $group_name, $matched_content ) { + $error = 'Detected a call to %s(). Use %s() instead.'; + $error_code = 'Found'; + $data = [ + $matched_content, + self::REPLACEMENT, + ]; + + $params = PassedParameters::getParameters( $this->phpcsFile, $stackPtr ); + + /* + * If no parameters were passed, we can safely replace the function call, even though + * the function call itself, as-is, is not correct/working (but that's not the concern of + * this sniff). + */ + if ( empty( $params ) ) { + /* + * Make sure this is not a PHP 8.1+ first class callable. If it is, throw the error, but don't autofix. + */ + $ignore = Tokens::$emptyTokens; + $ignore[ \T_OPEN_PARENTHESIS ] = \T_OPEN_PARENTHESIS; + + $first_in_call = $this->phpcsFile->findNext( $ignore, ( $stackPtr + 1 ), null, true ); + if ( $first_in_call !== false && $this->tokens[ $first_in_call ]['code'] === \T_ELLIPSIS ) { + $error_code .= 'InFirstClassCallable'; + $this->phpcsFile->addError( $error, $stackPtr, $error_code, $data ); + return; + } + + $fix = $this->phpcsFile->addFixableError( $error, $stackPtr, $error_code, $data ); + if ( $fix === true ) { + $this->fix_it( $stackPtr ); + } + + return; + } + + /* + * If there are function parameters, we need to verify that only the first ($value) parameter + * was passed, taking PHP 8.0+ function calls with named parameters into account. + * + * We also need to check for parameter unpacking being used as in that case, the + * parameter count will be unreliable. + */ + $value_param = PassedParameters::getParameterFromStack( $params, 1, self::PARAM_INFO[ $matched_content ] ); + if ( \is_array( $value_param ) && \count( $params ) === 1 ) { + $first_token = $this->phpcsFile->findNext( Tokens::$emptyTokens, $value_param['start'], ( $value_param['end'] + 1 ), true ); + if ( $first_token === false || $this->tokens[ $first_token ]['code'] !== \T_ELLIPSIS ) { + /* + * Okay, so this is a function call with only the first/$value parameter passed. + * This can be safely replaced. + */ + $fix = $this->phpcsFile->addFixableError( $error, $stackPtr, $error_code, $data ); + if ( $fix === true ) { + $this->fix_it( $stackPtr, $value_param ); + } + + return; + } + } + + /* + * In all other cases, we cannot auto-fix, only flag. + */ + $error_code .= 'WithAdditionalParams'; + + $this->phpcsFile->addError( $error, $stackPtr, $error_code, $data ); + } + + /** + * Auto-fix the function call to use the replacement function. + * + * @since 3.0.0 + * + * @param int $stackPtr The position of the current token in the stack. + * @param array|false $value_param Optional. Parameter information for the first/$value + * parameter if available, or false if not. + * + * @return void + */ + private function fix_it( $stackPtr, $value_param = false ) { + $this->phpcsFile->fixer->beginChangeset(); + + // Remove potential leading namespace separator for fully qualified function call. + $prev = $this->phpcsFile->findPrevious( Tokens::$emptyTokens, ( $stackPtr - 1 ), null, true ); + if ( $prev !== false && $this->tokens[ $prev ]['code'] === \T_NS_SEPARATOR ) { + $this->phpcsFile->fixer->replaceToken( $prev, '' ); + } + + // Replace the function call with a, potentially fully qualified, call to the replacement. + $namespaced = Namespaces::determineNamespace( $this->phpcsFile, $stackPtr ); + if ( empty( $namespaced ) ) { + $this->phpcsFile->fixer->replaceToken( $stackPtr, self::REPLACEMENT ); + } + else { + $this->phpcsFile->fixer->replaceToken( $stackPtr, '\\' . self::REPLACEMENT ); + } + + if ( \is_array( $value_param ) && isset( $value_param['name_token'] ) ) { + // Update the parameter name when the function call uses named parameters. + // `$data` is the parameter name used in the WPSEO_Utils::format_json_encode() function. + $this->phpcsFile->fixer->replaceToken( $value_param['name_token'], 'data' ); + } + + $this->phpcsFile->fixer->endChangeset(); + } +} diff --git a/Yoast/Tests/Commenting/CodeCoverageIgnoreDeprecatedUnitTest.inc b/Yoast/Tests/Commenting/CodeCoverageIgnoreDeprecatedUnitTest.inc index d9165e9e..3e39aeac 100644 --- a/Yoast/Tests/Commenting/CodeCoverageIgnoreDeprecatedUnitTest.inc +++ b/Yoast/Tests/Commenting/CodeCoverageIgnoreDeprecatedUnitTest.inc @@ -1,6 +1,6 @@ => + * @return array Key is the line number, value is the number of expected errors. */ - public function getErrorList() { + public function getErrorList(): array { return [ - 41 => 1, - 50 => 1, - 55 => 1, - 69 => 1, - 90 => 1, + 41 => 1, + 50 => 1, + 55 => 1, + 69 => 1, + 90 => 1, + 98 => 1, + 105 => 1, + 113 => 1, + 120 => 1, + 127 => 1, + 154 => 1, + 158 => 1, + 165 => 1, ]; } /** * Returns the lines where warnings should occur. * - * @return array => + * @return array Key is the line number, value is the number of expected warnings. */ - public function getWarningList() { + public function getWarningList(): array { return []; } } diff --git a/Yoast/Tests/Commenting/CoversTagUnitTest.inc b/Yoast/Tests/Commenting/CoversTagUnitTest.inc index 326e26b3..4ff3c918 100644 --- a/Yoast/Tests/Commenting/CoversTagUnitTest.inc +++ b/Yoast/Tests/Commenting/CoversTagUnitTest.inc @@ -1,30 +1,14 @@ - * @covers Class_Name:: - * @covers Class_Name:: - * @covers Class_Name:: - * @covers Class_Name:: - * @covers Class_Name:: - * @covers Class_Name:: * @covers Name\Space\Class_Name * @covers \Name\Space\Class_Name - * @covers Name\Space\Class_Name:: - * @covers Name\Space\Class_Name:: - * @covers Name\Space\Class_Name:: - * @covers Name\Space\Class_Name:: - * @covers Name\Space\Class_Name:: - * @covers Name\Space\Class_Name:: * @covers Class_Name::method_name * @covers \Class_name::method_name * @covers Name\Space\Class_Name::method_name @@ -42,8 +26,24 @@ class ClassNameTest { public function testCoversTag() {} /** - * Docblock. - * + * @covers Class_Name + * @covers Class_Name:: + * @covers Class_Name:: + * @covers Class_Name:: + * @covers Class_Name:: + * @covers Class_Name:: + * @covers Class_Name:: + * @covers \Name\Space\Class_Name + * @covers \Name\Space\Class_Name:: + * @covers Name\Space\Class_Name:: + * @covers Name\Space\Class_Name:: + * @covers \Name\Space\Class_Name:: + * @covers Name\Space\Class_Name:: + * @covers \Name\Space\Class_Name:: + */ + public function testDeprecatedRemovedTagTypes() {} + + /** * @covers ::global_functionA && ::other_functionA * @covers ::global_functionB & ::other_functionB * @covers ::global_functionC|::other_functionC @@ -164,4 +164,35 @@ class ClassNameTest { * @covers \Name\Space\ClassName::MethodName */ public function testRecognizingNamesNotFollowingWPNamingConventions() {} + + /** + * Docblock. + */ + public function testDocblockWithoutTags() {} + + /** + * Docblock. + * + * @covers This is not a valid annotation. + * @covers \ + * @covers + */ + public function testCompletelyInvalidAnnotations() {} +} + +/** + * The <*> formats can also be used in combination with a `@coversDefaultClass` tag. + * + * @coversDefaultClass Class_Name + */ +class BracketedPartialsTest { + /** + * @covers :: + * @covers :: + * @covers :: + * @covers :: + * @covers :: + * @covers :: + */ + public function testPartialTagTypes() {} } diff --git a/Yoast/Tests/Commenting/CoversTagUnitTest.inc.fixed b/Yoast/Tests/Commenting/CoversTagUnitTest.inc.fixed index 53a0b1a0..cc03668d 100644 --- a/Yoast/Tests/Commenting/CoversTagUnitTest.inc.fixed +++ b/Yoast/Tests/Commenting/CoversTagUnitTest.inc.fixed @@ -1,30 +1,14 @@ - * @covers Class_Name:: - * @covers Class_Name:: - * @covers Class_Name:: - * @covers Class_Name:: - * @covers Class_Name:: - * @covers Class_Name:: * @covers Name\Space\Class_Name * @covers \Name\Space\Class_Name - * @covers Name\Space\Class_Name:: - * @covers Name\Space\Class_Name:: - * @covers Name\Space\Class_Name:: - * @covers Name\Space\Class_Name:: - * @covers Name\Space\Class_Name:: - * @covers Name\Space\Class_Name:: * @covers Class_Name::method_name * @covers \Class_name::method_name * @covers Name\Space\Class_Name::method_name @@ -42,8 +26,24 @@ class ClassNameTest { public function testCoversTag() {} /** - * Docblock. - * + * @covers Class_Name + * @covers Class_Name:: + * @covers Class_Name:: + * @covers Class_Name:: + * @covers Class_Name:: + * @covers Class_Name:: + * @covers Class_Name:: + * @covers \Name\Space\Class_Name + * @covers \Name\Space\Class_Name:: + * @covers Name\Space\Class_Name:: + * @covers Name\Space\Class_Name:: + * @covers \Name\Space\Class_Name:: + * @covers Name\Space\Class_Name:: + * @covers \Name\Space\Class_Name:: + */ + public function testDeprecatedRemovedTagTypes() {} + + /** * @covers ::global_functionA * @covers ::other_functionA * @covers ::global_functionB @@ -163,4 +163,35 @@ class ClassNameTest { * @covers \Name\Space\ClassName::MethodName */ public function testRecognizingNamesNotFollowingWPNamingConventions() {} + + /** + * Docblock. + */ + public function testDocblockWithoutTags() {} + + /** + * Docblock. + * + * @covers This is not a valid annotation. + * @covers \ + * @covers + */ + public function testCompletelyInvalidAnnotations() {} +} + +/** + * The <*> formats can also be used in combination with a `@coversDefaultClass` tag. + * + * @coversDefaultClass Class_Name + */ +class BracketedPartialsTest { + /** + * @covers :: + * @covers :: + * @covers :: + * @covers :: + * @covers :: + * @covers :: + */ + public function testPartialTagTypes() {} } diff --git a/Yoast/Tests/Commenting/CoversTagUnitTest.php b/Yoast/Tests/Commenting/CoversTagUnitTest.php index 08be552c..f85d2463 100644 --- a/Yoast/Tests/Commenting/CoversTagUnitTest.php +++ b/Yoast/Tests/Commenting/CoversTagUnitTest.php @@ -7,26 +7,24 @@ /** * Unit test class for the CoversTag sniff. * - * @package Yoast\YoastCS + * @since 1.3.0 * - * @since 1.3.0 - * - * @covers YoastCS\Yoast\Sniffs\Commenting\CoversTagSniff + * @covers YoastCS\Yoast\Sniffs\Commenting\CoversTagSniff */ -class CoversTagUnitTest extends AbstractSniffUnitTest { +final class CoversTagUnitTest extends AbstractSniffUnitTest { /** * Returns the lines where errors should occur. * - * @return array => + * @return array Key is the line number, value is the number of expected errors. */ - public function getErrorList() { + public function getErrorList(): array { return [ - 36 => 1, - 37 => 1, - 38 => 1, - 39 => 1, - 40 => 1, + 20 => 1, + 21 => 1, + 22 => 1, + 23 => 1, + 24 => 1, 47 => 1, 48 => 1, 49 => 1, @@ -48,15 +46,39 @@ public function getErrorList() { 140 => 1, 150 => 1, 151 => 1, + 176 => 1, + 177 => 1, + 178 => 1, ]; } /** * Returns the lines where warnings should occur. * - * @return array => + * @return array Key is the line number, value is the number of expected warnings. */ - public function getWarningList() { - return []; + public function getWarningList(): array { + return [ + 29 => 1, + 30 => 1, + 31 => 1, + 32 => 1, + 33 => 1, + 34 => 1, + 35 => 1, + 36 => 1, + 37 => 1, + 38 => 1, + 39 => 1, + 40 => 1, + 41 => 1, + 42 => 1, + 190 => 1, + 191 => 1, + 192 => 1, + 193 => 1, + 194 => 1, + 195 => 1, + ]; } } diff --git a/Yoast/Tests/Commenting/FileCommentUnitTest.13.inc b/Yoast/Tests/Commenting/FileCommentUnitTest.13.inc index 8d5c53fc..d73859cf 100644 --- a/Yoast/Tests/Commenting/FileCommentUnitTest.13.inc +++ b/Yoast/Tests/Commenting/FileCommentUnitTest.13.inc @@ -5,6 +5,6 @@ declare(strict_types=1); namespace Yoast\Plugin\Sub; /** - * Class docblock. A file docblock is not needed in a namespaced file. + * Class docblock. A file docblock is not needed in a namespaced file containing an OO structure. */ class Testing {} diff --git a/Yoast/Tests/Commenting/FileCommentUnitTest.14.inc b/Yoast/Tests/Commenting/FileCommentUnitTest.14.inc index 2c68b4a1..f162ac7d 100644 --- a/Yoast/Tests/Commenting/FileCommentUnitTest.14.inc +++ b/Yoast/Tests/Commenting/FileCommentUnitTest.14.inc @@ -1,6 +1,9 @@ + * @copyright 2018 Squiz Pty Ltd (ABN 77 084 670 600) + */ + +/** + * Interface docblock. + */ +interface Testing { + public function test(); +} diff --git a/Yoast/Tests/Commenting/FileCommentUnitTest.24.inc b/Yoast/Tests/Commenting/FileCommentUnitTest.24.inc new file mode 100644 index 00000000..e6d24487 --- /dev/null +++ b/Yoast/Tests/Commenting/FileCommentUnitTest.24.inc @@ -0,0 +1,9 @@ + + * @copyright 2018 Squiz Pty Ltd (ABN 77 084 670 600) + */ + +/** + * Trait docblock. + */ +trait Testing { + public function test() { + echo namespace\SomeClass::$static_property; // This is not a namespace declaration, but use of the namespace operator. + } +} diff --git a/Yoast/Tests/Commenting/FileCommentUnitTest.28.inc b/Yoast/Tests/Commenting/FileCommentUnitTest.28.inc new file mode 100644 index 00000000..16d21100 --- /dev/null +++ b/Yoast/Tests/Commenting/FileCommentUnitTest.28.inc @@ -0,0 +1,6 @@ + + * @copyright 2018 Squiz Pty Ltd (ABN 77 084 670 600) + */ + +/** + * Enum docblock. + */ +enum Testing { + public function test() { + echo namespace\SomeClass::$static_property; // This is not a namespace declaration, but use of the namespace operator. + } +} diff --git a/Yoast/Tests/Commenting/FileCommentUnitTest.32.inc b/Yoast/Tests/Commenting/FileCommentUnitTest.32.inc new file mode 100644 index 00000000..1a6d7308 --- /dev/null +++ b/Yoast/Tests/Commenting/FileCommentUnitTest.32.inc @@ -0,0 +1,6 @@ + + * @copyright 2018 Squiz Pty Ltd (ABN 77 084 670 600) + */ + +do_something(); + +/** + * Function docblock. + */ +function test() { + echo namespace\SomeClass::$static_property; // This is not a namespace declaration, but use of the namespace operator. +} diff --git a/Yoast/Tests/Commenting/FileCommentUnitTest.36.inc b/Yoast/Tests/Commenting/FileCommentUnitTest.36.inc new file mode 100644 index 00000000..0066f145 --- /dev/null +++ b/Yoast/Tests/Commenting/FileCommentUnitTest.36.inc @@ -0,0 +1,8 @@ + + * @copyright 2018 Squiz Pty Ltd (ABN 77 084 670 600) + */ + +namespace Yoast\Plugin\Sub; + +use Ab\C; + +/** + * Function docblock. + */ +function test() {} diff --git a/Yoast/Tests/Commenting/FileCommentUnitTest.39.inc b/Yoast/Tests/Commenting/FileCommentUnitTest.39.inc new file mode 100644 index 00000000..310a38d7 --- /dev/null +++ b/Yoast/Tests/Commenting/FileCommentUnitTest.39.inc @@ -0,0 +1,19 @@ + + * @copyright 2018 Squiz Pty Ltd (ABN 77 084 670 600) + */ + +namespace Yoast\Plugin\Sub; + +use Ab\C; + +$anon = new class extends C {}; diff --git a/Yoast/Tests/Commenting/FileCommentUnitTest.4.inc b/Yoast/Tests/Commenting/FileCommentUnitTest.4.inc index 3108a255..0b705f1a 100644 --- a/Yoast/Tests/Commenting/FileCommentUnitTest.4.inc +++ b/Yoast/Tests/Commenting/FileCommentUnitTest.4.inc @@ -1,6 +1,6 @@ + * @copyright 2018 Squiz Pty Ltd (ABN 77 084 670 600) + */ + +namespace Yoast\Plugin\Sub; + +use Ab\C; + +$closure = function() { + return new C(); +}; diff --git a/Yoast/Tests/Commenting/FileCommentUnitTest.5.inc b/Yoast/Tests/Commenting/FileCommentUnitTest.5.inc index 39ff51e2..be522035 100644 --- a/Yoast/Tests/Commenting/FileCommentUnitTest.5.inc +++ b/Yoast/Tests/Commenting/FileCommentUnitTest.5.inc @@ -3,7 +3,7 @@ namespace Yoast\Plugin\Sub { /** - * Class docblock. A file docblock is not needed in a namespaced file, even when it contains scoped namespaces. + * Class docblock. A file docblock is not needed in a namespaced file containing an OO structure, even when it contains scoped namespaces. */ class Testing {} } diff --git a/Yoast/Tests/Commenting/FileCommentUnitTest.7.inc b/Yoast/Tests/Commenting/FileCommentUnitTest.7.inc index 23a5bd25..77f049e4 100644 --- a/Yoast/Tests/Commenting/FileCommentUnitTest.7.inc +++ b/Yoast/Tests/Commenting/FileCommentUnitTest.7.inc @@ -1,6 +1,6 @@ => + * @return array Key is the line number, value is the number of expected errors. */ - public function getErrorList( $testFile = '' ) { + public function getErrorList( string $testFile = '' ): array { switch ( $testFile ) { case 'FileCommentUnitTest.2.inc': case 'FileCommentUnitTest.8.inc': case 'FileCommentUnitTest.10.inc': case 'FileCommentUnitTest.12.inc': + case 'FileCommentUnitTest.22.inc': + case 'FileCommentUnitTest.24.inc': + case 'FileCommentUnitTest.28.inc': + case 'FileCommentUnitTest.32.inc': + case 'FileCommentUnitTest.36.inc': return [ 1 => 1, ]; + case 'FileCommentUnitTest.21.inc': + return [ + 2 => 1, + ]; + default: return []; } @@ -42,14 +50,17 @@ public function getErrorList( $testFile = '' ) { * * @param string $testFile The name of the file being tested. * - * @return array => + * @return array Key is the line number, value is the number of expected warnings. */ - public function getWarningList( $testFile = '' ) { + public function getWarningList( string $testFile = '' ): array { switch ( $testFile ) { case 'FileCommentUnitTest.4.inc': case 'FileCommentUnitTest.6.inc': case 'FileCommentUnitTest.14.inc': case 'FileCommentUnitTest.20.inc': + case 'FileCommentUnitTest.26.inc': + case 'FileCommentUnitTest.30.inc': + case 'FileCommentUnitTest.34.inc': return [ 2 => 1, ]; diff --git a/Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.inc b/Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.1.inc similarity index 65% rename from Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.inc rename to Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.1.inc index 63123a2b..0947d7d0 100644 --- a/Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.inc +++ b/Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.1.inc @@ -29,7 +29,7 @@ class ClassLevelCoversTest { /** * @coversNothing */ -class ClassLevelCoversNothingTest { +final class ClassLevelCoversNothingTest { /** * Docblock. @@ -85,7 +85,7 @@ class ClassNameTest { * * @test */ - public function annotatedTestMissingCoversTag() {} + public static function annotatedTestMissingCoversTag() {} /** * Docblock. @@ -101,14 +101,14 @@ class ClassNameTest { * @test * @covers ::globalFunction */ - public function annotatedTestHasCoversTag() {} + final public function annotatedTestHasCoversTag() {} } /** * @covers ClassName */ #[Some_Attribute] -class ClassLevelCoversWithAttributeTest { +final class ClassLevelCoversWithAttributeTest { /** * Docblock. @@ -154,3 +154,44 @@ abstract class ContainsTestMethodsTestCase { */ abstract function testToBeImplementedInChildClass(); } + +/** + * Docblock. + * + * @sometag + * @anothertag + */ +class DoesNotHaveOurTargetTagsTest { + /** + * Docblock with tags, but not any of the tags we're looking for. + * + * @param mixed $input + * @param string $expected + * + * @return void + */ + final public function testSomething($input, $expected) {} +} + +/** + * Global functions should be ignored as PHPUnit tests should always be in a class. + */ +function testMe() {} + +/** + * Docblocks above PHP 8.2 readonly test classes should be recognized correctly. + * + * @covers \Some\Class::methodName + */ +readonly class ReadonlyTest { + public function testMe() {} +} + +/** + * Docblocks above PHP 8.2 readonly test classes should be recognized correctly. + * + * @covers \Some\Class::methodName + */ +final readonly class FinalReadonlyTest { + public function testMe() {} +} diff --git a/Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.2.inc b/Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.2.inc new file mode 100644 index 00000000..c5d0b545 --- /dev/null +++ b/Yoast/Tests/Commenting/TestsHaveCoversTagUnitTest.2.inc @@ -0,0 +1,5 @@ + => + * @param string $testFile The name of the file being tested. + * + * @return array Key is the line number, value is the number of expected errors. */ - public function getErrorList() { - return [ - 49 => 1, - 59 => 1, - 88 => 1, - 126 => 1, - 142 => 1, - 150 => 1, - ]; + public function getErrorList( string $testFile = '' ): array { + + switch ( $testFile ) { + case 'TestsHaveCoversTagUnitTest.1.inc': + return [ + 49 => 1, + 59 => 1, + 88 => 1, + 126 => 1, + 142 => 1, + 150 => 1, + 173 => 1, + ]; + + default: + return []; + } } /** * Returns the lines where warnings should occur. * - * @return array => + * @return array Key is the line number, value is the number of expected warnings. */ - public function getWarningList() { + public function getWarningList(): array { return []; } } diff --git a/Yoast/Tests/ControlStructures/IfElseDeclarationUnitTest.inc b/Yoast/Tests/ControlStructures/IfElseDeclarationUnitTest.inc deleted file mode 100644 index fa42db12..00000000 --- a/Yoast/Tests/ControlStructures/IfElseDeclarationUnitTest.inc +++ /dev/null @@ -1,86 +0,0 @@ - => - */ - public function getErrorList() { - return [ - 22 => 1, - 28 => 1, - 30 => 1, - 34 => 1, - 37 => 1, - 44 => 1, - 51 => 1, - 76 => 1, - 84 => 2, - ]; - } - - /** - * Returns the lines where warnings should occur. - * - * @return array => - */ - public function getWarningList() { - return []; - } -} diff --git a/Yoast/Tests/Files/FileNameUnitTest.php b/Yoast/Tests/Files/FileNameUnitTest.php index f885e01a..0b04c8b0 100644 --- a/Yoast/Tests/Files/FileNameUnitTest.php +++ b/Yoast/Tests/Files/FileNameUnitTest.php @@ -8,18 +8,16 @@ /** * Unit test class for the FileName sniff. * - * @package Yoast\YoastCS + * @since 0.5 * - * @since 0.5 - * - * @covers YoastCS\Yoast\Sniffs\Files\FileNameSniff + * @covers YoastCS\Yoast\Sniffs\Files\FileNameSniff */ -class FileNameUnitTest extends AbstractSniffUnitTest { +final class FileNameUnitTest extends AbstractSniffUnitTest { /** * Error files with the expected nr of errors. * - * @var int[] + * @var array */ private $expected_results = [ @@ -27,6 +25,9 @@ class FileNameUnitTest extends AbstractSniffUnitTest { * In /FileNameUnitTests. */ + // Live coding/parse error test. + 'live-coding.inc' => 0, + // Exclusions. 'excluded-file.inc' => 0, 'ExcludedFile.inc' => 1, // Lowercase expected. @@ -36,6 +37,11 @@ class FileNameUnitTest extends AbstractSniffUnitTest { 'some_file.inc' => 1, // Dashes, not underscores. 'SomeFile.inc' => 1, // Lowercase expected. 'some-File.inc' => 1, // Lowercase expected. + 'short-open.inc' => 0, + 'ShortOpen.inc' => 1, // Lowercase expected. + 'short_Open.inc' => 1, // Lowercase expected, dashes, not underscores. + 'dot.not.underscore.inc' => 1, // Dashes, not other punctuation. + 'with#other+punctuation.inc' => 1, // Dashes, not other punctuation. // Class file names. 'my-class.inc' => 0, @@ -48,6 +54,11 @@ class FileNameUnitTest extends AbstractSniffUnitTest { 'yoast.inc' => 0, // Class name = prefix, so there would be nothing left otherwise. 'class-wpseo-some-class.inc' => 1, // Prefixes 'class' and 'wpseo' not necessary. 'excluded-CLASS-file.inc' => 1, // Lowercase expected. + 'excluded-backslash-file.inc' => 0, + 'excluded-class-wrong-case.inc' => 1, // Filename not in line with class name. File not excluded due to wrong case used. + 'excluded-illegal.inc' => 1, // Filename not in line with class name. File not excluded due to illegal path setting. + 'excluded-multiple.inc' => 0, + 'excluded-dot-prefixed.inc' => 0, // Interface file names. 'outline-something-interface.inc' => 0, @@ -65,30 +76,78 @@ class FileNameUnitTest extends AbstractSniffUnitTest { 'no-duplicate-trait.inc' => 0, // Check that 'Trait' in trait name does not cause duplication in filename. 'excluded-trait-file.inc' => 0, + // Enum file names. + 'something-enum.inc' => 0, + 'different-enum.inc' => 1, // Filename not in line with enum name. + 'something.inc' => 1, // Missing '-enum' suffix. + 'yoast-something.inc' => 1, // Prefix 'yoast' not needed. + 'no-duplicate-enum.inc' => 0, // Check that 'Enum' in enum name does not cause duplication in filename. + 'excluded-enum.inc' => 0, + // Functions file names. 'functions.inc' => 0, 'some-functions.inc' => 0, 'some-file.inc' => 1, // Missing '-functions' suffix. 'excluded-functions-file.inc' => 0, + // Ignore annotation handling. + 'blanket-disable.inc' => 0, + 'yoast-disable.inc' => 0, + 'category-disable.inc' => 0, + 'rule-disable.inc' => 0, + 'disable-matching-enable.inc' => 1, + 'disable-non-matching-enable.inc' => 0, + 'non-relevant-disable.inc' => 1, + 'partial-file-disable.inc' => 1, + 'Errorcode_Disable.inc' => 1, // The sniff can only be disabled completely, not by error code. + + // PSR4 file names. + 'Some_Class.inc' => 0, + 'Some_Enum.inc' => 0, + 'Some_Interface.inc' => 0, + 'Some_Trait.inc' => 0, + 'Wrong_Class.inc' => 1, // Filename not in line with class name. + 'Wrong_Enum.inc' => 1, // Filename not in line with enum name. + 'Wrong_Interface.inc' => 1, // Filename not in line with interface name. + 'Wrong_Trait.inc' => 1, // Filename not in line with trait name. + 'wrong_case.inc' => 1, // Names are case-sensitive. + 'Wrong_Case_Too.inc' => 1, // Names are case-sensitive. + 'WPSEO_Prefixed.inc' => 0, // Prefixes should not be stripped for PSR4 file names. + 'Yoast_Prefixed.inc' => 0, // Prefixes should not be stripped for PSR4 file names. + 'Prefix_Stripped.inc' => 1, // Prefixes should not be stripped for PSR4 file names. + 'Not_Excluded.inc' => 1, // Exclusions do not apply to files where prefix stripping is not supported. + 'no-oo.inc' => 0, // Files not containing OO should follow the normal rules for PSR4 dirs. + 'no-oo-functions.inc' => 0, // Files containing only functions should follow the normal rules for PSR4 dirs. + 'missing-suffix.inc' => 1, // Files containing only functions should follow the normal rules for PSR4 dirs. + 'Multiple_Paths.inc' => 0, + 'Dot_Prefixed_Path.inc' => 0, + 'illegal-psr4-path.inc' => 0, // PSR4 path ignored, so normal rules apply. + 'Illegal_PSR4_Path.inc' => 1, // PSR4 path ignored, so normal rules apply. + /* * In /. */ // Fall-back file in case glob() fails. 'FileNameUnitTest.inc' => 1, + + // PSR4 related, PSR4 dir is root dir. + 'not-in-psr4-path.inc' => 0, + 'not-in-psr4-path-wrong-name.inc' => 1, // Filename not in line with class name, non-PSR4. + 'PSR4_Path_Is_Root_Path.inc' => 0, + 'PSR4_Path_Root_Wrong_Name.inc' => 1, // Filename not in line with class name, PSR4. ]; /** * Set CLI values before the file is tested. * - * @param string $testFile The name of the file being tested. + * @param string $filename The name of the file being tested. * @param Config $config The config data for the test run. * * @return void */ - public function setCliValues( $testFile, $config ) { - if ( $testFile === 'no-basepath.inc' ) { + public function setCliValues( $filename, $config ): void { + if ( $filename === 'no-basepath.inc' || $filename === 'no-basepath-psr4.inc' ) { return; } @@ -100,9 +159,9 @@ public function setCliValues( $testFile, $config ) { * * @param string $testFileBase The base path that the unit tests files will have. * - * @return string[] + * @return array */ - protected function getTestFiles( $testFileBase ) { + protected function getTestFiles( $testFileBase ): array { $sep = \DIRECTORY_SEPARATOR; $test_files = \glob( \dirname( $testFileBase ) . $sep . 'FileNameUnitTests{' . $sep . ',' . $sep . '*' . $sep . '}*.inc', \GLOB_BRACE ); @@ -118,9 +177,9 @@ protected function getTestFiles( $testFileBase ) { * * @param string $testFile The name of the file being tested. * - * @return array => + * @return array Key is the line number, value is the number of expected errors. */ - public function getErrorList( $testFile = '' ) { + public function getErrorList( string $testFile = '' ): array { if ( isset( $this->expected_results[ $testFile ] ) ) { return [ @@ -136,10 +195,15 @@ public function getErrorList( $testFile = '' ) { * * @param string $testFile The name of the file being tested. * - * @return array => + * @return array Key is the line number, value is the number of expected warnings. */ - public function getWarningList( $testFile = '' ) { - if ( $testFile === 'no-basepath.inc' ) { + public function getWarningList( string $testFile = '' ): array { + /* + * Note: no warning for the 'no-basepath.inc' file as the warning will only be thrown once. + * Also note that in which file the warning is thrown, relies on the sorting order of the + * test files, which could change. + */ + if ( $testFile === 'no-basepath-psr4.inc' ) { return [ 1 => 1, ]; diff --git a/Yoast/Tests/Files/FileNameUnitTests/PSR4/Dot_Prefixed_Path.inc b/Yoast/Tests/Files/FileNameUnitTests/PSR4/Dot_Prefixed_Path.inc new file mode 100644 index 00000000..790bc3c4 --- /dev/null +++ b/Yoast/Tests/Files/FileNameUnitTests/PSR4/Dot_Prefixed_Path.inc @@ -0,0 +1,8 @@ + +phpcs:set Yoast.Files.FileName psr4_paths[] Prefix\\=>./PSR4 + + +phpcs:set Yoast.Files.FileName psr4_paths[] Prefix=>PSR4/../PSR4/../ + + +phpcs:set Yoast.Files.FileName psr4_paths[] PrefixA\=>Other_Path/,PrefixB=>/PSR4/,PrefixC\=>/Deep/Path/Sub/ + + +phpcs:set Yoast.Files.FileName psr4_paths[] Prefix=>/PSR4/ +phpcs:set Yoast.Files.FileName excluded_files_strict_check[] PSR4/Not_Excluded.inc + + +phpcs:set Yoast.Files.FileName psr4_paths[] Prefix=>/PSR4/ +phpcs:set Yoast.Files.FileName oo_prefixes[] wpseo,yoast + + +phpcs:set Yoast.Files.FileName psr4_paths[] Prefix=>/PSR4/ + + +phpcs:set Yoast.Files.FileName psr4_paths[] Prefix\=>PSR4 + + +phpcs:set Yoast.Files.FileName psr4_paths[] Prefix\\=>/PSR4 + + +phpcs:set Yoast.Files.FileName psr4_paths[] Prefix=>PSR4/ + + +phpcs:set Yoast.Files.FileName psr4_paths[] Prefix\\=>/PSR4/ +phpcs:set Yoast.Files.FileName oo_prefixes[] wpseo,yoast + + +phpcs:set Yoast.Files.FileName psr4_paths[] Prefix=>/PSR4/ + + +phpcs:set Yoast.Files.FileName psr4_paths[] Prefix\\=>/PSR4/ + + +phpcs:set Yoast.Files.FileName psr4_paths[] Prefix=>PSR4 + + +phpcs:set Yoast.Files.FileName psr4_paths[] Prefix=>/PSR4 + + +phpcs:set Yoast.Files.FileName psr4_paths[] Prefix=>PSR4/ + + +phpcs:set Yoast.Files.FileName psr4_paths[] PrefixA=>/src/,PrefixB=>/PSR4/ +phpcs:set Yoast.Files.FileName oo_prefixes[] wpseo,yoast + + +phpcs:set Yoast.Files.FileName psr4_paths[] Prefix\\=>PSR4/../PSR4/../ + + +phpcs:set Yoast.Files.FileName psr4_paths[] Prefix\\=>/PSR4/ + + +phpcs:set Yoast.Files.FileName psr4_paths[] Prefix=>/PSR4/ + + +phpcs:set Yoast.Files.FileName psr4_paths[] Prefix=>/PSR4/ + + +phpcs:set Yoast.Files.FileName psr4_paths[] Prefix\\=>/PSR4/ + + +phpcs:set Yoast.Files.FileName psr4_paths[] Prefix\\=>. + + +phpcs:set Yoast.Files.FileName psr4_paths[] Prefix\\=>. + + diff --git a/Yoast/Tests/Files/FileNameUnitTests/classes/excluded-backslash-file.inc b/Yoast/Tests/Files/FileNameUnitTests/classes/excluded-backslash-file.inc new file mode 100644 index 00000000..71c95b11 --- /dev/null +++ b/Yoast/Tests/Files/FileNameUnitTests/classes/excluded-backslash-file.inc @@ -0,0 +1,8 @@ + +phpcs:set Yoast.Files.FileName excluded_files_strict_check[] classes\excluded-backslash-file.inc + + +phpcs:set Yoast.Files.FileName excluded_files_strict_check[] classes/excluded-Class-wrong-Case.inc + + +phpcs:set Yoast.Files.FileName excluded_files_strict_check[] ./classes/excluded-dot-prefixed.inc + + +phpcs:set Yoast.Files.FileName excluded_files_strict_check[] classes/../classes/excluded-illegal.inc + + +phpcs:set Yoast.Files.FileName excluded_files_strict_check[] classes/excluded-CLASS-file.inc,classes/excluded-multiple.inc + + +phpcs:set Yoast.Files.FileName oo_prefixes[] wpseo,yoast + + +phpcs:set Yoast.Files.FileName excluded_files_strict_check[] enums/excluded-enum.inc + + +phpcs:set Yoast.Files.FileName oo_prefixes[] wpseo,yoast + + +phpcs:set Yoast.Files.FileName oo_prefixes[] wpseo,yoast + + +phpcs:set Yoast.Files.FileName oo_prefixes[] wpseo,yoast + + +phpcs:set Yoast.Files.FileName oo_prefixes[] wpseo,yoast + + -phpcs:set Yoast.Files.FileName excluded_files_strict_check[] excluded-file.inc +phpcs:set Yoast.Files.FileName excluded_files_strict_check[] excluded-file.inc,./excluded-file.inc,/excluded-file.inc +phpcs:set Yoast.Files.FileName psr4_paths[] Prefix\\=>/PSR4/ + + +phpcs:set Yoast.Files.FileName psr4_paths[] Prefix\\=>/PSR4/ + + +phpcs:set Yoast.Files.FileName psr4_paths[] Prefix\\=>/PSR4/ + + diff --git a/Yoast/Tests/Files/FileNameUnitTests/short_Open.inc b/Yoast/Tests/Files/FileNameUnitTests/short_Open.inc new file mode 100644 index 00000000..72fd545b --- /dev/null +++ b/Yoast/Tests/Files/FileNameUnitTests/short_Open.inc @@ -0,0 +1 @@ + diff --git a/Yoast/Tests/Files/FileNameUnitTests/traits/different-trait.inc b/Yoast/Tests/Files/FileNameUnitTests/traits/different-trait.inc index 2328951c..9e1942e2 100644 --- a/Yoast/Tests/Files/FileNameUnitTests/traits/different-trait.inc +++ b/Yoast/Tests/Files/FileNameUnitTests/traits/different-trait.inc @@ -3,6 +3,6 @@ phpcs:set Yoast.Files.FileName oo_prefixes[] wpseo,yoast */ - protected function getTestFiles( $testFileBase ) { + protected function getTestFiles( $testFileBase ): array { $sep = \DIRECTORY_SEPARATOR; - $test_files = \glob( \dirname( $testFileBase ) . $sep . 'TestDoublesUnitTests{' . $sep . ',' . $sep . '*' . $sep . '}*.inc', \GLOB_BRACE ); + $test_files = \glob( \dirname( $testFileBase ) . $sep . 'TestDoublesUnitTests' . $sep . 'tests{' . $sep . ',' . $sep . '*' . $sep . '}*.inc', \GLOB_BRACE ); if ( ! empty( $test_files ) ) { return $test_files; @@ -55,28 +53,19 @@ protected function getTestFiles( $testFileBase ) { * * @param string $testFile The name of the file being tested. * - * @return array => + * @return array Key is the line number, value is the number of expected errors. */ - public function getErrorList( $testFile = '' ) { + public function getErrorList( string $testFile = '' ): array { switch ( $testFile ) { // In tests/. case 'mock-not-in-correct-dir.inc': return [ - 3 => 1, - ]; - - case 'multiple-objects-in-file.inc': - return [ - 5 => 2, - ]; - - case 'multiple-objects-in-file-reverse.inc': - return [ - 7 => 2, + 4 => 1, ]; case 'non-existant-doubles-dir.inc': + case 'non-existant-doubles-dirs.inc': return [ 4 => 1, ]; @@ -87,43 +76,55 @@ public function getErrorList( $testFile = '' ) { ]; case 'not-in-correct-dir-double.inc': + case 'not-in-correct-dir-mock.inc': + case 'not-in-correct-dir-enum-double.inc': + case 'not-in-correct-dir-interface-double.inc': + case 'not-in-correct-dir-trait-double.inc': return [ - 3 => 1, + 4 => 1, ]; - case 'not-in-correct-dir-mock.inc': + // In tests/Doubles. + case 'correct-dir-not-double-or-mock.inc': return [ - 3 => 1, + 4 => 1, ]; - // In tests/doubles. - case 'correct-dir-not-double-or-mock.inc': + // In tests/DoublesNotCorrect. + case 'not-in-correct-subdir.inc': return [ - 3 => 1, + 4 => 1, ]; - case 'multiple-mocks-in-file.inc': + // In tests/Mocks. + case 'correct-custom-dir-not-mock.inc': return [ - 3 => 1, - 5 => 1, + 4 => 1, ]; - // In tests/doubles-not-correct. - case 'not-in-correct-subdir.inc': + // In tests/lowercase. + case 'correct-custom-dir-wrong-case.inc': return [ - 3 => 1, + 4 => 1, ]; - // In tests/mocks. - case 'correct-custom-dir-not-mock.inc': + // In tests/lowercase. + case 'correct-custom-lowercase-dir-not-mock.inc': return [ - 3 => 1, + 4 => 1, ]; + case 'live-coding.inc': // In tests. case 'not-double-or-mock.inc': // In tests. - case 'correct-dir-double.inc': // In tests/doubles. - case 'correct-dir-mock.inc': // In tests/doubles. - case 'correct-custom-dir.inc': // In tests/mocks. + case 'non-existant-doubles-dir-not-double.inc': // In tests. + case 'correct-dir-double.inc': // In tests/Doubles. + case 'correct-dir-mock.inc': // In tests/Doubles. + case 'correct-dir-enum.inc': // In tests/Doubles. + case 'correct-dir-interface.inc': // In tests/Doubles. + case 'correct-dir-trait-double.inc': // In tests/Doubles. + case 'correct-custom-dir.inc': // In tests/Mocks. + case 'correct-custom-lowercase-dir.inc': // In tests/lowercase. + case 'correct-custom-dir-not-mock-wrong-case.inc': // In tests/lowercase. default: return []; } @@ -134,9 +135,9 @@ public function getErrorList( $testFile = '' ) { * * @param string $testFile The name of the file being tested. * - * @return array => + * @return array Key is the line number, value is the number of expected warnings. */ - public function getWarningList( $testFile = '' ) { + public function getWarningList( string $testFile = '' ): array { switch ( $testFile ) { case 'no-basepath.inc': return [ diff --git a/Yoast/Tests/Files/TestDoublesUnitTests/tests/Doubles/correct-dir-double.inc b/Yoast/Tests/Files/TestDoublesUnitTests/tests/Doubles/correct-dir-double.inc new file mode 100644 index 00000000..f0fe58b9 --- /dev/null +++ b/Yoast/Tests/Files/TestDoublesUnitTests/tests/Doubles/correct-dir-double.inc @@ -0,0 +1,6 @@ +phpcs:set Yoast.Files.TestDoubles doubles_path[] /tests/Doubles + => - */ - public function getErrorList( $testFile = '' ) { - switch ( $testFile ) { - case 'NamespaceDeclarationUnitTest.2.inc': - return [ - 3 => 1, - 5 => 3, - 7 => 2, - 9 => 3, - 11 => 2, - ]; - - default: - return []; - } - } - - /** - * Returns the lines where warnings should occur. - * - * @return array => - */ - public function getWarningList() { - return []; - } -} diff --git a/Yoast/Tests/NamingConventions/NamespaceNameUnitTest.php b/Yoast/Tests/NamingConventions/NamespaceNameUnitTest.php index 19fadee3..a2add417 100644 --- a/Yoast/Tests/NamingConventions/NamespaceNameUnitTest.php +++ b/Yoast/Tests/NamingConventions/NamespaceNameUnitTest.php @@ -8,25 +8,23 @@ /** * Unit test class for the NamespaceName sniff. * - * @package Yoast\YoastCS + * @since 2.0.0 * - * @since 2.0.0 - * - * @covers YoastCS\Yoast\Sniffs\NamingConventions\NamespaceNameSniff - * @covers YoastCS\Yoast\Utils\CustomPrefixesTrait + * @covers YoastCS\Yoast\Sniffs\NamingConventions\NamespaceNameSniff + * @covers YoastCS\Yoast\Utils\CustomPrefixesTrait */ -class NamespaceNameUnitTest extends AbstractSniffUnitTest { +final class NamespaceNameUnitTest extends AbstractSniffUnitTest { /** * Set CLI values before the file is tested. * - * @param string $testFile The name of the file being tested. + * @param string $filename The name of the file being tested. * @param Config $config The config data for the test run. * * @return void */ - public function setCliValues( $testFile, $config ) { - if ( \strpos( $testFile, 'no-basepath' ) === 0 ) { + public function setCliValues( $filename, $config ): void { + if ( \strpos( $filename, 'no-basepath' ) === 0 ) { return; } @@ -38,11 +36,19 @@ public function setCliValues( $testFile, $config ) { * * @param string $testFileBase The base path that the unit tests files will have. * - * @return string[] + * @return array */ - protected function getTestFiles( $testFileBase ) { + protected function getTestFiles( $testFileBase ): array { $sep = \DIRECTORY_SEPARATOR; - $test_files = \glob( \dirname( $testFileBase ) . $sep . 'NamespaceNameUnitTests{' . $sep . ',' . $sep . '*' . $sep . '}*.inc', \GLOB_BRACE ); + $test_files = \glob( + \dirname( $testFileBase ) . $sep . 'NamespaceNameUnitTests{' + . $sep . ',' // Files in the "NamespaceNameUnitTests" directory. + . $sep . '*' . $sep . ',' // Files in first-level subdirectories. + . $sep . '*' . $sep . '*' . $sep . ',' // Files in second-level subdirectories. + . $sep . '*' . $sep . '*' . $sep . '*' . $sep // Files in third-level subdirectories. + . '}*.inc', + \GLOB_BRACE + ); if ( ! empty( $test_files ) ) { return $test_files; @@ -56,24 +62,32 @@ protected function getTestFiles( $testFileBase ) { * * @param string $testFile The name of the file being tested. * - * @return array => + * @return array Key is the line number, value is the number of expected errors. */ - public function getErrorList( $testFile = '' ) { + public function getErrorList( string $testFile = '' ): array { switch ( $testFile ) { // Level check tests. case 'no-basepath.inc': return [ - 12 => 1, - 21 => 1, - 24 => 1, - 33 => 1, - 44 => 1, - 53 => 1, - 54 => 1, - 66 => 1, - 70 => 1, - 74 => 1, + 12 => 1, + 21 => 1, + 23 => 1, + 24 => 2, + 33 => 1, + 44 => 1, + 53 => 1, + 54 => 1, + 66 => 1, + 70 => 1, + 74 => 1, + 81 => 1, + 90 => 1, + 91 => 1, + 92 => 1, + 103 => 1, + 104 => 1, + 105 => 1, ]; case 'no-basepath-scoped.inc': @@ -86,7 +100,7 @@ public function getErrorList( $testFile = '' ) { case 'path-translation-root.inc': return [ 11 => 1, - 14 => 1, + 14 => 2, ]; case 'path-translation-sub1.inc': @@ -104,6 +118,18 @@ public function getErrorList( $testFile = '' ) { 15 => 1, ]; + // Path translation with unconventional chars in directory name. + case 'path-translation-sub2-dot.inc': + case 'path-translation-sub2-underscore.inc': + return [ + 12 => 1, + ]; + + case 'path-translation-sub2-space.inc': + return [ + 1 => 1, // Invalid dir error. + ]; + // Path translation with $src_directory set tests. case 'path-translation-src.inc': return [ @@ -118,6 +144,7 @@ public function getErrorList( $testFile = '' ) { case 'path-translation-src-sub-b.inc': return [ 14 => 1, + 15 => 1, ]; // Path translation with multiple items in $src_directory tests. @@ -144,15 +171,80 @@ public function getErrorList( $testFile = '' ) { 14 => 1, ]; - // Path translation with no matching $src_directory. - case 'path-translation-mismatch.inc': + // Path translation with $psr4_paths set tests. + case 'path-translation-psr4.inc': + return [ + 11 => 2, + 12 => 2, + 21 => 2, + 22 => 1, + 23 => 1, + 33 => 2, + 34 => 2, + 35 => 1, + 36 => 1, + 37 => 1, + 53 => 2, + 54 => 2, + 70 => 2, + 71 => 2, + 72 => 2, + 73 => 2, + ]; + + case 'path-translation-psr4-case-sensitive-lower.inc': + return [ + 11 => 1, + ]; + + case 'path-translation-psr4-case-sensitive-proper.inc': + return [ + 12 => 3, + 13 => 2, + 14 => 1, + 15 => 1, + 16 => 1, + 26 => 2, + 27 => 2, + 28 => 1, + 29 => 1, + 39 => 3, + 40 => 2, + 41 => 1, + 42 => 1, + 52 => 2, + 53 => 2, + 54 => 3, + 55 => 1, + 56 => 1, + 66 => 2, + 67 => 3, + 68 => 1, + 69 => 1, + ]; + + case 'path-translation-src-deeper-than-psr4-sub.inc': return [ 13 => 1, + 14 => 1, + 15 => 1, + 16 => 3, + 17 => 3, ]; - case 'path-translation-mismatch-illegal.inc': + // PSR4 path translation with unconventional chars in directory name. + case 'path-translation-psr4-dash.inc': + case 'path-translation-psr4-dot.inc': + case 'path-translation-psr4-space.inc': return [ - 12 => 1, + 1 => 1, // Invalid dir error. + ]; + + // Path translation with no matching $src_directory. + case 'path-translation-mismatch.inc': + return [ + 14 => 1, + 24 => 1, ]; default: @@ -165,22 +257,48 @@ public function getErrorList( $testFile = '' ) { * * @param string $testFile The name of the file being tested. * - * @return array => + * @return array Key is the line number, value is the number of expected warnings. */ - public function getWarningList( $testFile = '' ) { + public function getWarningList( string $testFile = '' ): array { switch ( $testFile ) { // Level check tests. case 'no-basepath.inc': return [ - 8 => 1, - 20 => 1, - 23 => 1, - 32 => 1, - 43 => 1, - 65 => 1, - 69 => 1, - 72 => 1, - 73 => 1, + 8 => 1, + 20 => 1, + 23 => 1, + 32 => 1, + 43 => 1, + 65 => 1, + 69 => 1, + 72 => 1, + 80 => 1, + 90 => 1, + 103 => 1, + 122 => 1, + 123 => 1, + 124 => 1, + 125 => 1, + 126 => 1, + 127 => 1, + 128 => 1, + 129 => 1, + 147 => 1, + 148 => 1, + 149 => 1, + 150 => 1, + 151 => 1, + 152 => 1, + 153 => 1, + 154 => 1, + 172 => 1, + 173 => 1, + 174 => 1, + 175 => 1, + 176 => 1, + 177 => 1, + 178 => 1, + 179 => 1, ]; case 'no-basepath-scoped.inc': @@ -193,6 +311,15 @@ public function getWarningList( $testFile = '' ) { 14 => 1, ]; + case 'path-translation-psr4-case-sensitive-proper.inc': + return [ + 13 => 1, + 27 => 1, + 40 => 1, + 53 => 1, + 66 => 1, + ]; + default: return []; } diff --git a/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/PSR4_Path/ProperCase/SubPath/path-translation-src-deeper-than-psr4-sub.inc b/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/PSR4_Path/ProperCase/SubPath/path-translation-src-deeper-than-psr4-sub.inc new file mode 100644 index 00000000..8b1c5c4e --- /dev/null +++ b/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/PSR4_Path/ProperCase/SubPath/path-translation-src-deeper-than-psr4-sub.inc @@ -0,0 +1,22 @@ +PSR4_Path + */ + +namespace Prefix\ProperCase\SubPath; // Error. + +namespace Prefix\SubPath; // Error. +namespace Prefix\ProperCase\Subpath; // Error. +namespace Prefix\ProperCase\subpath; // Error. +namespace Yoast\WP\Plugin\SubPath; // Error. +namespace Yoast\WP\Plugin\PSR4_Path\ProperCase\SubPath; // Error. + +// Reset to default settings. +// phpcs:set Yoast.NamingConventions.NamespaceName prefixes[] +// phpcs:set Yoast.NamingConventions.NamespaceName src_directory[] +// phpcs:set Yoast.NamingConventions.NamespaceName psr4_paths[] diff --git a/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/PSR4_Path/ProperCase/path-translation-psr4-case-sensitive-proper.inc b/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/PSR4_Path/ProperCase/path-translation-psr4-case-sensitive-proper.inc new file mode 100644 index 00000000..81cf6293 --- /dev/null +++ b/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/PSR4_Path/ProperCase/path-translation-psr4-case-sensitive-proper.inc @@ -0,0 +1,74 @@ +PSR4_Path + */ +namespace Yoast\WP\Plugin\ProperCase; // OK. + +namespace Yoast\Plugin\PSR4_Path\ProperCase; // Error x 3. +namespace Yoast\Plugin\ProperCase; // Error x 2 + warning. +namespace Yoast\WP\Plugin\propercase; // Error. +namespace Yoast\WP\Plugin\Propercase; // Error. +namespace Yoast\WP\Plugin\PROPERCASE; // Error. + +/* + * Testing that PSR-4 path translation takes precedence over src_directory [1]. + * + * @phpcs:set Yoast.NamingConventions.NamespaceName src_directory[] PSR4_Path/ProperCase + * @phpcs:set Yoast.NamingConventions.NamespaceName psr4_paths[] Yoast\WP\Plugin=>PSR4_Path + */ +namespace Yoast\WP\Plugin\ProperCase; // OK. + +namespace Yoast\Plugin; // Error x 2. +namespace Yoast\Plugin\ProperCase; // Error x 2 + warning. +namespace Yoast\WP\Plugin; // Error. +namespace Yoast\WP\Plugin\PSR4_Path\ProperCase; // Error. + +/* + * Testing that PSR-4 path translation takes precedence over src_directory [2]. + * + * @phpcs:set Yoast.NamingConventions.NamespaceName src_directory[] PSR4_Path + * @phpcs:set Yoast.NamingConventions.NamespaceName psr4_paths[] Yoast\WP\Plugin\\=>. + */ +namespace Yoast\WP\Plugin\PSR4_Path\ProperCase; // OK. + +namespace Yoast\Plugin\PSR4_Path\ProperCase; // Error x 3. +namespace Yoast\Plugin\ProperCase; // Error x 2 + warning. +namespace Yoast\WP\Plugin; // Error. +namespace Yoast\WP\Plugin\ProperCase; // Error. + +/* + * Testing that PSR-4 path translation takes precedence over src_directory [3]. + * + * phpcs:set Yoast.NamingConventions.NamespaceName src_directory[] PSR4_Path + * phpcs:set Yoast.NamingConventions.NamespaceName psr4_paths[] Yoast\WP\Plugin=>PSR4_Path/ProperCase + */ +namespace Yoast\WP\Plugin; // OK. + +namespace Yoast\Plugin; // Error x 2. +namespace Yoast\Plugin\ProperCase; // Error x 2 + warning. +namespace Yoast\Plugin\PSR4_Path\ProperCase; // Error x 3. +namespace Yoast\WP\Plugin\ProperCase; // Error. +namespace Yoast\WP\Plugin\PSR4_Path\ProperCase; // Error. + +/* + * Testing that PSR-4 path translation takes precedence over src_directory [4]. + * + * phpcs:set Yoast.NamingConventions.NamespaceName src_directory[] . + * phpcs:set Yoast.NamingConventions.NamespaceName psr4_paths[] Yoast\WP\Plugin\\=>PSR4_Path/ + */ +namespace Yoast\WP\Plugin\ProperCase; // OK. + +namespace Yoast\Plugin\ProperCase; // Error x 2 + warning. +namespace Yoast\Plugin\PSR4_Path\ProperCase; // Error x 3. +namespace Yoast\WP\Plugin; // Error. +namespace Yoast\WP\Plugin\PSR4_Path\ProperCase; // Error. + +// Reset to default settings. +// phpcs:set Yoast.NamingConventions.NamespaceName prefixes[] +// phpcs:set Yoast.NamingConventions.NamespaceName src_directory[] +// phpcs:set Yoast.NamingConventions.NamespaceName psr4_paths[] diff --git a/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/PSR4_Path/lowercase/path-translation-psr4-case-sensitive-lower.inc b/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/PSR4_Path/lowercase/path-translation-psr4-case-sensitive-lower.inc new file mode 100644 index 00000000..f9221d38 --- /dev/null +++ b/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/PSR4_Path/lowercase/path-translation-psr4-case-sensitive-lower.inc @@ -0,0 +1,14 @@ +PSR4_Path + */ + +namespace Yoast\WP\Plugin\lowercase; // OK. + +namespace Yoast\WP\Plugin\Lowercase; // Error. + +// Reset to default settings. +// phpcs:set Yoast.NamingConventions.NamespaceName psr4_paths[] diff --git a/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/PSR4_Path/path-translation-psr4.inc b/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/PSR4_Path/path-translation-psr4.inc new file mode 100644 index 00000000..8c9a05ca --- /dev/null +++ b/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/PSR4_Path/path-translation-psr4.inc @@ -0,0 +1,78 @@ +PSR4_Path + */ +namespace PSR4_Path; // OK. + +// PSR-4 names are case-sensitive. +namespace psr4_Path; // Error x 2. +namespace Psr4_PATH; // Error x 2. + +/* + * Testing path translation in combination with psr4_paths, no src directory, no prefixes. + * + * phpcs:set Yoast.NamingConventions.NamespaceName psr4_paths[] PrefixA\\=>PSR4_Path,PrefixB\\=>OtherPath + */ +namespace PrefixA; // OK. + +namespace PrefixB; // Error x 2. +namespace PrefixA\PSR4_PATH; // Error. +namespace PrefixA\Psr4_path; // Error. + +/* + * Testing path translation in combination with multiple psr4_paths + src_directory, no prefixes. + * + * phpcs:set Yoast.NamingConventions.NamespaceName psr4_paths[] PrefixA=>Some_Other,PrefixB=>PSR4_Path,PrefixC=>MyMy + * phpcs:set Yoast.NamingConventions.NamespaceName src_directory[] PSR4_Path + */ +namespace PrefixB; // OK. + +namespace PrefixA; // Error x 2. +namespace PrefixC; // Error x 2. +namespace PrefixB\PSR4_Path; // Error. +namespace PrefixB\psr4_path; // Error. +namespace PrefixB\PSR4_PATH; // Error. + +/* + * Testing path translation in combination with psr4_paths + src_directory, with prefix, + * when there is no matching PSR4 path due to a case-mismatch. + * + * In that case, the "normal" rules apply. + * + * @phpcs:set Yoast.NamingConventions.NamespaceName prefixes[] Yoast\WP\Plugin + * phpcs:set Yoast.NamingConventions.NamespaceName src_directory[] . + * phpcs:set Yoast.NamingConventions.NamespaceName psr4_paths[] Prefix\\=>pSr4_path + */ +namespace Yoast\WP\Plugin\PSR4_Path; // OK. +namespace Yoast\WP\Plugin\Psr4_PATH; // OK. +namespace Yoast\WP\Plugin\psr4_path; // OK. + +namespace Prefix; // Error x 2. +namespace Prefix\PSR4_Path; // Error x 2. + +/* + * Testing path translation in combination with psr4_paths + src_directory, with prefix, + * when there is no matching PSR4 path. + * + * In that case, the "normal" rules apply. + * + * @phpcs:set Yoast.NamingConventions.NamespaceName prefixes[] Yoast\WP\Plugin + * phpcs:set Yoast.NamingConventions.NamespaceName src_directory[] . + * phpcs:set Yoast.NamingConventions.NamespaceName psr4_paths[] PrefixA\\=>not-this-path,PrefixB\\=>Invalid + */ +namespace Yoast\WP\Plugin\PSR4_Path; // OK. +namespace Yoast\WP\Plugin\Psr4_PATH; // OK. +namespace Yoast\WP\Plugin\psr4_path; // OK. + +namespace PrefixA; // Error x 2. +namespace PrefixA\PSR4_Path; // Error x 2. +namespace PrefixB; // Error x 2. +namespace PrefixB\PSR4_Path; // Error x 2. + +// Reset to default settings. +// phpcs:set Yoast.NamingConventions.NamespaceName prefixes[] +// phpcs:set Yoast.NamingConventions.NamespaceName src_directory[] +// phpcs:set Yoast.NamingConventions.NamespaceName psr4_paths[] diff --git a/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/PSR4_Path/sub path/path-translation-psr4-space.inc b/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/PSR4_Path/sub path/path-translation-psr4-space.inc new file mode 100644 index 00000000..e85ce7bc --- /dev/null +++ b/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/PSR4_Path/sub path/path-translation-psr4-space.inc @@ -0,0 +1,12 @@ +PSR4_Path + */ + +namespace Yoast\WP\Plugin\PSR4_Path\Sub Path; // Error: invalid path. Would cause parse error, illegal space in namespace. + +// Reset to default settings. +// phpcs:set Yoast.NamingConventions.NamespaceName psr4_paths[] diff --git a/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/PSR4_Path/sub#path/path-translation-psr4-hash.inc b/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/PSR4_Path/sub#path/path-translation-psr4-hash.inc new file mode 100644 index 00000000..ec19dff3 --- /dev/null +++ b/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/PSR4_Path/sub#path/path-translation-psr4-hash.inc @@ -0,0 +1,12 @@ +PSR4_Path + */ + +namespace Yoast\WP\Plugin\PSR4_Path\Sub#Path; // Error: invalid path. Would cause parse error, unexpected end of file, sniff won't be able to reliably retrieve the namespace. + +// Reset to default settings. +// phpcs:set Yoast.NamingConventions.NamespaceName psr4_paths[] diff --git a/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/PSR4_Path/sub-path/path-translation-psr4-dash.inc b/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/PSR4_Path/sub-path/path-translation-psr4-dash.inc new file mode 100644 index 00000000..fe0513e5 --- /dev/null +++ b/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/PSR4_Path/sub-path/path-translation-psr4-dash.inc @@ -0,0 +1,12 @@ +PSR4_Path + */ + +namespace Yoast\WP\Plugin\PSR4_Path\Sub-Path; // Error: invalid path. Would cause parse error, illegal dash in namespace. + +// Reset to default settings. +// phpcs:set Yoast.NamingConventions.NamespaceName psr4_paths[] diff --git a/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/PSR4_Path/sub.path/path-translation-psr4-dot.inc b/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/PSR4_Path/sub.path/path-translation-psr4-dot.inc new file mode 100644 index 00000000..040e4801 --- /dev/null +++ b/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/PSR4_Path/sub.path/path-translation-psr4-dot.inc @@ -0,0 +1,12 @@ +PSR4_Path + */ + +namespace Yoast\WP\Plugin\PSR4_Path\Sub.Path; // Error: invalid path. Would cause parse error, illegal concatenation in namespace. + +// Reset to default settings. +// phpcs:set Yoast.NamingConventions.NamespaceName psr4_paths[] diff --git a/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/ignore/src/path-translation-ignore-src.inc b/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/ignore/src/path-translation-ignore-src.inc index 6c7a78cc..64ff5ae8 100644 --- a/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/ignore/src/path-translation-ignore-src.inc +++ b/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/ignore/src/path-translation-ignore-src.inc @@ -2,11 +2,11 @@ /* * Testing path translation in combination with multi-level src_directory. + * + * phpcs:set Yoast.NamingConventions.NamespaceName prefixes[] Yoast\WP\Plugin + * phpcs:set Yoast.NamingConventions.NamespaceName src_directory[] ignore/src */ -// phpcs:set Yoast.NamingConventions.NamespaceName prefixes[] Yoast\WP\Plugin -// phpcs:set Yoast.NamingConventions.NamespaceName src_directory[] ignore/src - namespace Yoast\WP\Plugin; // OK. namespace Yoast\WP\Plugin\Ignore\Src; // Error. diff --git a/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/ignore/src/sub-path/path-translation-ignore-src-sub-path.inc b/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/ignore/src/sub-path/path-translation-ignore-src-sub-path.inc index 19acea65..6db9128b 100644 --- a/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/ignore/src/sub-path/path-translation-ignore-src-sub-path.inc +++ b/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/ignore/src/sub-path/path-translation-ignore-src-sub-path.inc @@ -2,11 +2,11 @@ /* * Testing path translation in combination with multi-level src_directory. + * + * phpcs:set Yoast.NamingConventions.NamespaceName prefixes[] Yoast\WP\Plugin + * phpcs:set Yoast.NamingConventions.NamespaceName src_directory[] ignore/src */ -// phpcs:set Yoast.NamingConventions.NamespaceName prefixes[] Yoast\WP\Plugin -// phpcs:set Yoast.NamingConventions.NamespaceName src_directory[] ignore/src - namespace Yoast\WP\Plugin\Sub_Path; // OK. namespace Yoast\WP\Plugin; // Error. diff --git a/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/mismatch/path-translation-mismatch-illegal.inc b/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/mismatch/path-translation-mismatch-illegal.inc deleted file mode 100644 index d144a19e..00000000 --- a/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/mismatch/path-translation-mismatch-illegal.inc +++ /dev/null @@ -1,16 +0,0 @@ - max). + * + * @phpcs:set Yoast.NamingConventions.NamespaceName max_levels 2 + * @phpcs:set Yoast.NamingConventions.NamespaceName recommended_max_levels 5 */ -// phpcs:set Yoast.NamingConventions.NamespaceName max_levels 2 -// phpcs:set Yoast.NamingConventions.NamespaceName recommended_max_levels 5 - namespace Yoast\WP\Plugin\Foo\Bar; // OK. namespace Yoast\WP\Plugin\Foo\Bar\Baz; // Error. namespace Yoast\WP\Plugin\Foo\Bar\Baz\Fro; // Error. @@ -70,11 +70,117 @@ namespace Yoast\WP\Plugin\Tests\Doubles\Foo\Bar\Baz; // Warning. namespace Yoast\WP\Plugin\Tests\Doubles\Foo\Bar\Baz\Fro; // Error. namespace Yoast\WP\Plugin\Foo\Tests\Bar; // Warning, `Tests\` counted as not directly after the prefix. -namespace Yoast\WP\Plugin\Tests\Foo\Doubles\Bar; // Warning, `Doubles\` counted as not directly after the prefix + `Tests\`. +namespace Yoast\WP\Plugin\Tests\Foo\Doubles\Bar; // OK. namespace Yoast\WP\Plugin\Doubles\Foo\Bar\Fro; // Error, `Doubles\` counted as not found in combination with `Tests\`. +/* + * Test handling of namespace names with reserved keywords in them (PHP 8.0+). + */ +namespace Yoast\WP\Plugin\Trait\Sub; // OK. +namespace Yoast\WP\Plugin\Foo\Bar\Global; // Warning. +namespace Yoast\WP\Plugin\Foo\Case\Bar\Default; // Error. + +/* + * Testing an error is thrown for not using a prefix from the validated prefixes array - single valid prefix. + * + * phpcs:set Yoast.NamingConventions.NamespaceName prefixes[] Yoast\WP\Plugin + */ + +namespace Yoast\WP\Plugin\Foo; // OK. +namespace Yoast\WP\Foo; // Error (+ warning for level depth as prefix not correct). +namespace Yoast\Foo; // Error. +namespace Foo\Bar; // Error. + +/* + * Testing an error is thrown for not using a prefix from the validated prefixes array - multiple valid prefixes. + * + * phpcs:set Yoast.NamingConventions.NamespaceName prefixes[] Yoast\WP\Plugin\SubA,Yoast\WP\Plugin\SubB,Yoast\WP\Plugin + */ + +namespace Yoast\WP\Plugin\SubA\Foo; // OK. +namespace Yoast\WP\Plugin\SubB; // OK. +namespace Yoast\WP\Plugin\Foo; // OK. +namespace Yoast\WP\Foo; // Error (+ warning for level depth as prefix not correct). +namespace Yoast\Foo; // Error. +namespace Foo\Bar; // Error. + +/* + * Test allowing for more variations of test directories and deeper double directories. + * + * phpcs:set Yoast.NamingConventions.NamespaceName prefixes[] Yoast\WP\Plugin + */ + +namespace Yoast\WP\Plugin\Tests\WP\Foo\Bar; // OK. +namespace Yoast\WP\Plugin\Tests\Unit\Foo\Bar; // OK. +namespace Yoast\WP\Plugin\Tests\WP\Doubles\Foo\Bar; // OK. +namespace Yoast\WP\Plugin\Tests\Unit\Doubles\Foo\Bar; // OK. +namespace Yoast\WP\Plugin\Tests\WP\Mocks\Foo\Bar; // OK. +namespace Yoast\WP\Plugin\Tests\Unit\Mocks\Foo\Bar; // OK. +namespace Yoast\WP\Plugin\Tests\WP\Fixtures\Foo\Bar; // OK. +namespace Yoast\WP\Plugin\Tests\Unit\Fixtures\Foo\Bar; // OK. + +namespace Yoast\WP\Plugin\Tests\WP\Foo\Bar\Baz; // Warning. +namespace Yoast\WP\Plugin\Tests\Unit\Foo\Bar\Baz; // Warning. +namespace Yoast\WP\Plugin\Tests\WP\Doubles\Foo\Bar\Baz; // Warning. +namespace Yoast\WP\Plugin\Tests\Unit\Doubles\Foo\Bar\Baz; // Warning. +namespace Yoast\WP\Plugin\Tests\WP\Mocks\Foo\Bar\Baz; // Warning. +namespace Yoast\WP\Plugin\Tests\Unit\Mocks\Foo\Bar\Baz; // Warning. +namespace Yoast\WP\Plugin\Tests\WP\Fixtures\Foo\Bar\Baz; // Warning. +namespace Yoast\WP\Plugin\Tests\Unit\Fixtures\Foo\Bar\Baz; // Warning. + +/* + * Test allowing for more variations of test directories and deeper double directories based on PSR4 paths. + * + * phpcs:set Yoast.NamingConventions.NamespaceName prefixes[] + * phpcs:set Yoast.NamingConventions.NamespaceName psr4_paths[] Prefix\Foo=>PSR4_Path,Prefix\Bar=>OtherPath + */ + +namespace Prefix\Foo\Tests\WP\Foo\Bar; // OK. +namespace Prefix\Bar\Tests\Unit\Foo\Bar; // OK. +namespace Prefix\Foo\Tests\WP\Doubles\Foo\Bar; // OK. +namespace Prefix\Bar\Tests\Unit\Doubles\Foo\Bar; // OK. +namespace Prefix\Foo\Tests\WP\Mocks\Foo\Bar; // OK. +namespace Prefix\Bar\Tests\Unit\Mocks\Foo\Bar; // OK. +namespace Prefix\Foo\Tests\WP\Fixtures\Foo\Bar; // OK. +namespace Prefix\Bar\Tests\Unit\Fixtures\Foo\Bar; // OK. + +namespace Prefix\Foo\Tests\WP\Foo\Bar\Baz; // Warning. +namespace Prefix\Bar\Tests\Unit\Foo\Bar\Baz; // Warning. +namespace Prefix\Foo\Tests\WP\Doubles\Foo\Bar\Baz; // Warning. +namespace Prefix\Bar\Tests\Unit\Doubles\Foo\Bar\Baz; // Warning. +namespace Prefix\Foo\Tests\WP\Mocks\Foo\Bar\Baz; // Warning. +namespace Prefix\Bar\Tests\Unit\Mocks\Foo\Bar\Baz; // Warning. +namespace Prefix\Foo\Tests\WP\Fixtures\Foo\Bar\Baz; // Warning. +namespace Prefix\Bar\Tests\Unit\Fixtures\Foo\Bar\Baz; // Warning. + +/* + * Test with PSR4 paths where the prefix already includes `Tests`. + * + * phpcs:set Yoast.NamingConventions.NamespaceName prefixes[] + * phpcs:set Yoast.NamingConventions.NamespaceName psr4_paths[] PrefixA\Tests\\=>PSR4_Path + */ + +namespace PrefixA\Tests\WP\Foo\Bar; // OK. +namespace PrefixA\Tests\Unit\Foo\Bar; // OK. +namespace PrefixA\Tests\WP\Doubles\Foo\Bar; // OK. +namespace PrefixA\Tests\Unit\Doubles\Foo\Bar; // OK. +namespace PrefixA\Tests\WP\Mocks\Foo\Bar; // OK. +namespace PrefixA\Tests\Unit\Mocks\Foo\Bar; // OK. +namespace PrefixA\Tests\WP\Fixtures\Foo\Bar; // OK. +namespace PrefixA\Tests\Unit\Fixtures\Foo\Bar; // OK. + +namespace PrefixA\Tests\WP\Foo\Bar\Baz; // Warning. +namespace PrefixA\Tests\Unit\Foo\Bar\Baz; // Warning. +namespace PrefixA\Tests\WP\Doubles\Foo\Bar\Baz; // Warning. +namespace PrefixA\Tests\Unit\Doubles\Foo\Bar\Baz; // Warning. +namespace PrefixA\Tests\WP\Mocks\Foo\Bar\Baz; // Warning. +namespace PrefixA\Tests\Unit\Mocks\Foo\Bar\Baz; // Warning. +namespace PrefixA\Tests\WP\Fixtures\Foo\Bar\Baz; // Warning. +namespace PrefixA\Tests\Unit\Fixtures\Foo\Bar\Baz; // Warning. + // Reset to default settings. // phpcs:set Yoast.NamingConventions.NamespaceName prefixes[] +// phpcs:set Yoast.NamingConventions.NamespaceName psr4_paths[] /* * Test against false positives for namespace operator and incorrect namespace declarations. diff --git a/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/path-translation-root.inc b/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/path-translation-root.inc index cd40436c..ad95da90 100644 --- a/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/path-translation-root.inc +++ b/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/path-translation-root.inc @@ -2,16 +2,16 @@ /* * Testing path translation. + * + * phpcs:set Yoast.NamingConventions.NamespaceName prefixes[] Yoast\WP\Plugin,yoast_plugin */ -// phpcs:set Yoast.NamingConventions.NamespaceName prefixes[] Yoast\WP\Plugin,yoast_plugin - namespace Yoast\WP\Plugin; // OK. namespace Yoast\WP\Plugin\Foo; // Error. -// Make sure an error is thrown when the wrong prefix is used. -namespace Yoast_Plugin; // Error. +// Make sure an error is thrown when the wrong type of prefix (non-namespace-like) is used. +namespace Yoast_Plugin; // Error for wrong path-to-namespace + error for not using valid prefix. // Reset to default settings. // phpcs:set Yoast.NamingConventions.NamespaceName prefixes[] diff --git a/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/path/path-translation-sub1.inc b/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/path/path-translation-sub1.inc index aa01e27b..786bfe27 100644 --- a/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/path/path-translation-sub1.inc +++ b/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/path/path-translation-sub1.inc @@ -2,10 +2,10 @@ /* * Testing path translation. + * + * @phpcs:set Yoast.NamingConventions.NamespaceName prefixes[] Yoast\WP\Plugin */ -// phpcs:set Yoast.NamingConventions.NamespaceName prefixes[] Yoast\WP\Plugin - namespace Yoast\WP\Plugin\Path; // OK. namespace Yoast\WP\Plugin; // Error. diff --git a/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/path/sub path/path-translation-sub2-space.inc b/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/path/sub path/path-translation-sub2-space.inc new file mode 100644 index 00000000..bc8dac55 --- /dev/null +++ b/Yoast/Tests/NamingConventions/NamespaceNameUnitTests/path/sub path/path-translation-sub2-space.inc @@ -0,0 +1,12 @@ + => + * @return array Key is the line number, value is the number of expected errors. */ - public function getErrorList( $testFile = '' ) { + public function getErrorList( string $testFile = '' ): array { switch ( $testFile ) { case 'ObjectNameDepthUnitTest.2.inc': @@ -44,6 +42,9 @@ public function getErrorList( $testFile = '' ) { 114 => 1, 115 => 1, 116 => 1, + 135 => 1, + 136 => 1, + 145 => 1, ]; default: @@ -56,9 +57,9 @@ public function getErrorList( $testFile = '' ) { * * @param string $testFile The name of the file being tested. * - * @return array => + * @return array Key is the line number, value is the number of expected warnings. */ - public function getWarningList( $testFile = '' ) { + public function getWarningList( string $testFile = '' ): array { switch ( $testFile ) { case 'ObjectNameDepthUnitTest.2.inc': return [ @@ -70,4 +71,3 @@ public function getWarningList( $testFile = '' ) { } } } - diff --git a/Yoast/Tests/NamingConventions/ValidHookNameUnitTest.inc b/Yoast/Tests/NamingConventions/ValidHookNameUnitTest.inc index 80882350..6704b7dc 100644 --- a/Yoast/Tests/NamingConventions/ValidHookNameUnitTest.inc +++ b/Yoast/Tests/NamingConventions/ValidHookNameUnitTest.inc @@ -140,5 +140,13 @@ apply_filters( "Yoast\\WP\\Plugin\\hookname", $var); // OK. apply_filters( "Yoast\WP\Plugin\hookname", $var); // Warning, missing escaping x 3. apply_filters( "Yoast\WP\\Plugin\hook{$name}", $var); // Warning, missing escaping x 2 + warning at severity 3 for word count. + +// Safeguard support for PHP 8.0+ named parameters. +do_action_ref_array( hook: 'My-Hook', args: $args ); // OK. Well, not really, but using the wrong parameter name, so not our concern. +do_action_ref_array( args: $args, hook_name: 'Yoast\WP\Plugin\Test\some_hook_name', ); // OK. +do_action_ref_array( args: $args, hook_name: "yoast_plugin_some_hook_name", ); // Warning - wrong prefix. +do_action_ref_array( args: $args, hook_name: 'Yoast\WP\Plugin\some_hook', ); // OK. +do_action_ref_array( args: $args, hook_name: 'Yoast\WP\Plugin\some_hook_name_which_is_too_long', ); // Error - too long. + // Reset to default settings. // phpcs:set Yoast.NamingConventions.ValidHookName prefixes[] diff --git a/Yoast/Tests/NamingConventions/ValidHookNameUnitTest.php b/Yoast/Tests/NamingConventions/ValidHookNameUnitTest.php index b49dd619..762b6e46 100644 --- a/Yoast/Tests/NamingConventions/ValidHookNameUnitTest.php +++ b/Yoast/Tests/NamingConventions/ValidHookNameUnitTest.php @@ -8,14 +8,12 @@ /** * Unit test class for the ValidHookName sniff. * - * @package Yoast\YoastCS + * @since 2.0.0 * - * @since 2.0.0 - * - * @covers YoastCS\Yoast\Sniffs\NamingConventions\ValidHookNameSniff - * @covers YoastCS\Yoast\Utils\CustomPrefixesTrait + * @covers YoastCS\Yoast\Sniffs\NamingConventions\ValidHookNameSniff + * @covers YoastCS\Yoast\Utils\CustomPrefixesTrait */ -class ValidHookNameUnitTest extends AbstractSniffUnitTest { +final class ValidHookNameUnitTest extends AbstractSniffUnitTest { /** * Set warnings level to 3 to trigger suggestions as warnings. @@ -25,16 +23,16 @@ class ValidHookNameUnitTest extends AbstractSniffUnitTest { * * @return void */ - public function setCliValues( $filename, $config ) { + public function setCliValues( $filename, $config ): void { $config->warningSeverity = 3; } /** * Returns the lines where errors should occur. * - * @return array => + * @return array Key is the line number, value is the number of expected errors. */ - public function getErrorList() { + public function getErrorList(): array { return [ 14 => 1, @@ -52,15 +50,16 @@ public function getErrorList() { 108 => 1, 111 => 1, 119 => 1, + 149 => 1, ]; } /** * Returns the lines where warnings should occur. * - * @return array => + * @return array Key is the line number, value is the number of expected warnings. */ - public function getWarningList() { + public function getWarningList(): array { return [ 16 => 1, 19 => 1, @@ -81,7 +80,7 @@ public function getWarningList() { 136 => 1, // Severity: 3. 140 => 1, 141 => 2, // Severity: 3 + 5. + 147 => 1, ]; } } - diff --git a/Yoast/Tests/NonSniffTestCase.php b/Yoast/Tests/NonSniffTestCase.php new file mode 100644 index 00000000..0c9035be --- /dev/null +++ b/Yoast/Tests/NonSniffTestCase.php @@ -0,0 +1,44 @@ +config = $config; + + return $phpcsFile; + } +} diff --git a/Yoast/Tests/Reports/ThresholdReportTest.php b/Yoast/Tests/Reports/ThresholdReportTest.php new file mode 100644 index 00000000..138e9588 --- /dev/null +++ b/Yoast/Tests/Reports/ThresholdReportTest.php @@ -0,0 +1,192 @@ +get_mock_file(); + $report = [ + 'errors' => $errors, + 'warnings' => $warnings, + ]; + + $threshold_report = new Threshold(); + + $this->assertSame( $expected, $threshold_report->generateFileReport( $report, $phpcsFile ) ); + } + + /** + * Data provider. + * + * @see test_generate_file_report() For the array format. + * + * @return array> + */ + public static function data_generate_file_report() { + return [ + 'no errors, no warnings' => [ + 'errors' => 0, + 'warnings' => 0, + 'expected' => false, + ], + 'has errors, no warnings' => [ + 'errors' => 10, + 'warnings' => 0, + 'expected' => true, + ], + 'no errors, has warnings' => [ + 'errors' => 0, + 'warnings' => 5, + 'expected' => true, + ], + 'has errors and warnings' => [ + 'errors' => 3, + 'warnings' => 5, + 'expected' => true, + ], + ]; + } + + /** + * Test creating the threshold report. + * + * @dataProvider data_generate + * @covers ::generate + * + * @param int $error_threshold Allowed nr of errors. + * @param int $warning_threshold Allowed nr of warnings. + * @param string $expected_output Expected screen output regex. + * + * @return void + */ + public function test_generate( $error_threshold, $warning_threshold, $expected_output ) { + // phpcs:disable WordPress.PHP.DiscouragedPHPFunctions + \putenv( "YOASTCS_THRESHOLD_ERRORS=$error_threshold" ); + \putenv( "YOASTCS_THRESHOLD_WARNINGS=$warning_threshold" ); + // phpcs:enable + + $this->expectOutputRegex( $expected_output ); + + $report = new Threshold(); + $report->generate( + '', + 10, // Files. + 150, // Errors. + 300, // Warnings. + 10 // Fixable. + ); + } + + /** + * Data provider. + * + * @see test_generate() For the array format. + * + * @return array> + */ + public static function data_generate() { + return [ + 'Threshold: no errors, no warnings allowed' => [ + 'error_threshold' => 0, + 'warning_threshold' => 0, + 'expected_output' => '`\s+' + . '\\033\[1mPHP CODE SNIFFER THRESHOLD COMPARISON\\033\[0m\s+' + . '-{80}\s+' + . '\\033\[31mCoding standards ERRORS: 150/0\.\\033\[0m\s+' + . '\\033\[33mCoding standards WARNINGS: 300/0\.\\033\[0m\s+' + . '\\033\[31mPlease fix any errors introduced in your code and run PHPCS again to verify\.\\033\[0m\s+' + . '\\033\[33mPlease fix any warnings introduced in your code and run PHPCS again to verify\.\\033\[0m\s+' + . '`', + ], + 'Threshold: both errors and warnings below threshold' => [ + 'error_threshold' => 160, + 'warning_threshold' => 320, + 'expected_output' => '`\s+' + . '\\033\[1mPHP CODE SNIFFER THRESHOLD COMPARISON\\033\[0m\s+' + . '-{80}\s+' + . '\\033\[32mCoding standards ERRORS: 150/160\.\\033\[0m\s+' + . '\\033\[32mCoding standards WARNINGS: 300/320\.\\033\[0m\s+' + . '\\033\[32mFound less errors than the threshold, great job!\\033\[0m\s+' + . 'Please update the ERRORS threshold in the composer\.json file to \\033\[32m150\.\\033\[0m\s+' + . '\\033\[32mFound less warnings than the threshold, great job!\\033\[0m\s+' + . 'Please update the WARNINGS threshold in the composer.json file to \\033\[32m300\.\\033\[0m\s+' + . 'Coding standards checks have passed!\s+' + . '`', + ], + 'Threshold: both errors and warnings exactly at threshold' => [ + 'error_threshold' => 150, + 'warning_threshold' => 300, + 'expected_output' => '`\s+' + . '\\033\[1mPHP CODE SNIFFER THRESHOLD COMPARISON\\033\[0m\s+' + . '-{80}\s+' + . '\\033\[32mCoding standards ERRORS: 150/150\.\\033\[0m\s+' + . '\\033\[32mCoding standards WARNINGS: 300/300\.\\033\[0m\s+' + . 'Coding standards checks have passed!\s+' + . '`', + ], + 'Threshold: errors below threshold, warnings above' => [ + 'error_threshold' => 155, + 'warning_threshold' => 250, + 'expected_output' => '`\s+' + . '\\033\[1mPHP CODE SNIFFER THRESHOLD COMPARISON\\033\[0m\s+' + . '-{80}\s+' + . '\\033\[32mCoding standards ERRORS: 150/155\.\\033\[0m\s+' + . '\\033\[33mCoding standards WARNINGS: 300/250\.\\033\[0m\s+' + . '\\033\[32mFound less errors than the threshold, great job!\\033\[0m\s+' + . 'Please update the ERRORS threshold in the composer\.json file to \\033\[32m150\.\\033\[0m\s+' + . '\\033\[33mPlease fix any warnings introduced in your code and run PHPCS again to verify\.\\033\[0m\s+' + . '`', + ], + 'Threshold: errors above threshold, warnings below' => [ + 'error_threshold' => 100, + 'warning_threshold' => 500, + 'expected_output' => '`\s+' + . '\\033\[1mPHP CODE SNIFFER THRESHOLD COMPARISON\\033\[0m\s+' + . '-{80}\s+' + . '\\033\[31mCoding standards ERRORS: 150/100\.\\033\[0m\s+' + . '\\033\[32mCoding standards WARNINGS: 300/500\.\\033\[0m\s+' + . '\\033\[31mPlease fix any errors introduced in your code and run PHPCS again to verify\.\\033\[0m\s+' + . '\\033\[32mFound less warnings than the threshold, great job!\\033\[0m\s+' + . 'Please update the WARNINGS threshold in the composer.json file to \\033\[32m300\.\\033\[0m\s+' + . '`', + ], + 'Threshold: both errors and warnings above threshold' => [ + 'error_threshold' => 100, + 'warning_threshold' => 200, + 'expected_output' => '`\s+' + . '\\033\[1mPHP CODE SNIFFER THRESHOLD COMPARISON\\033\[0m\s+' + . '-{80}\s+' + . '\\033\[31mCoding standards ERRORS: 150/100\.\\033\[0m\s+' + . '\\033\[33mCoding standards WARNINGS: 300/200\.\\033\[0m\s+' + . '\\033\[31mPlease fix any errors introduced in your code and run PHPCS again to verify\.\\033\[0m\s+' + . '\\033\[33mPlease fix any warnings introduced in your code and run PHPCS again to verify\.\\033\[0m\s+' + . '`', + ], + ]; + } +} diff --git a/Yoast/Tests/Tools/BrainMonkeyRaceConditionUnitTest.inc b/Yoast/Tests/Tools/BrainMonkeyRaceConditionUnitTest.inc index bce0ba9f..cb7ddb3f 100644 --- a/Yoast/Tests/Tools/BrainMonkeyRaceConditionUnitTest.inc +++ b/Yoast/Tests/Tools/BrainMonkeyRaceConditionUnitTest.inc @@ -153,4 +153,41 @@ class BrainMonkeyBasedTest extends TestCase { // Do something to satisfy expectations. } + + /* + * Tests involving PHP 8.0+ named parameters. + */ + public function test_NOT_race_condition_wrong_param_name() { + Functions\expect( not_functionname: "apply_filters")->times(5); + + Filters\ExpectDone ( 'filter_name' ); + + // Do something to satisfy expectations. + } + + public function test_NOT_race_condition_named_param_not_for_hook() { + Functions\expect( function_name: "other_function")->times(5); + + Filters\ExpectDone ( 'filter_name' ); + + // Do something to satisfy expectations. + } + + public function test_filter_race_condition_named_param() { + Functions\expect( function_name: "apply_filters")->twice(); + + Filters\ExpectApplied ( 'filter_name' ) + ->with( true ); + + // Do something to satisfy expectations. + } + + public function test_action_race_condition_named_param_unconventional_order() { + \Brain\Monkey\Functions\Expect( something: foo, function_name: 'do_action' )->once(); + + Actions\expectDone( 'yoast\action_name' ) + ->with( $param ); + + // Do something to satisfy expectations. + } } diff --git a/Yoast/Tests/Tools/BrainMonkeyRaceConditionUnitTest.php b/Yoast/Tests/Tools/BrainMonkeyRaceConditionUnitTest.php index 979f8969..c3db33de 100644 --- a/Yoast/Tests/Tools/BrainMonkeyRaceConditionUnitTest.php +++ b/Yoast/Tests/Tools/BrainMonkeyRaceConditionUnitTest.php @@ -7,32 +7,34 @@ /** * Unit test class for the BrainMonkeyRaceCondition sniff. * - * @package Yoast\YoastCS + * @since 2.3.0 * - * @covers YoastCS\Yoast\Sniffs\Tools\BrainMonkeyRaceConditionSniff + * @covers YoastCS\Yoast\Sniffs\Tools\BrainMonkeyRaceConditionSniff */ -class BrainMonkeyRaceConditionUnitTest extends AbstractSniffUnitTest { +final class BrainMonkeyRaceConditionUnitTest extends AbstractSniffUnitTest { /** * Returns the lines where errors should occur. * - * @return array => + * @return array Key is the line number, value is the number of expected errors. */ - public function getErrorList() { + public function getErrorList(): array { return [ 104 => 1, 113 => 1, 122 => 1, 131 => 1, + 176 => 1, + 185 => 1, ]; } /** * Returns the lines where warnings should occur. * - * @return array => + * @return array Key is the line number, value is the number of expected warnings. */ - public function getWarningList() { + public function getWarningList(): array { return []; } } diff --git a/Yoast/Tests/Utils/PSR4PathsTraitTest.php b/Yoast/Tests/Utils/PSR4PathsTraitTest.php new file mode 100644 index 00000000..4ba41730 --- /dev/null +++ b/Yoast/Tests/Utils/PSR4PathsTraitTest.php @@ -0,0 +1,545 @@ +psr4_paths = []; + $this->previous_psr4_paths = []; + $this->validated_psr4_paths = []; + } + + /** + * Test validating the $psr4_paths property + * + * @dataProvider data_is_get_psr4_info + * @covers ::is_in_psr4_path + * + * @param array $psr4_paths The initial input for the $psr4_paths property. + * @param string $file_path The file path to evaluate. + * @param array $expected The expected function output. + * + * @return void + */ + public function test_is_in_psr4_path( $psr4_paths, $file_path, $expected ) { + $phpcsFile = $this->get_mock_file(); + $phpcsFile->config->basepath = self::CLEAN_BASEPATH; + $phpcsFile->path = $file_path; + + $this->psr4_paths = $psr4_paths; + + $this->assertSame( $expected['is'], $this->is_in_psr4_path( $phpcsFile ) ); + } + + /** + * Test validating the $psr4_paths property + * + * @dataProvider data_is_get_psr4_info + * @covers ::get_psr4_info + * + * @param array $psr4_paths The initial input for the $psr4_paths property. + * @param string $file_path The file path to evaluate. + * @param array $expected The expected function output. + * + * @return void + */ + public function test_get_psr4_info( $psr4_paths, $file_path, $expected ) { + $phpcsFile = $this->get_mock_file(); + $phpcsFile->config->basepath = self::DIRTY_BASEPATH; + $phpcsFile->path = $file_path; + + $this->psr4_paths = $psr4_paths; + + $this->assertSame( $expected['get'], $this->get_psr4_info( $phpcsFile ) ); + } + + /** + * Test validating the $psr4_paths property + * + * @dataProvider data_is_get_psr4_info + * @covers ::get_psr4_info + * + * @param array $psr4_paths The initial input for the $psr4_paths property. + * @param string $file_path The file path to evaluate. + * @param array $expected The expected function output. + * + * @return void + */ + public function test_get_psr4_info_explicit( $psr4_paths, $file_path, $expected ) { + $phpcsFile = $this->get_mock_file(); + $phpcsFile->config->basepath = self::DIRTY_BASEPATH; + + $this->psr4_paths = $psr4_paths; + + $this->assertSame( $expected['get'], $this->get_psr4_info( $phpcsFile, $file_path ) ); + } + + /** + * Data provider. + * + * @see test_is_in_psr4_path() For the array format. + * @see test_get_psr4_info() For the array format. + * @see test_get_psr4_info_explicit() For the array format. + * + * @return array>>> + */ + public static function data_is_get_psr4_info() { + $default_psr4_paths = self::data_validate_psr4_paths()['multiple prefixes, variation of paths']['psr4_paths']; + + return [ + 'path is unknown' => [ + 'psr4_paths' => [], + 'file_path' => '', + 'expected' => [ + 'is' => false, + 'get' => false, + ], + ], + 'path is STDIN' => [ + 'psr4_paths' => [], + 'file_path' => 'STDIN', + 'expected' => [ + 'is' => false, + 'get' => false, + ], + ], + 'path is single-quoted STDIN' => [ + 'psr4_paths' => [], + 'file_path' => "'STDIN'", + 'expected' => [ + 'is' => false, + 'get' => false, + ], + ], + 'path is double-quoted STDIN' => [ + 'psr4_paths' => [], + 'file_path' => '"STDIN"', + 'expected' => [ + 'is' => false, + 'get' => false, + ], + ], + 'PSR-4 paths is empty/not set' => [ + 'psr4_paths' => [], + 'file_path' => 'path/is/not/relevant', + 'expected' => [ + 'is' => false, + 'get' => false, + ], + ], + 'Linux style file path, not matching' => [ + 'psr4_paths' => $default_psr4_paths, + 'file_path' => 'path/will/not/match/filename.ext', + 'expected' => [ + 'is' => false, + 'get' => false, + ], + ], + 'Linux style file path, matching' => [ + 'psr4_paths' => $default_psr4_paths, + 'file_path' => self::DIRTY_BASEPATH . '/config/subdir/filename.ext', + 'expected' => [ + 'is' => true, + 'get' => [ + 'prefix' => 'Plugin\PrefixB', + 'basepath' => self::CLEAN_BASEPATH . 'config/', + 'relative' => 'subdir', + ], + ], + ], + 'Windows style file path, not matching' => [ + 'psr4_paths' => $default_psr4_paths, + 'file_path' => 'path\will\not\match\filename.ext', + 'expected' => [ + 'is' => false, + 'get' => false, + ], + ], + 'Windows style file path, matching' => [ + 'psr4_paths' => $default_psr4_paths, + 'file_path' => '\base\path\config\subdir\filename.ext', + 'expected' => [ + 'is' => true, + 'get' => [ + 'prefix' => 'Plugin\PrefixB', + 'basepath' => self::CLEAN_BASEPATH . 'config/', + 'relative' => 'subdir', + ], + ], + ], + 'Mixed slashes in file path, not matching' => [ + 'psr4_paths' => $default_psr4_paths, + 'file_path' => self::DIRTY_BASEPATH . '\subdir\filename.ext', + 'expected' => [ + 'is' => false, + 'get' => false, + ], + ], + 'Mixed slashes in file path, matching' => [ + 'psr4_paths' => $default_psr4_paths, + 'file_path' => self::CLEAN_BASEPATH . 'subA\sub/deeper/filename.ext', + 'expected' => [ + 'is' => true, + 'get' => [ + 'prefix' => 'Plugin\PrefixC', + 'basepath' => self::CLEAN_BASEPATH . 'subA/sub/', + 'relative' => 'deeper', + ], + ], + ], + 'Exact match, no file name' => [ + 'psr4_paths' => $default_psr4_paths, + 'file_path' => self::CLEAN_BASEPATH . 'tests/', + 'expected' => [ + 'is' => true, + 'get' => [ + 'prefix' => 'Plugin\PrefixB', + 'basepath' => self::CLEAN_BASEPATH . 'tests/', + 'relative' => '.', + ], + ], + ], + 'Exact match, no file name, no trailing slash' => [ + 'psr4_paths' => $default_psr4_paths, + 'file_path' => self::CLEAN_BASEPATH . 'subC', + 'expected' => [ + 'is' => true, + 'get' => [ + 'prefix' => 'Plugin\PrefixC', + 'basepath' => self::CLEAN_BASEPATH . 'subC/', + 'relative' => '.', + ], + ], + ], + 'Exact match, with file name' => [ + 'psr4_paths' => $default_psr4_paths, + 'file_path' => self::CLEAN_BASEPATH . 'subC/file.ext', + 'expected' => [ + 'is' => true, + 'get' => [ + 'prefix' => 'Plugin\PrefixC', + 'basepath' => self::CLEAN_BASEPATH . 'subC/', + 'relative' => '.', + ], + ], + ], + 'Long subdir, matching' => [ + 'psr4_paths' => $default_psr4_paths, + 'file_path' => self::CLEAN_BASEPATH . 'src/something/else/and/more/file.ext', + 'expected' => [ + 'is' => true, + 'get' => [ + 'prefix' => 'Plugin\PrefixA', + 'basepath' => self::CLEAN_BASEPATH . 'src/', + 'relative' => 'something/else/and/more', + ], + ], + ], + ]; + } + + /** + * Test validating the $psr4_paths property when no basepath is present. + * + * @covers ::validate_psr4_paths + * + * @return void + */ + public function test_validate_psr4_paths_without_basepath_doesnt_set_validated() { + $phpcsFile = $this->get_mock_file(); + + $this->psr4_paths = self::data_validate_psr4_paths()['multiple prefixes, variation of paths']['psr4_paths']; + + $this->validate_psr4_paths( $phpcsFile ); + + $this->assertSame( [], $this->previous_psr4_paths, 'Previous paths has been set' ); + $this->assertSame( [], $this->validated_psr4_paths, 'Validated paths has been set' ); + } + + /** + * Test validating the $psr4_paths property when no basepath is present while a previous run did have a basepath. + * + * @covers ::validate_psr4_paths + * + * @return void + */ + public function test_validate_psr4_paths_without_basepath_resets_validated() { + $phpcsFile = $this->get_mock_file(); + + // Initial test with basepath. + $phpcsFile->config->basepath = self::DIRTY_BASEPATH; + + $input = self::data_validate_psr4_paths()['multiple prefixes, variation of paths']; + $this->psr4_paths = $input['psr4_paths']; + + $this->validate_psr4_paths( $phpcsFile ); + + $this->assertSame( $input['psr4_paths'], $this->previous_psr4_paths, 'Previous paths has not been set correctly' ); + $this->assertSame( $input['expected'], $this->validated_psr4_paths, 'Validated paths has not been set correctly' ); + + // Now make sure that the missing basepath resets the validated paths. + $phpcsFile->config->basepath = null; + + $this->validate_psr4_paths( $phpcsFile ); + + $this->assertSame( $input['psr4_paths'], $this->previous_psr4_paths, 'Previous paths is not still the same' ); + $this->assertSame( [], $this->validated_psr4_paths, 'Validated paths has not been reset' ); + } + + /** + * Test validating the $psr4_paths property results in an exception when no prefixes are passed. + * + * @covers ::validate_psr4_paths + * + * @return void + */ + public function test_validate_psr4_paths_throws_exception_on_missing_prefixes() { + $phpcsFile = $this->get_mock_file(); + $phpcsFile->config->basepath = self::CLEAN_BASEPATH; + + // PSR-4 paths contains the same path for two different prefixes. + $this->psr4_paths = [ + 'src', + 'tests', + ]; + + $this->expectException( RuntimeException::class ); + $this->expectExceptionMessage( + 'Invalid value passed for `psr4_paths`. Path "src" is not associated with a namespace prefix' + ); + + $this->validate_psr4_paths( $phpcsFile ); + } + + /** + * Test validating the $psr4_paths property results in an exception when the same path is passed for multiple prefixes. + * + * @covers ::validate_psr4_paths + * + * @return void + */ + public function test_validate_psr4_paths_throws_exception_on_duplicate_paths_for_different_prefixes() { + $phpcsFile = $this->get_mock_file(); + $phpcsFile->config->basepath = self::DIRTY_BASEPATH; + + // PSR-4 paths contains the same path for two different prefixes. + $this->psr4_paths = [ + 'Plugin\Prefix\\' => 'src/', + 'Plugin\Prefix\Tests\\' => 'src/,tests/', + ]; + + $this->expectException( RuntimeException::class ); + $this->expectExceptionMessage( + 'Invalid value passed for `psr4_paths`. Multiple prefixes include the same path. Problem path: ' . self::CLEAN_BASEPATH . 'src/' + ); + + $this->validate_psr4_paths( $phpcsFile ); + } + + /** + * Test validating the $psr4_paths property + * + * @dataProvider data_validate_psr4_paths + * @covers ::validate_psr4_paths + * + * @param array $psr4_paths The initial input for the $psr4_paths property. + * @param array $expected The expected value for the validated property. + * + * @return void + */ + public function test_validate_psr4_paths( $psr4_paths, $expected ) { + $phpcsFile = $this->get_mock_file(); + $phpcsFile->config->basepath = self::CLEAN_BASEPATH; + + $this->psr4_paths = $psr4_paths; + + $this->validate_psr4_paths( $phpcsFile ); + + $this->assertSame( $psr4_paths, $this->previous_psr4_paths ); + $this->assertSame( $expected, $this->validated_psr4_paths ); + } + + /** + * Data provider. + * + * @see test_validate_psr4_paths() For the array format. + * + * @return array>> + */ + public static function data_validate_psr4_paths() { + return [ + 'empty array' => [ + 'psr4_paths' => [], + 'expected' => [], + ], + 'array with only empty values' => [ + 'psr4_paths' => [ + 'Plugin\PrefixA' => '', + 'Plugin\PrefixB' => ' ', + 'Plugin\PrefixC' => ' ', + 'Plugin\PrefixD' => ' ,, " ", \' \' ', + ], + 'expected' => [], + ], + + 'single prefix, single path; no trailing slash at end of prefix' => [ + 'psr4_paths' => [ + 'Plugin\Prefix' => 'src', + ], + 'expected' => [ + self::CLEAN_BASEPATH . 'src/' => 'Plugin\Prefix', + ], + ], + 'single prefix, single path; trailing slash at end of prefix' => [ + 'psr4_paths' => [ + 'Plugin\Prefix\\' => 'src', + ], + 'expected' => [ + self::CLEAN_BASEPATH . 'src/' => 'Plugin\Prefix', + ], + ], + 'single prefix, single path; double slashes within prefix' => [ + 'psr4_paths' => [ + 'Plugin\\Prefix\\Sub\\' => 'src', + ], + 'expected' => [ + self::CLEAN_BASEPATH . 'src/' => 'Plugin\Prefix\Sub', + ], + ], + + 'single prefix, single path; path has leading slash' => [ + 'psr4_paths' => [ + 'Plugin\Prefix' => '/src', + ], + 'expected' => [ + self::CLEAN_BASEPATH . 'src/' => 'Plugin\Prefix', + ], + ], + 'single prefix, single path; path has trailing slash' => [ + 'psr4_paths' => [ + 'Plugin\Prefix\\' => 'src/', + ], + 'expected' => [ + self::CLEAN_BASEPATH . 'src/' => 'Plugin\Prefix', + ], + ], + 'single prefix, single path; path has leading dot-slash and trailing slash' => [ + 'psr4_paths' => [ + 'Plugin\Prefix\\' => './src/', + ], + 'expected' => [ + self::CLEAN_BASEPATH . 'src/' => 'Plugin\Prefix', + ], + ], + + 'single prefix, multiple paths; simple comma-separated value' => [ + 'psr4_paths' => [ + 'Plugin\Prefix' => 'src,tests,config', + ], + 'expected' => [ + self::CLEAN_BASEPATH . 'src/' => 'Plugin\Prefix', + self::CLEAN_BASEPATH . 'tests/' => 'Plugin\Prefix', + self::CLEAN_BASEPATH . 'config/' => 'Plugin\Prefix', + ], + ], + 'single prefix, multiple paths; leading/trailing slash variations in comma-separated value' => [ + 'psr4_paths' => [ + 'Plugin\Prefix\\' => 'src/,./tests/,/config', + ], + 'expected' => [ + self::CLEAN_BASEPATH . 'src/' => 'Plugin\Prefix', + self::CLEAN_BASEPATH . 'tests/' => 'Plugin\Prefix', + self::CLEAN_BASEPATH . 'config/' => 'Plugin\Prefix', + ], + ], + 'single prefix, multiple paths; brackets around comma-separated value and spaces within' => [ + 'psr4_paths' => [ + 'Plugin\Prefix' => '[src, tests, config]', + ], + 'expected' => [ + self::CLEAN_BASEPATH . 'src/' => 'Plugin\Prefix', + self::CLEAN_BASEPATH . 'tests/' => 'Plugin\Prefix', + self::CLEAN_BASEPATH . 'config/' => 'Plugin\Prefix', + ], + ], + 'single prefix, multiple paths; brackets around comma-separated value and spaces and single quotes within' => [ + 'psr4_paths' => [ + 'Plugin\Prefix\\' => "[ 'src', 'tests' , ' config ']", + ], + 'expected' => [ + self::CLEAN_BASEPATH . 'src/' => 'Plugin\Prefix', + self::CLEAN_BASEPATH . 'tests/' => 'Plugin\Prefix', + self::CLEAN_BASEPATH . 'config/' => 'Plugin\Prefix', + ], + ], + 'single prefix, multiple paths; brackets around comma-separated value and spaces and double quotes within' => [ + 'psr4_paths' => [ + 'Plugin\Prefix' => '[ "src/", " ./tests", "/config/ " ]', + ], + 'expected' => [ + self::CLEAN_BASEPATH . 'src/' => 'Plugin\Prefix', + self::CLEAN_BASEPATH . 'tests/' => 'Plugin\Prefix', + self::CLEAN_BASEPATH . 'config/' => 'Plugin\Prefix', + ], + ], + 'single prefix, multiple paths; duplicate values' => [ + 'psr4_paths' => [ + 'Plugin\Prefix\\' => 'src, "./src" , /src/', + ], + 'expected' => [ + self::CLEAN_BASEPATH . 'src/' => 'Plugin\Prefix', + ], + ], + + 'multiple prefixes, variation of paths' => [ + 'psr4_paths' => [ + 'Plugin\PrefixA' => 'src', + 'Plugin\PrefixB\\' => 'tests/,config/', + 'Plugin\PrefixC' => '[ \'subA\sub\\\', "subB" , subC]', + 'Plugin\PrefixD\\' => ' ,, " ", \' \' ', + ], + 'expected' => [ + self::CLEAN_BASEPATH . 'src/' => 'Plugin\PrefixA', + self::CLEAN_BASEPATH . 'tests/' => 'Plugin\PrefixB', + self::CLEAN_BASEPATH . 'config/' => 'Plugin\PrefixB', + self::CLEAN_BASEPATH . 'subA/sub/' => 'Plugin\PrefixC', + self::CLEAN_BASEPATH . 'subB/' => 'Plugin\PrefixC', + self::CLEAN_BASEPATH . 'subC/' => 'Plugin\PrefixC', + ], + ], + ]; + } +} diff --git a/Yoast/Tests/Utils/PathHelperTest.php b/Yoast/Tests/Utils/PathHelperTest.php new file mode 100644 index 00000000..70aec34e --- /dev/null +++ b/Yoast/Tests/Utils/PathHelperTest.php @@ -0,0 +1,413 @@ +assertSame( $exp_absolute, PathHelper::normalize_absolute_path( $input ) ); + } + + /** + * Test normalizing a relative directory path. + * + * @dataProvider data_normalize_path + * @covers ::normalize_relative_path + * + * @param string $input The input string. + * @param string $unused Unused param. + * @param string $exp_relative The expected function output. + * + * @return void + */ + public function test_normalize_relative_path( $input, $unused, $exp_relative ) { + $this->assertSame( $exp_relative, PathHelper::normalize_relative_path( $input ) ); + } + + /** + * Data provider. + * + * @see test_normalize_absolute_path() For the array format. + * @see test_normalize_relative_path() For the array format. + * + * @return array> + */ + public static function data_normalize_path() { + return [ + 'path is dot' => [ + 'input' => '.', + 'exp_absolute' => './', + 'exp_relative' => './', + ], + 'path containing forward slashes only with trailing slash' => [ + 'input' => 'my/path/to/', + 'exp_absolute' => 'my/path/to/', + 'exp_relative' => 'my/path/to/', + ], + 'path containing forward slashes only without trailing slash' => [ + 'input' => 'my/path/to', + 'exp_absolute' => 'my/path/to/', + 'exp_relative' => 'my/path/to/', + ], + 'path containing forward slashes only with leading and trailing slash' => [ + 'input' => '/my/path/to/', + 'exp_absolute' => '/my/path/to/', + 'exp_relative' => 'my/path/to/', + ], + 'path containing back-slashes only with trailing slash' => [ + 'input' => 'my\path\to\\', + 'exp_absolute' => 'my/path/to/', + 'exp_relative' => 'my/path/to/', + ], + 'path containing back-slashes only without trailing slash' => [ + 'input' => 'my\path\to', + 'exp_absolute' => 'my/path/to/', + 'exp_relative' => 'my/path/to/', + ], + 'path containing back-slashes only with leading, no trailing slash' => [ + 'input' => '\my\path\to', + 'exp_absolute' => '/my/path/to/', + 'exp_relative' => 'my/path/to/', + ], + 'path containing a mix of forward and backslashes with leading and trailing slash' => [ + 'input' => '/my\path/to\\', + 'exp_absolute' => '/my/path/to/', + 'exp_relative' => 'my/path/to/', + ], + 'path containing a mix of forward and backslashes without trailing slash' => [ + 'input' => 'my\path/to', + 'exp_absolute' => 'my/path/to/', + 'exp_relative' => 'my/path/to/', + ], + ]; + } + + /** + * Test normalizing the directory separators in a path. + * + * @dataProvider data_normalize_directory_separators + * @covers ::normalize_directory_separators + * + * @param string $input The input string. + * @param string $expected The expected function output. + * + * @return void + */ + public function test_normalize_directory_separators( $input, $expected ) { + $this->assertSame( $expected, PathHelper::normalize_directory_separators( $input ) ); + } + + /** + * Data provider. + * + * @see test_normalize_directory_separators() For the array format. + * + * @return array> + */ + public static function data_normalize_directory_separators() { + return [ + 'path is dot' => [ + 'input' => '.', + 'expected' => '.', + ], + 'path containing forward slashes only' => [ + 'input' => 'my/path/to/', + 'expected' => 'my/path/to/', + ], + 'path containing back-slashes only' => [ + 'input' => 'my\path\to\\', + 'expected' => 'my/path/to/', + ], + 'path containing a mix of forward and backslashes' => [ + 'input' => 'my\path/to\\', + 'expected' => 'my/path/to/', + ], + ]; + } + + /** + * Test ensuring that a directory path does not start with a leading slash. + * + * @dataProvider data_remove_leading_slash + * @covers ::remove_leading_slash + * + * @param string $input The input string. + * @param string $expected The expected function output. + * + * @return void + */ + public function test_remove_leading_slash( $input, $expected ) { + $this->assertSame( $expected, PathHelper::remove_leading_slash( $input ) ); + } + + /** + * Data provider. + * + * @see test_remove_leading_slash() For the array format. + * + * @return array> + */ + public static function data_remove_leading_slash() { + return [ + 'path is dot' => [ + 'input' => '.', + 'expected' => '.', + ], + 'path with leading forward slash' => [ + 'input' => '/my/path/to/', + 'expected' => 'my/path/to/', + ], + 'path with leading back slash' => [ + 'input' => '\my\path\to', + 'expected' => 'my\path\to', + ], + 'path without leading slash' => [ + 'input' => 'my/path/to', + 'expected' => 'my/path/to', + ], + 'path to a file with leading slash' => [ + 'input' => '/file.ext', + 'expected' => 'file.ext', + ], + ]; + } + + /** + * Test ensuring that a directory path ends on a trailing slash. + * + * @dataProvider data_trailingslashit + * @covers ::trailingslashit + * + * @param string|bool $input The input string. + * @param string $expected The expected function output. + * + * @return void + */ + public function test_trailingslashit( $input, $expected ) { + $this->assertSame( $expected, PathHelper::trailingslashit( $input ) ); + } + + /** + * Data provider. + * + * @see test_trailingslashit() For the array format. + * + * @return array> + */ + public static function data_trailingslashit() { + return [ + 'path is non-string' => [ + 'input' => false, + 'expected' => '', + ], + 'path is empty string' => [ + 'input' => '', + 'expected' => '', + ], + 'path is dot' => [ + 'input' => '.', + 'expected' => './', + ], + 'path with trailing forward slash' => [ + 'input' => 'my/path/to/', + 'expected' => 'my/path/to/', + ], + 'path with trailing back slash' => [ + 'input' => 'my\path\to\\', + 'expected' => 'my\path\to/', + ], + 'path without trailing slash' => [ + 'input' => 'my/path/to', + 'expected' => 'my/path/to/', + ], + 'path to a file with an extension' => [ + 'input' => 'my/path/to/filename.ext', + 'expected' => 'my/path/to/filename.ext', + ], + 'path to a dot file' => [ + 'input' => 'my/path/to/.gitignore', + 'expected' => 'my/path/to/.gitignore', + ], + 'path ending on a dot' => [ + 'input' => 'my/path/to/dot.', + 'expected' => 'my/path/to/dot./', + ], + 'path with trailing forward slash and the last dir contains a dot' => [ + 'input' => 'my/path/to.ext/', + 'expected' => 'my/path/to.ext/', + ], + ]; + } + + /** + * Test stripping one path from the start of another path. + * + * @dataProvider data_strip_basepath + * @covers ::strip_basepath + * + * @param string $path The path of the file. + * @param string $basepath The base path to remove. + * @param string $expected The expected function output. + * + * @return void + */ + public function test_strip_basepath( $path, $basepath, $expected ) { + $this->assertSame( $expected, PathHelper::strip_basepath( $path, $basepath ) ); + } + + /** + * Data provider. + * + * @see test_strip_basepath() For the array format. + * + * @return array> + */ + public static function data_strip_basepath() { + return [ + 'basepath is empty' => [ + 'path' => '/my/path/too/', + 'basepath' => '', + 'expected' => '/my/path/too/', + ], + 'path NOT starting with other path' => [ + 'path' => '/my/path/too/', + 'basepath' => '/my/path/to/', + 'expected' => '/my/path/too/', + ], + 'path equal to other path, forward slashes' => [ + 'path' => '/my/path/to/', + 'basepath' => '/my/path/to/', + 'expected' => '.', + ], + 'path starting with other path, forward slashes' => [ + 'path' => '/my/path/to/some/sub/directory', + 'basepath' => '/my/path/to/', + 'expected' => 'some/sub/directory', + ], + 'path equal to other path, forward slashes, basepath does not end on slash' => [ + 'path' => '/my/path/to/', + 'basepath' => '/my/path/to', + 'expected' => '.', + ], + 'path starting with other path, forward slashes, basepath does not end on slash' => [ + 'path' => '/my/path/to/some/sub/file.ext', + 'basepath' => '/my/path/to', + 'expected' => 'some/sub/file.ext', + ], + 'path equal to other path, back slashes' => [ + 'path' => 'C:\my\path\to\\', + 'basepath' => 'C:\my\path\to\\', + 'expected' => '.', + ], + 'path starting with other path, back slashes' => [ + 'path' => 'C:\my\path\to\some\sub\directory', + 'basepath' => 'C:\my\path\to\\', + 'expected' => 'some\sub\directory', + ], + 'path equal to other path, back slashes, basepath does not end on slash' => [ + 'path' => 'C:\my\path\to\\', + 'basepath' => 'C:\my\path\to', + 'expected' => '.', + ], + 'path starting with other path, back slashes, basepath does not end on slash' => [ + 'path' => 'C:\my\path\to\some\sub\directory', + 'basepath' => 'C:\my\path\to', + 'expected' => 'some\sub\directory', + ], + ]; + } + + /** + * Test verifying whether a certain path starts with another path. + * + * @dataProvider data_path_starts_with + * @covers ::path_starts_with + * + * @param string $haystack Directory path to search in. + * @param string $needle Path the haystack path should start with. + * @param string $expected The expected function output. + * + * @return void + */ + public function test_path_starts_with( $haystack, $needle, $expected ) { + $this->assertSame( $expected, PathHelper::path_starts_with( $haystack, $needle ) ); + } + + /** + * Data provider. + * + * @see test_path_starts_with() For the array format. + * + * @return array> + */ + public static function data_path_starts_with() { + return [ + 'path equal to other path, forward slashes' => [ + 'haystack' => '/my/path/to/', + 'needle' => '/my/path/to/', + 'expected' => true, + ], + 'path starting with other path, forward slashes' => [ + 'haystack' => '/my/path/to/some/sub/directory', + 'needle' => '/my/path/to/', + 'expected' => true, + ], + 'path equal to other path, back slashes' => [ + 'haystack' => 'C:\my\path\to\\', + 'needle' => 'C:\my\path\to\\', + 'expected' => true, + ], + 'path starting with other path, back slashes' => [ + 'haystack' => 'C:\my\path\to\some\sub\directory', + 'needle' => 'C:\my\path\to\\', + 'expected' => true, + ], + 'path starting with other path, but slashes are different' => [ + 'haystack' => '\my\path\to\some\sub\directory', + 'needle' => '/my/path/to/', + 'expected' => false, + ], + 'path starting with other path, but case is different' => [ + 'haystack' => '/My/path/To/some/Sub/directory', + 'needle' => '/my/path/to/', + 'expected' => false, + ], + 'path NOT starting with other path, forward slashes' => [ + 'haystack' => '/my/path/too/some/sub/directory', + 'needle' => '/my/path/to/', + 'expected' => false, + ], + 'path NOT starting with other path, back slashes' => [ + 'haystack' => 'C:\my\path\too\some\sub\directory', + 'needle' => 'C:\my\path\to\\', + 'expected' => false, + ], + 'completely different paths' => [ + 'haystack' => '/your/subdir/', + 'needle' => 'my/path/to/', + 'expected' => false, + ], + ]; + } +} diff --git a/Yoast/Tests/Utils/PathValidationHelperTest.php b/Yoast/Tests/Utils/PathValidationHelperTest.php new file mode 100644 index 00000000..b464305a --- /dev/null +++ b/Yoast/Tests/Utils/PathValidationHelperTest.php @@ -0,0 +1,149 @@ +get_mock_file(); + + $this->assertSame( [], PathValidationHelper::relative_to_absolute( $phpcsFile, [ 'path' ] ) ); + } + + /** + * Test converting a set of relative paths to absolute paths when no paths where passed. + * + * @dataProvider data_relative_to_absolute_no_paths + * @covers ::relative_to_absolute + * + * @param array $input The input array. + * + * @return void + */ + public function test_relative_to_absolute_no_paths( $input ) { + $phpcsFile = $this->get_mock_file(); + $phpcsFile->config->basepath = self::DIRTY_BASEPATH; + + $this->assertSame( [], PathValidationHelper::relative_to_absolute( $phpcsFile, $input ) ); + } + + /** + * Data provider. + * + * @see test_relative_to_absolute_no_paths() For the array format. + * + * @return array>> + */ + public static function data_relative_to_absolute_no_paths() { + return [ + 'empty array' => [ + 'input' => [], + ], + 'array with only empty values' => [ + 'input' => [ + '', + ' ', + ' ', + ], + ], + ]; + } + + /** + * Test converting a set of relative paths to absolute paths. + * + * @dataProvider data_relative_to_absolute + * @covers ::relative_to_absolute + * + * @param array $input The input array. + * @param array $expected The expected function output. + * + * @return void + */ + public function test_relative_to_absolute( $input, $expected ) { + $phpcsFile = $this->get_mock_file(); + $phpcsFile->config->basepath = self::DIRTY_BASEPATH; + + $this->assertSame( $expected, PathValidationHelper::relative_to_absolute( $phpcsFile, $input ) ); + } + + /** + * Data provider. + * + * @see test_relative_to_absolute() For the array format. + * + * @return array>> + */ + public static function data_relative_to_absolute() { + return [ + 'all cases' => [ + 'input' => [ + '../walking/up/', + '/walking/../up/', + '/walking/up/../', + ' . ', + '.', + './', + '.\\', + './some/path', + './some/path/to/file.ext', + '/some/path', + '/some/path/to/file.ext', + 'some/path', + 'some/path/to/file.ext', + '\some\path', + '\some\path\to\file.ext', + '.\some\path', + '.\some\path\to\file.ext', + ], + 'expected' => [ + ' . ' => self::CLEAN_BASEPATH, + '.' => self::CLEAN_BASEPATH, + './' => self::CLEAN_BASEPATH, + '.\\' => self::CLEAN_BASEPATH, + './some/path' => self::CLEAN_BASEPATH . 'some/path/', + './some/path/to/file.ext' => self::CLEAN_BASEPATH . 'some/path/to/file.ext', + '/some/path' => self::CLEAN_BASEPATH . 'some/path/', + '/some/path/to/file.ext' => self::CLEAN_BASEPATH . 'some/path/to/file.ext', + 'some/path' => self::CLEAN_BASEPATH . 'some/path/', + 'some/path/to/file.ext' => self::CLEAN_BASEPATH . 'some/path/to/file.ext', + '\some\path' => self::CLEAN_BASEPATH . 'some/path/', + '\some\path\to\file.ext' => self::CLEAN_BASEPATH . 'some/path/to/file.ext', + '.\some\path' => self::CLEAN_BASEPATH . 'some/path/', + '.\some\path\to\file.ext' => self::CLEAN_BASEPATH . 'some/path/to/file.ext', + ], + ], + ]; + } +} diff --git a/Yoast/Tests/WhiteSpace/FunctionSpacingUnitTest.inc b/Yoast/Tests/WhiteSpace/FunctionSpacingUnitTest.inc index 5703b7bb..4729b406 100644 --- a/Yoast/Tests/WhiteSpace/FunctionSpacingUnitTest.inc +++ b/Yoast/Tests/WhiteSpace/FunctionSpacingUnitTest.inc @@ -88,3 +88,17 @@ $util->setLogger(new class { private function b(){} protected function c(){} }); + +enum MyTrait: int +{ + function func1() { + + }//end func1() + + + + function func3() { + + } + +} diff --git a/Yoast/Tests/WhiteSpace/FunctionSpacingUnitTest.inc.fixed b/Yoast/Tests/WhiteSpace/FunctionSpacingUnitTest.inc.fixed index 67ec6b30..1230ea2a 100644 --- a/Yoast/Tests/WhiteSpace/FunctionSpacingUnitTest.inc.fixed +++ b/Yoast/Tests/WhiteSpace/FunctionSpacingUnitTest.inc.fixed @@ -88,3 +88,15 @@ $util->setLogger(new class { protected function c(){} }); + +enum MyTrait: int +{ + + function func1() { + + }//end func1() + + function func3() { + + } +} diff --git a/Yoast/Tests/WhiteSpace/FunctionSpacingUnitTest.php b/Yoast/Tests/WhiteSpace/FunctionSpacingUnitTest.php index 228018bf..157a09cc 100644 --- a/Yoast/Tests/WhiteSpace/FunctionSpacingUnitTest.php +++ b/Yoast/Tests/WhiteSpace/FunctionSpacingUnitTest.php @@ -7,40 +7,41 @@ /** * Unit test class for the FunctionSpacing sniff. * - * @package Yoast\YoastCS + * @since 1.0.0 * - * @since 1.0.0 - * - * @covers YoastCS\Yoast\Sniffs\WhiteSpace\FunctionSpacingSniff + * @covers YoastCS\Yoast\Sniffs\WhiteSpace\FunctionSpacingSniff */ -class FunctionSpacingUnitTest extends AbstractSniffUnitTest { +final class FunctionSpacingUnitTest extends AbstractSniffUnitTest { /** * Returns the lines where errors should occur. * - * @return array => + * @return array Key is the line number, value is the number of expected errors. */ - public function getErrorList() { + public function getErrorList(): array { return [ - 31 => 1, - 33 => 1, - 39 => 1, - 46 => 1, - 53 => 2, - 57 => 1, - 60 => 1, - 74 => 1, - 87 => 2, - 88 => 1, + 31 => 1, + 33 => 1, + 39 => 1, + 46 => 1, + 53 => 2, + 57 => 1, + 60 => 1, + 74 => 1, + 87 => 2, + 88 => 1, + 94 => 1, + 96 => 1, + 102 => 1, ]; } /** * Returns the lines where warnings should occur. * - * @return array => + * @return array Key is the line number, value is the number of expected warnings. */ - public function getWarningList() { + public function getWarningList(): array { return []; } } diff --git a/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc b/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc deleted file mode 100644 index d4790b4a..00000000 --- a/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc +++ /dev/null @@ -1,22 +0,0 @@ -wp_json_encode( $thing ); // OK. -$json = MyNamespace\json_encode( $thing ); // OK. -$json = namespace\wp_json_encode( $thing ); // OK. - -// The sniff should trigger on these. -$json = json_encode( $thing ); // Error. -$json = wp_json_encode( $thing ); // Error. -return function_call(nested_call(json_encode( $thing ))); // Error. - -$json = json_encode ($value, $options); // Error, non-fixable. -$json = wp_json_encode( $data, $options, $depth ); // Error, non-fixable. - -namespace Yoast\CMS\Plugin\Dir; // Non-relevant parse error. - -$json = json_encode( $thing ); // Error. -$json = wp_json_encode( $thing ); // Error. diff --git a/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc.fixed b/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc.fixed deleted file mode 100644 index 6a16d0f5..00000000 --- a/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.inc.fixed +++ /dev/null @@ -1,22 +0,0 @@ -wp_json_encode( $thing ); // OK. -$json = MyNamespace\json_encode( $thing ); // OK. -$json = namespace\wp_json_encode( $thing ); // OK. - -// The sniff should trigger on these. -$json = WPSEO_Utils::format_json_encode( $thing ); // Error. -$json = WPSEO_Utils::format_json_encode( $thing ); // Error. -return function_call(nested_call(WPSEO_Utils::format_json_encode( $thing ))); // Error. - -$json = json_encode ($value, $options); // Error, non-fixable. -$json = wp_json_encode( $data, $options, $depth ); // Error, non-fixable. - -namespace Yoast\CMS\Plugin\Dir; // Non-relevant parse error. - -$json = \WPSEO_Utils::format_json_encode( $thing ); // Error. -$json = \WPSEO_Utils::format_json_encode( $thing ); // Error. diff --git a/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.php b/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.php deleted file mode 100644 index 702eade0..00000000 --- a/Yoast/Tests/Yoast/AlternativeFunctionsUnitTest.php +++ /dev/null @@ -1,43 +0,0 @@ - => - */ - public function getErrorList() { - return [ - 12 => 1, - 13 => 1, - 14 => 1, - 16 => 1, - 17 => 1, - 21 => 1, - 22 => 1, - ]; - } - - /** - * Returns the lines where warnings should occur. - * - * @return array => - */ - public function getWarningList() { - return []; - } -} diff --git a/Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.inc b/Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.inc new file mode 100644 index 00000000..4b6a254e --- /dev/null +++ b/Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.inc @@ -0,0 +1,52 @@ +wp_json_encode( $thing ); // OK. +$json = MyNamespace\json_encode( $thing ); // OK. +$json = namespace\wp_json_encode( $thing ); // OK. +$json = $obj?->wp_json_encode( $thing ); // OK. + +// The sniff should trigger on these. +$json = \json_encode( $thing, ); // Error. +$json = wp_json_encode( $thing ); // Error. +return function_call(nested_call(json_encode( $thing ))); // Error. + +$json = json_encode ($value, $options,); // Error, non-fixable. +$json = wp_json_encode( $data, $options, $depth ); // Error, non-fixable. + +namespace Yoast\CMS\Plugin\Dir; // Non-relevant parse error. + +$json = json_encode( $thing ); // Error. +$json = wp_json_encode( $thing ); // Error. + +// Safeguard that the leading \ does not get doubled. +$json = \json_encode( $thing ); // Error. +$json = \wp_json_encode( $thing ); // Error. + +// Safeguard that parameter unpacking gets recognized and makes the error non-fixable. +$json = json_encode (...$params); // Error, non-fixable. +$json = wp_json_encode( ...$params ); // Error, non-fixable. + +// Safeguard support for PHP 8.0+ function calls using named parameters. +$json = wp_json_encode(); // Error - not useful as required param is missing, but that's not the concern of this sniff. +$json = json_encode( something: $thing ); // Error, non-fixable - not useful as required param is missing, but that's not the concern of this sniff. +$json = \wp_json_encode( depth: 256, options: 0 ); // Error, non-fixable - not useful as required param is missing, but that's not the concern of this sniff. +$json = json_encode( depths: 256, value: $thing ); // Error, non-fixable - typo in optional param, but that's not the concern of this sniff. +$json = json_encode( value: ); // Error, - missing the actual parameter value, but that's not the concern of this sniff. + +$json = \json_encode( value: $thing ); // Error. +$json = wp_json_encode( data: $thing ); // Error. + +$json = json_encode( flags: 0, depth: 256, value: $thing ); // Error, non-fixable. +$json = wp_json_encode( depth: 256, options: 0, data: $thing ); // Error, non-fixable. + +// Safeguard handling of the functions when used as PHP 8.1+ first class callables. +call_user_func(json_encode(...), $something); // Error, non-fixable. +\call_user_func(\wp_json_encode(...), $something); // Error, non-fixable. + +// Live coding/parse error test. +// This must be the last test in the file. +$json = wp_json_encode( diff --git a/Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.inc.fixed b/Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.inc.fixed new file mode 100644 index 00000000..ac0724ba --- /dev/null +++ b/Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.inc.fixed @@ -0,0 +1,52 @@ +wp_json_encode( $thing ); // OK. +$json = MyNamespace\json_encode( $thing ); // OK. +$json = namespace\wp_json_encode( $thing ); // OK. +$json = $obj?->wp_json_encode( $thing ); // OK. + +// The sniff should trigger on these. +$json = WPSEO_Utils::format_json_encode( $thing, ); // Error. +$json = WPSEO_Utils::format_json_encode( $thing ); // Error. +return function_call(nested_call(WPSEO_Utils::format_json_encode( $thing ))); // Error. + +$json = json_encode ($value, $options,); // Error, non-fixable. +$json = wp_json_encode( $data, $options, $depth ); // Error, non-fixable. + +namespace Yoast\CMS\Plugin\Dir; // Non-relevant parse error. + +$json = \WPSEO_Utils::format_json_encode( $thing ); // Error. +$json = \WPSEO_Utils::format_json_encode( $thing ); // Error. + +// Safeguard that the leading \ does not get doubled. +$json = \WPSEO_Utils::format_json_encode( $thing ); // Error. +$json = \WPSEO_Utils::format_json_encode( $thing ); // Error. + +// Safeguard that parameter unpacking gets recognized and makes the error non-fixable. +$json = json_encode (...$params); // Error, non-fixable. +$json = wp_json_encode( ...$params ); // Error, non-fixable. + +// Safeguard support for PHP 8.0+ function calls using named parameters. +$json = \WPSEO_Utils::format_json_encode(); // Error - not useful as required param is missing, but that's not the concern of this sniff. +$json = json_encode( something: $thing ); // Error, non-fixable - not useful as required param is missing, but that's not the concern of this sniff. +$json = \wp_json_encode( depth: 256, options: 0 ); // Error, non-fixable - not useful as required param is missing, but that's not the concern of this sniff. +$json = json_encode( depths: 256, value: $thing ); // Error, non-fixable - typo in optional param, but that's not the concern of this sniff. +$json = \WPSEO_Utils::format_json_encode( data: ); // Error, - missing the actual parameter value, but that's not the concern of this sniff. + +$json = \WPSEO_Utils::format_json_encode( data: $thing ); // Error. +$json = \WPSEO_Utils::format_json_encode( data: $thing ); // Error. + +$json = json_encode( flags: 0, depth: 256, value: $thing ); // Error, non-fixable. +$json = wp_json_encode( depth: 256, options: 0, data: $thing ); // Error, non-fixable. + +// Safeguard handling of the functions when used as PHP 8.1+ first class callables. +call_user_func(json_encode(...), $something); // Error, non-fixable. +\call_user_func(\wp_json_encode(...), $something); // Error, non-fixable. + +// Live coding/parse error test. +// This must be the last test in the file. +$json = \WPSEO_Utils::format_json_encode( diff --git a/Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.php b/Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.php new file mode 100644 index 00000000..e42a8735 --- /dev/null +++ b/Yoast/Tests/Yoast/JsonEncodeAlternativeUnitTest.php @@ -0,0 +1,57 @@ + Key is the line number, value is the number of expected errors. + */ + public function getErrorList(): array { + return [ + 13 => 1, + 14 => 1, + 15 => 1, + 17 => 1, + 18 => 1, + 22 => 1, + 23 => 1, + 26 => 1, + 27 => 1, + 30 => 1, + 31 => 1, + 34 => 1, + 35 => 1, + 36 => 1, + 37 => 1, + 38 => 1, + 40 => 1, + 41 => 1, + 43 => 1, + 44 => 1, + 47 => 1, + 48 => 1, + 52 => 1, + ]; + } + + /** + * Returns the lines where warnings should occur. + * + * @return array Key is the line number, value is the number of expected warnings. + */ + public function getWarningList(): array { + return []; + } +} diff --git a/Yoast/Utils/CustomPrefixesTrait.php b/Yoast/Utils/CustomPrefixesTrait.php index 6b54fa07..e17f18d0 100644 --- a/Yoast/Utils/CustomPrefixesTrait.php +++ b/Yoast/Utils/CustomPrefixesTrait.php @@ -6,15 +6,12 @@ * Trait to add a custom `$prefixes` property to sniffs and utility functions * to validate the property value. * - * @package Yoast\YoastCS - * @author Juliette Reinders Folmer - * - * @since 2.0.0 + * @since 2.0.0 */ trait CustomPrefixesTrait { /** - * The prefix which are allowed to be used. + * The prefixes which are allowed to be used. * * The prefix(es) should be in the exact case as expected. * @@ -24,16 +21,16 @@ trait CustomPrefixesTrait { * hook names are being deprecated and the new hooks put in place -, * two prefixes (old and new) will be allowed. * At a future point in time, this property should be changed - * to allow only a single string.}} + * to allow only a single string.} * - * @var string[]|string + * @var array */ public $prefixes = []; /** * Target prefixes after validation. * - * @var string[] + * @var array */ protected $validated_prefixes = []; @@ -42,7 +39,7 @@ trait CustomPrefixesTrait { * * Prevents having to do the same prefix validation over and over again. * - * @var string[] + * @var array */ protected $previous_prefixes = []; @@ -55,7 +52,7 @@ trait CustomPrefixesTrait { * * @return void */ - protected function validate_prefixes() { + final protected function validate_prefixes() { if ( $this->previous_prefixes === $this->prefixes ) { return; } @@ -97,9 +94,9 @@ protected function validate_prefixes() { /** * Overloadable method to do custom prefix filtering prior to validation. * - * @param string[] $prefixes The unvalidated prefixes. + * @param array $prefixes The unvalidated prefixes. * - * @return string[] + * @return array */ protected function filter_prefixes( $prefixes ) { return $prefixes; @@ -108,11 +105,11 @@ protected function filter_prefixes( $prefixes ) { /** * Filter out all prefixes which don't contain a namespace separator. * - * @param string[] $prefixes The unvalidated prefixes. + * @param array $prefixes The unvalidated prefixes. * - * @return string[] + * @return array */ - protected function filter_allow_only_namespace_prefixes( $prefixes ) { + final protected function filter_allow_only_namespace_prefixes( $prefixes ) { $filtered = []; foreach ( $prefixes as $prefix ) { if ( \strpos( $prefix, '\\' ) === false ) { @@ -128,11 +125,11 @@ protected function filter_allow_only_namespace_prefixes( $prefixes ) { /** * Filter out all prefixes which only contain lowercase characters. * - * @param string[] $prefixes The unvalidated prefixes. + * @param array $prefixes The unvalidated prefixes. * - * @return string[] + * @return array */ - protected function filter_exclude_lowercase_prefixes( $prefixes ) { + final protected function filter_exclude_lowercase_prefixes( $prefixes ) { $filtered = []; foreach ( $prefixes as $prefix ) { if ( \strtolower( $prefix ) === $prefix ) { diff --git a/Yoast/Utils/PSR4PathsTrait.php b/Yoast/Utils/PSR4PathsTrait.php new file mode 100644 index 00000000..d40f5451 --- /dev/null +++ b/Yoast/Utils/PSR4PathsTrait.php @@ -0,0 +1,173 @@ + + * + * + * + * + * + * + * + * + * + * ``` + * + * Note: paths are handled case-sensitively! + * + * @var array Key should be the prefix, value a comma-separated list of relative paths. + */ + public $psr4_paths = []; + + /** + * Cache of previously set list of psr4 paths. + * + * Prevents having to do the same path validation over and over again. + * + * @var array + */ + private $previous_psr4_paths = []; + + /** + * Validated & cleaned up list of absolute paths to the directories expecting PSR-4 file names + * with their associated prefixes. + * + * @var array Key is the absolute path, value the applicable prefix without trailing slash. + */ + private $validated_psr4_paths = []; + + /** + * Check if the file is in one of the PSR4 directories. + * + * @param File $phpcsFile The file being scanned. + * @param string $path_to_file Optional The absolute path to the file currently being examined. + * If not provided, the file name will be retrieved from the File object. + * + * @return bool + */ + final protected function is_in_psr4_path( File $phpcsFile, $path_to_file = '' ) { + return \is_array( $this->get_psr4_info( $phpcsFile, $path_to_file ) ); + } + + /** + * Retrieve all applicable information for a PSR-4 path. + * + * @param File $phpcsFile The file being scanned. + * @param string $path_to_file Optional The absolute path to the file currently being examined. + * If not provided, the file name will be retrieved from the File object. + * + * @return array|false Array with information about the PSR-4 path. Otherwise FALSE. + */ + final protected function get_psr4_info( File $phpcsFile, $path_to_file = '' ) { + if ( $path_to_file === '' ) { + $path_to_file = TextStrings::stripQuotes( $phpcsFile->getFileName() ); + if ( $path_to_file === 'STDIN' ) { + return false; + } + } + + $this->validate_psr4_paths( $phpcsFile ); + if ( empty( $this->validated_psr4_paths ) ) { + return false; + } + + $path_to_file = PathHelper::normalize_absolute_path( $path_to_file ); + + foreach ( $this->validated_psr4_paths as $psr4_path => $prefix ) { + $remainder = PathHelper::strip_basepath( $path_to_file, $psr4_path ); + if ( $remainder === $path_to_file ) { + // Nothing was stripped, so this wasn't a match. + continue; + } + + return [ + 'prefix' => $prefix, + 'basepath' => $psr4_path, + 'relative' => \dirname( $remainder ), + ]; + } + + return false; + } + + /** + * Validate the list of PSR-4 paths passed from a custom ruleset. + * + * This will only need to be done once in a normal PHPCS run, though for + * tests the function may be called multiple times. + * + * @param File $phpcsFile The file being scanned. + * + * @return void + * + * @throws RuntimeException When the `psr4_paths` array is missing keys. + * @throws RuntimeException When the `psr4_paths` array contains duplicate paths in multiple entries. + */ + private function validate_psr4_paths( File $phpcsFile ) { + // The basepath check needs to be done first as otherwise the previous/current comparison would be broken. + if ( ! isset( $phpcsFile->config->basepath ) ) { + // Only relevant for the tests: make sure previously set validated paths are cleared out. + $this->validated_psr4_paths = []; + + // No use continuing as we can't turn relative paths into absolute paths. + return; + } + + if ( $this->previous_psr4_paths === $this->psr4_paths ) { + return; + } + + // Set the cache *before* validation so as to not break the above compare. + $this->previous_psr4_paths = $this->psr4_paths; + + $validated_paths = []; + + foreach ( $this->psr4_paths as $prefix => $paths ) { + if ( \is_string( $prefix ) === false || $prefix === '' ) { + throw new RuntimeException( + 'Invalid value passed for `psr4_paths`. Path "' . $paths . '" is not associated with a namespace prefix' + ); + } + + $prefix = \rtrim( $prefix, '\\' ); + + $paths = \rtrim( \ltrim( $paths, '[' ), ']' ); // Trim off potential [] copied over from Composer.json. + $paths = \explode( ',', $paths ); + $paths = \array_map( 'trim', $paths ); + $paths = \array_map( [ TextStrings::class, 'stripQuotes' ], $paths ); // Trim off potential quotes around the paths copied over. + $paths = \array_map( 'trim', $paths ); + $paths = PathValidationHelper::relative_to_absolute( $phpcsFile, $paths ); + $paths = \array_unique( $paths ); // Filter out multiple of the same paths for the same prefix. + + foreach ( $paths as $path ) { + if ( isset( $validated_paths[ $path ] ) ) { + throw new RuntimeException( + 'Invalid value passed for `psr4_paths`. Multiple prefixes include the same path. Problem path: ' . $path + ); + } + + $validated_paths[ $path ] = $prefix; + } + } + + // Set the validated value. + $this->validated_psr4_paths = $validated_paths; + } +} diff --git a/Yoast/Utils/PathHelper.php b/Yoast/Utils/PathHelper.php new file mode 100644 index 00000000..866d41d0 --- /dev/null +++ b/Yoast/Utils/PathHelper.php @@ -0,0 +1,147 @@ + $relative_paths Array of relative paths which should become absolute paths. + * Paths are expected to be relative to the "basepath" setting. + * + * @return array Array of absolute paths or an empty array if the conversion could not be executed. + * The array will contain the original relative paths as the keys and the absolute paths + * as the values. + * Note: multiple relative paths may result in the same absolute path. + * The values are not guaranteed to be unique! + */ + public static function relative_to_absolute( File $phpcsFile, array $relative_paths ) { + $absolute = []; + + if ( ! isset( $phpcsFile->config->basepath ) ) { + // No use continuing as we can't turn relative paths into absolute paths. + return $absolute; + } + + $base_path = PathHelper::normalize_absolute_path( $phpcsFile->config->basepath ); + + foreach ( $relative_paths as $path ) { + $result_path = \trim( $path ); + $result_path = PathHelper::normalize_relative_path( $result_path ); + + if ( $result_path === '' ) { + continue; + } + + if ( \strpos( $result_path, '..' ) !== false ) { + // Ignore paths containing path walking. + continue; + } + + if ( $result_path === './' ) { + $absolute[ $path ] = $base_path; + continue; + } + + if ( \strpos( $result_path, './' ) === 0 ) { + $result_path = \substr( $result_path, 2 ); + } + + $absolute[ $path ] = $base_path . $result_path; + } + + return $absolute; + } +} diff --git a/Yoast/ruleset.xml b/Yoast/ruleset.xml index 084e6b76..344b7769 100644 --- a/Yoast/ruleset.xml +++ b/Yoast/ruleset.xml @@ -6,7 +6,7 @@ @@ -18,6 +18,22 @@ */node_modules/* */vendor(_prefixed)?/* + + */wp-content/plugins/* + + + + + ./../autoload-bootstrap.php + + + + - + - - @@ -66,13 +89,13 @@ - + - + + + + + + + + + error @@ -105,21 +137,16 @@ - - + + + - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + - - - - - - - - - @@ -160,6 +215,12 @@ + + + + + + @@ -169,43 +230,264 @@ - - - + + - - - + + - - - - + + - - - - - - + + + + + + + + + + + + + - - - + + + + + + + + + + + + + + + - + + + + + + + + - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + 5 + + + 5 + + + + + 5 + + + 5 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + */tests/*\.php$ + + + + + + - */index\.php + */index\.php$ - */index\.php + */index\.php$ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + 5 + + + + + + + + + + + + + */tests/*\.php$ + + + + + */tests/*\.php$ + + + + + */tests(/*)?/Doubles/*\.php$ + + + + + */tests/*\.php$ + + + + + + + + */tests/*\.php$ + + */tests(/*)?/Doubles/*\.php$ diff --git a/autoload-bootstrap.php b/autoload-bootstrap.php new file mode 100644 index 00000000..561550fe --- /dev/null +++ b/autoload-bootstrap.php @@ -0,0 +1,23 @@ +=5.4", - "squizlabs/php_codesniffer": "^3.7.2", - "wp-coding-standards/wpcs": "^2.3.0", - "phpcompatibility/phpcompatibility-wp": "^2.1.4", - "dealerdirect/phpcodesniffer-composer-installer": "^0.6.2 || ^0.7 || ^1.0", + "php": ">=7.2", + "ext-tokenizer": "*", + "automattic/vipwpcs": "^3.0.0", + "php-parallel-lint/php-console-highlighter": "^1.0.0", "php-parallel-lint/php-parallel-lint": "^1.3.2", - "php-parallel-lint/php-console-highlighter": "^1.0.0" + "phpcompatibility/phpcompatibility-wp": "^2.1.4", + "phpcsstandards/phpcsextra": "^1.2.1", + "phpcsstandards/phpcsutils": "^1.0.9", + "sirbrillig/phpcs-variable-analysis": "^2.11.17", + "slevomat/coding-standard": "^8.14.0", + "squizlabs/php_codesniffer": "^3.8.0", + "wp-coding-standards/wpcs": "^3.0.1" }, "require-dev": { "phpcompatibility/php-compatibility": "^9.3.5", - "roave/security-advisories": "dev-master", - "phpunit/phpunit": "^4.0 || ^5.0 || ^6.0 || ^7.0", - "phpcsstandards/phpcsdevtools": "^1.2.0" + "phpcsstandards/phpcsdevtools": "^1.2.1", + "phpunit/phpunit": "^8.0 || ^9.0", + "roave/security-advisories": "dev-master" }, + "minimum-stability": "dev", + "prefer-stable": true, "config": { "allow-plugins": { "dealerdirect/phpcodesniffer-composer-installer": true } }, - "minimum-stability": "dev", - "prefer-stable": true, "scripts": { - "config-yoastcs" : [ - "Dealerdirect\\Composer\\Plugin\\Installers\\PHPCodeSniffer\\Plugin::run", - "@php ./vendor/squizlabs/php_codesniffer/bin/phpcs --config-set default_standard Yoast" - ], "lint": [ "@php ./vendor/php-parallel-lint/php-parallel-lint/parallel-lint . -e php --show-deprecated --exclude vendor --exclude .git" ], "check-cs": [ - "@php ./vendor/squizlabs/php_codesniffer/bin/phpcs --runtime-set testVersion 5.4-" + "@php ./vendor/squizlabs/php_codesniffer/bin/phpcs" ], "fix-cs": [ "@php ./vendor/squizlabs/php_codesniffer/bin/phpcbf" ], "test": [ - "@php ./vendor/phpunit/phpunit/phpunit --filter Yoast --bootstrap=\"./vendor/squizlabs/php_codesniffer/tests/bootstrap.php\" ./vendor/squizlabs/php_codesniffer/tests/AllTests.php" + "@php ./vendor/phpunit/phpunit/phpunit --filter Yoast ./vendor/squizlabs/php_codesniffer/tests/AllTests.php --no-coverage" + ], + "coverage": [ + "@php ./vendor/phpunit/phpunit/phpunit --filter Yoast ./vendor/squizlabs/php_codesniffer/tests/AllTests.php" ], "check-complete": [ "@php ./vendor/phpcsstandards/phpcsdevtools/bin/phpcs-check-feature-completeness ./Yoast" ] + }, + "scripts-descriptions": { + "lint": "Check the PHP files for parse errors.", + "check-cs": "Check the PHP files for code style violations and best practices.", + "fix-cs": "Auto-fix code style violations in the PHP files.", + "test": "Run the unit tests without code coverage.", + "coverage": "Run the unit tests with code coverage.", + "check-complete": "Check if all the sniffs have tests and XML documentation." } } diff --git a/phpstan.neon.dist b/phpstan.neon.dist new file mode 100644 index 00000000..1795848f --- /dev/null +++ b/phpstan.neon.dist @@ -0,0 +1,102 @@ +parameters: + phpVersion: 70200 + level: 9 + paths: + - Yoast + bootstrapFiles: + - phpunit-bootstrap.php + scanDirectories: + - vendor/wp-coding-standards/wpcs/WordPress +# treatPhpDocTypesAsCertain: false +# reportUnmatchedIgnoredErrors: false + dynamicConstantNames: + - YOASTCS_ABOVE_THRESHOLD + + ignoreErrors: + # Level 3 + - + # This is an issue with the PHPCS docs. Can't be helped. + message: '`^Property PHP_CodeSniffer\\Config::\$basepath \(string\) does not accept null\.$`' + path: Yoast/Tests/Utils/PSR4PathsTraitTest.php + count: 1 + - + # This is a test specifically checking the handling when an invalid value is passed, so this is okay. + message: '`^Property YoastCS\\Yoast\\Tests\\Utils\\PSR4PathsTraitTest::\$psr4_paths \(array\\) does not accept array\\.$`' + path: Yoast/Tests/Utils/PSR4PathsTraitTest.php + count: 1 + + # Level 4 + - + # Bug in PHPStan: PHPStan doesn't seem to like uninitialized properties... + message: '`^Property \S+Sniff::\$target_paths \(array\) in isset\(\) is not nullable\.$`' + path: Yoast/Sniffs/Files/TestDoublesSniff.php + count: 1 + - + # Defensive coding in the PSR4PathsTrait as the property value is user provided via the ruleset. This is okay. + message: '`^Strict comparison using === between true and false will always evaluate to false\.$`' + paths: + - Yoast/Sniffs/Files/FileNameSniff.php + - Yoast/Sniffs/NamingConventions/NamespaceNameSniff.php + - Yoast/Tests/Utils/PSR4PathsTraitTest.php + + # Level 5 + # We're not using strict types, so this will be juggled without any issues. + - '#^Parameter \#3 \$value of method \S+File::recordMetric\(\) expects string, \(?(float|int|bool)(<[^>]+>)?(\|(float|int|bool)(<[^>]+>)?)*\)? given\.$#' + + # Level 7 + - + # False positive: the explode will never return `false` as it is never given an empty string separator. + message: '`^Parameter #2 \$array of function array_map expects array, array\|false given\.$`' + path: Yoast/Sniffs/Commenting/CoversTagSniff.php + count: 1 + - + # False positive: the preg_replace will never return `null` as the regex is valid. + message: '`^Parameter #1 \$str of function strtolower expects string, string\|null given\.$`' + path: Yoast/Sniffs/Files/FileNameSniff.php + count: 1 + - + # False positive: the preg_replace will never return `null` as the regex is valid. + message: '`^Parameter #2 \$str of function explode expects string, string\|null given\.$`' + path: Yoast/Sniffs/NamingConventions/NamespaceNameSniff.php + count: 1 + - + # False positive: the passed value will only ever be an integer, PHPStan just doesn't know the shape of the array. + message: '`^Binary operation "\+" between int\|string and 1 results in an error\.$`' + count: 2 + path: Yoast/Sniffs/NamingConventions/ValidHookNameSniff.php + - + # False positive: the passed value will only ever be an integer, PHPStan just doesn't know the shape of the array. + message: '`^Parameter #2 \$start of method PHP_CodeSniffer\\Files\\File::findNext\(\) expects int, int\|string given\.$`' + count: 2 + path: Yoast/Sniffs/NamingConventions/ValidHookNameSniff.php + - + # False positive: the passed value will only ever be an integer, PHPStan just doesn't know the shape of the array. + message: '`^Parameter #1 \$stackPtr of method PHP_CodeSniffer\\Fixer::replaceToken\(\) expects int, int\|string given\.$`' + path: Yoast/Sniffs/Yoast/JsonEncodeAlternativeSniff.php + count: 1 + - + # This is a test specifically checking type handling, so this is okay. + message: '`^Parameter #1 \$path of static method YoastCS\\Yoast\\Utils\\PathHelper::trailingslashit\(\) expects string, bool\|string given\.$`' + path: Yoast/Tests/Utils/PathHelperTest.php + count: 1 + + # Not a real issue (x 4), PHPstan just doesn't know the format of the array well enough. + - + message: '`^Binary operation "\+" between int\|string and 1 results in an error\.$`' + count: 2 + path: Yoast/Sniffs/Tools/BrainMonkeyRaceConditionSniff.php + + - + message: '`^Parameter #2 \$start of method PHP_CodeSniffer\\Files\\File::findNext\(\) expects int, int\|string given\.$`' + count: 2 + path: Yoast/Sniffs/Tools/BrainMonkeyRaceConditionSniff.php + + - + message: '`^Binary operation "\+" between int\|string and 1 results in an error\.$`' + count: 1 + path: Yoast/Sniffs/Yoast/JsonEncodeAlternativeSniff.php + + - + message: '`^Parameter #2 \$start of method PHP_CodeSniffer\\Files\\File::findNext\(\) expects int, int\|string given\.$`' + count: 1 + path: Yoast/Sniffs/Yoast/JsonEncodeAlternativeSniff.php diff --git a/phpunit-bootstrap.php b/phpunit-bootstrap.php new file mode 100644 index 00000000..50bc6a59 --- /dev/null +++ b/phpunit-bootstrap.php @@ -0,0 +1,84 @@ + true, +]; + +$all_standards = Standards::getInstalledStandards(); +$all_standards[] = 'Generic'; + +$standards_to_ignore = []; +foreach ( $all_standards as $standard ) { + if ( isset( $yoast_standards[ $standard ] ) === true ) { + continue; + } + + $standards_to_ignore[] = $standard; +} + +$standards_to_ignore_string = implode( ',', $standards_to_ignore ); + +// phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.runtime_configuration_putenv -- This is not production, but test code. +putenv( "PHPCS_IGNORE_TESTS={$standards_to_ignore_string}" ); + +// Clean up. +unset( $phpcs_dir, $composer_phpcs_path, $all_standards, $standards_to_ignore, $standard, $standards_to_ignore_string ); diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 0aa18327..362273a1 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -1,30 +1,34 @@ - ./Yoast/Tests/ + ./Yoast/Tests/ + Yoast/Reports/ Yoast/Sniffs/ + Yoast/Utils/ - - - + + + +