-
Notifications
You must be signed in to change notification settings - Fork 3.3k
Tests / Build Scripts: Configure PHPStan level 0 #10419
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: trunk
Are you sure you want to change the base?
Changes from all commits
0308f9e
b427ca6
a741131
88c5426
03b4080
cd1149a
aec7e74
acfeb8d
d429181
de6d304
d43edb1
8e5e8b0
75c8c54
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,96 @@ | ||
| name: PHPStan Static Analysis | ||
|
|
||
| on: | ||
| # PHPStan testing was introduced in @todo. | ||
| push: | ||
| branches: | ||
| - trunk | ||
| - '[7-9].[0-9]' | ||
| tags: | ||
| - '[7-9].[0-9]' | ||
| - '[7-9]+.[0-9].[0-9]+' | ||
| pull_request: | ||
| branches: | ||
| - trunk | ||
| - '[7-9].[0-9]' | ||
| paths: | ||
| # This workflow only scans PHP files. | ||
| - '**.php' | ||
| # These files configure Composer. Changes could affect the outcome. | ||
| - 'composer.*' | ||
| # These files configure PHPStan. Changes could affect the outcome. | ||
| - 'phpstan.neon.dist' | ||
| - 'tests/phpstan/base.neon' | ||
| # Confirm any changes to relevant workflow files. | ||
| - '.github/workflows/php-static-analysis.yml' | ||
| - '.github/workflows/reusable-php-static-analysis.yml' | ||
| workflow_dispatch: | ||
|
|
||
| # Cancels all previous workflow runs for pull requests that have not completed. | ||
| concurrency: | ||
| # The concurrency group contains the workflow name and the branch name for pull requests | ||
| # or the commit hash for any other events. | ||
| group: ${{ github.workflow }}-${{ github.event_name == 'pull_request' && github.head_ref || github.sha }} | ||
| cancel-in-progress: true | ||
|
|
||
| # Disable permissions for all available scopes by default. | ||
| # Any needed permissions should be configured at the job level. | ||
| permissions: {} | ||
|
|
||
| jobs: | ||
| # Runs PHPStan Static Analysis. | ||
| phpstan: | ||
| name: PHP static analysis | ||
| uses: ./.github/workflows/reusable-php-static-analysis.yml | ||
| permissions: | ||
| contents: read | ||
| if: ${{ github.repository == 'WordPress/wordpress-develop' || ( github.event_name == 'pull_request' && github.actor != 'dependabot[bot]' ) }} | ||
|
|
||
| slack-notifications: | ||
| name: Slack Notifications | ||
| uses: ./.github/workflows/slack-notifications.yml | ||
| permissions: | ||
| actions: read | ||
| contents: read | ||
| needs: [ phpstan ] | ||
| if: ${{ github.repository == 'WordPress/wordpress-develop' && github.event_name != 'pull_request' && always() }} | ||
| with: | ||
| calling_status: ${{ contains( needs.*.result, 'cancelled' ) && 'cancelled' || contains( needs.*.result, 'failure' ) && 'failure' || 'success' }} | ||
| secrets: | ||
| SLACK_GHA_SUCCESS_WEBHOOK: ${{ secrets.SLACK_GHA_SUCCESS_WEBHOOK }} | ||
| SLACK_GHA_CANCELLED_WEBHOOK: ${{ secrets.SLACK_GHA_CANCELLED_WEBHOOK }} | ||
| SLACK_GHA_FIXED_WEBHOOK: ${{ secrets.SLACK_GHA_FIXED_WEBHOOK }} | ||
| SLACK_GHA_FAILURE_WEBHOOK: ${{ secrets.SLACK_GHA_FAILURE_WEBHOOK }} | ||
|
|
||
| failed-workflow: | ||
| name: Failed workflow tasks | ||
| runs-on: ubuntu-24.04 | ||
| permissions: | ||
| actions: write | ||
| needs: [ slack-notifications ] | ||
| if: | | ||
| always() && | ||
| github.repository == 'WordPress/wordpress-develop' && | ||
| github.event_name != 'pull_request' && | ||
| github.run_attempt < 2 && | ||
| ( | ||
| contains( needs.*.result, 'cancelled' ) || | ||
| contains( needs.*.result, 'failure' ) | ||
| ) | ||
|
|
||
| steps: | ||
| - name: Dispatch workflow run | ||
| uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1 | ||
| with: | ||
| retries: 2 | ||
| retry-exempt-status-codes: 418 | ||
| script: | | ||
| github.rest.actions.createWorkflowDispatch({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| workflow_id: 'failed-workflow.yml', | ||
| ref: 'trunk', | ||
| inputs: { | ||
| run_id: `${context.runId}`, | ||
| } | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,109 @@ | ||
| ## | ||
| # A reusable workflow that runs PHP Static Analysis tests. | ||
| ## | ||
| name: PHP Static Analysis | ||
|
|
||
| on: | ||
| workflow_call: | ||
| inputs: | ||
| php-version: | ||
| description: 'The PHP version to use.' | ||
| required: false | ||
| type: 'string' | ||
| default: 'latest' | ||
|
|
||
| # Disable permissions for all available scopes by default. | ||
| # Any needed permissions should be configured at the job level. | ||
| permissions: {} | ||
|
|
||
| jobs: | ||
| # Runs PHP static analysis tests. | ||
| # | ||
| # Violations are reported inline with annotations. | ||
| # | ||
| # Performs the following steps: | ||
| # - Checks out the repository. | ||
| # - Sets up PHP. | ||
| # - Logs debug information. | ||
| # - Installs Composer dependencies. | ||
| # - Configures caching for PHP static analysis scans. | ||
| # - Make Composer packages available globally. | ||
| # - Runs PHPStan static analysis (with Pull Request annotations). | ||
| # - Saves the PHPStan result cache. | ||
| # - Ensures version-controlled files are not modified or deleted. | ||
| phpstan: | ||
| name: Run PHP static analysis | ||
| runs-on: ubuntu-24.04 | ||
| permissions: | ||
| contents: read | ||
| timeout-minutes: 20 | ||
|
|
||
| steps: | ||
| - name: Checkout repository | ||
| uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 | ||
| with: | ||
| show-progress: ${{ runner.debug == '1' && 'true' || 'false' }} | ||
| persist-credentials: false | ||
|
|
||
| - name: Set up Node.js | ||
| uses: actions/setup-node@2028fbc5c25fe9cf00d9f06a71cc4710d4507903 # v6.0.0 | ||
| with: | ||
| node-version-file: '.nvmrc' | ||
| cache: npm | ||
|
|
||
| - name: Set up PHP | ||
| uses: shivammathur/setup-php@20529878ed81ef8e78ddf08b480401e6101a850f # v2.35.3 | ||
| with: | ||
| php-version: ${{ inputs.php-version }} | ||
| coverage: none | ||
| tools: cs2pr | ||
|
|
||
| # This date is used to ensure that the Composer cache is cleared at least once every week. | ||
| # http://man7.org/linux/man-pages/man1/date.1.html | ||
| - name: "Get last Monday's date" | ||
| id: get-date | ||
| run: echo "date=$(/bin/date -u --date='last Mon' "+%F")" >> "$GITHUB_OUTPUT" | ||
|
|
||
| - name: General debug information | ||
| run: | | ||
| npm --version | ||
| node --version | ||
| composer --version | ||
|
|
||
| # Since Composer dependencies are installed using `composer update` and no lock file is in version control, | ||
| # passing a custom cache suffix ensures that the cache is flushed at least once per week. | ||
| - name: Install Composer dependencies | ||
| uses: ramsey/composer-install@3cf229dc2919194e9e36783941438d17239e8520 # v3.1.1 | ||
| with: | ||
| custom-cache-suffix: ${{ steps.get-date.outputs.date }} | ||
|
|
||
| - name: Make Composer packages available globally | ||
| run: echo "${PWD}/vendor/bin" >> "$GITHUB_PATH" | ||
|
|
||
| - name: Install npm dependencies | ||
| run: npm ci | ||
|
|
||
| - name: Build WordPress | ||
| run: npm run build:dev | ||
|
|
||
| - name: Cache PHP Static Analysis scan cache | ||
| uses: actions/cache@5a3ec84eff668545956fd18022155c47e93e2684 # v4.2.3 | ||
| with: | ||
| path: .cache # This is defined in the base.neon file. | ||
| key: "phpstan-result-cache-${{ github.run_id }}" | ||
| restore-keys: | | ||
| phpstan-result-cache- | ||
|
|
||
| - name: Run PHP static analysis tests | ||
| id: phpstan | ||
| run: phpstan analyse -vvv --error-format=checkstyle | cs2pr | ||
|
|
||
| - name: "Save result cache" | ||
| uses: actions/cache@0400d5f644dc74513175e3cd8d07132dd4860809 # v4.2.4 | ||
| if: ${{ !cancelled() }} | ||
| with: | ||
| path: .cache | ||
| key: "phpstan-result-cache-${{ github.run_id }}" | ||
|
|
||
| - name: Ensure version-controlled files are not modified or deleted | ||
| run: git diff --exit-code |
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -23,6 +23,7 @@ | |||||||||
| "squizlabs/php_codesniffer": "3.13.5", | ||||||||||
| "wp-coding-standards/wpcs": "~3.3.0", | ||||||||||
| "phpcompatibility/phpcompatibility-wp": "~2.1.3", | ||||||||||
| "phpstan/phpstan": "~2.1.33", | ||||||||||
|
||||||||||
| "phpstan/phpstan": "~2.1.33", | |
| "phpstan/phpstan": "~2.1.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@westonruter what are your thoughts on this one?
I definitely think we should pin at the version we commit, but I wouldn't want to Semver because contextually "nonbreaking enhancements" are breaking from an implementation POV if they create a new quality gate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense to me. So switching to ~2.1.0 would keep it at 2.1.x. This seems necessary because there is no composer.lock, which actually is curious since we package-lock.json. If we had a composer.lock then we'd be free to use ^2.1. But I suppose can't use it because of the different versions of PHP which may end up getting used when doing composer install.
So yeah, I guess go with ~2.1.0 and not ^2.1. When PHPStan 2.2 comes out, we'll have to manually upgrade.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😛
| "analyse": "@php ./vendor/bin/phpstan analyse --memory-limit=2G", | |
| "analyze": "@php ./vendor/bin/phpstan analyse --memory-limit=2G", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But I wonder, why not just:
| "analyse": "@php ./vendor/bin/phpstan analyse --memory-limit=2G", | |
| "phpstan": "@php ./vendor/bin/phpstan analyse --memory-limit=2G", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be closer to test:php:stan.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm torn. I prefer the US spelling, but the name of the PHPStan command is analyse and the muscle memory of folks who use PHPStan. 2 years ago this would have been a nit, but it also affects LLMs since the inconsistency can get in the way of their inference
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about just phpstan then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a compat script which isn't a verb already.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -126,6 +126,7 @@ | |
| "env:logs": "node ./tools/local-env/scripts/docker.js logs", | ||
| "env:pull": "node ./tools/local-env/scripts/docker.js pull", | ||
| "test:performance": "wp-scripts test-playwright --config tests/performance/playwright.config.js", | ||
| "test:php:stan": "node ./tools/local-env/scripts/docker.js run --rm php ./vendor/bin/phpstan analyse --memory-limit=2G", | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder about this one. There isn't an npm script for PHPCS. Is that an oversight? It's in a grunt task instead: That said, I'm not sure about Relatedly: I want to add TypeScript
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm good with whatever here 🤷. I prefixed it with
Agreed, the motivation here is just that |
||
| "test:php": "node ./tools/local-env/scripts/docker.js run --rm php ./vendor/bin/phpunit", | ||
| "test:coverage": "npm run test:php -- --coverage-html ./coverage/html/ --coverage-php ./coverage/php/report.php --coverage-text=./coverage/text/report.txt", | ||
| "test:e2e": "wp-scripts test-playwright --config tests/e2e/playwright.config.js", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| # PHPStan configuration for WordPress Core. | ||
| # | ||
| # To overload this configuration, copy this file to phpstan.neon and adjust as needed. | ||
| # | ||
| # https://phpstan.org/config-reference | ||
|
|
||
| includes: | ||
| # The base configuration file for using PHPStan with the WordPress core codebase. | ||
| - tests/phpstan/base.neon | ||
|
|
||
| # The baseline file includes preexisting errors in the codebase that should be ignored. | ||
| # https://phpstan.org/user-guide/baseline | ||
| - tests/phpstan/baseline.php | ||
|
|
||
| parameters: | ||
| # https://phpstan.org/user-guide/rule-levels | ||
| level: 0 | ||
| reportUnmatchedIgnoredErrors: true | ||
|
|
||
| ignoreErrors: | ||
| # Level 0: | ||
| - # Inner functions aren't supported by PHPStan. | ||
| message: '#Function wxr_[a-z_]+ not found#' | ||
| path: src/wp-admin/includes/export.php | ||
| - | ||
| identifier: function.inner | ||
| path: src/wp-admin/includes/export.php | ||
| count: 13 | ||
| - | ||
| identifier: function.inner | ||
| path: src/wp-admin/includes/file.php | ||
| count: 1 | ||
| - | ||
| identifier: function.inner | ||
| path: src/wp-includes/canonical.php | ||
| count: 1 |
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -28,6 +28,7 @@ final class WP_Customize_Background_Image_Setting extends WP_Customize_Setting { | |||||||
| * @since 3.4.0 | ||||||||
| * | ||||||||
| * @param mixed $value The value to update. Not used. | ||||||||
| * @return bool|void Nothing is returned. | ||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
| */ | ||||||||
| public function update( $value ) { | ||||||||
| remove_theme_mod( 'background_image_thumb' ); | ||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Even though I agree this is the real fix, I'm nervous about making any src code changes in a tooling PR. Am I overthinking?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can commit these fixes separately if you like. |
||||||||
|
|
||||||||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -24,6 +24,7 @@ class WP_Customize_Filter_Setting extends WP_Customize_Setting { | |||||||||
| * @since 3.4.0 | ||||||||||
| * | ||||||||||
| * @param mixed $value The value to update. | ||||||||||
| * @return bool|void Nothing is returned. | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
| */ | ||||||||||
| public function update( $value ) {} | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
| } | ||||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -32,6 +32,7 @@ final class WP_Customize_Header_Image_Setting extends WP_Customize_Setting { | |||||
| * @global Custom_Image_Header $custom_image_header | ||||||
| * | ||||||
| * @param mixed $value The value to update. | ||||||
| * @return bool|void Nothing is returned. | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| */ | ||||||
| public function update( $value ) { | ||||||
| global $custom_image_header; | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment contains a TODO placeholder "@todo" that should be replaced with the actual version number when PHPStan testing is introduced