search

10up / headstartwp

Build a headless website fast with WordPress, the world’s most popular CMS, and Next.js, the most popular React framework. A free and open source solution by the experts at 10up.
https://headstartwp.10up.com
161 stars 17 forks source link

Types are not set for parameters or return values in the WordPress plugin #625

Open claytoncollie opened 1 year ago

claytoncollie commented 1 year ago

Is your enhancement related to a problem? Please describe.

As I was looking through the codebase, I was impressed with how much time and effort was put into defining all of the parameter and return types in JavaScript. The work is very organized. Bravo! But then when I looked at the WordPress plugin, I thought there could be a bit more effort put into this codebase to bring it up to the same standard. Most of the time we are defining the types in a docblock which is good but should be taken a step further and codified as well such as function get_something( string $param ): string { return 'something'; }.

I think a good first step is to all parameter and return types to each method and function in the WordPress plugin ( including void for functions that do not return a value) and then, as a second step, introduce PHPstan and a GitHub action to help future contributors adhere to the standard. We have an open pull request in the wp-scaffold repo that would be easy to copy from to introduce this functionality.

I am happy to make a pull request for one or both of these ideas.

Designs

No response

Describe alternatives you've considered

No response

Code of Conduct

tobeycodes commented 12 months ago

We could run https://github.com/rectorphp/rector to automate a lot of what you're suggesting

claytoncollie commented 12 months ago

@tobeycodes Is this a one-time event to refactor the codebase or can we use rector on a continual basis? Do you think the codebase is large enough to warrant an automated solution versus going through the plugin file by file?

tobeycodes commented 12 months ago

Both. You can run it once or part of CI pipelines. In my opinion the size of a codebase should not be a factor in deciding whether to use tooling like this. I think there should still be a file by file review but automating things will catch things now we miss and keep us up to date in the future too.

nicholasio commented 11 months ago

@claytoncollie You're definitely right that the plugin could get a bit more love. I'd love to see changes on that front too, feel free to submit a PR!

I don't have experience with phpstan but I'm all for automated process to improve the codebase.

claytoncollie commented 8 months ago

@nicholasio I created four pull requests to address this issue. I think we can add a fifth PR in the future to introduce PHPStan but, for now, I would like to get your eyes on these changes and hear your feedback. I am not as familiar with this platform as you are so I will need guidance on how to test each of the changes.

PRs 1,2,3 are focused on PHPCS with automatic and manual fixes. They are all chained so that you are not looking at one giant PR.

The fourth PR is to introduce Rector and enforce those coding standards. @tobeycodes please have a look at the config of that tool to see if it matches up with how you are using it.

  1. 688

  2. 689

  3. 696

  4. 695