diff --git a/src/Command/ControllerCommand.php b/src/Command/ControllerCommand.php index 2b88cf0f..82ca60d0 100644 --- a/src/Command/ControllerCommand.php +++ b/src/Command/ControllerCommand.php @@ -129,6 +129,12 @@ public function bake(string $controllerName, Arguments $args, ConsoleIo $io): vo $singularHumanName = $this->_singularHumanName($controllerName); $pluralHumanName = $this->_variableName($controllerName); + // Handle cases where singular and plural are identical (e.g., "news", "sheep") + // to avoid variable collisions in generated controller code + if ($singularName === $pluralName) { + $singularName .= 'Entity'; + } + $defaultModel = sprintf('%s\Model\Table\%sTable', $namespace, $controllerName); if (!class_exists($defaultModel)) { $defaultModel = null; diff --git a/src/Command/TemplateCommand.php b/src/Command/TemplateCommand.php index 239eb576..64619f00 100644 --- a/src/Command/TemplateCommand.php +++ b/src/Command/TemplateCommand.php @@ -325,6 +325,12 @@ protected function _loadController(ConsoleIo $io): array $pluralVar = Inflector::variable($this->controllerName); $pluralHumanName = $this->_pluralHumanName($this->controllerName); + // Handle cases where singular and plural are identical (e.g., "news", "sheep") + // to avoid generating invalid code like `foreach ($news as $news)` + if ($singularVar === $pluralVar) { + $singularVar .= 'Entity'; + } + return compact( 'modelObject', 'modelClass', diff --git a/templates/bake/Template/view.twig b/templates/bake/Template/view.twig index e4234c0e..a6841f35 100644 --- a/templates/bake/Template/view.twig +++ b/templates/bake/Template/view.twig @@ -120,6 +120,9 @@ {% set relations = associations.BelongsToMany|merge(associations.HasMany) %} {% for alias, details in relations %} {%~ set otherSingularVar = alias|singularize|variable %} + {%~ if otherSingularVar == singularVar %} + {%~ set otherSingularVar = otherSingularVar ~ 'Related' %} + {%~ endif %} {%~ set otherPluralHumanName = details.controller|underscore|humanize %}

