Skip to content

Conversation

@justlevine
Copy link
Collaborator

What does this implement/fix? Explain your changes.

This PR takes advantages of the numerous improvements to type-safety over the last several releases to clean up unnecessary code primarily used as type guards. Additionally, the PHPStan config has been improved to prevent future code quality regressions.

More specifically:

  • Toggled multiple PHPStan config flags for stricter analyis.
  • Removed "Always-true" checks in conditionals (e.g. true === true, isset( true ), etc).
  • Removed unnecessary type casts ( e.g. if ($foo === true ) { return (bool) $foo }; )
  • Removed the usage of $self === $this in Model::wrap_fields(), since we can use the class instance directly in the closure.

This PR contains no changes to internal/external APIs or any expected functionality.

Does this close any currently open issues?

Any relevant logs, error output, GraphiQL screenshots, etc?

Any other comments?

The checkAlwaysTrueInstanceof flag in phpstan.neon.dist can be toggled on once #3008 is merged.

Where has this been tested?

Operating System: Ubuntu 20.04 (wsl2 + devilbox + php 8.1.15)

WordPress Version: 6.4.2

@coveralls
Copy link

coveralls commented Jan 2, 2024

Coverage Status

coverage: 84.756% (-0.04%) from 84.798%
when pulling 19440ea on justlevine:chore/strict-phpstan-fixes
into 3d3d723 on wp-graphql:develop.

@justlevine justlevine added type: chore Maintenance tasks, refactoring, and other non-functional changes scope: code quality Refactoring, linting, and enforcing coding standards status: in review Awaiting review before merging or closing labels Jan 3, 2024
@justlevine justlevine requested a review from jasonbahl January 3, 2024 01:18
jasonbahl
jasonbahl previously approved these changes Jan 23, 2024
@jasonbahl
Copy link
Collaborator

jasonbahl commented Jan 23, 2024

The checkAlwaysTrueInstanceof flag in phpstan.neon.dist can be toggled on once #3008 is merged.

@justlevine IIRC, you said you might have had some more changes you were going to add to #3008?

UPDATE:

I see that you pushed up a new commit today. will check it 👀

…rict-phpstan-fixes

- update phpstan.neon.dist
@qlty-cloud-legacy
Copy link

Code Climate has analyzed commit 19440ea and detected 3 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 1
Duplication 2

View more on Code Climate.

@jasonbahl jasonbahl merged commit 5db11fb into wp-graphql:develop Jan 23, 2024
This was referenced Jan 23, 2024
@justlevine justlevine deleted the chore/strict-phpstan-fixes branch February 10, 2024 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

scope: code quality Refactoring, linting, and enforcing coding standards status: in review Awaiting review before merging or closing type: chore Maintenance tasks, refactoring, and other non-functional changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants