Honour prefixes in the Str macros, fix Configs::make() and read config lazily

CHANGELOG.md

@@ -36,6 +36,28 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
   `Illuminate\Http\Request`. It used to drop the code its one caller passed, leaving `0`, and type-hint the
   `Request` facade, which a real request never is.
 
+- The `sku` and `ticketNumber` `Str` macros, scoped and bare, pass their `$prefix` argument through to
+  `Identifiers`. 0.1.0 accepted it and dropped it, which commit `5a0461b` (2022) introduced when it moved the
+  bodies out of the macros; before that the prefix was honoured. A call that passed a prefix now gets it as the
+  leading segment: `Str::sku('laravel', '-', 'pfx')` returned `LAR-8056449213` and now returns
+  `PFX-LAR-8056449213`. Calls without a prefix, or with `null` or `''`, are unchanged.
+
+- `ConfigsInterface::setPrefix()` and `Configs::setPrefix()` both take `?string`. The interface took `string`,
+  refusing the `null` that `getPrefix()` returns, and the class took `mixed`. A class implementing
+  `ConfigsInterface` itself must widen its parameter to `?string`. Calling `Configs::setPrefix()` from a file with
+  `declare(strict_types=1)` with a non-string now throws a `TypeError` at the call; a numeric `prefix` in the
+  config file still works.
+
+- The generator and configs bindings read `simtabi.lacommerce.generator` when they resolve instead of once at
+  boot, so a `config()` change made at runtime, in a test or a tenant switch for example, applies to the next
+  value generated. It was silently ignored before.
+
+- A custom generator naming its own `Str` macro as `$strMixin` has that macro called with the configured prefix
+  as a third argument: `Str::yourMacro($source, $separator, $prefix)`, `$prefix` being `null` when none is
+  configured. It got only the source and separator, so a configured prefix never reached it. A macro declaring
+  two parameters ignores the extra argument; one whose third parameter meant something else now receives the
+  prefix there.
+
 ### Deprecated
 
 - The bare macros `Str::sku()`, `Str::orderNumber()` and `Str::ticketNumber()`. `Str`'s macro registry is one
@@ -57,6 +79,15 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
   against a model. A test fails if the documented example stops matching its fixture.
 - A custom generator that implemented `GeneratorInterface` without a `__toString()` threw
   `Object ... could not be converted to string` on every save.
+- PHPStan reported `InvalidOptionException::invalidArgument()`'s `new static` as unsafe. The class now declares
+  `@phpstan-consistent-constructor` and stays open, so a subclass still gets an instance of itself. Nothing in
+  the package extends it; a subclass elsewhere must keep a constructor callable as `new static($message, $code)`,
+  which PHPStan now checks.
+- `Configs::make()` resolved the base `Configs` class, which the container cannot build, so calling it on the
+  base, or on a subclass that inherited it, threw `BindingResolutionException` about an unresolvable
+  `array $config`. It now resolves the class it is called on (`static::class`), and on the base class throws
+  `InvalidOptionException` naming the subclasses to call instead. `SkuConfigs::make()` and its siblings are
+  unchanged.
 - `Configs::getPrefix()`, and `skuConfig('prefix')` and its siblings, threw "must not be accessed before
   initialization" unless `setPrefix()` had been called. The prefix now defaults to `null`.
 

UPGRADING.md

@@ -30,11 +30,41 @@ Two things changed for a custom generator that already works:
   returned something other than `render()` now has `render()` used.
 - A `generator` key naming a class that does not implement `GeneratorInterface` throws
   `InvalidOptionException` naming the key, instead of a `TypeError`.
+- A generator naming its own `Str` macro as `$strMixin` has it called as `($source, $separator, $prefix)`, with
+  the configured prefix or `null`. It was called with two arguments. Check the macro's third parameter, if it
+  has one.
 
 ### Prefixes
 
 Each generator block takes a new `prefix` key. A config published before this release does not have it, which
 is the same as `null`: values are generated exactly as before. Add it to the block to prefix the values.
-`Identifiers::sku()` and `Identifiers::ticketNumber()` take the prefix as an optional third argument. The `Str`
-macros still ignore their `$prefix` argument for SKUs and ticket numbers, as 0.1.0 did.
+`Identifiers::sku()` and `Identifiers::ticketNumber()` take the prefix as an optional third argument.
 
