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

{capture $link}Foo:bar{/capture} throws "link() expects string" on level 7 #398

Closed
spaze opened this issue Jul 14, 2023 · 11 comments · Fixed by #410
Closed

{capture $link}Foo:bar{/capture} throws "link() expects string" on level 7 #398

spaze opened this issue Jul 14, 2023 · 11 comments · Fixed by #410

Comments

@spaze
Copy link
Contributor

spaze commented Jul 14, 2023

This is something I'm not sure is a bug, or something that can be fixed or changed in phpstan-latte but you tell me.

My template contains this:

<p class="separated">
	{capture $link}Trainings:application#{_html.id.application}{/capture}
	<a n:href="$link $name">{_messages.trainings.addanother}</a>  <-- line 54

which can probably be minimized to just:

{capture $link}Foo:bar{/capture}
<a n:href="$link">something</a>

and I get this:

 ------ ---------- -------------------------------------------------------------------------------------------------------------------------------------
  Line   Compiled   Www/Presenters/templates/Trainings/success.latte rendered from MichalSpacekCz\Www\Presenters\TrainingsPresenter::success
         line       See compiled template: /tmp/phpstan-latte/app/Www/Presenters/templates/Trainings/success.latte.accbee637de3e9eee62368a83384f885.php
 ------ ---------- -------------------------------------------------------------------------------------------------------------------------------------
  54     470        Parameter #1 $destination of method Nette\Application\UI\Component::link() expects string, Latte\Runtime\Html|string|false given.
 ------ ---------- -------------------------------------------------------------------------------------------------------------------------------------

The compiled code looks like this

<p class="separated">
		';
        /* line 53 */
        \ob_start(fn() => '');
        try {
            echo 'Trainings:application#';
            /* line 53 */
            echo \Latte\Runtime\Filters::escapeHtmlText($__filter__translate->translate('html.id.application'));
        } finally {
            $ʟ_tmp = \ob_get_length() ? new \Latte\Runtime\Html(\ob_get_clean()) : \ob_get_clean();
        }
        $ʟ_fi = new \Latte\Runtime\FilterInfo('html');
        $link = $ʟ_tmp;
        echo '
		<a href="';
        /* line 54 */
        echo \Latte\Runtime\Filters::escapeHtmlAttr($this->global->uiControl->link($link, [$name]));  // line <- 470
        echo '">';
        /* line 54 */
        echo \Latte\Runtime\Filters::escapeHtmlText($__filter__translate->translate('messages.trainings.addanother'));
        echo '</a>

now the error seems more or less correct because of these lines

$ʟ_tmp = \ob_get_length() ? new \Latte\Runtime\Html(\ob_get_clean()) : \ob_get_clean();
...
$link = $ʟ_tmp;
...
$this->global->uiControl->link($link, [$name]);

So in theory $ʟ_tmp can indeed be new \Latte\Runtime\Html(\ob_get_clean()) or a result of \ob_get_clean(); which is a string or false.

But, is there something I can do with that? I can change it to something like

<a href="{link Trainings:training $name}#{_html.id.application}">

and the error will disappear but the original code seems to be a legal (and working) Latte expression.

@lulco
Copy link
Contributor

lulco commented Jul 15, 2023

So problem is only false? Not Latte\Runtime\Html?

Well we can transform the compiled code.
Replace \ob_get_clean(); with \ob_get_clean() ?: '';
Not sure if it is completelly correct because maybe we will hide some real bugs (I don’t know if it is true).

@lulco
Copy link
Contributor

lulco commented Jul 15, 2023

With strict types enabled for Engine your code will even not work.

https://github.com/nette/latte/blob/master/tests/common/Engine.strictTypes.phpt

Maybe you should cast capture output to string? Or not using capture at all here.

@spaze
Copy link
Contributor Author

spaze commented Jul 15, 2023

This is one of the bugs where I don't know where the problem is and who should fix it. You're correct that with strict types it won't work and that casting to string would fix it.

spaze added a commit to spaze/michalspacek.cz that referenced this issue Jul 15, 2023
…d one in the link

A workaround efabrica-team/phpstan-latte#398 but I have to say I like this better.
spaze added a commit to spaze/michalspacek.cz that referenced this issue Jul 15, 2023
The issue here is very similar to efabrica-team/phpstan-latte#398 and the workaround is a bit different although type-casting would also work for the case reported in that bug.

Fixes errors like "Parameter #2 ...$args of method MichalSpacekCz\Templating\Filters::format() expects int|string, Latte\Runtime\Html|string|false given."
@lulco
Copy link
Contributor

lulco commented Jul 15, 2023

I think this is not the bug of phpstan-latte extension. I would rather say it is possible bug found by it :)

@spaze
Copy link
Contributor Author

spaze commented Jul 15, 2023

A possible bug where though? 😅

@lulco
Copy link
Contributor

lulco commented Jul 15, 2023

In your code O:-) and also it is weird that capture returns Html and not string :) I think it is not some public knowledge

@spaze
Copy link
Contributor Author

spaze commented Jul 15, 2023

It's not even documented I can't use a captured variable for links or that I have to type-cast it to string.

@lulco
Copy link
Contributor

lulco commented Jul 15, 2023

Maybe we should check the original compiled template :)) but I don’t think it is added by our extension.

Also maybe it is not documented because it works - Html is automatically casted thanks to non-strict template classes.

Then it is wrong phpstan behavior? Should it use Stringable classes / classes containing __toString() as strings?

@spaze
Copy link
Contributor Author

spaze commented Jul 15, 2023

I've checked the original Latte-compiled template and the code is basically the same.

It could also be Latte, it could cast whatever comes to link() for example to strings. Or document that what I was doing is not supported. Or throw an exception.

This is one of the things I'm not sure what's wrong and where. I could just write that part in a different way (and I've already done that) and be done with it but that doesn't help the thing (we just don't know which one) get better.

@dg
Copy link

dg commented Aug 8, 2023

You're capturing HTML and pasting it into the place where plain text is expected, so it doesn't work.

It could be done like this:

{if}
<script type="text/plain">
    {capture $link}Foo:bar{/capture}
</script>
{/if false}

Then $link would be plain text stored in a string. But it's not a very nice solution.

@spaze
Copy link
Contributor Author

spaze commented Aug 8, 2023

You're capturing HTML and pasting it into the place where plain text is expected, so it doesn't work.

It actually works (the template compiles just fine and works as expected), and I understand why, but PHPStan Latte says it doesn't, which I also understand why. But the difference is why I've filed the bug in the first place :-)

I've already found a nice solution: I've stopped using {capture} output for links :-)

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 a pull request may close this issue.

3 participants