From df883fbfd128221c8b03f025f5667200cd9ad76f Mon Sep 17 00:00:00 2001 From: Michal Lulco Date: Mon, 5 Jun 2023 17:18:53 +0200 Subject: [PATCH 1/2] Added support for form fields in containers --- CHANGELOG.md | 6 ++ docs/how_it_works.md | 93 ++++++++++++++++++- .../NodeVisitor/AddFormClassesNodeVisitor.php | 14 +-- .../Form/CollectedFormControl.php | 10 +- .../Collector/FormControlCollector.php | 4 +- src/LatteContext/Finder/FormControlFinder.php | 49 +++++++++- .../Form/Behavior/ControlHolderBehavior.php | 11 ++- src/Template/Form/ControlHolderInterface.php | 2 + .../Fixtures/templates/Forms/default.latte | 1 + .../LatteTemplatesRuleForPresenterTest.php | 19 +++- 10 files changed, 186 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f499de13..918783ff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,12 @@ ## [Unreleased][unreleased] +### Added +- Support for form fields in containers + +### Fixed +- Report errors when numeric container names are used in latte + ## [0.13.0] - 2023-06-05 ### Changed - Separated collection of Form Containers diff --git a/docs/how_it_works.md b/docs/how_it_works.md index 62d00856..4d87a126 100644 --- a/docs/how_it_works.md +++ b/docs/how_it_works.md @@ -125,10 +125,95 @@ It is important to check the context first (text after path of latte file - rend Now the type of `$baz` will be `'bar'|null` and isset() in condition will be valid. - - + +Forms are collected from PHP classes (e.g. Presenters or Controls) when they are registered as components via `createComponent*` or `createComponent` method if this method returns instance of `Nette\Forms\Form`. +Form fields and form containers are also collected and can be then analysed. + +**IMPORTANT NOTE**: Container fields are currently assigned to containers if the name of container is the same as the name of variable which adds some field to this container. + +### Common errors + +#### Form control with name "xxx" probably does not exist. +Let's say you register form like this: + +```php + +use Nette\Application\UI\Form; + +protected function createComponentContainerForm(): Form +{ + $form = new Form(); + $form->setMethod('get'); + $form->addCheckbox('checkbox', 'Checkbox'); + $part1 = $form->addContainer('part1'); + $part1->addText('text1', 'Text 1'); + $part1->addSubmit('submit1', 'Submit 1'); + + $part2 = $form->addContainer('part2'); + $part2->addText('text2', 'Text 2'); + $part2->addSubmit('submit2', 'Submit 2'); + + return $form; +} +``` + + + +Then you can access all registered fields in latte this way: +```latte +{form containerForm} + {$form[part1][text1]->getHtmlId()} + {input part1-text1} + {input part1-submit1} + + {input part2-text2} + {input part2-submit2} + + {input checkbox:} + + {input xxx} <-- this field is not registered in createComponent method therefore it is marked as non-existing +{/form} +``` diff --git a/src/Compiler/NodeVisitor/AddFormClassesNodeVisitor.php b/src/Compiler/NodeVisitor/AddFormClassesNodeVisitor.php index b531b782..4c65dc4c 100644 --- a/src/Compiler/NodeVisitor/AddFormClassesNodeVisitor.php +++ b/src/Compiler/NodeVisitor/AddFormClassesNodeVisitor.php @@ -134,11 +134,8 @@ public function enterNode(Node $node): ?Node $itemArgument = $node->getArgs()[0] ?? null; $itemArgumentValue = $itemArgument ? $itemArgument->value : null; - if ($itemArgumentValue instanceof String_) { - $controlName = $itemArgumentValue->value; - // TODO remove when container are supported - $controlNameParts = explode('-', $controlName); - $controlName = end($controlNameParts); + if ($itemArgumentValue instanceof String_ || $itemArgumentValue instanceof LNumber) { + $controlName = (string)$itemArgumentValue->value; $formControl = $this->actualForm->getControl($controlName); if ($formControl === null) { $this->errorControlNodes[] = [ @@ -178,11 +175,8 @@ public function enterNode(Node $node): ?Node return null; } - if ($node->dim instanceof String_) { - $controlName = $node->dim->value; - // TODO remove when container are supported - $controlNameParts = explode('-', $controlName); - $controlName = end($controlNameParts); + if ($node->dim instanceof String_ || $node->dim instanceof LNumber) { + $controlName = (string)$node->dim->value; $formControl = $this->actualForm->getControl($controlName); if ($formControl === null) { $this->errorControlNodes[] = [ diff --git a/src/LatteContext/CollectedData/Form/CollectedFormControl.php b/src/LatteContext/CollectedData/Form/CollectedFormControl.php index 3cd30080..52615041 100644 --- a/src/LatteContext/CollectedData/Form/CollectedFormControl.php +++ b/src/LatteContext/CollectedData/Form/CollectedFormControl.php @@ -16,14 +16,17 @@ final class CollectedFormControl extends CollectedLatteContextObject private ControlInterface $formControl; + private string $parentName; + /** * @param class-string $className */ - public function __construct(string $className, string $methodName, ControlInterface $formControl) + public function __construct(string $className, string $methodName, ControlInterface $formControl, string $parentName) { $this->className = $className; $this->methodName = $methodName; $this->formControl = $formControl; + $this->parentName = $parentName; } public function getClassName(): string @@ -40,4 +43,9 @@ public function getFormControl(): ControlInterface { return $this->formControl; } + + public function getParentName(): string + { + return $this->parentName; + } } diff --git a/src/LatteContext/Collector/FormControlCollector.php b/src/LatteContext/Collector/FormControlCollector.php index 2ff580f8..74b80276 100644 --- a/src/LatteContext/Collector/FormControlCollector.php +++ b/src/LatteContext/Collector/FormControlCollector.php @@ -127,6 +127,7 @@ public function collectData(Node $node, Scope $scope): ?array return null; } + $parentName = $this->nameResolver->resolve($node->var) ?: 'form'; $formControls = []; foreach ($controlNames as $controlName) { $controlName = (string)$controlName; @@ -141,7 +142,8 @@ public function collectData(Node $node, Scope $scope): ?array $formControls[] = new CollectedFormControl( $classReflection->getName(), $methodName, - $formControl + $formControl, + $parentName ); } return $formControls; diff --git a/src/LatteContext/Finder/FormControlFinder.php b/src/LatteContext/Finder/FormControlFinder.php index b5a40974..fc42c0a3 100644 --- a/src/LatteContext/Finder/FormControlFinder.php +++ b/src/LatteContext/Finder/FormControlFinder.php @@ -6,7 +6,9 @@ use Efabrica\PHPStanLatte\Analyser\LatteContextData; use Efabrica\PHPStanLatte\LatteContext\CollectedData\Form\CollectedFormControl; +use Efabrica\PHPStanLatte\Template\Form\Container; use Efabrica\PHPStanLatte\Template\Form\ControlInterface; +use Efabrica\PHPStanLatte\Template\Form\Field; use Efabrica\PHPStanLatte\Template\ItemCombinator; use PHPStan\Reflection\ReflectionProvider; @@ -28,14 +30,55 @@ public function __construct(LatteContextData $latteContext, ReflectionProvider $ $collectedFormControls = $latteContext->getCollectedData(CollectedFormControl::class); + /** @var array>> $containers */ + $containers = []; + + /** @var array>> $controls */ + $controls = []; + /** @var CollectedFormControl $collectedFormControl */ foreach ($collectedFormControls as $collectedFormControl) { $className = $collectedFormControl->getClassName(); $methodName = $collectedFormControl->getMethodName(); - if (!isset($this->assignedFormControls[$className][$methodName])) { - $this->assignedFormControls[$className][$methodName] = []; + $formControl = $collectedFormControl->getFormControl(); + if (!$formControl instanceof Container) { + $parentName = $collectedFormControl->getParentName(); + if (!isset($controls[$className][$methodName][$parentName])) { + $controls[$className][$methodName][$parentName] = []; + } + $controls[$className][$methodName][$parentName][] = $formControl; + continue; + } + + $containerName = $formControl->getName(); + $containers[$className][$methodName][$containerName] = $formControl; + } + + foreach ($containers as $className => $classContainers) { + foreach ($classContainers as $methodName => $methodContainers) { + foreach ($methodContainers as $containerName => $container) { + $containerControls = $controls[$className][$methodName][$containerName] ?? []; + $container->addControls($containerControls); + unset($controls[$className][$methodName][$containerName]); + + if (!isset($this->assignedFormControls[$className][$methodName])) { + $this->assignedFormControls[$className][$methodName] = []; + } + + $this->assignedFormControls[$className][$methodName][] = $container; + } + } + } + + foreach ($controls as $className => $classControls) { + foreach ($classControls as $methodName => $methodControls) { + foreach ($methodControls as $parentControls) { + if (!isset($this->assignedFormControls[$className][$methodName])) { + $this->assignedFormControls[$className][$methodName] = []; + } + $this->assignedFormControls[$className][$methodName] = array_merge($this->assignedFormControls[$className][$methodName], $parentControls); + } } - $this->assignedFormControls[$className][$methodName][] = $collectedFormControl->getFormControl(); } } diff --git a/src/Template/Form/Behavior/ControlHolderBehavior.php b/src/Template/Form/Behavior/ControlHolderBehavior.php index be87d021..4ffa1989 100644 --- a/src/Template/Form/Behavior/ControlHolderBehavior.php +++ b/src/Template/Form/Behavior/ControlHolderBehavior.php @@ -4,6 +4,7 @@ namespace Efabrica\PHPStanLatte\Template\Form\Behavior; +use Efabrica\PHPStanLatte\Template\Form\ControlHolderInterface; use Efabrica\PHPStanLatte\Template\Form\ControlInterface; trait ControlHolderBehavior @@ -21,13 +22,19 @@ public function getControls(): array public function getControl(string $name): ?ControlInterface { - return $this->controls[$name] ?? null; + $nameParts = explode('-', $name); + $controlName = array_shift($nameParts); + $control = $this->controls[$controlName] ?? null; + if ($control instanceof ControlHolderInterface && $nameParts !== []) { + return $control->getControl(implode('-', $nameParts)); + } + return $control; } /** * @param ControlInterface[] $controls */ - private function addControls(array $controls): void + public function addControls(array $controls): void { foreach ($controls as $control) { $this->controls[$control->getName()] = $control; diff --git a/src/Template/Form/ControlHolderInterface.php b/src/Template/Form/ControlHolderInterface.php index 34e72844..7f04ba52 100644 --- a/src/Template/Form/ControlHolderInterface.php +++ b/src/Template/Form/ControlHolderInterface.php @@ -10,4 +10,6 @@ interface ControlHolderInterface * @return ControlInterface[] */ public function getControls(): array; + + public function getControl(string $name): ?ControlInterface; } diff --git a/tests/Rule/LatteTemplatesRule/PresenterWithoutModule/Fixtures/templates/Forms/default.latte b/tests/Rule/LatteTemplatesRule/PresenterWithoutModule/Fixtures/templates/Forms/default.latte index c16759b9..4b03222c 100644 --- a/tests/Rule/LatteTemplatesRule/PresenterWithoutModule/Fixtures/templates/Forms/default.latte +++ b/tests/Rule/LatteTemplatesRule/PresenterWithoutModule/Fixtures/templates/Forms/default.latte @@ -58,6 +58,7 @@ {form containerForm} {$form[part1][text1]->getHtmlId()} {input part1-text1} + {input part1-text2} {input part1-submit1} {input part2-text2} diff --git a/tests/Rule/LatteTemplatesRule/PresenterWithoutModule/LatteTemplatesRuleForPresenterTest.php b/tests/Rule/LatteTemplatesRule/PresenterWithoutModule/LatteTemplatesRuleForPresenterTest.php index e00f562a..0564c2bf 100644 --- a/tests/Rule/LatteTemplatesRule/PresenterWithoutModule/LatteTemplatesRuleForPresenterTest.php +++ b/tests/Rule/LatteTemplatesRule/PresenterWithoutModule/LatteTemplatesRuleForPresenterTest.php @@ -444,14 +444,29 @@ public function testForms(): void 48, 'default.latte', ], + [ + 'Form control with name "part1-text2" probably does not exist.', + 61, + 'default.latte', + ], + [ + 'Form control with name "5" probably does not exist.', + 87, + 'default.latte', + ], [ 'Form control with name "5" probably does not exist.', - 90, + 91, + 'default.latte', + ], + [ + 'Form control with name "1" probably does not exist.', + 105, 'default.latte', ], [ 'Form control with name "1" probably does not exist.', - 108, + 109, 'default.latte', ], ]); From 567bdf79dba9552b29df6065117ac72b1f0f9609 Mon Sep 17 00:00:00 2001 From: Michal Lulco Date: Mon, 5 Jun 2023 23:18:41 +0200 Subject: [PATCH 2/2] =?UTF-8?q?Added=20replace=20$form[=E2=80=98foo-bar?= =?UTF-8?q?=E2=80=99]=20to=20$form['foo']['bar']?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../NodeVisitor/AddFormClassesNodeVisitor.php | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/Compiler/NodeVisitor/AddFormClassesNodeVisitor.php b/src/Compiler/NodeVisitor/AddFormClassesNodeVisitor.php index 4c65dc4c..91fef295 100644 --- a/src/Compiler/NodeVisitor/AddFormClassesNodeVisitor.php +++ b/src/Compiler/NodeVisitor/AddFormClassesNodeVisitor.php @@ -190,6 +190,29 @@ public function enterNode(Node $node): ?Node if ($formControlType instanceof ObjectType && ($formControlType->isInstanceOf('Nette\Forms\Controls\CheckboxList')->yes() || $formControlType->isInstanceOf('Nette\Forms\Controls\RadioList')->yes())) { $this->possibleAlwaysTrueLabels[] = $this->findParentStmt($node); } + + /** + * Replace: + * + * $form['foo-bar']->getControl(); + * + * + * With: + * + * $form['foo']['bar']->getControl(); + * + * + * if foobar exists in actual form + */ + if (str_contains($controlName, '-')) { + $controlNameParts = explode('-', $controlName); + $tmpDim = new ArrayDimFetch(new Variable('form'), new String_(array_shift($controlNameParts))); + foreach ($controlNameParts as $controlNamePart) { + $tmpDim = new ArrayDimFetch($tmpDim, new String_($controlNamePart)); + } + $tmpDim->setAttributes($node->getAttributes()); + return $tmpDim; + } } elseif ($node->dim instanceof Variable) { // dynamic control } else {