diff --git a/templates/bake/element/Controller/add.twig b/templates/bake/element/Controller/add.twig index 593ae4d4..4986c99c 100644 --- a/templates/bake/element/Controller/add.twig +++ b/templates/bake/element/Controller/add.twig @@ -40,6 +40,9 @@ {%- for assoc in associations %} {%~ set otherName = Bake.getAssociatedTableAlias(modelObj, assoc) %} {%~ set otherPlural = otherName|variable %} + {%~ if otherPlural == singularName %} + {%~ set otherPlural = otherPlural ~ 'List' %} + {%~ endif %} ${{ otherPlural }} = $this->{{ currentModelName }}->{{ otherName }}->find('list', limit: 200)->all(); {%~ set compact = compact|merge(["'#{otherPlural}'"]) %} {% endfor %} diff --git a/templates/bake/element/Controller/edit.twig b/templates/bake/element/Controller/edit.twig index 4e0b97d9..d68e4a39 100644 --- a/templates/bake/element/Controller/edit.twig +++ b/templates/bake/element/Controller/edit.twig @@ -41,6 +41,9 @@ {% for assoc in belongsTo|merge(belongsToMany) %} {%~ set otherName = Bake.getAssociatedTableAlias(modelObj, assoc) %} {%~ set otherPlural = otherName|variable %} + {%~ if otherPlural == singularName %} + {%~ set otherPlural = otherPlural ~ 'List' %} + {%~ endif %} ${{ otherPlural }} = $this->{{ currentModelName }}->{{ otherName }}->find('list', limit: 200)->all(); {%~ set compact = compact|merge(["'#{otherPlural}'"]) %} {% endfor %} diff --git a/tests/Fixture/NewsFixture.php b/tests/Fixture/NewsFixture.php new file mode 100644 index 00000000..030835c5 --- /dev/null +++ b/tests/Fixture/NewsFixture.php @@ -0,0 +1,28 @@ + 'First News', 'body' => 'First News Body'], + ['title' => 'Second News', 'body' => 'Second News Body'], + ]; +} diff --git a/tests/TestCase/Command/ControllerCommandTest.php b/tests/TestCase/Command/ControllerCommandTest.php index ae6ec044..00e73762 100644 --- a/tests/TestCase/Command/ControllerCommandTest.php +++ b/tests/TestCase/Command/ControllerCommandTest.php @@ -39,6 +39,7 @@ class ControllerCommandTest extends TestCase 'plugin.Bake.BakeArticlesBakeTags', 'plugin.Bake.BakeComments', 'plugin.Bake.BakeTags', + 'plugin.Bake.News', 'plugin.Bake.Users', ]; @@ -435,4 +436,34 @@ public function testMainWithPluginOption() $this->assertFileContains('use Company\Pastry\Controller\AppController;', $this->generatedFile); $this->assertFileContains('BakeArticlesController extends AppController', $this->generatedFile); } + + /** + * Test baking controller for models where singular and plural are identical. + * + * This tests the fix for generating variable collisions like `$news` for both + * the entity and the paginated collection when the model name is both singular + * and plural (e.g., "news", "sheep", "fish"). + * + * @return void + */ + public function testBakeControllerWithSingularPluralCollision(): void + { + $this->generatedFile = APP . 'Controller/NewsController.php'; + if (file_exists($this->generatedFile)) { + unlink($this->generatedFile); + } + $this->exec('bake controller --connection test --no-test News'); + + $this->assertExitCode(CommandInterface::CODE_SUCCESS); + $this->assertFileExists($this->generatedFile); + + $result = file_get_contents($this->generatedFile); + + // In view/edit/add/delete actions, the entity should use 'newsEntity' to avoid collision + $this->assertStringContainsString('$newsEntity = $this->News->get(', $result); + + // In index action, the paginated collection should still use 'news' + $this->assertStringContainsString('$news = $this->paginate(', $result); + $this->assertStringContainsString("compact('news')", $result); + } } diff --git a/tests/TestCase/Command/TemplateCommandTest.php b/tests/TestCase/Command/TemplateCommandTest.php index 846386e7..7b1ed0fe 100644 --- a/tests/TestCase/Command/TemplateCommandTest.php +++ b/tests/TestCase/Command/TemplateCommandTest.php @@ -54,6 +54,7 @@ class TemplateCommandTest extends TestCase 'plugin.Bake.BakeTemplateProfiles', 'plugin.Bake.CategoryThreads', 'plugin.Bake.HiddenFields', + 'plugin.Bake.News', ]; /** @@ -891,4 +892,29 @@ public function testMainWithMissingTable() $this->assertExitCode(CommandInterface::CODE_ERROR); } + + /** + * Test baking templates for models where singular and plural are identical. + * + * This tests the fix for generating invalid code like `foreach ($news as $news)` + * when the model name is both singular and plural (e.g., "news", "sheep", "fish"). + * + * @return void + */ + public function testBakeIndexWithSingularPluralCollision() + { + $this->generatedFile = ROOT . 'templates/News/index.php'; + $this->exec('bake template News index'); + + $this->assertExitCode(CommandInterface::CODE_SUCCESS); + $this->assertFileExists($this->generatedFile); + + $result = file_get_contents($this->generatedFile); + + // Should NOT have `foreach ($news as $news)` which would overwrite the collection + $this->assertStringNotContainsString('foreach ($news as $news)', $result); + + // Should have `foreach ($news as $newsEntity)` instead + $this->assertStringContainsString('foreach ($news as $newsEntity)', $result); + } } diff --git a/tests/schema.php b/tests/schema.php index 5e650f4b..336c7d84 100644 --- a/tests/schema.php +++ b/tests/schema.php @@ -587,4 +587,16 @@ 'unique_self_referencing_parent' => ['type' => 'unique', 'columns' => ['parent_id']], ], ], + // "news" is both singular and plural - tests variable collision fix + [ + 'table' => 'news', + 'columns' => [ + 'id' => ['type' => 'integer'], + 'title' => ['type' => 'string', 'length' => 255, 'null' => false], + 'body' => ['type' => 'text'], + 'created' => ['type' => 'datetime'], + 'modified' => ['type' => 'datetime'], + ], + 'constraints' => ['primary' => ['type' => 'primary', 'columns' => ['id']]], + ], ];