From 2921e723fa652597eb9e468964b0640e242f4f15 Mon Sep 17 00:00:00 2001 From: Imani Manyara Date: Wed, 7 Oct 2026 08:52:02 -0400 Subject: [PATCH 1/6] Pass the prefix through the SKU and ticket-number Str macros The scoped and deprecated bare macros accepted a $prefix argument and dropped it, a leftover of the 2022 move of their bodies into Helpers. They now forward it to Identifiers, so a caller passing a prefix gets it as the leading segment. Calls without one are unchanged. --- CHANGELOG.md | 6 ++++++ UPGRADING.md | 17 +++++++++++++++-- docs/tools/generators.md | 8 ++++---- src/Supports/StrMacros.php | 11 ++++++----- tests/Feature/LegacyStrMacrosTest.php | 17 ++++++++++++++--- tests/Feature/ScopedStrMacrosTest.php | 10 +++++++++- 6 files changed, 54 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 09d9005..2be9cc3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,12 @@ 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. + ### Deprecated - The bare macros `Str::sku()`, `Str::orderNumber()` and `Str::ticketNumber()`. `Str`'s macro registry is one diff --git a/UPGRADING.md b/UPGRADING.md index 2795e42..7553479 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -35,6 +35,19 @@ Two things changed for a custom generator that already works: 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. diff --git a/docs/tools/generators.md b/docs/tools/generators.md index d6c3300..e828f8a 100644 --- a/docs/tools/generators.md +++ b/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 diff --git a/src/Supports/StrMacros.php b/src/Supports/StrMacros.php index bd8eadf..15e754b 100644 --- a/src/Supports/StrMacros.php +++ b/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) { diff --git a/tests/Feature/LegacyStrMacrosTest.php b/tests/Feature/LegacyStrMacrosTest.php index e37dfe5..8d89183 100644 --- a/tests/Feature/LegacyStrMacrosTest.php +++ b/tests/Feature/LegacyStrMacrosTest.php @@ -12,7 +12,8 @@ /** * Pins the behaviour of the bare Str macros shipped in 0.1.0 (`Str::sku`, `Str::orderNumber`, * `Str::ticketNumber`). They are deprecated, not removed: their output format and arguments must not - * change until the release that removes them. + * change until the release that removes them. One change is deliberate: the SKU and ticket-number macros + * pass their `$prefix` through again, which 0.1.0 accepted and dropped (see CHANGELOG, Unreleased). */ final class LegacyStrMacrosTest extends TestCase { @@ -53,9 +54,12 @@ public function sku_falls_back_to_the_configured_separator_for_an_empty_one(): v } #[Test] - public function sku_ignores_its_prefix_argument(): void + public function sku_leads_with_its_prefix_argument(): void { - $this->assertMatchesRegularExpression('/^LAR-\d{10}$/', @Str::sku('laravel', '-', 'PFX')); + // 0.1.0 accepted the prefix and dropped it. It is honoured again, as it was before 2022. + $this->assertMatchesRegularExpression('/^PFX-LAR-\d{10}$/', @Str::sku('laravel', '-', 'pfx')); + $this->assertMatchesRegularExpression('/^LAR-\d{10}$/', @Str::sku('laravel', '-', null)); + $this->assertMatchesRegularExpression('/^LAR-\d{10}$/', @Str::sku('laravel', '-', '')); } #[Test] @@ -78,6 +82,13 @@ public function ticket_number_takes_the_first_three_studly_characters_of_the_sou $this->assertMatchesRegularExpression('/^SUP\.\d{10}$/', @Str::ticketNumber('support', '.')); } + #[Test] + public function ticket_number_leads_with_its_prefix_argument(): void + { + $this->assertMatchesRegularExpression('/^HD-SUP-\d{10}$/', @Str::ticketNumber('support', '-', 'hd')); + $this->assertMatchesRegularExpression('/^HD-SUP-\d{10}$/', @Str::ticketNumber('support', null, 'hd')); + } + #[Test] #[WithoutErrorHandler] public function each_bare_macro_raises_one_deprecation_per_boot_and_still_returns_a_value(): void diff --git a/tests/Feature/ScopedStrMacrosTest.php b/tests/Feature/ScopedStrMacrosTest.php index 12a9527..bc865ff 100644 --- a/tests/Feature/ScopedStrMacrosTest.php +++ b/tests/Feature/ScopedStrMacrosTest.php @@ -48,13 +48,21 @@ public function no_scoped_name_shadows_a_real_str_method(): void public function the_scoped_macros_keep_the_bare_signatures(): void { $this->assertMatchesRegularExpression('/^LAR-\d{10}$/', Str::simtabiLacommerceSku('laravel is awesome')); - $this->assertMatchesRegularExpression('/^LAR_\d{10}$/', Str::simtabiLacommerceSku('laravel', '_', 'ignored')); + $this->assertMatchesRegularExpression('/^LAR_\d{10}$/', Str::simtabiLacommerceSku('laravel', '_')); $this->assertMatchesRegularExpression('/^LAR-\d{10}$/', Str::simtabiLacommerceSku('laravel', '')); $this->assertMatchesRegularExpression('/^ORD-\d{10}$/', Str::simtabiLacommerceOrderNumber('ignored')); $this->assertMatchesRegularExpression('/^INV\/\d{10}$/', Str::simtabiLacommerceOrderNumber(null, '/', 'inv')); $this->assertMatchesRegularExpression('/^SUP-\d{10}$/', Str::simtabiLacommerceTicketNumber('support request')); } + #[Test] + public function the_scoped_macros_pass_the_prefix_through(): void + { + $this->assertMatchesRegularExpression('/^ACME_LAR_\d{10}$/', Str::simtabiLacommerceSku('laravel', '_', 'acme')); + $this->assertMatchesRegularExpression('/^HD-SUP-\d{10}$/', Str::simtabiLacommerceTicketNumber('support', '', 'hd')); + $this->assertMatchesRegularExpression('/^SUP-\d{10}$/', Str::simtabiLacommerceTicketNumber('support', '-', null)); + } + #[Test] #[WithoutErrorHandler] public function the_scoped_macros_raise_no_deprecation(): void From c8859b0a3ec93d1c968d178bdc0947f030cbeb05 Mon Sep 17 00:00:00 2001 From: Imani Manyara Date: Wed, 7 Oct 2026 08:52:38 -0400 Subject: [PATCH 2/6] Make Configs::make() resolve the class it is called on On the base class it resolved Configs itself, which the container cannot build, so it failed with an unresolvable-dependency error; a subclass inheriting make() failed the same way. It now resolves static::class and throws an InvalidOptionException naming the subclasses on the base. --- CHANGELOG.md | 5 ++ src/Generators/Services/Configs.php | 18 +++++++- tests/Feature/ConfigsMakeTest.php | 46 +++++++++++++++++++ .../Fixtures/Configs/WarehouseSkuConfigs.php | 14 ++++++ 4 files changed, 82 insertions(+), 1 deletion(-) create mode 100644 tests/Feature/ConfigsMakeTest.php create mode 100644 tests/Fixtures/Configs/WarehouseSkuConfigs.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 2be9cc3..a3f48a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -63,6 +63,11 @@ 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. +- `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`. diff --git a/src/Generators/Services/Configs.php b/src/Generators/Services/Configs.php index da40f5f..d58badf 100644 --- a/src/Generators/Services/Configs.php +++ b/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); } /** diff --git a/tests/Feature/ConfigsMakeTest.php b/tests/Feature/ConfigsMakeTest.php new file mode 100644 index 0000000..68dc6eb --- /dev/null +++ b/tests/Feature/ConfigsMakeTest.php @@ -0,0 +1,46 @@ +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()); + } +} diff --git a/tests/Fixtures/Configs/WarehouseSkuConfigs.php b/tests/Fixtures/Configs/WarehouseSkuConfigs.php new file mode 100644 index 0000000..2f4df97 --- /dev/null +++ b/tests/Fixtures/Configs/WarehouseSkuConfigs.php @@ -0,0 +1,14 @@ + Date: Wed, 7 Oct 2026 08:53:13 -0400 Subject: [PATCH 3/6] Declare setPrefix() as ?string on both the contract and Configs The interface took string, refusing the null getPrefix() returns, while the class took mixed. Both now take ?string. --- CHANGELOG.md | 6 ++ UPGRADING.md | 8 +++ src/Generators/Services/Configs.php | 7 ++- .../Services/Contracts/ConfigsInterface.php | 6 +- tests/Feature/ConfigsPrefixSignatureTest.php | 57 +++++++++++++++++++ 5 files changed, 79 insertions(+), 5 deletions(-) create mode 100644 tests/Feature/ConfigsPrefixSignatureTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index a3f48a0..7d46986 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), 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. + ### Deprecated - The bare macros `Str::sku()`, `Str::orderNumber()` and `Str::ticketNumber()`. `Str`'s macro registry is one diff --git a/UPGRADING.md b/UPGRADING.md index 7553479..eb07ee2 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -51,3 +51,11 @@ now put it first: 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. diff --git a/src/Generators/Services/Configs.php b/src/Generators/Services/Configs.php index d58badf..bb3e8e1 100644 --- a/src/Generators/Services/Configs.php +++ b/src/Generators/Services/Configs.php @@ -125,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; diff --git a/src/Generators/Services/Contracts/ConfigsInterface.php b/src/Generators/Services/Contracts/ConfigsInterface.php index 20c40ee..5ff285f 100644 --- a/src/Generators/Services/Contracts/ConfigsInterface.php +++ b/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 diff --git a/tests/Feature/ConfigsPrefixSignatureTest.php b/tests/Feature/ConfigsPrefixSignatureTest.php new file mode 100644 index 0000000..ce569ce --- /dev/null +++ b/tests/Feature/ConfigsPrefixSignatureTest.php @@ -0,0 +1,57 @@ + $config]; + } + + #[Test] + public function the_interface_and_the_class_declare_the_same_nullable_string_parameter(): void + { + foreach ([ConfigsInterface::class, Configs::class] as $class) { + $type = (new ReflectionMethod($class, 'setPrefix'))->getParameters()[0]->getType(); + + $this->assertInstanceOf(ReflectionNamedType::class, $type, "{$class}::setPrefix()"); + $this->assertSame('string', $type->getName(), "{$class}::setPrefix()"); + $this->assertTrue($type->allowsNull(), "{$class}::setPrefix()"); + } + } + + #[Test] + public function the_prefix_can_be_set_and_cleared_through_the_interface(): void + { + $configs = SkuConfigs::make(); + + $this->assertSame('ACME', $configs->setPrefix('ACME')->getPrefix()); + $this->assertNull($configs->setPrefix(null)->getPrefix()); + } + + #[Test] + public function a_numeric_prefix_in_the_config_file_still_works(): void + { + $this->assertSame('42', SkuConfigs::make()->getPrefix()); + $this->assertMatchesRegularExpression('/^42-BLU-\d{10}$/', DummyProduct::create(['name' => 'Blue shirt'])->sku); + } +} From 4fdfe65ded3dda0dc924c69f90312d3e55818573 Mon Sep 17 00:00:00 2001 From: Imani Manyara Date: Wed, 7 Oct 2026 08:53:41 -0400 Subject: [PATCH 4/6] Declare a consistent constructor on InvalidOptionException invalidArgument() builds new static on a non-final class, which PHPStan reports as unsafe. The class stays open so subclasses keep returning themselves, and @phpstan-consistent-constructor makes PHPStan check that a subclass keeps a compatible constructor. --- CHANGELOG.md | 4 ++ .../Exceptions/InvalidOptionException.php | 8 ++++ tests/Unit/InvalidOptionExceptionTest.php | 41 +++++++++++++++++++ 3 files changed, 53 insertions(+) create mode 100644 tests/Unit/InvalidOptionExceptionTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d46986..c1a0849 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -69,6 +69,10 @@ 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 diff --git a/src/Generators/Exceptions/InvalidOptionException.php b/src/Generators/Exceptions/InvalidOptionException.php index be074e9..1d1d376 100644 --- a/src/Generators/Exceptions/InvalidOptionException.php +++ b/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 { diff --git a/tests/Unit/InvalidOptionExceptionTest.php b/tests/Unit/InvalidOptionExceptionTest.php new file mode 100644 index 0000000..1927963 --- /dev/null +++ b/tests/Unit/InvalidOptionExceptionTest.php @@ -0,0 +1,41 @@ +assertInstanceOf($subclass::class, $exception); + $this->assertSame('bad', $exception->getMessage()); + $this->assertSame(422, $exception->getCode()); + $this->assertSame(500, InvalidOptionException::invalidArgument('bad')->getCode()); + } + + #[Test] + public function the_class_stays_extendable_and_declares_a_consistent_constructor(): void + { + $class = new ReflectionClass(InvalidOptionException::class); + + $this->assertFalse($class->isFinal()); + $this->assertStringContainsString('@phpstan-consistent-constructor', (string) $class->getDocComment()); + } +} From 665c8d9e63c903aa8a366b6b61dd8346c1e5bb48 Mon Sep 17 00:00:00 2001 From: Imani Manyara Date: Wed, 7 Oct 2026 08:54:23 -0400 Subject: [PATCH 5/6] Read the generator config when the bindings resolve, not at boot bindGenerator() and bindConfigs() closed over the config read during boot, so a later config() change never reached a generator. The closures now read it on each resolve; they were already non-singleton binds. --- CHANGELOG.md | 4 ++ UPGRADING.md | 6 +++ docs/configuration.md | 5 +++ src/Providers/LacommerceServiceProvider.php | 29 ++++++------- tests/Feature/RuntimeConfigTest.php | 47 +++++++++++++++++++++ 5 files changed, 76 insertions(+), 15 deletions(-) create mode 100644 tests/Feature/RuntimeConfigTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index c1a0849..18c3aa4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), `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. + ### Deprecated - The bare macros `Str::sku()`, `Str::orderNumber()` and `Str::ticketNumber()`. `Str`'s macro registry is one diff --git a/UPGRADING.md b/UPGRADING.md index eb07ee2..c7fe5d4 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -59,3 +59,9 @@ stop passing it. `Str::orderNumber()` always used its prefix and is unchanged. `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. diff --git a/docs/configuration.md b/docs/configuration.md index 87ebc19..1d082b5 100644 --- a/docs/configuration.md +++ b/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. diff --git a/src/Providers/LacommerceServiceProvider.php b/src/Providers/LacommerceServiceProvider.php index 5115cc3..67662d4 100644 --- a/src/Providers/LacommerceServiceProvider.php +++ b/src/Providers/LacommerceServiceProvider.php @@ -155,13 +155,13 @@ 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', @@ -169,8 +169,8 @@ protected function bindGenerator() ]; 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'); }); - } /** diff --git a/tests/Feature/RuntimeConfigTest.php b/tests/Feature/RuntimeConfigTest.php new file mode 100644 index 0000000..7213d44 --- /dev/null +++ b/tests/Feature/RuntimeConfigTest.php @@ -0,0 +1,47 @@ +set(self::GENERATOR . '.sku.prefix', 'live'); + config()->set(self::GENERATOR . '.default.separator', '_'); + + $configs = SkuConfigs::make(); + + $this->assertSame('live', $configs->getPrefix()); + $this->assertSame('_', $configs->getSeparator()); + $this->assertMatchesRegularExpression('/^LIVE_BLU_\d{10}$/', DummyProduct::create(['name' => 'Blue shirt'])->sku); + } + + #[Test] + public function a_generator_class_configured_after_boot_is_the_one_resolved(): void + { + config()->set(self::GENERATOR . '.sku.generator', FallbackSkuGenerator::class); + + $this->assertInstanceOf( + FallbackSkuGenerator::class, + resolve(SkuGeneratorInterface::class, ['model' => new DummyProduct(['name' => 'Blue shirt'])]), + ); + $this->assertMatchesRegularExpression('/^ITE-\d{10}$/', DummyProduct::create(['name' => ''])->sku); + } +} From 1dc4ff467f8fe85e5d9d214839c94272bdd3fe45 Mon Sep 17 00:00:00 2001 From: Imani Manyara Date: Wed, 7 Oct 2026 08:54:55 -0400 Subject: [PATCH 6/6] Pass the configured prefix to a custom generator's Str macro Generator::makeValue() called a custom $strMixin macro with the source and separator only, so a configured prefix never reached it while the shipped generators honoured it. It now passes the prefix, or null, as a third argument. --- CHANGELOG.md | 6 +++ UPGRADING.md | 3 ++ src/Generators/Services/Generator.php | 5 +- tests/Feature/CustomStrMixinPrefixTest.php | 51 +++++++++++++++++++ .../Fixtures/Generators/MacroSkuGenerator.php | 22 ++++++++ 5 files changed, 85 insertions(+), 2 deletions(-) create mode 100644 tests/Feature/CustomStrMixinPrefixTest.php create mode 100644 tests/Fixtures/Generators/MacroSkuGenerator.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 18c3aa4..02d14a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), 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 diff --git a/UPGRADING.md b/UPGRADING.md index c7fe5d4..c400305 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -30,6 +30,9 @@ 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 diff --git a/src/Generators/Services/Generator.php b/src/Generators/Services/Generator.php index 2892636..bb3b1a2 100644 --- a/src/Generators/Services/Generator.php +++ b/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), }; } diff --git a/tests/Feature/CustomStrMixinPrefixTest.php b/tests/Feature/CustomStrMixinPrefixTest.php new file mode 100644 index 0000000..8cd36d7 --- /dev/null +++ b/tests/Feature/CustomStrMixinPrefixTest.php @@ -0,0 +1,51 @@ + $config]; + } + + protected function setUp(): void + { + parent::setUp(); + + Str::macro( + MacroSkuGenerator::MACRO, + static fn (string $source, string $separator, ?string $prefix = null): string => implode('|', [$prefix ?? 'none', $source, $separator]), + ); + } + + #[Test] + public function the_configured_prefix_reaches_a_custom_str_mixin(): void + { + $this->assertSame('acme|Blue shirt|-', DummyProduct::create(['name' => 'Blue shirt'])->sku); + } + + #[Test] + public function a_null_prefix_reaches_it_as_null(): void + { + config()->set('simtabi.lacommerce.generator.sku.prefix', null); + + $this->assertSame('none|Blue shirt|-', DummyProduct::create(['name' => 'Blue shirt'])->sku); + } +} diff --git a/tests/Fixtures/Generators/MacroSkuGenerator.php b/tests/Fixtures/Generators/MacroSkuGenerator.php new file mode 100644 index 0000000..43acd42 --- /dev/null +++ b/tests/Fixtures/Generators/MacroSkuGenerator.php @@ -0,0 +1,22 @@ +