Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Statical analysis for Latte [WIP] #297

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Statical analysis for Latte [WIP] #297

wants to merge 3 commits into from

Conversation

dg
Copy link
Member

@dg dg commented May 18, 2022

The purpose of this PR is to enable statical analysis of resulting code by analysers like PHPStan, Psalm, etc.

Related #262, #275, #276

First commit propagates known types infomation from {var type}, {varType}, {define}, {templateType} and {parameters} into compiled PHP code in form of /** @var type $var */ annotations.

The second commit propagates filter parameter in the form of @property annotations and stub class:

/**
* @property FiltersTemplate4d1f38ae82 $filters
*/
final class Template4d1f38ae82 extends Latte\Runtime\Template
{

	public function main(array $ʟ_args): void
	{
		echo LR\Filters::escapeHtmlText(($this->filters->lower)(($this->filters->upper)($a))) /* line 1 */;
	}
}

/**
* @property callable(mixed): string $lower
* @property callable(mixed): string $upper
*/
class FiltersTemplate4d1f38ae82
{
}

@JanTvrdik
Copy link
Contributor

Very much looking forward to this. I did some initial testing and the main issue I encountered is, that it does not support "closure" variables, i.e. $foo would be reported as undefined in the template bellow.

{define outer, Foo $foo}
	{embed wrapper}
		{block inner}{$foo->text}{/block}
	{/embed}
{/define}

@dg dg force-pushed the master branch 9 times, most recently from eef7b89 to c292484 Compare June 13, 2022 21:03
@MartinMystikJonas
Copy link

MartinMystikJonas commented Jun 17, 2022

@dg: I tried fist attempts to migrate my tests to your implementation. SO far I notived few things I would need to solve before I continue:

  1. There are many failing tests currently some of them are tests I want to extend by additional checks. It would be best if you could fix these tests befaore I start adding new checks.
  2. When you print variable type comments for multiple variable it is printen on single line like this /** @var string $a *//** @var string $c */ I think it would be best to print each var on separate line for better readability and also it would simplify some tests expects.
  3. When I prepared my implementation I stumbled upon one problem. Tag {varType} does not diffrentiate between specifying global varibles (passed to template) and local variables (created inside template). This becomes problem when you starts to use blocks. Because block see only global variables and not local variables (unless explicitly passed). Your implementation adds all types from {varType} to global scope so it is printet also inside block where these variables are not available. I solved this issue by adding variables to global scope only if {varType} is used in template head and othervise it will just output type comment in place. This is snippet of my code:
$comment = "/** @var $type $variable */\n";
		if ($this->getCompiler()->isInHead()) {
			$this->getCompiler()->paramsExtraction .= $comment;
			return "";
		} else {
			return $writer->write($comment);
		}

@MartinMystikJonas
Copy link

@dg Example of global/local sope test I wrote in my implementation:

$template = <<<'XX'

{varType string $a}

{$a}

{varType string $c}
{var $c = 10}

{include test}

{define test}
  {varType int $b}
  {var $b = 5}
  {$a}{$b}
{/define}

XX;

Assert::matchFile(
	__DIR__ . '/expected/varType.phtml',
	$latte->compile($template)
);

Expects vartype.phtml:

<?php
%A%
	public function main(): array
	{
		extract($this->params);
		/** @var string $a */
%A%
	public function blockTest(array $ʟ_args): void
	{
		extract($this->params);
		/** @var string $a */
%A%
		/** @var int $b */
		$b = 5%a%;
%A%

Notice that in blockTest there is no variable type comment for $c, which is local variable.

@MartinMystikJonas
Copy link

MartinMystikJonas commented Jun 21, 2022

@dg: Here you can find my first patch https://gist.github.com/MartinMystikJonas/05a11cbbb404184493b2353f4041a6bc

It includes:

  1. Fix for most tests
  2. When there is multiple variable comments each comment is printed on separate line
  3. Backward compatible whitespaces when variable types are not used (prevents need to update may test expects)
  4. Fixed buf with {default} - it called addExpression but with string parameter not expression
  5. Draft of new test for {varType}

@dg dg force-pushed the master branch 9 times, most recently from 8d92896 to 1267554 Compare May 27, 2024 11:56
@dg dg force-pushed the master branch 4 times, most recently from ee646d6 to b80c3a6 Compare June 18, 2024 21:40
@dg dg force-pushed the master branch 10 times, most recently from 6c48587 to bd46751 Compare August 9, 2024 02:30
@dg dg force-pushed the master branch 7 times, most recently from 6baf2c5 to 4db7a55 Compare October 8, 2024 00:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants