MDL-43896 admin: Fail validation of plugins with $module in version.php

diff --git a/admin/tool/installaddon/classes/validator.php b/admin/tool/installaddon/classes/validator.php
index cafd9b9..0bb3a41 100644 (file)
--- a/admin/tool/installaddon/classes/validator.php
+++ b/admin/tool/installaddon/classes/validator.php
@@ -300,29 +300,21 @@ class tool_installaddon_validator {
         $this->versionphp = array();
         $info = $this->parse_version_php($fullpath);
 
-        if ($this->assertions['plugintype'] === 'mod') {
-            $type = 'module';
-        } else {
-            $type = 'plugin';
+        if (isset($info['module->version'])) {
+            $this->add_message(self::ERROR, 'versionphpsyntax', '$module');
+            return false;
         }
 
-        if (!isset($info[$type.'->version'])) {
-            if ($type === 'module' and isset($info['plugin->version'])) {
-                // Expect the activity module using $plugin in version.php instead of $module.
-                $type = 'plugin';
-                $this->versionphp['version'] = $info[$type.'->version'];
-                $this->add_message(self::INFO, 'pluginversion', $this->versionphp['version']);
-            } else {
-                $this->add_message(self::ERROR, 'missingversion');
-                return false;
-            }
-        } else {
-            $this->versionphp['version'] = $info[$type.'->version'];
+        if (isset($info['plugin->version'])) {
+            $this->versionphp['version'] = $info['plugin->version'];
             $this->add_message(self::INFO, 'pluginversion', $this->versionphp['version']);
+        } else {
+            $this->add_message(self::ERROR, 'missingversion');
+            return false;
         }
 
-        if (isset($info[$type.'->requires'])) {
-            $this->versionphp['requires'] = $info[$type.'->requires'];
+        if (isset($info['plugin->requires'])) {
+            $this->versionphp['requires'] = $info['plugin->requires'];
             if ($this->versionphp['requires'] > $this->assertions['moodleversion']) {
                 $this->add_message(self::ERROR, 'requiresmoodle', $this->versionphp['requires']);
                 return false;
@@ -330,8 +322,8 @@ class tool_installaddon_validator {
             $this->add_message(self::INFO, 'requiresmoodle', $this->versionphp['requires']);
         }
 
-        if (isset($info[$type.'->component'])) {
-            $this->versionphp['component'] = $info[$type.'->component'];
+        if (isset($info['plugin->component'])) {
+            $this->versionphp['component'] = $info['plugin->component'];
             list($reqtype, $reqname) = core_component::normalize_component($this->versionphp['component']);
             if ($reqtype !== $this->assertions['plugintype']) {
                 $this->add_message(self::ERROR, 'componentmismatchtype', array(
@@ -346,8 +338,8 @@ class tool_installaddon_validator {
             $this->add_message(self::INFO, 'componentmatch', $this->versionphp['component']);
         }
 
-        if (isset($info[$type.'->maturity'])) {
-            $this->versionphp['maturity'] = $info[$type.'->maturity'];
+        if (isset($info['plugin->maturity'])) {
+            $this->versionphp['maturity'] = $info['plugin->maturity'];
             if ($this->versionphp['maturity'] === 'MATURITY_STABLE') {
                 $this->add_message(self::INFO, 'maturity', $this->versionphp['maturity']);
             } else {
@@ -355,8 +347,8 @@ class tool_installaddon_validator {
             }
         }
 
-        if (isset($info[$type.'->release'])) {
-            $this->versionphp['release'] = $info[$type.'->release'];
+        if (isset($info['plugin->release'])) {
+            $this->versionphp['release'] = $info['plugin->release'];
             $this->add_message(self::INFO, 'release', $this->versionphp['release']);
         }
 

diff --git a/admin/tool/installaddon/lang/en/tool_installaddon.php b/admin/tool/installaddon/lang/en/tool_installaddon.php
index 6f1ad09..ade477a 100644 (file)
--- a/admin/tool/installaddon/lang/en/tool_installaddon.php
+++ b/admin/tool/installaddon/lang/en/tool_installaddon.php
@@ -91,6 +91,7 @@ $string['validationmsg_rootdirinvalid_help'] = 'The name of the root directory i
 $string['validationmsg_targetexists'] = 'Target location already exists';
 $string['validationmsg_targetexists_help'] = 'The directory that the plugin is to be installed to must not yet exist.';
 $string['validationmsg_unknowntype'] = 'Unknown plugin type';
+$string['validationmsg_versionphpsyntax'] = 'Unsupported syntax detected in version.php file';
 $string['validationmsglevel_debug'] = 'Debug';
 $string['validationmsglevel_error'] = 'Error';
 $string['validationmsglevel_info'] = 'OK';

diff --git a/admin/tool/installaddon/tests/fixtures/nolang/bah/version.php b/admin/tool/installaddon/tests/fixtures/nolang/bah/version.php
index 6c648f9..3b473d4 100644 (file)
--- a/admin/tool/installaddon/tests/fixtures/nolang/bah/version.php
+++ b/admin/tool/installaddon/tests/fixtures/nolang/bah/version.php
@@ -1,4 +1,3 @@
 <?php
 
-$module->version = 2014122455;
 $plugin->version = 2014122455;

diff --git a/admin/tool/installaddon/tests/fixtures/plugindir/foobar/version.php b/admin/tool/installaddon/tests/fixtures/plugindir/foobar/version.php
index ebad339..5b76ce2 100644 (file)
--- a/admin/tool/installaddon/tests/fixtures/plugindir/foobar/version.php
+++ b/admin/tool/installaddon/tests/fixtures/plugindir/foobar/version.php
@@ -1,9 +1,7 @@
 <?php
 
-$module->version = 10; // Ignored, this should use $plugin
 $plugin->version = 2013031900;
 $plugin->component = 'local_foobar';
 $plugin->requires = 2013031200;
-$module->release = 'We are not an activity module!';
 $plugin->maturity = MATURITY_ALPHA;
 //$plugin->release = 'And this is commented';

diff --git a/admin/tool/installaddon/tests/fixtures/plugindir/legacymod/lang/en/legacymod.php b/admin/tool/installaddon/tests/fixtures/plugindir/legacymod/lang/en/legacymod.php
new file mode 100644 (file)
index 0000000..01e066e
--- /dev/null
+++ b/admin/tool/installaddon/tests/fixtures/plugindir/legacymod/lang/en/legacymod.php
@@ -0,0 +1,3 @@
+<?php
+
+$string['modulename'] = 'Legacy activity module with $module in version.php';

diff --git a/admin/tool/installaddon/tests/fixtures/plugindir/legacymod/version.php b/admin/tool/installaddon/tests/fixtures/plugindir/legacymod/version.php
new file mode 100644 (file)
index 0000000..3434317
--- /dev/null
+++ b/admin/tool/installaddon/tests/fixtures/plugindir/legacymod/version.php
@@ -0,0 +1,4 @@
+<?php
+
+// Support for the $module has been dropped in Moodle 3.0.
+$module->version = 2013031900;

diff --git a/admin/tool/installaddon/tests/validator_test.php b/admin/tool/installaddon/tests/validator_test.php
index d0342f5..709f2cc 100644 (file)
--- a/admin/tool/installaddon/tests/validator_test.php
+++ b/admin/tool/installaddon/tests/validator_test.php
@@ -134,6 +134,17 @@ class tool_installaddon_validator_testcase extends basic_testcase {
         $this->assertFalse($validator->execute());
         $this->assertTrue($this->has_message($validator->get_messages(), $validator::ERROR, 'missingversionphp'));
 
+        $validator = testable_tool_installaddon_validator::instance($fixtures.'/plugindir', array(
+            'legacymod/' => true,
+            'legacymod/version.php' => true,
+            'legacymod/lang/' => true,
+            'legacymod/lang/en/' => true,
+            'legacymod/lang/en/legacymod.php' => true));
+        $validator->assert_plugin_type('mod');
+        $validator->assert_moodle_version(0);
+        $this->assertFalse($validator->execute());
+        $this->assertTrue($this->has_message($validator->get_messages(), $validator::ERROR, 'versionphpsyntax', '$module'));
+
         $validator = testable_tool_installaddon_validator::instance($fixtures.'/plugindir', array(
             'foobar/' => true,
             'foobar/version.php' => true,