POC write output for varType and templateType macros #1
Conversation
|
problems:
|
| $reflectionClass = new ReflectionClass($node->args); | ||
| foreach ($reflectionClass->getProperties() as $property) { | ||
| $propertyName = $property->getName(); | ||
| $type = $property->getType(); |
There was a problem hiding this comment.
this works only for php >= 7.4
There was a problem hiding this comment.
and only for native typed properties (php doc is not read)
|
|
||
| $templateType = ''; | ||
| if ($type) { | ||
| $templateType .= "/** @var {$type->getName()} */\n"; |
There was a problem hiding this comment.
getName() can be called only on ReflectionNamedType, it could be also ReflectionUnionType here
| $templateTypes = []; | ||
| try { | ||
| $reflectionClass = new ReflectionClass($node->args); | ||
| foreach ($reflectionClass->getProperties() as $property) { |
There was a problem hiding this comment.
getProperties returns only real properties, it is not working for @property / @property-read annotation
|
|
||
| $templateType = ''; | ||
| if ($type) { | ||
| $templateType .= "/** @var {$type->getName()} */\n"; |
| if ($type) { | ||
| $templateType .= "/** @var {$type->getName()} */\n"; | ||
| } | ||
| $templateType .= "\$$propertyName = \$this->params['$propertyName'];"; |
There was a problem hiding this comment.
TODO handle nullable with $this->params['$propertyName'] ?? null
|
|
||
| $varName = $variable[0]; | ||
| $paramName = ltrim($varName, '$'); | ||
| return $writer->write("/** @var $type */\n$varName = \$this->params['$paramName']"); |
There was a problem hiding this comment.
TODO handle nullable with $this->params['$paramName'] ?? null
|
|
||
| $varName = $variable[0]; | ||
| $paramName = ltrim($varName, '$'); | ||
| return $writer->write("/** @var $type */\n$varName = \$this->params['$paramName']"); |
There was a problem hiding this comment.
missing semicolon at the end of this write
|
looking forward to this 👍 |
|
Nobody wants this :( https://blog.nette.org/cs/vyhodnoceni-nette-pruzkumu |
|
What needs to be done for this to be merged? Static analysis of templates would be huge help during rectorings. |
|
There are many problems. I've freezed it. Now I am trying different approach. It is based on running phpstan for latte templates in context of Control (Presenter later). It creates php parser nodes and feed them to phpstan. See https://github.com/efabrica-team/phpstan-latte and follow my Twitter. |
|
Well that new approach seems a bit overcomplicated to me. and it will work only with phpstan but not other tools. Can you summary what are problems you encountered with this approach? I would like to look at it myself to see if it can be (at least partially) solved. Main problem is propagation of type information to subsequent blocks, right? |
Probably it is, it starts as challenge. It is my first phpstan rule. I really don't know what I'm doing :D previously I used phpstan just as user, not developer.
Yes, I don't use any other tools so I don't care.
Yes propagation of types to all blocks is first problem. Next problem will be how to resolve filters, components etc. Then you need to map line numbers from compiled template to latte's line numbers. This is solved with the new approach because nodes are generated with line info from latte. I believe the best would be some combination of existing compiled template and my approach. |
|
Ok I will try to look at when I would have some free time. Problem with filters I plan to solve by dumping known filters from app latte engine that I already use to convert latte to php. |
|
Have you seen macro {parameters} and how it works by modifying Compiler->paramsExtraction? It seems to me that it will be enough to just add generation of phpdoc comments with types there and implement similar behaviour in varType where it will append appropriate phpdoc to paramsExtraction. |
|
No, I haven't seen it. Go for it :) |
|
@lulco Check this: nette#275 What do you think? Next step will by support for {templateType} but it should be relatively easy. I just have one question I saw your note about @Property / @property-read annotations - is it really needed? It seems that it is not propagated to template by Latte anyway. Or am I wrong? |
|
I think this way (my and also yours) is screwing static analysis. varType nor templateType not propagate variables to latte. They are only for latte plugin. Just control / presenter propagate variables to latte. It is much better prepare compiled template for static analysis by adding assigned variables to it. Ad @Property: it was inspired by https://forum.nette.org/en/32893-custom-template-class-for-presenter-and-control-experimental-feature |
|
Check this repo https://github.com/symplify/symplify/tree/main/packages/phpstan-rules/packages and this rule https://github.com/symplify/symplify/tree/main/packages/phpstan-rules/packages/nette/tests/Rules/LatteCompleteCheckRule it is similar to my solution in phpstan-latte, but it use compiled template with some changes and postprocessing for mapping php lines back to latte lines. It can have good results if we teach it to check all controls and presenters. Now it works only if you have |
|
Closing as not relevant anymore |
|
Imho static analysis should be done in two separate layers. First is static analysis of template itself (my approach) that checks internal consistency of template and is independent of used tool and works even for non standard rendering code and second is static analysis of template rendering in caller done by tool plugin. |
|
Also propagation of known type information to compiled template can be also used to guide these plugins you and Tomáš develops. |
No description provided.