Skip to content

Commit

Permalink
Fix LegacyProxy::get_instance_of for classesd having an instance me…
Browse files Browse the repository at this point in the history
…thod.

Pass the arguments as `...$args` instead of `$args`.
Also fix related unit test, and remove unnecessary `is_function`.
  • Loading branch information
Konamiman committed Jul 24, 2020
1 parent bd1e6a5 commit 4082957
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ private function verify_class_and_concrete( string $class_name, $concrete ) {
} 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.' );
}

Expand Down
2 changes: 1 addition & 1 deletion src/Proxies/LegacyProxy.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public function get_instance_of( string $class_name, ...$args ) {

// 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.
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}
}
12 changes: 7 additions & 5 deletions tests/php/src/Proxies/LegacyProxyTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,15 @@ public function test_get_instance_of_can_be_used_to_get_a_non_namespaced_class_w
}

/**
* @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 );
}

/**
Expand Down

0 comments on commit 4082957

Please sign in to comment.