Add Twig template exists rule #405

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

Jump to bottom

Open

zacharylund wants to merge 3 commits into phpstan:1.4.x

from zacharylund:twig-template-exists-rule

Open

Add Twig template exists rule #405

zacharylund wants to merge 3 commits into phpstan:1.4.x from zacharylund:twig-template-exists-rule

Conversation

zacharylund

Copy link

Contributor

@zacharylund zacharylund commented Aug 21, 2024

Fixes #257

@zacharylund


 Add Twig template exists rule

396dc78

@zacharylund zacharylund force-pushed the twig-template-exists-rule branch from d0a7ea1 to 396dc78 Compare

August 21, 2024 13:23

ruudk

ruudk reviewed

Aug 21, 2024

View reviewed changes

tests/Rules/Symfony/ExampleTwigController.php Show resolved Hide resolved

@zacharylund


 Handle Twig namespaces

d366e51

ruudk

ruudk reviewed

Aug 21, 2024

View reviewed changes

Copy link

Contributor

@ruudk ruudk left a comment

Choose a reason for hiding this comment

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

I would use the Twig environment using a loader, then it auto picks up all namespaces.

extension.neon Show resolved Hide resolved

@zacharylund


 Use Twig environment loader

9b97b95

@zacharylund zacharylund force-pushed the twig-template-exists-rule branch from ba45518 to 9b97b95 Compare

August 25, 2024 20:07

@zacharylund

Copy link

Contributor Author

zacharylund commented Aug 25, 2024

I would use the Twig environment using a loader, then it auto picks up all namespaces.

@ruudk, I've updated this to use a loader, but I'm not loving it. Because twig is a private service, the loader would need to look something like:

use App\Kernel;
use App\Kernel;
use Symfony\Component\Dotenv\Dotenv;
require __DIR__ . '/../vendor/autoload.php';
(new Dotenv())->bootEnv(__DIR__ . '/../.env');
$kernel = new Kernel('test', (bool) $_SERVER['APP_DEBUG']);
$kernel->boot();
return $kernel->getContainer()->get('test.service_container')->get('twig');

@ruudk

Copy link

Contributor

ruudk commented Aug 26, 2024

Yes. Downside is that now you are using the test environment.

In TwigStan I came up with this solution:
https://github.com/twigstan/twigstan/blob/90e1792a68dc0cd9471ce87a1e1f0145467299f9/twig-loader-symfony.php.dist#L18-L26

VincentLanglet

VincentLanglet reviewed

Oct 1, 2024

View reviewed changes

src/Rules/Symfony/TwigTemplateExistsRule.php

Comment on lines +50 to +58

if ($templateArg->value instanceof Variable && is_string($templateArg->value->name)) {

$varType = $scope->getVariableType($templateArg->value->name);

foreach ($varType->getConstantStrings() as $constantString) {

$templateNames[] = $constantString->getValue();

}

} elseif ($templateArg->value instanceof String_) {

$templateNames[] = $templateArg->value->value;

}

Copy link

Contributor

@VincentLanglet VincentLanglet Oct 1, 2024

Choose a reason for hiding this comment

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

I think you can avoid the if/else and write

Suggested change

if ($templateArg->value instanceof Variable && is_string($templateArg->value->name)) {

$varType = $scope->getVariableType($templateArg->value->name);

foreach ($varType->getConstantStrings() as $constantString) {

$templateNames[] = $constantString->getValue();

}

} elseif ($templateArg->value instanceof String_) {

$templateNames[] = $templateArg->value->value;

}

$argType = $scope->getType($templateArg->value);

foreach ($varType->getConstantStrings() as $constantString) {

$templateNames[] = $constantString->getValue();

}

src/Rules/Symfony/TwigTemplateExistsRule.php

Comment on lines +66 to +75

foreach ($templateNames as $templateName) {

if ($this->twigEnvironmentResolver->templateExists($templateName)) {

continue;

}

$errors[] = RuleErrorBuilder::message(sprintf(

'Twig template "%s" does not exist.',

$templateName

))->line($templateArg->getStartLine())->identifier('twig.templateNotFound')->build();

}

Copy link

Contributor

@VincentLanglet VincentLanglet Oct 1, 2024

Choose a reason for hiding this comment

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

I dunno if the error strategy need to depends on the PHPStan Level.

For instance, maybe existingTemplate|nonExistingTemplate should only be reported at level 7 or more.
In the same way unions/maybe are reported on level 7 for classic rules.

@ondrejmirtes ondrejmirtes force-pushed the 1.4.x branch from 0f833b0 to 270c2ee Compare

October 30, 2024 12:07

@landsman landsman mentioned this pull request

Jan 20, 2025

verify that path used in twig actually exist #425

Open

Labels

None yet

3 participants

@zacharylund @ruudk @VincentLanglet

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add Twig template exists rule #405

Are you sure you want to change the base?

Add Twig template exists rule #405

Uh oh!

Conversation

@zacharylund zacharylund commented Aug 21, 2024

Uh oh!

Uh oh!

@ruudk ruudk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zacharylund commented Aug 25, 2024

Uh oh!

ruudk commented Aug 26, 2024

Uh oh!

@VincentLanglet VincentLanglet Oct 1, 2024

Choose a reason for hiding this comment

Uh oh!

@VincentLanglet VincentLanglet Oct 1, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!