-
Notifications
You must be signed in to change notification settings - Fork 384
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
Raise PHPStan level to 4 in lib/common #4686
Conversation
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.
Does this warrant being part of v1.5.4?
if (200 < $statusCode || $statusCode >= 300) { | ||
if ($statusCode < 200 || $statusCode >= 300) { |
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 bug means that the fallback stylesheet would almost always be used?
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.
Yes, we should backport this.
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.
Cool. Then I'll squash merge and cherry-pick.
- | ||
message: '#^If condition is always false\.$#' | ||
path: 'src/Dom/Document.php' | ||
# See https://github.com/phpstan/phpstan/issues/3291 |
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 assume your PR on the phpstan repo (phpstan/phpstan-src#203) fixes this. Once a new release is made with that change we should be able to remove this.
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.
Yes. The general idea is to update to latest PHPStan and revisit the exceptions here each time we bump the level.
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.
FWIW I think PHPStan will complain if an exception is no longer encountered. In other words, it will tell you that you can remove it :-)
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.
Yes, indeed, it does.
* Bump level to 4 * Add additional null typehint * Add additional null typehint * Fix bug with status code comparison * Rewrite logic and add exception for false positive when checking libxml version
…ve-collection-schema-type * 'develop' of github.com:ampproject/amp-wp: Raise PHPStan level to 4 in lib/common (#4686) Update dependency uuid to v8 Update dependency webpack to v4.43.0 Update dependency eslint-plugin-jest to v23.11.0 Update dependency eslint-plugin-react to v7.20.0 (#4691) Update dependency postcss to v7.0.30 (#4649) Update dependency moment to v2.25.3 (#4639) Update dependency babel-jest to v26 (#4652) Update dependency uuid to v7.0.3 (#4490) Update dependency wp-coding-standards/wpcs to v2.3.0 (#4721) Suppress Site Health ICU test if site or home URL is not an IDN (#4698) Pin amp-experiment to v0.1 (#4690) Strip multiple BOM characters (#4683) Strip leading BOM and whitespace and trailing HTML comment before parsing validation response JSON (#4679) Disable share icons Use amp-embedly-card for Tiktok embeds Update dependency xwp/wp-dev-lib to v1.6.4 Update dependency eslint to v7 Update dependency terser-webpack-plugin to v2.3.6
…-phpstan-to-level-4 * 'develop' of github.com:ampproject/amp-wp: Update dependency @wordpress/scripts to v9.1.0 Update dependency @wordpress/jest-puppeteer-axe to v1.8.0 Update dependency @wordpress/url to v2.15.0 Update dependency @wordpress/plugins to v2.16.0 Raise PHPStan level to 4 in lib/common (#4686) Update dependency uuid to v8 Update dependency webpack to v4.43.0 Update dependency eslint-plugin-jest to v23.11.0 Update dependency eslint-plugin-react to v7.20.0 (#4691) Update dependency postcss to v7.0.30 (#4649) Update dependency moment to v2.25.3 (#4639) Update dependency babel-jest to v26 (#4652) Update dependency uuid to v7.0.3 (#4490) Update dependency wp-coding-standards/wpcs to v2.3.0 (#4721) Update dependency @wordpress/e2e-test-utils to v4.7.0 Suppress Site Health ICU test if site or home URL is not an IDN (#4698) Pin amp-experiment to v0.1 (#4690) Disable share icons Use amp-embedly-card for Tiktok embeds Update dependency terser-webpack-plugin to v2.3.6
Summary
This PR fixes the following issues that PHPStan detected at level 4:
Fixes #4379
Checklist