WordPress / plugin-check

A repository for the new Plugin Check plugin from the WordPress Performance and Plugins Team.
https://wordpress.org/plugins/plugin-check/
GNU General Public License v2.0
198 stars 39 forks source link

Allow static checks to be run against an arbitrary path #490

Open davidperezgar opened 2 days ago

davidperezgar commented 2 days ago

What we need for the submission form from wordpress.org is run static checks in an arbitrary path. It's helpful as well for any developer user to directly run static checks in any directory.

Resolves #478

github-actions[bot] commented 2 days ago

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: davidperezgar <davidperez@git.wordpress.org>
Co-authored-by: ernilambar <rabmalin@git.wordpress.org>
Co-authored-by: swissspidy <swissspidy@git.wordpress.org>
Co-authored-by: joemcgill <joemcgill@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

swissspidy commented 2 days ago

I'm out of office until Monday but happy to review then.

At first glance I'd say the is_dir logic is at the wrong level. If you pull it higher up, then you don't to worry about naming or anything. For example doing that in Abstract_Check_Runner::get_plugin_basename would be better. No need to touch get_plugin_basename_from_input() at all.

Also, this would be an excellent candidate to write Behat tests for.

Happy to help with both once I'm back.

ernilambar commented 2 days ago

I tested PR and it is not working for plugin in external location as expected. Few checks are working though.

In Plugin_Context there is protected $main_file; which is used in several checks. This expects the main file of the plugin but in external plugin, this variable just has plugin path. So there are few errors in the output.

Screenshot 2024-07-04 at 4 51 43 PM

Like in Plugin_Header_Text_Domain_Check, there is like this:

$plugin_main_file = WP_PLUGIN_DIR . '/' . $result->plugin()->basename();

May be we should fix $main_file in the beginning and access same property in all checks rather than evaluating main file like above.

davidperezgar commented 2 days ago

Thanks @ernilambar for your feedback. I've detected that problem, and I've committed a fix. Update the PR and it should work.