Extract CodeGen namespace from LintMixinCommand#1716
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 3.1 #1716 +/- ##
=========================================
Coverage 99.45% 99.45%
Complexity 1015 1015
=========================================
Files 194 194
Lines 2384 2384
=========================================
Hits 2371 2371
Misses 13 13 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 44 out of 44 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Replace hardcoded validator class lists with a declarative #[Mixin] attribute and extract the mixin generation logic into a reusable CodeGen namespace under src-dev/CodeGen/. The new MixinGenerator discovers prefix definitions and filtering rules by scanning #[Mixin] attributes on the target namespace's classes, removing the need for hardcoded configuration. It supports configurable interface types (Builder for __callStatic, Chain for __call) with custom suffixes, return types, and root extends. This is the first step toward extracting the code generation into a standalone package that can map __call/__callStatic to any namespace, possibly for Respect/StringFormatter and any kind of project in the future.
henriquemoody
left a comment
henriquemoody
left a comment
There was a problem hiding this comment.
I think the CodeGen part could be extract, but I'm not super sure about it. The Mixin attribute seems more like a Validation concern than a CodeGen concern. If we want to make it as a CodeGen concern, perhaps it should be something more generic, like tags or labels that we could use when generating those mixings. For example, another CodeGen tag/label could be: prefix and having other tags for describing the order of the arguments.
|
@henriquemoody I'm not sure I understand your comment. The While this was sitting in review, I got to think about the naming though, and it's possible that I will rename |
|
@alganet, sorry if my previous comment was a bit vague. I like this change, and not I'm saying we shouldn't have a generic CodeGen, quite the opposite, I think we should do it, and be more generic. My main concern is actually more structural regarding separation of concerns. Right now, the attribute design feels heavily tied to how Respect/Validation behaves (and granted, like other packages we have, like When I mentioned generic concepts like tags or labels, I meant that the CodeGen tool should rely on domain-agnostic metadata. Instead of an attribute that implies our specific use case, we could use composable instructions. #[CodeGen\Tag('exclude', value: ['all', 'key', 'property'])]
#[CodeGen\Tag('prefix', value: 'key')]
#[CodeGen\Tag('prefixParameter', value: true)]
final readonly class Key implements KeyRelatedThis way, Respect/CodeGen remains completely unopinionated. The client code, that uses CodeGen, would make decisions based on those "tags". Does that make more sense regarding why I think the current approach feels too coupled to the Validation domain? |
|
I still don't get it! You described the shape of how the Attributes should be: separate instead of a single Attrribute with many named parameters. Current: Your proposal: I don't understand how that makes it more generic. It's just a different signature. All of these (exclude, prefix, prefixParameter) are needed for I'm a big fan of decoupling, but in this case I'm not seeing the actual proposal here. How would we change or refactor what |
|
Before I answer anything, let me take a step back and ask you this question: Do you want to extract the CodeGen part into a library that serves only our Respect packages or would it be aimed at generic code generation, like generating method signatures from a constructor, for example? Is it generic or just for us to reuse code around? |
|
@henriquemoody my goal is what I've described in the PR description. I want to start shaping a CodeGen library that could be generic later down the line, but first I want to exclusively solve maintenance and reuse for Validation and StringFormatter. It's an iterative approach to design. I don't want to think of everything beforehand, I want the real use cases we have to guide us. If it ends up being reusable beyond Respect, cool. If not, then it's fine too. If you believe there's an use case for which there's a clear gap in the PHP ecosystem and this somehow intersects with that gap, I'd love to know what that use case is! It would certainly affect how I approach it. |
|
I understand that we don't want to think about all scenarios, but since this change will be shipped in the next version, that attribute will be something we'd need to think twice before changing. The clear gap in the PHP ecosystem is the same reason why we built this. Have we had a PHP library that would extract the signature of a method and allow you to regenerate it with modifications we wouldn't have created this. A similar problem would be true for other builders that use I can stand behind just focusing on the use case we have, but if we want to move that into a different package, and if youdon't want to have a |
Why? Attributes are inert in PHP unless loaded. It doesn't change the public API of Respect\Validation. It also doesn't change how people write custom validators. We can change it as much as we want, this is
But what are the other use cases though? Knowing them would be precious to me, because I could think of design further for next iterations. Perhaps you have something in mind, but I don't know what it is. My current approach solves the case where a namespace (such as Validators/) has classes that map to nodes in a chain (via __call/__callStatic). The parametrization happens in the constructor of
The naming is not what I'm talking about, right? Nor details about separate attributes vs parameters. I understand those are relevant cosmetic details, but don't change much the abstractions involved, they're details. You said it yourself, right?
Mine are too! However, the actual implementation is not in the attributes, is within If you can comment there on your idea for separating how |
Fair point, I'm fine with keeping it like that and refactoring later.
I can only think about our use-cases.
No it's not. I was just talking about the fact that when we extract it to a different package we'd have to rename it, unless Respect\CodeGen namespace will start with Respect\Dev. As I said, I'm fine with keeping it like that and refactoring later.
For context on where my head was: I'm planning on adding I will approve this PR, there's nothing blocking from my perspective. Let's merge and see how it will evolve. |
|
@henriquemoody thanks! Any further improvement to support automated My next steps are:
|
This change introduces no BC break.
Attributes from
src-devare not used in runtime, so it's fine for them to be referenced by files insrcfor now.Goal here is to start shaping a Respect/CodeGen project that can move all of this complexity outside of Validation and also support projects like Respect/StringFormatter later.
Replace hardcoded validator class lists with a declarative #[Mixin] attribute and extract the mixin generation logic into a reusable CodeGen namespace under src-dev/CodeGen/.
The new MixinGenerator discovers prefix definitions and filtering rules by scanning #[Mixin] attributes on the target namespace's classes, removing the need for hardcoded configuration. It supports configurable interface types (Builder for __callStatic, Chain for __call) with custom suffixes, return types, and root extends.
This is the first step toward extracting the code generation into a standalone package that can map __call/__callStatic to any namespace, possibly for Respect/StringFormatter and any kind of project in the future.