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

Add support for PHP 8s nullsafe operator. #860

Open
wants to merge 1 commit into
base: smarty5
Choose a base branch
from

Conversation

j-applese3d
Copy link
Contributor

@j-applese3d j-applese3d commented Feb 8, 2023

I'm not sure that I got everything right, so please review and let me know what I need to change :)

For instance the test will fail if you run it with PHP <8.0, and I wasn't certain that specific test file was the right one to update or if I should make a new one. I didn't know where to put it...

Thanks!
Related Issue: #633

@j-applese3d j-applese3d changed the title Add support for PHP 8s nullsafe operator. #633 Add support for PHP 8s nullsafe operator. Feb 8, 2023
@wisskid wisskid changed the base branch from smarty5 to master March 18, 2024 14:32
@wisskid wisskid changed the base branch from master to smarty5 March 18, 2024 14:36
@wisskid
Copy link
Contributor

wisskid commented Mar 18, 2024

@j-applese3d is there any way to compile this into PHP7-compatible PHP code? I.e. to implement the nullsafe operator in Smarty without actually using the nullsafe operator in the compiled code? If not, we'll have to drop support for PHP7.x and we cannot release this until v6.

@j-applese3d
Copy link
Contributor Author

j-applese3d commented Mar 18, 2024

@wisskid I believe that there are 2 different ways ?-> can be used:

  1. to read an object property
  2. to call a method

Both of these cases should be easy enough to implement in PHP compatible with 7.0.
Here are some example functions:

/** {$obj?->doSomething($p, $p2)} ➡ nullSafeMethod($obj, "doSomething", [$p, $p2]); */
function nullSafeMethod($object, $function, ...$params)
{
    if ($object === null) {
        return null;
    }
    return $object->{$function}($params);
}

/** {$obj?->someProperty} ➡ nullSafeProperty($obj, "someProperty"); */
function nullSafeProperty($object, $property)
{
    if ($object === null) {
        return null;
    }
    return $object->{$property};
}

@wisskid
Copy link
Contributor

wisskid commented Aug 17, 2024

I'm trying to implement your suggestion, but it requires significant changes in the parser. While at it, I'm wondering if it would be even better to make -> nullsafe by default in Smarty. And also array member access. That way, we could ditch the custom error handler used to suppress warnings for "undefined index", "undefined array key" and "trying to read property of null".

@j-applese3d
Copy link
Contributor Author

it requires significant changes in the parser.

Yes... I did try to fix my solution, but I couldn't find where/how to insert the custom logic (as opposed to simply mapping ?-> to ?->)

I'm wondering if it would be even better to make -> nullsafe by default in Smarty

I guess it depends on the main purpose of Smarty.
In my situation, I code the backend and frontend, so I think of Smarty as being syntactic sugar, and a nice way to separate my "View".

Coming from this point of view, it doesn't make sense to magically treat -> as if it were ?->, rather the developer (templater 🙂) should receive the same error as they would in PHP. This lets them know they made a mistake, and if they want ?-> they can adjust it; but maybe they forgot to assign the variable -- or used the wrong name.

Call to a member function [function name]() on null

However, I understand that I'm not the only person using this library. [1]


TL;DR: I like the idea of ditching the custom error handler, but I'd prefer if the "undefined index" errors show up the same as when PHP runs into them. (i.e. leave error handling up to PHP)


[1] I would be curious to know how many projects have a team of template editors that cannot edit the backend code, as it feel like many things are directed toward preventing unauthorized execution php code. For instance, deprecating usage of implode: for a php dev it is the name they are familiar with. Using |join:', ' makes sense, it's just not very natural to use join in templates when I use implode in php.

Thank you for all the work you put into this project. It is much appreciated. ❤️

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.

2 participants