Refactor following PR review

Based on @cproensa's suggestion, see
#1565 (comment)

Invalid Plugin classes changes:
- Define a new InvalidDefinitionPlugin class for the case of missing
  required properties (name and version)
- InvalidPlugin becomes a generic, base class for invalid plugins
- Initialize with InvalidPlugin::setInvalidPlugin() method

MantisPlugin base class changes:
- New properties $status (with related STATUS_xxx constants) and
  $status_message to MantisPlugin class
- Method isValid() returns a loaded plugin's actual validity status
- Method getInvalidPlugin() creates an InvalidPlugin object from the
  current plugin and initializes it with setInvalidPlugin().

Plugin API, management and admin checks page have been modified to
leverage the new class, properties and methods.

admin/check/check_plugins_inc.php

@@ -86,14 +86,19 @@ function( $p ) { return plugin_is_registered( $p->basename ); }
 # Check for invalid or missing plugins
 $t_invalid_plugins = array_filter(
 	$t_plugins,
-	function( $p ) { return $p instanceof InvalidPlugin; }
+	function( $p ) {
+		return $p instanceof InvalidPlugin;
+	}
 );
 foreach( $t_invalid_plugins as $t_plugin ) {
 	$t_description = "'$t_plugin->name': $t_plugin->description";
+	if( $t_plugin->status_message ) {
+		$t_description .= "<br>$t_plugin->status_message";
+	}
 	$t_msg_contact = "Contact the Plugin's author.";
 
-	switch( get_class( $t_plugin ) ) {
-		case 'MissingPlugin':
+	switch( $t_plugin->status ) {
+		case MantisPlugin::STATUS_MISSING_PLUGIN:
 			check_print_test_row(
 				$t_description,
 				false,
@@ -104,7 +109,7 @@ function( $p ) { return $p instanceof InvalidPlugin; }
 				)
 			);
 			break;
-		case 'MissingClassPlugin':
+		case MantisPlugin::STATUS_MISSING_BASE_CLASS:
 			# Issue a warning instead of a failure, to cover the case of a directory
 			# created under plugins/ for other purposes than storing a plugin.
 			# https://github.com/mantisbt/mantisbt/pull/1565#discussion_r329311260

core/classes/InvalidDefinitionPlugin.class.php

@@ -0,0 +1,65 @@
+<?php
+# MantisBT - A PHP based bugtracking system
+
+# MantisBT is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 2 of the License, or
+# (at your option) any later version.
+#
+# MantisBT is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with MantisBT.  If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * MantisBT Invalid Incomplete Definition Plugin
+ * @copyright Copyright 2020  MantisBT Team - mantisbt-dev@lists.sourceforge.net
+ * @link http://www.mantisbt.org
+ * @package MantisBT
+ * @subpackage classes
+ */
+
+/**
+ * MantisBT Invalid Definition Plugin class
+ *
+ * The purpose of this class is to handle incomplete plugin definitions, i.e.
+ * having undefined 'name' or 'version' properties.
+ *
+ * For Plugin API internal use only.
+ */
+class InvalidDefinitionPlugin extends InvalidPlugin {
+
+	function register() {
+		$this->name = $this->basename;
+		$this->description = lang_get( 'plugin_invalid_description' );
+
+		$this->status = self::STATUS_INCOMPLETE_DEFINITION;
+	}
+
+	public function setInvalidPlugin( MantisPlugin $p_plugin ) {
+		parent::setInvalidPlugin( $p_plugin );
+
+		$t_missing = array();
+
+		# Add the reference plugin's name, if defined
+		if( $p_plugin->name ) {
+			$this->name .= " ($p_plugin->name)";
+		} else {
+			$t_missing[] = 'name';
+		}
+
+		if( !$p_plugin->version ) {
+			$t_missing[] = 'version';
+		}
+
+		if( !empty( $t_missing ) ) {
+			$this->status_message = sprintf(
+					lang_get( 'plugin_invalid_description_details' ),
+					implode( ', ', $t_missing )
+				);
+		}
+	}
+}

core/classes/InvalidPlugin.class.php

@@ -23,38 +23,48 @@
  */
 
 /**
- * MantisBT Invalid Plugin class
+ * MantisBT Generic Invalid Plugin class
  *
- * The purpose of this class is to handle incomplete plugin definitions, i.e.
- * undefined 'name' or 'version' properties.
+ * The purpose of this class is to handle invalid plugins. It is used as a base
+ * for other, specialized invalid plugin classes, e.g.
+ * @see InvalidIncompleteDefinitionPlugin
+ * @see MissingPlugin
+ * @see MissingClassPlugin
  *
  * For Plugin API internal use only.
  */
 class InvalidPlugin extends MantisPlugin {
+	/**
+	 * The reference, invalid Plugin.
+	 *
+	 * This is used for plugins that are considered invalid even though they
+	 * can be loaded, so we can query the reference plugin's properties.
+	 *
+	 * @var MantisPlugin $ref_plugin
+	 */
+	public $ref_plugin;
+
+	/**
+	 * Flag indicating whether the plugin can be removed from manage plugins page.
+	 * @var bool $removable True if it can be removed,
+	 *                      False if manual intervention is required.
+	 */
+	public $removable = true;
+
 	function register() {
 		$this->name = $this->basename;
 		$this->description = lang_get( 'plugin_invalid_description' );
-	}
 
-	public function set( MantisPlugin $p_plugin ) {
-		$t_missing = array();
-
-		if( $p_plugin->name ) {
-			$this->name .= " ($p_plugin->name)";
-		} else {
-			$t_missing[] = 'name';
-		}
-
-		if( !$p_plugin->version ) {
-			$t_missing[] = 'version';
-		}
+		$this->status = self::STATUS_INVALID;
+	}
 
-		if( !empty( $t_missing ) ) {
-			$this->description .= '<br>'
-				. sprintf(
-					lang_get( 'plugin_invalid_description_details' ),
-					implode( ', ', $t_missing )
-				);
-		}
+	/**
+	 * Initialize the invalid plugin.
+	 * @see MantisPlugin::getInvalidPlugin()
+	 *
+	 * @param MantisPlugin $p_plugin Reference, invalid plugin
+	 */
+	public function setInvalidPlugin( MantisPlugin $p_plugin ) {
+		$this->ref_plugin = $p_plugin;
 	}
 }

core/classes/MantisPlugin.class.php

@@ -29,6 +29,26 @@
  * more information.
  */
 abstract class MantisPlugin {
+	/**
+	 * Constants indicating the Plugin's validity status
+	 *
+	 * - VALID - Plugin is valid
+	 * - INVALID - Generic invalid status
+	 * - INCOMPLETE_DEFINITION - The plugin's definition is incomplete
+	 *   (i.e. required properties 'name' or 'version' are missing)
+	 * - MISSING_BASE_CLASS - Plugin directory exists but does not contain a
+	 *   matching Class
+	 * - MISSING_PLUGIN - The plugin has been installed, but its source code
+	 *   is no longer available in plugin_path directory
+	 *
+	 * @see MantisPlugin::$status
+	 */
+	const STATUS_VALID = 0;
+	const STATUS_INVALID = 1;
+	const STATUS_INCOMPLETE_DEFINITION = 2;
+	const STATUS_MISSING_BASE_CLASS = 3;
+	const STATUS_MISSING_PLUGIN = 4;
+
 	/**
 	 * name - Your plugin's full name. Required value.
 	 */
@@ -67,6 +87,18 @@ abstract class MantisPlugin {
 	 */
 	public $url			= null;
 
+	/**
+	 * Plugin's validity status
+	 * @var int $status
+	 */
+	public $status = self::STATUS_VALID;
+
+	/**
+	 * Explanation of the reason why the plugin is not valid
+	 * @var string $status_message
+	 */
+	public $status_message = '';
+
 	/**
 	 * this function registers your plugin - must set at least name and version
 	 * @return void
@@ -259,4 +291,31 @@ final public function __init() {
 
 		$this->init();
 	}
+
+	/**
+	 * Check the plugin's validity status.
+	 *
+	 * @return bool True if the plugin is valid.
+	 */
+	public function isValid() {
+		return $this->name !== null && $this->version !== null;
+	}
+
+	/**
+	 * Creates an InvalidPlugin object from the current plugin.
+	 *
+	 * Invalid Plugin objects are used by manage_plugin_page.php to present
+	 * relevant information about the invalid plugin to the administrator so
+	 * they can take appropriate action to fix the problem.
+	 *
+	 * By default, it returns an InvalidDefinitionPlugin object.
+	 *
+	 * @return InvalidPlugin
+	 */
+	public function getInvalidPlugin() {
+		$t_plugin = new InvalidDefinitionPlugin( $this->basename );
+		$t_plugin->setInvalidPlugin( $this );
+
+		return $t_plugin;
+	}
 }

core/classes/MissingClassPlugin.class.php

@@ -34,8 +34,10 @@
 final class MissingClassPlugin extends InvalidPlugin {
 	function register() {
 		$this->name = $this->basename;
-		$this->description = lang_get( 'plugin_missing_class_description' )
-			. '<br>'
-			. lang_get( 'plugin_missing_class_description_details' );
+		$this->description = lang_get( 'plugin_missing_class_description' );
+
+		$this->status = self::STATUS_MISSING_BASE_CLASS;
+		$this->status_message = lang_get( 'plugin_missing_class_description_details' );
+		$this->removable = false;
 	}
 }

core/classes/MissingPlugin.class.php

@@ -34,5 +34,7 @@ final class MissingPlugin extends InvalidPlugin {
 	function register() {
 		$this->name = $this->basename;
 		$this->description = lang_get( 'plugin_missing_description' );
+
+		$this->status = self::STATUS_MISSING_PLUGIN;
 	}
 }

core/plugin_api.php

@@ -797,7 +797,7 @@ function plugin_upgrade( MantisPlugin $p_plugin ) {
 function plugin_uninstall( MantisPlugin $p_plugin ) {
 	access_ensure_global_level( config_get_global( 'manage_plugin_threshold' ) );
 
-	if( !$p_plugin instanceof MissingClassPlugin &&
+	if( !$p_plugin->status == MantisPlugin::STATUS_MISSING_BASE_CLASS &&
 		( !plugin_is_installed( $p_plugin->basename ) || plugin_protected( $p_plugin->basename ) )
 	) {
 		return;
@@ -951,19 +951,17 @@ function plugin_register( $p_basename, $p_return = false, $p_child = null ) {
 		# Make sure the class exists and that it's of the right type.
 		if( class_exists( $t_classname ) && is_subclass_of( $t_classname, 'MantisPlugin' ) ) {
 			plugin_push_current( $t_basename );
+			/** @var MantisPlugin $t_plugin */
 			$t_plugin = new $t_classname( $t_basename );
 			plugin_pop_current();
 
 			# Final check on the class
-			if( is_null( $t_plugin->name ) || is_null( $t_plugin->version ) ) {
-				$t_invalid = new InvalidPlugin( $t_basename);
-				$t_invalid->set( $t_plugin );
-
+			if( !$t_plugin->isValid() ) {
 				log_event(
 					LOG_PLUGIN,
 					"Plugin '$t_basename' is invalid (undefined Name or Version)"
 				);
-				return $t_invalid;
+				return $t_plugin->getInvalidPlugin();
 			}
 
 			if( $p_return ) {

manage_plugin_page.php

@@ -273,7 +273,7 @@ function ( $p_p1, $p_p2 ) {
 		);
 
 		foreach( $t_plugins as $t_basename => $t_plugin ) {
-			if( $t_plugin instanceof InvalidPlugin ) {
+			if( !$t_plugin->isValid() ) {
 				$this->invalid[$t_basename] = new InvalidPluginForDisplay( $t_plugin );
 			} elseif( plugin_is_registered( $t_basename ) ) {
 				$this->installed[$t_basename] = new InstalledPlugin( $t_plugin );
@@ -365,8 +365,11 @@ public function __construct( MantisPlugin $p_plugin ) {
 
 		# Descriptions from InvalidPlugin classes are trusted input
 		$this->description = $p_plugin->description;
+		if( $p_plugin->status_message ) {
+			$this->description .= '<br>' . $p_plugin->status_message;
+		}
 
-		$this->can_remove = ! $p_plugin instanceof MissingClassPlugin;
+		$this->can_remove = $p_plugin->removable;
 	}
 
 	protected function renderColumns() {

manage_plugin_uninstall.php

@@ -59,14 +59,15 @@
 $f_basename = gpc_get_string( 'name' );
 $t_plugin = plugin_register( $f_basename, true );
 
-if(    $t_plugin instanceof MissingPlugin
-	|| $t_plugin instanceof MissingClassPlugin
-) {
-	$t_message = 'plugin_remove_message';
-	$t_button = 'remove_link';
-} else {
-	$t_message = 'plugin_uninstall_message';
-	$t_button = 'plugin_uninstall';
+switch( $t_plugin->status ) {
+	case MantisPlugin::STATUS_MISSING_PLUGIN:
+	case MantisPlugin::STATUS_MISSING_BASE_CLASS:
+		$t_message = 'plugin_remove_message';
+		$t_button = 'remove_link';
+		break;
+	default:
+		$t_message = 'plugin_uninstall_message';
+		$t_button = 'plugin_uninstall';
 }
 
 helper_ensure_confirmed(

manage_plugin_upgrade.php

@@ -51,7 +51,7 @@
 $f_basename = gpc_get_string( 'name' );
 $t_plugin = plugin_register( $f_basename, true );
 
-if( !is_null( $t_plugin ) && ! $t_plugin instanceof MissingPlugin ) {
+if( !is_null( $t_plugin ) && $t_plugin->status != MantisPlugin::STATUS_MISSING_PLUGIN ) {
 	$t_status = plugin_upgrade( $t_plugin );
 }