Small changes after review feedback:

- Method and class renames.
- Removed unnecessary autoloader registration.
- Add a unit test for classes with non-object type hints
  in constructor arguments.

includes/wc-core-functions.php

@@ -1954,10 +1954,10 @@ function wc_print_r( $expression, $return = false ) {
 		if ( function_exists( $alternative['func'] ) ) {
 			$res = $alternative['func']( ...$alternative['args'] );
 			if ( $return ) {
-				return $res; // phpcs:disable WordPress.Security.EscapeOutput.OutputNotEscaped
+				return $res; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped
 			}
 
-			echo $res;
+			echo $res; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped
 			return true;
 		}
 	}
@@ -2126,7 +2126,7 @@ function wc_make_phone_clickable( $phone ) {
  * @return mixed Value sanitized by wc_clean.
  */
 function wc_get_post_data_by_key( $key, $default = '' ) {
-	// phpcs:ignore
+	// phpcs:ignore WordPress.Security.ValidatedSanitizedInput, WordPress.Security.NonceVerification.Missing
 	return wc_clean( wp_unslash( wc_get_var( $_POST[ $key ], $default ) ) );
 }
 

src/Autoloader.php

@@ -57,8 +57,7 @@ class Autoloader {
 	 * Define a PSR4 autoloader for the dependency injection engine to work.
 	 * Function grabbed from https://container.thephpleague.com/3.x
 	 *
-	 * TODO: Assess if this is still needed after https://github.com/Automattic/jetpack/pull/15106 is merged.
-	 *       If it still is, remove this notice. If it isn't, remove the method.
+	 * TODO: Remove after the JetPack Autoloader dependency has been updated to v2.
 	 */
 	protected static function register_psr4_autoloader() {
 		spl_autoload_register(

src/Internal/DependencyManagement/AbstractServiceProvider.php

@@ -40,7 +40,7 @@ abstract class AbstractServiceProvider extends \League\Container\ServiceProvider
 		try {
 			$reflector = new \ReflectionClass( $class_name );
 		} catch ( \ReflectionException $ex ) {
-			throw new \Exception( "AbstractServiceProvider::addWithAutoArguments: error when reflecting class '$class_name': {$ex->getMessage()}" );
+			throw new \Exception( "AbstractServiceProvider::add_with_auto_arguments: error when reflecting class '$class_name': {$ex->getMessage()}" );
 		}
 
 		$definition = new Definition( $class_name, $concrete );
@@ -49,7 +49,7 @@ abstract class AbstractServiceProvider extends \League\Container\ServiceProvider
 
 		if ( ! is_null( $constructor ) ) {
 			if ( ! $constructor->isPublic() ) {
-				throw new \Exception( "AbstractServiceProvider::addWithAutoArguments: constructor of class '$class_name' isn't public, instances can't be created." );
+				throw new \Exception( "AbstractServiceProvider::add_with_auto_arguments: constructor of class '$class_name' isn't public, instances can't be created." );
 			}
 
 			$constructor_arguments = $constructor->getParameters();
@@ -60,7 +60,7 @@ abstract class AbstractServiceProvider extends \League\Container\ServiceProvider
 				} else {
 					$argument_class = $argument->getClass();
 					if ( is_null( $argument_class ) ) {
-						throw new \Exception( "AbstractServiceProvider::addWithAutoArguments: constructor argument '{$argument->getName()}' of class '$class_name' doesn't have a type hint or has one that doesn't specify a class." );
+						throw new \Exception( "AbstractServiceProvider::add_with_auto_arguments: constructor argument '{$argument->getName()}' of class '$class_name' doesn't have a type hint or has one that doesn't specify a class." );
 					}
 
 					$definition->addArgument( $argument_class->name );

src/Internal/DependencyManagement/ExtendedContainer.php

@@ -75,7 +75,7 @@ class ExtendedContainer extends \League\Container\Container {
 	/**
 	 * Reset all the cached resolutions, so any further "get" for shared definitions will generate the instance again.
 	 */
-	public function reset_resolved() {
+	public function reset_all_resolved() {
 		foreach ( $this->definitions->getIterator() as $definition ) {
 			// setConcrete causes the cached resolved value to be forgotten.
 			$concrete = $definition->getConcrete();

tests/Tools/CodeHacking/CodeHacker.php

@@ -118,7 +118,7 @@ final class CodeHacker {
 	public static function add_hack( $hack ) {
 		if ( ! self::is_valid_hack_object( $hack ) ) {
 			$class = get_class( $hack );
-			throw new \Exception( "CodeHacker::addhack for instance of $class: Hacks must be objects having a 'process(\$text, \$path)' method and a 'reset()' method." );
+			throw new \Exception( "CodeHacker::add_hack for instance of $class: Hacks must be objects having a 'process(\$text, \$path)' method and a 'reset()' method." );
 		}
 
 		self::$hacks[] = $hack;

tests/Tools/CodeHacking/Hacks/StaticMockerHack.php

@@ -144,8 +144,6 @@ final class StaticMockerHack extends CodeHack {
 	 * @param array $mocks Mocks as an associative array of class name => associative array of method name => mock method with the same arguments as the original method.
 	 *
 	 * @throws \Exception Invalid input.
-	 *
-	 * @oublic
 	 */
 	public static function add_method_mocks( $mocks ) {
 		self::$instance->register_method_mocks( $mocks );

tests/legacy/bootstrap.php

@@ -39,8 +39,6 @@ class WC_Unit_Tests_Bootstrap {
 		$this->tests_dir  = dirname( __FILE__ );
 		$this->plugin_dir = dirname( dirname( $this->tests_dir ) );
 
-		$this->register_autoloader_for_testing_tools();
-
 		$this->initialize_code_hacker();
 
 		ini_set( 'display_errors', 'on' ); // phpcs:ignore WordPress.PHP.IniSet.display_errors_Blacklisted
@@ -122,34 +120,11 @@ class WC_Unit_Tests_Bootstrap {
 		$inner_container = $inner_container_property->getValue( wc_get_container() );
 
 		$inner_container->replace( LegacyProxy::class, MockableLegacyProxy::class );
-		$inner_container->reset_resolved();
+		$inner_container->reset_all_resolved();
 
 		$GLOBALS['wc_container'] = $inner_container;
 	}
 
-	/**
-	 * Register autoloader for the files in the 'tests/tools' directory, for the root namespace 'Automattic\WooCommerce\Testing\Tools'.
-	 */
-	protected static function register_autoloader_for_testing_tools() {
-		return spl_autoload_register(
-			function ( $class ) {
-				$prefix   = 'Automattic\\WooCommerce\\Testing\\Tools\\';
-				$base_dir = dirname( dirname( __FILE__ ) ) . '/Tools';
-				$len      = strlen( $prefix );
-				if ( strncmp( $prefix, $class, $len ) !== 0 ) {
-					// no, move to the next registered autoloader.
-					return;
-				}
-				$relative_class = substr( $class, $len );
-				$file           = $base_dir . str_replace( '\\', '/', $relative_class ) . '.php';
-				if ( ! file_exists( $file ) ) {
-					throw new \Exception( 'Autoloader for unit tests: file not found: ' . $file );
-				}
-				require $file;
-			}
-		);
-	}
-
 	/**
 	 * Load WooCommerce.
 	 *

tests/legacy/framework/class-wc-unit-test-case.php

@@ -183,7 +183,7 @@ class WC_Unit_Test_Case extends WP_HTTP_TestCase {
 	 * This may be needed when registering mocks for already resolved shared classes.
 	 */
 	public function reset_container_resolutions() {
-		wc_get_container()->reset_resolved();
+		wc_get_container()->reset_all_resolved();
 	}
 
 	/**

tests/php/src/Internal/DependencyManagement/AbstractServiceProviderTest.php

@@ -12,13 +12,14 @@ use Automattic\WooCommerce\Internal\DependencyManagement\ExtendedContainer;
 use Automattic\WooCommerce\Tests\Internal\DependencyManagement\ExampleClasses\ClassWithConstructorArgumentWithoutTypeHint;
 use Automattic\WooCommerce\Tests\Internal\DependencyManagement\ExampleClasses\ClassWithDependencies;
 use Automattic\WooCommerce\Tests\Internal\DependencyManagement\ExampleClasses\ClassWithPrivateConstructor;
+use Automattic\WooCommerce\Tests\Internal\DependencyManagement\ExampleClasses\ClassWithScalarConstructorArgument;
 use Automattic\WooCommerce\Tests\Internal\DependencyManagement\ExampleClasses\DependencyClass;
 use League\Container\Definition\DefinitionInterface;
 
 /**
  * Tests for AbstractServiceProvider.
  */
-class AbstractServiceProviderTests extends \WC_Unit_Test_Case {
+class AbstractServiceProviderTest extends \WC_Unit_Test_Case {
 
 	/**
 	 * The system under test.
@@ -69,7 +70,7 @@ class AbstractServiceProviderTests extends \WC_Unit_Test_Case {
 	 */
 	public function test_add_with_auto_arguments_throws_on_non_class_passed() {
 		$this->expectException( \Exception::class );
-		$this->expectExceptionMessage( "AbstractServiceProvider::addWithAutoArguments: error when reflecting class 'foobar': Class foobar does not exist" );
+		$this->expectExceptionMessage( "AbstractServiceProvider::add_with_auto_arguments: error when reflecting class 'foobar': Class foobar does not exist" );
 
 		$this->sut->add_with_auto_arguments( 'foobar' );
 	}
@@ -81,7 +82,7 @@ class AbstractServiceProviderTests extends \WC_Unit_Test_Case {
 	 */
 	public function test_add_with_auto_arguments_throws_on_private_constructor() {
 		$this->expectException( \Exception::class );
-		$this->expectExceptionMessage( "AbstractServiceProvider::addWithAutoArguments: constructor of class '" . ClassWithPrivateConstructor::class . "' isn't public, instances can't be created." );
+		$this->expectExceptionMessage( "AbstractServiceProvider::add_with_auto_arguments: constructor of class '" . ClassWithPrivateConstructor::class . "' isn't public, instances can't be created." );
 
 		$this->sut->add_with_auto_arguments( ClassWithPrivateConstructor::class );
 	}
@@ -93,11 +94,23 @@ class AbstractServiceProviderTests extends \WC_Unit_Test_Case {
 	 */
 	public function test_add_with_auto_arguments_throws_on_constructor_argument_without_type_hint() {
 		$this->expectException( \Exception::class );
-		$this->expectExceptionMessage( "AbstractServiceProvider::addWithAutoArguments: constructor argument 'argument_without_type_hint' of class '" . ClassWithConstructorArgumentWithoutTypeHint::class . "' doesn't have a type hint or has one that doesn't specify a class." );
+		$this->expectExceptionMessage( "AbstractServiceProvider::add_with_auto_arguments: constructor argument 'argument_without_type_hint' of class '" . ClassWithConstructorArgumentWithoutTypeHint::class . "' doesn't have a type hint or has one that doesn't specify a class." );
 
 		$this->sut->add_with_auto_arguments( ClassWithConstructorArgumentWithoutTypeHint::class );
 	}
 
+	/**
+	 * @testdox 'add_with_auto_arguments' should throw an exception if the passed class has a constructor argument with a scalar type hint.
+	 *
+	 * @throws \Exception The passed class has a constructor argument with a scalar type hint.
+	 */
+	public function test_add_with_auto_arguments_throws_on_constructor_argument_with_scalar_type_hint() {
+		$this->expectException( \Exception::class );
+		$this->expectExceptionMessage( "AbstractServiceProvider::add_with_auto_arguments: constructor argument 'scalar_argument_without_default_value' of class '" . ClassWithScalarConstructorArgument::class . "' doesn't have a type hint or has one that doesn't specify a class." );
+
+		$this->sut->add_with_auto_arguments( ClassWithScalarConstructorArgument::class );
+	}
+
 	/**
 	 * @testdox 'add_with_auto_arguments' should properly register the supplied class.
 	 *

tests/php/src/Internal/DependencyManagement/ExampleClasses/ClassWithScalarConstructorArgument.php

@@ -0,0 +1,24 @@
+<?php
+/**
+ * ClassWithConstructorArgumentWithoutTypeHint class file.
+ *
+ * @package Automattic\WooCommerce\Tests\Internal\DependencyManagement\ExampleClasses
+ */
+
+namespace Automattic\WooCommerce\Tests\Internal\DependencyManagement\ExampleClasses;
+
+/**
+ * An example class that has a constructor argument with a scalar type but without a default value.
+ */
+class ClassWithScalarConstructorArgument {
+
+	// phpcs:disable Squiz.Commenting.FunctionComment.InvalidTypeHint
+
+	/**
+	 * Class constructor.
+	 *
+	 * @param mixed $scalar_argument_without_default_value Anything, really.
+	 */
+	public function __construct( int $scalar_argument_without_default_value ) {
+	}
+}

tests/php/src/Internal/DependencyManagement/ExtendedContainerTest.php

@@ -14,7 +14,7 @@ use Automattic\WooCommerce\Tests\Internal\DependencyManagement\ExampleClasses\De
 /**
  * Tests for ExtendedContainer.
  */
-class ExtendedContainerTests extends \WC_Unit_Test_Case {
+class ExtendedContainerTest extends \WC_Unit_Test_Case {
 
 	/**
 	 * The system under test.
@@ -82,9 +82,9 @@ class ExtendedContainerTests extends \WC_Unit_Test_Case {
 	}
 
 	/**
-	 * @testdox 'reset_resolved' should discard cached resolutions for classes registered as 'shared'.
+	 * @testdox 'reset_all_resolved' should discard cached resolutions for classes registered as 'shared'.
 	 */
-	public function test_reset_resolved_discards_cached_shared_resolutions() {
+	public function test_reset_all_resolved_discards_cached_shared_resolutions() {
 		$this->sut->add( DependencyClass::class );
 		$this->sut->add( ClassWithDependencies::class, null, true )->addArgument( DependencyClass::class );
 		ClassWithDependencies::$instances_count = 0;
@@ -94,7 +94,7 @@ class ExtendedContainerTests extends \WC_Unit_Test_Case {
 		$this->sut->get( ClassWithDependencies::class );
 		$this->assertEquals( 1, ClassWithDependencies::$instances_count );
 
-		$this->sut->reset_resolved();
+		$this->sut->reset_all_resolved();
 
 		$this->sut->get( ClassWithDependencies::class );
 		$this->assertEquals( 2, ClassWithDependencies::$instances_count );

tests/php/src/Proxies/LegacyProxyTest.php

@@ -13,7 +13,7 @@ use Automattic\WooCommerce\Tests\Internal\DependencyManagement\ExampleClasses\De
 /**
  * Tests for LegacyProxy
  */
-class LegacyProxyTests extends \WC_Unit_Test_Case {
+class LegacyProxyTest extends \WC_Unit_Test_Case {
 	/**
 	 * The system under test.
 	 *

tests/php/src/Proxies/MockableLegacyProxyTest.php

@@ -13,7 +13,7 @@ use Automattic\WooCommerce\Tests\Internal\DependencyManagement\ExampleClasses\De
 /**
  * Tests for MockableLegacyProxy
  */
-class MockableLegacyProxyTests extends \WC_Unit_Test_Case {
+class MockableLegacyProxyTest extends \WC_Unit_Test_Case {
 
 	/**
 	 * The system under test.