+### The `Str` macros honour `$prefix` for SKUs and ticket numbers
+
+**This changes output.** In 0.1.0, `Str::sku()` and `Str::ticketNumber()` accepted a third `$prefix` argument
+and dropped it. They, and the scoped `Str::simtabiLacommerceSku()` and `Str::simtabiLacommerceTicketNumber()`,
+now put it first:
+
+| Call | 0.1.0 | Now |
+|------|-------|-----|
+| `Str::sku('laravel', '-', 'pfx')` | `LAR-8056449213` | `PFX-LAR-8056449213` |
+| `Str::ticketNumber('support', '-', 'hd')` | `SUP-8056449213` | `HD-SUP-8056449213` |
+
+A call passing no prefix, `null` or `''` is unchanged. If you passed a prefix and relied on it being dropped,
+stop passing it. `Str::orderNumber()` always used its prefix and is unchanged.
+
+### `setPrefix()` takes `?string`
+
+`ConfigsInterface::setPrefix()` took `string` and `Configs::setPrefix()` took `mixed`; both take `?string` now.
+
+- If you implement `ConfigsInterface` yourself, change your `setPrefix(string $prefix)` to
+  `setPrefix(?string $prefix)`, or PHP refuses to load the class.
+- If you call `setPrefix()` with an `int` or another non-string from a file declaring `strict_types=1`, cast
+  it to a string first. A numeric `prefix` in the config file still works.
+
+### Runtime config changes now apply
+
+The generator and configs bindings used to read `simtabi.lacommerce.generator` once, at boot. They read it
+when they resolve now, so `config()->set('simtabi.lacommerce.generator.sku.prefix', 'X')` after boot changes
+the next SKU generated. If code changed that config at runtime expecting nothing to happen, it now takes effect.

docs/configuration.md

