Conversation
FormatterBuilder now extends Fluent's Append with #[FluentNamespace] attribute, replacing the manual __call/__callStatic reflection logic. Mixin generation uses FluentGen via a LintMixinCommand. Mixins moved from src/Mixin/ to src/Mixins/ to match the Validation's output convention.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #21 +/- ##
============================================
- Coverage 99.55% 99.54% -0.01%
+ Complexity 199 196 -3
============================================
Files 29 29
Lines 446 439 -7
============================================
- Hits 444 437 -7
Misses 2 2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR migrates FormatterBuilder to use Respect/Fluent for fluent formatter resolution, and introduces FluentGen-based tooling to generate/validate the fluent mixin interfaces under src/Mixins.
Changes:
- Replace
FormatterBuilder’s manual__call/__callStaticreflection logic with Respect/Fluent’sAppendbuilder +#[FluentNamespace]lookup. - Add a Symfony Console devtool command (
lint:mixin) powered by FluentGen to (re)generate and validate mixin interfaces. - Move/adjust mixin interfaces to
Respect\StringFormatter\Mixinsand update method signatures to match formatter constructors.
Reviewed changes
Copilot reviewed 8 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/Integration/FormatterBuilderTest.php | Updates exception expectations to align with Fluent-based resolution errors. |
| src/FormatterBuilder.php | Switches builder implementation to Respect\Fluent\Builders\Append with attribute-based namespace lookup. |
| src/Internal/CompiledPattern.php | Minor tweak to quantifier compilation fallback handling. |
| src/Mixins/Chain.php | Updates fluent chain interface namespace and method signatures. |
| src/Mixins/Builder.php | Updates fluent static builder interface namespace and method signatures. |
| src-dev/Commands/LintMixinCommand.php | Adds a dev command to generate/compare mixin interfaces using FluentGen. |
| bin/console | Adds a CLI entrypoint for running dev console commands. |
| phpcs.xml.dist | Adds exclusions (line length) for generated mixin files (and a tests path). |
| composer.json | Adds Respect/Fluent (runtime) and FluentGen + Symfony Console (dev) dependencies; adds dev autoload for DevTools. |
| composer.lock | Locks new dependencies and updated versions. |
Comments suppressed due to low confidence (2)
src/Mixins/Chain.php:45
- The mixin signature allows
placeholder(..., Modifier|null $modifier = null), butPlaceholderFormatter::__construct()requires a non-nullModifier(it has a default instance, but passing explicitnullwill cause aTypeError). Either updatePlaceholderFormatterto accept?Modifierand substitute the default when null is passed, or change the mixin signature so it doesn't advertisenullas a valid value.
src/Mixins/Builder.php:46 - The mixin signature allows
placeholder(..., Modifier|null $modifier = null), butPlaceholderFormatter::__construct()requires a non-nullModifier(it has a default instance, but passing explicitnullwill cause aTypeError). Either updatePlaceholderFormatterto accept?Modifierand substitute the default when null is passed, or change the mixin signature so it doesn't advertisenullas a valid value.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| <!-- Exclusions --> | ||
| <rule ref="Generic.Files.LineLength.TooLong"> | ||
| <exclude-pattern>src/Mixins/</exclude-pattern> | ||
| <exclude-pattern>tests/feature/</exclude-pattern> |
There was a problem hiding this comment.
tests/feature/ is excluded from line-length checks, but there is no tests/feature directory in this repo (current test dirs are tests/Helper, tests/Integration, tests/Unit). If this is meant to exclude a real path, update it to the correct directory; otherwise it should be removed to avoid stale config. Also consider removing the trailing whitespace on the blank line above the exclusions.
| <!-- Exclusions --> | |
| <rule ref="Generic.Files.LineLength.TooLong"> | |
| <exclude-pattern>src/Mixins/</exclude-pattern> | |
| <exclude-pattern>tests/feature/</exclude-pattern> | |
| <!-- Exclusions --> | |
| <rule ref="Generic.Files.LineLength.TooLong"> | |
| <exclude-pattern>src/Mixins/</exclude-pattern> |
There was a problem hiding this comment.
Fair enough. I'll wait fore more reviews and batch this change with other possible suggestions.
FormatterBuilder now extends Fluent's Append with #[FluentNamespace] attribute, replacing the manual __call/__callStatic reflection logic. Mixin generation uses FluentGen via a LintMixinCommand. Mixins moved from src/Mixin/ to src/Mixins/ to match the Validation's output convention.
See: Respect/Validation#1729
This also serves as reusability proof of Fluent and FluentGen. Some aspects of it, such as the
LintMixinCommandare still not fully grown as an idea (would require formalizing diff abstractions, linter abstractions better), so they are copied (it's kind of a smaller price to pay than copy the entire CodeGen or the older procedural mixin generator).Similarly to Validation's PR, this also unlocks full fluent static analysis for StringFormatter fluent usage using the FluentAnalysis project, but shipping
fluent.neonand wiring the extensibility is out of scope for this PR now.