[wp-trac] [WordPress Trac] #43992: Prevent activation of a plugin if its required PHP version is too high

[WordPress Trac]

#43992: Prevent activation of a plugin if its required PHP version is too high
-------------------------------------------------+-------------------------
 Reporter:  flixos90                             |       Owner:  desrosj
     Type:  task (blessed)                       |      Status:  reviewing
 Priority:  normal                               |   Milestone:  5.2
Component:  Plugins                              |     Version:
 Severity:  major                                |  Resolution:
 Keywords:  needs-unit-tests servehappy dev-     |     Focuses:
  feedback has-patch needs-testing 2nd-opinion   |
-------------------------------------------------+-------------------------
Changes (by desrosj):

 * keywords:  needs-unit-tests servehappy dev-feedback has-patch needs-
     testing =>
     needs-unit-tests servehappy dev-feedback has-patch needs-testing 2nd-
     opinion


Comment:

 This is testing pretty well for me. After some of the items below are
 solidified I can add some unit tests.

 At first look, the thought that "we should pass a `$context` of `plugin`
 or `plugin_validation` to the `get_file_data()` call in
 `get_plugin_validation_data()`" came to mind. This would allow extra
 headers to be added and detected. But, if this was added, then a filter
 would need to be added to the end of `get_plugin_validation()`. Otherwise,
 detecting any additional headers would be useless. I can't think of a good
 use case right now, though, so let's leave it as is.

 In the current state, any empty headers in the `readme.txt` will be
 ignored in favor of the plugin's headers, even if they are defined and
 empty. Just confirming that this is intended.

 Can you confirm that I am understanding the precedence correctly:
 - Non empty values defined in `readme.txt`.
 - Plugin file header values when `readme.txt` does not exist.
 - When `readme.txt` exists but has empty (or undefined) values for either
 header, the empty values will fall back to the plugin file headers.

 I am also a little concerned with the function names `is_wp_compatible()`
 and `is_php_compatible()` being too generic. I did some brief searching
 around, and `is_wp_compatible()` seems to be pretty common among plugins.
 I am mostly seeing `is_wp_compatible()` as private or protected class
 methods, but I'd like a few more opinions on whether we should adjust the
 names a bit.

 For the `unmet_requirements` error, maybe the approach could be:
 - Add a new function that returns a boolean indicating a plugin can be
 activated.
 - `validate_plugin_requirements()` returns an array indicating if PHP
 passed, WP passed, and if either fails, a string with the correct
 contextual error message.

 Something like this could let us have different messages for each
 scenario. Examples: `Plugin does not meet minimum WordPress
 requirements.`, `Plugin does not meet minimum PHP requirements.`, `Plugin
 does not meet minimum WordPress and PHP requirements.`.

 Some other minor items:
 - `@uses` [https://make.wordpress.org/core/handbook/best-practices/inline-
 documentation-standards/php/#phpdoc-tags should no longer be used] within
 PHP docblocks. `get_plugin_validation_data()` description needs to be
 revised to be more clear about priority that data is used.
 - Can the short description for `get_plugin_validation_data()` be more
 specific about what is being validated?

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/43992#comment:38>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform