docs: final-service composition + array-over-regex idiom#67
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
github-actions
Bot
added
documentation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds documentation guidelines for PHP modernization and Symfony patterns, introducing a rule favoring array functions over regular expressions for token lists and a pattern for wrapping final services in tagged collections. The review feedback suggests improving the token list example to handle empty strings robustly, and addresses a dependency injection anti-pattern in the Symfony pattern guide by recommending constructor injection of the upstream service combined with clearing its tag in a compiler pass instead of manual instantiation.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Pull request overview
This PR promotes two PHP/Symfony patterns into the php-modernization skill’s reference docs: (1) guidance for wrapping final services used in tagged collections, and (2) an idiom favoring array operations over regex when manipulating token lists in delimited strings.
Changes:
- Adds a Symfony DI subsection documenting pitfalls of
->decorate()with tagged iterators and proposing a wrapper-based approach. - Adds a core rule recommending
explode/array_filter/implodeoverpreg_replacefor adding/removing specific tokens.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| skills/php-modernization/references/symfony-patterns.md | Documents a pattern for wrapping final services used in tagged collections, including a compiler-pass snippet to avoid duplicate tag entries. |
| skills/php-modernization/references/core-rules.md | Adds a rule/idiom for manipulating token lists using array functions instead of regex. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
The failing
|
e2e347c to
bda0c9f
Compare
bda0c9f to
0539d12
Compare
…corate() Symfony ->decorate() keeps the inner service registered, so a final upstream service in a tagged collection ends up tagged twice (inner + outer); the duplicate registration broke a phpdoc.guides.directive collection on PHP 8.1. Document manual composition — construct and delegate to the upstream instance, then removeDefinition() it in a compiler pass so only the wrapper carries the tag. From a /retro promote pass over local memory (patterns.md). Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
Removing specific tokens from a delimited string (CSS classes, scopes) reads more clearly as explode/array_filter/implode than preg_replace, and avoids word-boundary and partial-match traps. From a /retro promote pass over local memory (patterns.md). Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
0539d12 to
b47a5d4
Compare
|
Summary
Two PHP patterns promoted from local memory: composing a
finaltagged service instead of decorating it, and preferring array functions over regex for token lists.Came from
/retro promotepass over local memory (2026-07-04), draining apatterns.mdnote upward to the owning skill.Change
references/symfony-patterns.md— new DI subsection: for afinalupstream service in a tagged collection,->decorate()leaves the inner service registered so the collection gets tagged twice (broke aphpdoc.guides.directivecollection on PHP 8.1). Documents composing the wrapper by injecting the upstream service and delegating, then clearing its tag in a compiler pass ($container->has()+findDefinition()+clearTag(), which resolve aliases) so the upstream stays a container service and only its tag is removed — preferred overremoveDefinition(), which would break other consumers that inject it.references/core-rules.md— new idiom entry:explode/array_filter/implodeoverpreg_replacefor adding/removing specific tokens in a delimited string, with a$c !== ''guard that drops empty tokens.Two atomic commits, one per pattern.
Test plan
markdownlint-cli2clean on both changed files