From d6ae1b4c5ac7a298ee304b4090c3f27b889975fc Mon Sep 17 00:00:00 2001
From: Matthew Weier O'Phinney <matthew@zend.com>
Date: Wed, 1 Nov 2017 10:48:46 -0500
Subject: [PATCH] Test for a `Module` class before testing for a class named
 after the module

Per #5, non-namespaced classes can create issues within applications if
they conflict with a module. In particular, `Generator` is one (as the
class is non-instantiable via `new`), but this can also happen in legacy
applications where a new module may exist under the same name as a
defined class.

This patch adds tests to ensure that the resolver prefers `Module`
classes if they exist, and never loads known uninstantiable classes that
always exist (specifically, `Generator`).
---
 composer.json                                |  4 +++-
 phpcs.xml                                    |  5 ++++
 src/Listener/ModuleResolverListener.php      | 25 ++++++++++++++------
 test/Listener/ModuleResolverListenerTest.php | 19 +++++++++++++++
 test/TestAsset/ModuleAsClass.php             | 10 ++++++++
 test/TestAsset/ModuleAsClass/Module.php      | 12 ++++++++++
 6 files changed, 67 insertions(+), 8 deletions(-)
 create mode 100644 test/TestAsset/ModuleAsClass.php
 create mode 100644 test/TestAsset/ModuleAsClass/Module.php

diff --git a/composer.json b/composer.json
index 76f596d..649e568 100644
--- a/composer.json
+++ b/composer.json
@@ -41,10 +41,12 @@
     },
     "autoload-dev": {
         "files": [
-            "test/autoload.php"
+            "test/autoload.php",
+            "test/TestAsset/ModuleAsClass.php"
         ],
         "psr-4": {
             "ListenerTestModule\\": "test/TestAsset/ListenerTestModule/",
+            "ModuleAsClass\\": "test/TestAsset/ModuleAsClass/",
             "ZendTest\\ModuleManager\\": "test/"
         }
     },
diff --git a/phpcs.xml b/phpcs.xml
index 9e6a751..dfd8512 100644
--- a/phpcs.xml
+++ b/phpcs.xml
@@ -5,4 +5,9 @@
     <!-- Paths to check -->
     <file>src</file>
     <file>test</file>
+
+    <!-- Exclusions -->
+    <rule ref="PSR1.Classes.ClassDeclaration">
+        <exclude-pattern>*/test/TestAsset/ModuleAsClass.php</exclude-pattern>
+    </rule>
 </ruleset>
diff --git a/src/Listener/ModuleResolverListener.php b/src/Listener/ModuleResolverListener.php
index 5229c3f..09ca188 100644
--- a/src/Listener/ModuleResolverListener.php
+++ b/src/Listener/ModuleResolverListener.php
@@ -9,6 +9,7 @@
 
 namespace Zend\ModuleManager\Listener;
 
+use Generator;
 use Zend\ModuleManager\ModuleEvent;
 
 /**
@@ -16,6 +17,15 @@
  */
 class ModuleResolverListener extends AbstractListener
 {
+    /**
+     * Class names that are invalid as module classes, due to inability to instantiate.
+     *
+     * @var string[]
+     */
+    protected $invalidClassNames = [
+        Generator::class,
+    ];
+
     /**
      * @param  ModuleEvent $e
      * @return object|false False if module class does not exist
@@ -24,16 +34,17 @@ public function __invoke(ModuleEvent $e)
     {
         $moduleName = $e->getModuleName();
 
-        if (class_exists($moduleName)) {
-            return new $moduleName;
+        $class = sprintf('%s\Module', $moduleName);
+        if (class_exists($class)) {
+            return new $class;
         }
 
-        $class      = $moduleName . '\Module';
-
-        if (! class_exists($class)) {
-            return false;
+        if (class_exists($moduleName)
+            && ! in_array($moduleName, $this->invalidClassNames, true)
+        ) {
+            return new $moduleName;
         }
 
-        return new $class;
+        return false;
     }
 }
diff --git a/test/Listener/ModuleResolverListenerTest.php b/test/Listener/ModuleResolverListenerTest.php
index 3b3d0c1..af6346c 100644
--- a/test/Listener/ModuleResolverListenerTest.php
+++ b/test/Listener/ModuleResolverListenerTest.php
@@ -10,6 +10,7 @@
 namespace ZendTest\ModuleManager\Listener;
 
 use ListenerTestModule;
+use ModuleAsClass;
 use Zend\ModuleManager\Listener\ModuleResolverListener;
 use Zend\ModuleManager\ModuleEvent;
 
@@ -49,4 +50,22 @@ public function testModuleResolverListenerReturnFalseIfCannotResolveModuleClasse
         $e->setModuleName('DoesNotExist');
         $this->assertFalse($moduleResolver($e));
     }
+
+    public function testModuleResolverListenerPrefersModuleClassesInModuleNamespaceOverNamedClasses()
+    {
+        $moduleResolver = new ModuleResolverListener;
+        $e = new ModuleEvent;
+
+        $e->setModuleName('ModuleAsClass');
+        $this->assertInstanceOf(ModuleAsClass\Module::class, $moduleResolver($e));
+    }
+
+    public function testModuleResolverListenerWillNotAttemptToResolveModuleAsClassNameGenerator()
+    {
+        $moduleResolver = new ModuleResolverListener;
+        $e = new ModuleEvent;
+
+        $e->setModuleName('Generator');
+        $this->assertFalse($moduleResolver($e));
+    }
 }
diff --git a/test/TestAsset/ModuleAsClass.php b/test/TestAsset/ModuleAsClass.php
new file mode 100644
index 0000000..e3588f9
--- /dev/null
+++ b/test/TestAsset/ModuleAsClass.php
@@ -0,0 +1,10 @@
+<?php
+/**
+ * @see       https://github.com/zendframework/zend-modulemanager for the canonical source repository
+ * @copyright Copyright (c) 2017 Zend Technologies USA Inc. (http://www.zend.com)
+ * @license   https://github.com/zendframework/zend-modulemanager/blob/master/LICENSE.md New BSD License
+ */
+
+class ModuleAsClass
+{
+}
diff --git a/test/TestAsset/ModuleAsClass/Module.php b/test/TestAsset/ModuleAsClass/Module.php
new file mode 100644
index 0000000..81c0848
--- /dev/null
+++ b/test/TestAsset/ModuleAsClass/Module.php
@@ -0,0 +1,12 @@
+<?php
+/**
+ * @see       https://github.com/zendframework/zend-modulemanager for the canonical source repository
+ * @copyright Copyright (c) 2017 Zend Technologies USA Inc. (http://www.zend.com)
+ * @license   https://github.com/zendframework/zend-modulemanager/blob/master/LICENSE.md New BSD License
+ */
+
+namespace ModuleAsClass;
+
+class Module
+{
+}