@@ -60,6 +60,11 @@ return [
 
 Override any of these per model via the trait's config method — see [Generators](tools/generators.md).
 
+The generator and configs bindings read `simtabi.lacommerce.generator` each time they resolve, so a
+`config()->set('simtabi.lacommerce.generator.…', …)` made after boot applies to the next model saved. Set the
+`simtabi.lacommerce` key: the deprecated `lacommerce` mirror is copied once at boot and is not read back. The
+`Str` macros' default separator is still read once, at boot.
+
 The published file replaces the defaults block by block, not key by key: a top-level key you keep
 (`generator`) must carry every nested key the package reads, so publish the whole file and edit it.
 

docs/tools/generators.md

@@ -67,13 +67,13 @@ configured separator when you pass none:
 
 | Macro | Forwards to |
 |-------|-------------|
-| `Str::simtabiLacommerceSku(string $source, ?string $separator = null)` | `Identifiers::sku()` |
+| `Str::simtabiLacommerceSku(string $source, ?string $separator = null, ?string $prefix = null)` | `Identifiers::sku()` |
 | `Str::simtabiLacommerceOrderNumber(?string $source, ?string $separator = null, ?string $prefix = null)` | `Identifiers::orderNumber($prefix, …)`; `$source` is ignored |
-| `Str::simtabiLacommerceTicketNumber(string $source, ?string $separator = null)` | `Identifiers::ticketNumber()` |
+| `Str::simtabiLacommerceTicketNumber(string $source, ?string $separator = null, ?string $prefix = null)` | `Identifiers::ticketNumber()` |
 
 Each takes the same arguments as the bare macro it replaces, so migrating is a rename. The `sku` and
-`ticketNumber` macros accept a third `$prefix` argument and ignore it, as the 0.1.0 macros did; to prefix a
-SKU or ticket number, call `Identifiers` or set `prefix` in the config.
+`ticketNumber` macros pass their third `$prefix` argument through, so `Str::simtabiLacommerceSku('laravel',
+'-', 'acme')` returns e.g. `ACME-LAR-8056449213`. The 0.1.0 macros accepted it and dropped it.
 
 > The bare `Str::sku()`, `Str::orderNumber()` and `Str::ticketNumber()` from 0.1.0 are deprecated.
 > `Str`'s macros are one flat map keyed by name, so another package or your application registering

src/Generators/Exceptions/InvalidOptionException.php

@@ -8,6 +8,14 @@
 use Illuminate\Http\Response;
 use Illuminate\Http\Request;
 
+/**
+ * Thrown for a configuration value the package cannot use.
+ *
+ * Not final, so a subclass keeps working, and invalidArgument() returns an instance of the class it is called
+ * on. A subclass must keep a constructor callable as `new static($message, $code)`.
+ *
+ * @phpstan-consistent-constructor
+ */
 class InvalidOptionException extends Exception
 {
 

src/Generators/Services/Configs.php

@@ -77,11 +77,27 @@ public function __construct(array $config, string $key)
     }
 
     /**
+     * Resolve the class this is called on from the container.
+     *
+     * Call it on a subclass the container can build, such as SkuConfigs::make(). The base class cannot be
+     * resolved, because its constructor needs a config array and a key, so calling make() on it throws an
+     * exception naming the subclasses rather than a container error about an unresolvable `array $config`.
+     *
      * @return ConfigsInterface
+     *
+     * @throws InvalidOptionException when called on Configs itself
      */
     public static function make(): ConfigsInterface
     {
-        return resolve(self::class);
+        if (static::class === self::class) {
+            throw InvalidOptionException::invalidArgument(sprintf(
+                '%s::make() cannot build the base class; call it on a subclass bound in the container, such as '
+                . 'SkuConfigs::make(), OrderNumberConfigs::make() or TicketNumberConfigs::make().',
+                self::class,
+            ));
+        }
+
+        return resolve(static::class);
     }
 
     /**
@@ -109,10 +125,13 @@ public function getSourceColumn(): array|string
      * Set the prefix: a leading part of the generated value. Null or an empty string for none. For an
      * order number it replaces the default `ORD`.
      *
-     * @param mixed $prefix
+     * Declared `?string`, as ConfigsInterface declares it. It took `mixed` before; a scalar from a config
+     * file is still coerced to a string here, because this file does not declare strict types.
+     *
+     * @param ?string $prefix
      * @return $this
      */
-    public function setPrefix(mixed $prefix): self
+    public function setPrefix(?string $prefix): self
     {
         $this->prefix = $prefix;
 

src/Generators/Services/Contracts/ConfigsInterface.php

@@ -35,12 +35,12 @@ public function setSourceColumn(array|string $sourceColumn): self;
     public function getSourceColumn(): array|string;
 
     /**
-     * Set the prefix.
+     * Set the prefix: a leading part of the generated value. Null or an empty string for none.
      *
-     * @param string $prefix
+     * @param ?string $prefix
      * @return $this
      */
-    public function setPrefix(string $prefix): self;
+    public function setPrefix(?string $prefix): self;
 
     /**
      * @return ?string

src/Generators/Services/Generator.php

@@ -117,7 +117,8 @@ protected function generate(string $source, string $separator, bool $unique = fa
      * The three shipped generators call Identifiers directly rather than the `Str` macro named by
      * `$strMixin`: Str's macro registry is a flat, host-owned map, so going through it let any package
      * or application that registered its own `Str::sku()` replace what every model generated. A
-     * subclass naming any other `$strMixin` still has that macro called, as before.
+     * subclass naming any other `$strMixin` still has that macro called, as before, with the configured
+     * prefix (or null) as a third argument, the order the package's own macros take.
      *
      * An empty separator falls back to the configured default, as the 0.1.0 macros did. The configured
      * prefix, when there is one, leads the value; for an order number it replaces `ORD`.
@@ -139,7 +140,7 @@ protected function makeValue(string $source, string $separator): string
             'sku'          => Identifiers::sku($source, $separator, $prefix),
             'orderNumber'  => Identifiers::orderNumber($prefix, $separator),
             'ticketNumber' => Identifiers::ticketNumber($source, $separator, $prefix),
-            default        => Str::{$this->strMixin}($source, $separator),
+            default        => Str::{$this->strMixin}($source, $separator, $prefix),
         };
     }
 

src/Providers/LacommerceServiceProvider.php

@@ -155,22 +155,22 @@ private function registerConsoles(): static
      * argument, and it must implement GeneratorInterface: the observer calls render() on it. Extending
      * Generators\Services\Generator satisfies both. See docs/tools/generators.md.
      *
+     * The config is read when the binding resolves, not at boot, so a `config()` change made after boot
+     * applies to the next generator built.
+     *
      * @return void
      */
     protected function bindGenerator()
     {
-
-        $config = $this->getConfig();
-
         $bindings = [
             SkuGeneratorInterface::class          => 'sku',
             OrderNumberGeneratorInterface::class  => 'order_number',
             TicketNumberGeneratorInterface::class => 'ticket_number',
         ];
 
         foreach ($bindings as $interface => $key) {
-            $this->app->bind($interface, function ($app, array $parameters) use ($config, $key) {
-                return self::makeGenerator($config[$key]['generator'] ?? null, $key, head($parameters));
+            $this->app->bind($interface, function ($app, array $parameters) use ($key) {
+                return self::makeGenerator($this->getConfig()[$key]['generator'] ?? null, $key, head($parameters));
             });
         }
     }
@@ -205,25 +205,24 @@ private function getConfig(): array
     /**
      * Bind Configs.
      *
+     * Not singletons, and the config is read when the binding resolves rather than at boot, so a `config()`
+     * change made after boot applies to the next Configs built.
+     *
      * @return void
      */
     protected function bindConfigs()
     {
-
-        $config = $this->getConfig();
-
-        $this->app->bind(SkuConfigs::class, function ($app) use ($config) {
-            return new SkuConfigs($config, 'sku');
+        $this->app->bind(SkuConfigs::class, function ($app) {
+            return new SkuConfigs($this->getConfig(), 'sku');
         });
 
-        $this->app->bind(OrderNumberConfigs::class, function ($app) use ($config) {
-            return new OrderNumberConfigs($config, 'order_number');
+        $this->app->bind(OrderNumberConfigs::class, function ($app) {
+            return new OrderNumberConfigs($this->getConfig(), 'order_number');
         });
 
-        $this->app->bind(TicketNumberConfigs::class, function ($app) use ($config) {
-            return new TicketNumberConfigs($config, 'ticket_number');
+        $this->app->bind(TicketNumberConfigs::class, function ($app) {
+            return new TicketNumberConfigs($this->getConfig(), 'ticket_number');
         });
-
     }
 
     /**

src/Supports/StrMacros.php

@@ -17,7 +17,8 @@
  *
  * The bare names shipped in 0.1.0 (`Str::sku()`, `Str::orderNumber()`, `Str::ticketNumber()`) are
  * still registered, unless `simtabi.lacommerce.register_legacy_macros` is false, and forward to the
- * scoped macro with the same arguments.
+ * scoped macro with the same arguments. A `$prefix` passed to the SKU or ticket-number macro leads the
+ * value (`PFX-LAR-8056449213`); 0.1.0 accepted it and dropped it.
  */
 final class StrMacros
 {
@@ -63,8 +64,8 @@ public static function register(string $defaultSeparator, bool $registerLegacy =
         self::$warned = [];
 
         Str::macro(self::SKU, static function (string $source, ?string $separator = null, ?string $prefix = null) use ($defaultSeparator): string {
-            // $prefix is accepted and ignored, as it was by the 0.1.0 Str::sku() this replaces.
-            return Identifiers::sku($source, $separator ?: $defaultSeparator);
+            // 0.1.0 accepted $prefix and dropped it; it leads the value again, as it did before 2022.
+            return Identifiers::sku($source, $separator ?: $defaultSeparator, $prefix);
         });
 
         Str::macro(self::ORDER_NUMBER, static function (?string $source, ?string $separator = null, ?string $prefix = null) use ($defaultSeparator): string {
@@ -73,8 +74,8 @@ public static function register(string $defaultSeparator, bool $registerLegacy =
         });
 
         Str::macro(self::TICKET_NUMBER, static function (string $source, ?string $separator = null, ?string $prefix = null) use ($defaultSeparator): string {
-            // $prefix is accepted and ignored, as it was by the 0.1.0 Str::ticketNumber() this replaces.
-            return Identifiers::ticketNumber($source, $separator ?: $defaultSeparator);
+            // 0.1.0 accepted $prefix and dropped it; it leads the value again, as it did before 2022.
+            return Identifiers::ticketNumber($source, $separator ?: $defaultSeparator, $prefix);
         });
 
         if ($registerLegacy) {

tests/Feature/ConfigsMakeTest.php

@@ -0,0 +1,46 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Simtabi\Lacommerce\Tests\Feature;
+
+use PHPUnit\Framework\Attributes\Test;
+use Simtabi\Lacommerce\Generators\Concerns\Sku\SkuConfigs;
+use Simtabi\Lacommerce\Generators\Exceptions\InvalidOptionException;
+use Simtabi\Lacommerce\Generators\Services\Configs;
+use Simtabi\Lacommerce\Providers\LacommerceServiceProvider;
+use Simtabi\Lacommerce\Tests\Fixtures\Configs\WarehouseSkuConfigs;
+use Simtabi\Lacommerce\Tests\TestCase;
+
+/**
+ * Configs::make() on the base class used to resolve Configs itself, which the container cannot build: its
+ * constructor needs a config array and a key. It now says so, and a subclass that inherits make() gets itself.
+ */
+final class ConfigsMakeTest extends TestCase
+{
+    #[Test]
+    public function make_on_the_base_class_throws_an_exception_that_names_the_fix(): void
+    {
+        $this->expectException(InvalidOptionException::class);
+        $this->expectExceptionMessage('SkuConfigs::make()');
+
+        Configs::make();
+    }
+
+    #[Test]
+    public function make_on_a_shipped_subclass_still_returns_that_subclass(): void
+    {
+        $this->assertInstanceOf(SkuConfigs::class, SkuConfigs::make());
+    }
+
+    #[Test]
+    public function a_subclass_that_inherits_make_resolves_itself(): void
+    {
+        $this->app->bind(WarehouseSkuConfigs::class, static fn (): WarehouseSkuConfigs => new WarehouseSkuConfigs(
+            (array) config(LacommerceServiceProvider::CONFIG_KEY . '.generator'),
+            'sku',
+        ));
+
+        $this->assertInstanceOf(WarehouseSkuConfigs::class, WarehouseSkuConfigs::make());
+    }
+}