fix(singleton)!: restore protected static $instance storage

CHANGELOG.md

@@ -7,7 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
-Nothing yet.
+### Changed
+
+- `Singleton` returns to the ecosystem-standard storage shape:
+  `protected static $instance`, stored by `get_instance()` once the constructor
+  returns. The class-string-keyed private map introduced for 1.0.0 broke a real
+  consumer contract — a heavy constructor assigning `static::$instance = $this;`
+  first so work done during construction can re-enter `get_instance()`. That
+  early-assignment guard is now the documented, tested pattern. Trade-off, also
+  documented on the trait: a class using the trait and its subclasses share one
+  storage slot, so do not call `get_instance()` on a subclass of a singleton.
 
 ## [1.0.0] - 2026-07-28
 

inc/Contracts/Traits/Singleton.php

@@ -18,16 +18,22 @@
  */
 trait Singleton {
 	/**
-	 * Instances of the classes using this trait, keyed by concrete class name.
+	 * The single instance of the class using this trait.
 	 *
-	 * A trait's `static` property is a single storage slot shared by a class and
-	 * its subclasses, so a plain `static $instance` lets a parent and a child that
-	 * both use this trait collide (the child would receive the parent's instance).
-	 * Keying by class name gives each concrete class its own instance.
+	 * Protected on purpose: it is part of the trait's contract. A singleton whose
+	 * constructor does real work (a plugin/theme `Main` running a Loader) should
+	 * assign `static::$instance = $this;` as its first statement, so anything
+	 * built during that work can call `get_instance()` re-entrantly and receive
+	 * this same object instead of triggering a second construction.
 	 *
-	 * @var array<class-string, static>
+	 * Limitation: a trait `static` property is one storage slot shared by a class
+	 * and its subclasses. Do not extend a class that uses this trait and call
+	 * `get_instance()` on the child — whichever side is resolved first occupies
+	 * the shared slot for both.
+	 *
+	 * @var ?static
 	 */
-	private static array $instances = [];
+	protected static $instance;
 
 	/**
 	 * The single constructor.
@@ -40,15 +46,19 @@ protected function __construct() {
 
 	/**
 	 * Get the instance of the class.
+	 *
+	 * The instance is stored once the constructor returns. If the constructor
+	 * does work that can call back into `get_instance()`, assign
+	 * `static::$instance = $this;` as its first statement (see the property
+	 * docblock) — otherwise the re-entrant call finds nothing stored yet and
+	 * constructs a second instance, recursing until the stack blows.
 	 */
 	public static function get_instance(): static {
-		$class = static::class;
-
-		if ( ! isset( self::$instances[ $class ] ) ) {
-			self::$instances[ $class ] = new static();
+		if ( ! isset( static::$instance ) ) {
+			static::$instance = new static();
 		}
 
-		return self::$instances[ $class ];
+		return static::$instance;
 	}
 
 	/**

tests/Fixtures/FlakySingleton.php

@@ -0,0 +1,33 @@
+<?php
+/**
+ * Singleton fixture whose constructor throws on the first attempt.
+ *
+ * Exercises the rollback path: a failed construction must not leave a
+ * half-built instance published, and a later call must construct again.
+ *
+ * @package rtCamp\WPFramework\Tests\Fixtures
+ */
+
+declare( strict_types = 1 );
+
+namespace rtCamp\WPFramework\Tests\Fixtures;
+
+use rtCamp\WPFramework\Contracts\Traits\Singleton;
+
+/**
+ * Class - FlakySingleton
+ */
+class FlakySingleton {
+	use Singleton;
+
+	/**
+	 * Throw on construction while true.
+	 */
+	public static bool $fail = true;
+
+	protected function __construct() {
+		if ( self::$fail ) {
+			throw new \RuntimeException( 'construction failed' );
+		}
+	}
+}

tests/Fixtures/ReentrantSingleton.php

@@ -0,0 +1,58 @@
+<?php
+/**
+ * Singleton fixture using the documented heavy-constructor pattern.
+ *
+ * Models the supported consumer shape: a theme/plugin Main whose constructor
+ * assigns `static::$instance = $this;` first, then runs a Loader — and a class
+ * built by that Loader calls Main::get_instance() while Main is still
+ * constructing. The early assignment is the trait's documented re-entrancy
+ * guard; this fixture pins that the property stays protected and assignable.
+ *
+ * @package rtCamp\WPFramework\Tests\Fixtures
+ */
+
+declare( strict_types = 1 );
+
+namespace rtCamp\WPFramework\Tests\Fixtures;
+
+use rtCamp\WPFramework\Contracts\Traits\Singleton;
+
+/**
+ * Class - ReentrantSingleton
+ */
+class ReentrantSingleton {
+	use Singleton;
+
+	/**
+	 * How many times the constructor ran — must stay at 1.
+	 */
+	public static int $construct_count = 0;
+
+	/**
+	 * What the re-entrant get_instance() call returned mid-construction.
+	 */
+	public static ?self $seen_during_construction = null;
+
+	/**
+	 * Set before the re-entrant call, so the test can prove the re-entrant
+	 * caller observed the constructor's progress on the same object.
+	 */
+	public string $ready = 'no';
+
+	protected function __construct() {
+		// The documented pattern: publish first, then do the real work.
+		static::$instance = $this;
+
+		++self::$construct_count;
+
+		if ( self::$construct_count > 3 ) {
+			// A regression would recurse forever; fail loudly instead.
+			throw new \RuntimeException( 'Singleton re-entered its own construction.' );
+		}
+
+		$this->ready = 'yes';
+
+		// What a Loader-built class does from inside Main::__construct().
+		self::$seen_during_construction = self::get_instance();
+	}
+}

tests/SingletonTest.php

@@ -10,9 +10,9 @@
 namespace rtCamp\WPFramework\Tests;
 
 use rtCamp\WPFramework\Tests\Fixtures\BareSingleton;
-use rtCamp\WPFramework\Tests\Fixtures\SingletonChild;
+use rtCamp\WPFramework\Tests\Fixtures\FlakySingleton;
+use rtCamp\WPFramework\Tests\Fixtures\ReentrantSingleton;
 use rtCamp\WPFramework\Tests\Fixtures\SingletonExample;
-use rtCamp\WPFramework\Tests\Fixtures\SingletonParent;
 use rtCamp\WPFramework\Tests\TestCase;
 
 final class SingletonTest extends TestCase {
@@ -59,32 +59,38 @@ public function test_default_constructor_is_used_when_not_overridden(): void {
 		$this->assertInstanceOf( BareSingleton::class, BareSingleton::get_instance() );
 	}
 
-	public function test_parent_and_subclass_get_separate_instances(): void {
-		// Regression: the instance store is keyed by class-string, so a subclass
-		// that inherits get_instance() must not be handed the parent's object
-		// (or vice versa, depending on which one is resolved first).
-		$parent = SingletonParent::get_instance();
-		$child  = SingletonChild::get_instance();
-
-		$this->assertNotSame( $parent, $child );
-		$this->assertInstanceOf( SingletonParent::class, $parent );
-		$this->assertInstanceOf( SingletonChild::class, $child );
-
-		// The subclass must resolve to its own concrete type, not the parent's.
-		$this->assertNotInstanceOf( SingletonChild::class, $parent );
+	public function test_the_documented_early_assignment_guard_supports_reentrant_construction(): void {
+		// Regression: the trait documents that a heavy constructor should assign
+		// `static::$instance = $this;` first, so work done during construction (a
+		// Main running a Loader) can call get_instance() re-entrantly and receive
+		// the same object. This pins that contract — the property must stay
+		// protected and assignable, and the guard must short-circuit a second
+		// construction. Changing the storage broke a real consumer once already.
+		$instance = ReentrantSingleton::get_instance();
+
+		$this->assertSame( 1, ReentrantSingleton::$construct_count );
+		$this->assertSame( $instance, ReentrantSingleton::$seen_during_construction );
+
+		// The re-entrant caller saw the constructor's progress on this object,
+		// not a fresh copy with default property values.
+		$this->assertSame( 'yes', ReentrantSingleton::$seen_during_construction->ready );
 	}
 
-	public function test_parent_and_subclass_each_construct_once(): void {
-		SingletonParent::get_instance();
-		SingletonParent::get_instance();
-		SingletonChild::get_instance();
-		SingletonChild::get_instance();
+	public function test_a_throwing_constructor_is_not_left_published(): void {
+		FlakySingleton::$fail = true;
 
-		$this->assertSame( 1, SingletonParent::$construct_counts[ SingletonParent::class ] ?? 0 );
-		$this->assertSame( 1, SingletonParent::$construct_counts[ SingletonChild::class ] ?? 0 );
-	}
+		try {
+			FlakySingleton::get_instance();
+			$this->fail( 'Expected the constructor exception to propagate.' );
+		} catch ( \RuntimeException $e ) {
+			$this->assertSame( 'construction failed', $e->getMessage() );
+		}
+
+		// Nothing is stored until the constructor returns, so the failed attempt
+		// cached nothing: once construction can succeed, get_instance() builds a
+		// fresh, working instance.
+		FlakySingleton::$fail = false;
 
-	public function test_subclass_instance_is_stable_across_calls(): void {
-		$this->assertSame( SingletonChild::get_instance(), SingletonChild::get_instance() );
+		$this->assertInstanceOf( FlakySingleton::class, FlakySingleton::get_instance() );
 	}
 }