From 408295720c9673e43aadaee2d3ee2176bc793eba Mon Sep 17 00:00:00 2001
From: Nestor Soriano <nestor.soriano@automattic.com>
Date: Tue, 21 Jul 2020 09:07:50 +0200
Subject: [PATCH] Fix LegacyProxy::get_instance_of for classesd having an
 `instance` method.

Pass the arguments as `...$args` instead of `$args`.
Also fix related unit test, and remove unnecessary `is_function`.
---
 .../AbstractServiceProvider.php               |  2 +-
 src/Proxies/LegacyProxy.php                   |  2 +-
 .../ExampleClasses/ClassWithSingleton.php     | 39 +++++++++++++++++++
 tests/php/src/Proxies/LegacyProxyTest.php     | 12 +++---
 4 files changed, 48 insertions(+), 7 deletions(-)
 create mode 100644 tests/php/src/Internal/DependencyManagement/ExampleClasses/ClassWithSingleton.php

diff --git a/src/Internal/DependencyManagement/AbstractServiceProvider.php b/src/Internal/DependencyManagement/AbstractServiceProvider.php
index 76a104eeedc..4f932062fdc 100644
--- a/src/Internal/DependencyManagement/AbstractServiceProvider.php
+++ b/src/Internal/DependencyManagement/AbstractServiceProvider.php
@@ -88,7 +88,7 @@ abstract class AbstractServiceProvider extends \League\Container\ServiceProvider
 			} catch ( \ReflectionException $ex ) {
 				throw new ContainerException( "AbstractServiceProvider::add_with_auto_arguments: error when reflecting class '$class': {$ex->getMessage()}" );
 			}
-		} elseif ( ! is_object( $concrete ) && ! is_callable( $concrete ) && ! function_exists( $concrete ) ) {
+		} elseif ( ! is_object( $concrete ) && ! is_callable( $concrete ) ) {
 			throw new ContainerException( 'AbstractServiceProvider::add_with_auto_arguments: concrete must be a valid class name, function name, object, or callable.' );
 		}
 
diff --git a/src/Proxies/LegacyProxy.php b/src/Proxies/LegacyProxy.php
index 9584dc2f3e3..c110b686f92 100644
--- a/src/Proxies/LegacyProxy.php
+++ b/src/Proxies/LegacyProxy.php
@@ -49,7 +49,7 @@ class LegacyProxy {
 
 		// If the class is a singleton, use the "instance" method.
 		if ( method_exists( $class_name, 'instance' ) ) {
-			return $class_name::instance( $args );
+			return $class_name::instance( ...$args );
 		}
 
 		// Fallback to simply creating a new instance of the class.
diff --git a/tests/php/src/Internal/DependencyManagement/ExampleClasses/ClassWithSingleton.php b/tests/php/src/Internal/DependencyManagement/ExampleClasses/ClassWithSingleton.php
new file mode 100644
index 00000000000..44205c8009e
--- /dev/null
+++ b/tests/php/src/Internal/DependencyManagement/ExampleClasses/ClassWithSingleton.php
@@ -0,0 +1,39 @@
+<?php
+/**
+ * ClassWithSingleton class file.
+ *
+ * @package Automattic\WooCommerce\Tests\Internal\DependencyManagement\ExampleClasses
+ */
+
+// This class is in the root namespace on purpose, since it simulates being a legacy class in the 'includes' directory.
+
+/**
+ * An example of a class that holds a singleton instance.
+ */
+class ClassWithSingleton {
+
+	/**
+	 * @var ClassWithSingleton The singleton instance of the class.
+	 */
+	public static $instance;
+
+	/**
+	 * @var array The arguments supplied to 'instance'.
+	 */
+	public static $instance_args;
+
+	/**
+	 * Gets the singleton instance of the class.
+	 *
+	 * @param mixed ...$args Any arguments required by the method.
+	 *
+	 * @return ClassWithSingleton The singleton instance of the class.
+	 */
+	public static function instance( ...$args ) {
+		if ( is_null( self::$instance ) ) {
+			self::$instance      = new ClassWithSingleton();
+			self::$instance_args = $args;
+		}
+		return self::$instance;
+	}
+}
diff --git a/tests/php/src/Proxies/LegacyProxyTest.php b/tests/php/src/Proxies/LegacyProxyTest.php
index cab79459b4e..aebc9e80794 100644
--- a/tests/php/src/Proxies/LegacyProxyTest.php
+++ b/tests/php/src/Proxies/LegacyProxyTest.php
@@ -50,13 +50,15 @@ class LegacyProxyTest extends \WC_Unit_Test_Case {
 	}
 
 	/**
-	 * @testdox 'get_instance_of' uses the 'instance' static method in classes that implement it.
+	 * @testdox 'get_instance_of' uses the 'instance' static method in classes that implement it, passing the supplied arguments.
 	 */
 	public function test_get_instance_of_class_with_instance_method_gets_an_instance_of_the_appropriate_class() {
-		$instance1 = $this->sut->get_instance_of( \WooCommerce::class );
-		$instance2 = $this->sut->get_instance_of( \WooCommerce::class );
-		$this->assertSame( \WooCommerce::instance(), $instance1 );
-		$this->assertSame( $instance1, $instance2 );
+		// ClassWithSingleton is in the root namespace and thus can't be autoloaded.
+		require_once dirname( __DIR__ ) . '/Internal/DependencyManagement/ExampleClasses/ClassWithSingleton.php';
+
+		$instance = $this->sut->get_instance_of( \ClassWithSingleton::class, 'foo', 'bar' );
+		$this->assertSame( \ClassWithSingleton::$instance, $instance );
+		$this->assertEquals( array( 'foo', 'bar' ), \ClassWithSingleton::$instance_args );
 	}
 
 	/**