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.

Sign up for GitHub

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

Support unnamed function arguments to compiler plugins in Smarty 5 #1088

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

Support unnamed function arguments to compiler plugins in Smarty 5 #1088

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b85616c
Fix compiler plugin support for unnamed arguments
gkreitz Nov 29, 2024
ff12140
Add test coverage of unnamed arguments in compiler plugins
gkreitz Nov 29, 2024
0d24f1d
Change type annotation to mixed due to lack of guarantees on type
gkreitz Jan 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions src/Compile/Base.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,17 @@ protected function formatParamsArray(array $_attr): array {
return $_paramsArray;
}

/**
* Returns attribute index for unnamed ("shorthand") attribute, or null if not allowed.
*
* @param string|int|null $key Index of the argument. Type should probably be narrowed to int
*
* @return string|int|null
*/
protected function getShorthandOrder($key) {
return $this->shorttag_order[$key] ?? null;
}

/**
* This function checks if the attributes passed are valid
* The attributes passed for the tag to compile are checked against the list of required and
Expand All @@ -91,8 +102,8 @@ protected function getAttributes($compiler, $attributes) {
if (isset($options[trim($mixed, '\'"')])) {
$_indexed_attr[trim($mixed, '\'"')] = true;
// shorthand attribute ?
} elseif (isset($this->shorttag_order[$key])) {
$_indexed_attr[$this->shorttag_order[$key]] = $mixed;
} elseif (!is_null($this->getShorthandOrder($key))) {
Copy link
Contributor Author

@gkreitz gkreitz Nov 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I think the type of $key is always int here, based on the fact that it looks like $attributes is a list<mixed> from the usage, but I do not know enough about the code base to know. If my assumption is incorrect, the type of the argument to getShorthandOrder needs to be changed to avoid crashing.

Copy link
Member

Choose a reason for hiding this comment

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

Well, all the unit tests pass and I haven't found any references to shorttag_order in the source that suggest other keys. But as you noted, it only looks that way and is not guaranteed. Just to be safe, I'd prefer to allow a mixed type for the key param to getShorthandOrder.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for the slow response. I removed the php type annotations to be on the safe side.

$_indexed_attr[$this->getShorthandOrder($key)] = $mixed;
} else {
// too many shorthands
$compiler->trigger_template_error('too many shorthand attributes', null, true);
Expand Down
16 changes: 15 additions & 1 deletion src/Compile/Tag/BCPluginWrapper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,25 @@ public function __construct($callback, bool $cacheable = true) {
$this->cacheable = $cacheable;
}

/**
* Returns attribute index for unnamed ("shorthand") attribute, or null if not allowed.
*
* For compiler plugins, we allow arbitrarily many unnamed attributes,
* and just make them accessible in the order they are set.
*
* @param string|int|null $key Index of the argument. Type should probably be narrowed to int
*
* @return string|int|null
*/
protected function getShorthandOrder($key) {
return $key;
}

/**
* @inheritDoc
*/
public function compile($args, \Smarty\Compiler\Template $compiler, $parameter = [], $tag = null, $function = null): string
{
return call_user_func($this->callback, $this->getAttributes($compiler, $args), $compiler->getSmarty());
}
}
}
4 changes: 2 additions & 2 deletions tests/UnitTests/SmartyMethodsTests/RegisterCompiler/RegisterCompilerFunctionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public function testRegisterCompilerFunction()
{
$this->smarty->registerPlugin(Smarty::PLUGIN_COMPILER, 'testcompilerfunction', 'mycompilerfunction');
$this->assertEquals('mycompilerfunction', $this->smarty->getRegisteredPlugin('compiler', 'testcompilerfunction')[0]);
$this->assertEquals('hello world 1', $this->smarty->fetch('eval:{testcompilerfunction var=1}'));
$this->assertEquals('hello world 1 2', $this->smarty->fetch('eval:{testcompilerfunction 1 var=2}'));
}

/**
Expand Down Expand Up @@ -99,7 +99,7 @@ public function testUnregisterCompilerFunctionOtherRegistered()

function mycompilerfunction($params, $smarty)
{
return "<?php echo 'hello world {$params['var']}';?>";
return "<?php echo 'hello world {$params[0]} {$params['var']}';?>";
}

function mycompilerfunctionopen($params, $smarty)
Expand Down
Loading