-
-
Notifications
You must be signed in to change notification settings - Fork 138
chore: setup PHPStan #746
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
chore: setup PHPStan #746
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.
@justlevine Thanks for the quick work, bro 🚀 .
Code Climate has analyzed commit bb5cdcc and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 31.5% (50% is the threshold). This pull request will bring the total coverage in the repository to 82.3% (0.0% change). View more on Code Climate. |
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.
Okay I flagged any possible changes in behavior I noticed, but best would be to see what the GraphQL error is that we're getting from the tests
@@ -89,7 +89,7 @@ public function get_loader_name() { | |||
*/ | |||
public function get_node_by_id( $id ) { | |||
$post = get_post( $id ); | |||
if ( empty( $post ) || is_wp_error( $post ) ) { | |||
if ( empty( $post ) ) { |
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 is possibly a change in behavior.
@@ -472,7 +472,7 @@ public function sanitize_input_fields( array $where_args ) { | |||
} | |||
|
|||
// Set sub tax query relation. | |||
if ( 1 > count( $sub_tax_query ) ) { | |||
if ( 1 > count( $sub_tax_query ) && ! empty( $relation ) ) { |
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 is possibly a change in behavior
if ( ! empty( $input['keys'] ) && ! isset( $items ) ) { | ||
$items = []; | ||
|
||
if ( ! empty( $input['keys'] ) && ! empty( $items ) ) { |
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 change means that if WC()->cart->get_cart()
returns empty, the foreach loop will no longer run.
If that's not intentional, then initialize $items
to null
, and then change this to && null !== $items
@@ -149,6 +143,13 @@ private static function get_value( $value ) { | |||
'value' => 'uri', | |||
'description' => __( 'Identify a resource by the URI.', 'wp-graphql-woocommerce' ), | |||
]; | |||
case 'id': | |||
default: |
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 lack of a 'default' case is a possibl change in behavior
] | ||
); | ||
|
||
if ( ! WooCommerce_Filters::is_session_handler_disabled() ) { |
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 is a change in behavior
caf471b
to
bb01736
Compare
@kidunot89 after my latest commit, the last two level-4 errors are:
|
Your checklist for this pull request
Thanks for sending a pull request! Please make sure you click the link above to view the contribution guidelines, then fill out the blanks below.
🚨Please review the guidelines for contributing to this repository.
What does this implement/fix? Explain your changes.
This PR implements PHPStan static analysis for the plugin.
Incidentally, the platform req in
composer.json
has been set to PHP 7.3 to ensure Composer deps are built against the same WordPress version.To run (following the convention of the other composer script commands)
Does this close any currently open issues?
…
Any relevant logs, error output, GraphiQL screenshots, etc?
(If it’s long, please paste to https://ghostbin.com/ and insert the link here.)
Any other comments?
Problems
panel, you must install the dev dependences and then reload the window.Where has this been tested?