Skip to content

Magic __get method fails for data() #88

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

Closed
cxj opened this issue May 26, 2016 · 17 comments
Closed

Magic __get method fails for data() #88

cxj opened this issue May 26, 2016 · 17 comments

Comments

@cxj
Copy link
Contributor

cxj commented May 26, 2016

Accessing object properties via the magic __get() method does not work. Note that no exception is thrown, hence I don't think it is even called. This might be a PHP syntax special case, such as described in this block part way down the PHP Overloading page (php.net/__get):

Note:
The return value of __set() is ignored because of the way PHP processes the assignment operator. Similarly, __get() is never called when chaining assignments together like this:
$a = $obj->b = 8;

Can this be made to work? Is there a variation on calling data() in the TSS that can make it work in the current code? I tried a few variations, and can call __get explicitly via data(__get) but don't know how to pass the argument.

<?php
require_once "vendor/autoload.php";

$xml = '<h1>Original Title</h1>';

class Data {
    protected $data = array();
    public    $baz = 'Baz';

    public function __construct() {
        $this->data['foo'] = 'Foo';
        $this->data['bar'] = 'Bar';
    }

    public function __get($name) {
        if (array_key_exists($name, $this->data)) {
            return $this->data[$name];
        }
        throw new Exception("no such property");
    }

    public function getBar() {
        return $this->data['bar'];
    }
}

$output = new Data;

// this fails, using __get magic method.
$tss      = 'h1 {content: data(foo); }';
$template = new \Transphporm\Builder($xml, $tss);
echo $template->output($output)->body . PHP_EOL;

// this works, using explicit method.
$tss      = 'h1 {content: data(getBar); }';
$template = new \Transphporm\Builder($xml, $tss);
echo $template->output($output)->body . PHP_EOL;

// this works, using public property.
$tss      = 'h1 {content: data(baz); }';
$template = new \Transphporm\Builder($xml, $tss);
echo $template->output($output)->body . PHP_EOL;
@solleer
Copy link
Collaborator

solleer commented May 26, 2016

When it checks to see if the method exists the check doesn't validate magic methods. I remember when this was removed a while ago. I will see why I requested that support be removed.

@cxj
Copy link
Contributor Author

cxj commented May 26, 2016

I grabbed a copy of the new-parser branch and ran my sample above against it.

The magic get of property foo via data(foo) stills fails as before, but now the data(getBar) calls also fails to return anything. I'm assuming there is an intentional backward compatibility break in the new parser.

If I change the data(getBar) to data(getBar()), then it works. That is an acceptable change.

Interestingly, that also means I can now call __get() directly via data(__get("foo")) and it works. It's kind of a hack to do that, but at least there is way to display my object's properties via Transphporm now with the new parser.

@solleer
Copy link
Collaborator

solleer commented May 26, 2016

Correct. With the new parser, function calls must be explicit.

@cxj
Copy link
Contributor Author

cxj commented May 26, 2016

Thank you for the responses. I'd like to see the new parser merged to master. :)

@cxj
Copy link
Contributor Author

cxj commented Jul 1, 2016

Would still like to see PHP magic methods work, too.

@solleer
Copy link
Collaborator

solleer commented Jul 1, 2016

I think this may have caused an issue with maphper earlier with magic methods being called at the wrong time

@solleer
Copy link
Collaborator

solleer commented Jul 1, 2016

I think the problem before was that since functions weren't explicit it would call properties

@solleer
Copy link
Collaborator

solleer commented Jul 4, 2016

This test passes

public function testDataObjCallMagicMethod() {
    $xml = "
    <div></div>
    ";

    $data = new Foo;

    $tss = "div { content: data(testCall()); }";

    $template = new \Transphporm\Builder($xml, $tss);

    $this->assertEquals('<div>test</div>', $template->output($data)->body);
}

class Foo {
    public $model;

    public function __construct() {
        $this->model = new Bar();
    }
    public function getBar($bar) {
        return $bar;
    }

    public function returnFalse() {
        return false;
    }

    public function returnTrue() {
        return true;
    }

    public function returnOne() {
        return 1;
    }

    public function add($a, $b) {
        return $a+$b;
    }

    public function __call($name, $args) {
        if ($name == "testCall") return "test";
        throw new \Exception();
    }
}

@solleer
Copy link
Collaborator

solleer commented Jul 26, 2016

I assume this can be closed

@cxj
Copy link
Contributor Author

cxj commented Jul 26, 2016

I'm not completely sure. What's the proper syntax to access an object's public property under the new parser? That is, if I have:

    class Foo {
        public $bar = "bar";
    }
    $data = new Foo;

How do I write TSS to ask for $bar to be used as the content?

@solleer
Copy link
Collaborator

solleer commented Jul 26, 2016

If foo is being passed as root data then
data(bar)

@cxj
Copy link
Contributor Author

cxj commented Jul 26, 2016

Does not work with magic method __get(). I admit it may be impossible, or more difficult than is worth the trouble to make it work. Here is my test case:

require 'vendor/autoload.php';

class getTest {

    private $data = array('a' => 'a', 1 => 1);

    public function __get($name) {
        echo " >>  Getting property '$name'\n";

        if (array_key_exists($name, $this->data)) {
            return $this->data[$name];
        }

        throw new Exception("no such property");

        return null;
    }
}

$data = new getTest;

$xml = "<div></div>\n";

$tss = "div { content: data(__get('a')); }";        // this works
$tss = "div { content: data(__get(1)); }";          // this works
$tss = "div { content: data(a); }";                 // fails
$tss = "div { content: data(1); }";                 // fails

$template = new \Transphporm\Builder($xml, $tss);

echo $template->output($data)->body;

@solleer
Copy link
Collaborator

solleer commented Jul 27, 2016

The reason __get isn't called is because when traversing it uses isset to check if the property exists and isset doesn't call __get it does call __isset though

@cxj
Copy link
Contributor Author

cxj commented Jul 27, 2016

Ah! Thanks! Adding an __isset() magic method to my class makes my test work. :-)

    public function __isset($name) {
        if (array_key_exists($name, $this->data)) {
            return true;
        }
        return false;
    }

@cxj
Copy link
Contributor Author

cxj commented Jul 27, 2016

I just added __isset() functions to my production code. My unit testing passes. So I think this can be closed now.

@cxj cxj closed this as completed Jul 27, 2016
@solleer
Copy link
Collaborator

solleer commented Jul 27, 2016

This should definitely be documented to avoid future confusing.

@solleer
Copy link
Collaborator

solleer commented Jul 28, 2016

I added some light documentation

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

No branches or pull requests

2 participants