Skip to content

Comments

Prevent exception when dynamically extending a class to add behaviors.#225

Merged
LukeTowers merged 3 commits intodevelopfrom
fix-implement
Feb 20, 2026
Merged

Prevent exception when dynamically extending a class to add behaviors.#225
LukeTowers merged 3 commits intodevelopfrom
fix-implement

Conversation

@mjauvin
Copy link
Member

@mjauvin mjauvin commented Feb 20, 2026

Fixes wintercms/winter#1460

Summary by CodeRabbit

  • Bug Fixes
    • Fixed an issue where the same extension could be applied multiple times; extensions are now applied once.
    • This reduces redundant processing and prevents unintended duplicate behavior, improving reliability and consistency when multiple identical extensions are present.

@mjauvin mjauvin added this to the 1.2.12 milestone Feb 20, 2026
@mjauvin mjauvin requested a review from LukeTowers February 20, 2026 06:23
@mjauvin mjauvin self-assigned this Feb 20, 2026
@mjauvin mjauvin added the maintenance PRs that fix bugs, are translation changes or make only minor changes label Feb 20, 2026
@coderabbitai
Copy link

coderabbitai bot commented Feb 20, 2026

No actionable comments were generated in the recent review. 🎉


Walkthrough

Deduplicates the $uses array in ExtendableTrait before iterating in both extendableConstruct() and extendableCallStatic(), so each identical extension is processed only once and duplicate extension attempts are avoided.

Changes

Cohort / File(s) Summary
ExtendableTrait
src/Extension/ExtendableTrait.php
Call to array_unique() added to deduplicate $uses before iterating in extendableConstruct() and extendableCallStatic(), preventing repeated processing of identical extensions and duplicate-extension exceptions.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: preventing an exception when dynamically extending a class to add behaviors, which directly addresses the core issue.
Linked Issues check ✅ Passed The code change (deduplicating the $uses array before iteration) directly addresses the root cause in issue #1460 by preventing duplicate behavior implementations.
Out of Scope Changes check ✅ Passed All changes are strictly focused on fixing the duplicate extension issue in ExtendableTrait; no unrelated modifications are present.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-implement

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/Extension/ExtendableTrait.php (1)

492-502: Consider applying the same array_unique() treatment here for consistency.

The static method flow has a similar loop processing $uses without deduplication. While this won't throw an exception (it just overwrites self::$extendableStaticMethods entries), applying array_unique() here would maintain consistency with the fix at line 87 and avoid redundant processing.

Suggested fix
-                foreach ($uses as $use) {
+                foreach (array_unique($uses) as $use) {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/Extension/ExtendableTrait.php` around lines 492 - 502, The static-methods
loop processes $uses without deduplication causing redundant work and overwrites
in self::$extendableStaticMethods; modify the loop that builds $useClassName and
iterates ReflectionClass->getMethods(ReflectionMethod::IS_STATIC) to first apply
array_unique() to $uses (the same way used earlier) so each $use is handled once
before creating $useClassName and populating
self::$extendableStaticMethods[$className][$method->getName()] = $useClassName.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/Extension/ExtendableTrait.php`:
- Around line 492-502: The static-methods loop processes $uses without
deduplication causing redundant work and overwrites in
self::$extendableStaticMethods; modify the loop that builds $useClassName and
iterates ReflectionClass->getMethods(ReflectionMethod::IS_STATIC) to first apply
array_unique() to $uses (the same way used earlier) so each $use is handled once
before creating $useClassName and populating
self::$extendableStaticMethods[$className][$method->getName()] = $useClassName.

@LukeTowers LukeTowers merged commit 21e5110 into develop Feb 20, 2026
11 checks passed
@LukeTowers LukeTowers deleted the fix-implement branch February 20, 2026 06:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintenance PRs that fix bugs, are translation changes or make only minor changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

isClassExtendedWith doesnt work in Controller::extend

2 participants