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

BUG Incorrect type inspection on many_many through #13

Closed
tractorcow opened this issue Jun 6, 2018 · 3 comments
Closed

BUG Incorrect type inspection on many_many through #13

tractorcow opened this issue Jun 6, 2018 · 3 comments

Comments

@tractorcow
Copy link

As per comment at silverstripe/recipe-cms#12 (comment)

https://github.com/silbinarywolf/silverstripe-phpstan/blob/7fadf204193dcfa313666509a3e5d20098ab970a/src/Reflection/MethodClassReflectionExtension.php#L129-L138

The type of the list is not the 'through' array value. That's the 'internal type' only; You'll need to use both 'through' and 'to' to get the actual list class type.

See DataObjectSchema for how we do this internally.

// Normal many-many
            if ($manyManySpec === $parentClass) {
                return $inverseComponentName;
            }
            // many-many through, inspect 'to' for the many_many
            if (is_array($manyManySpec)) {
                $toClass = $this->hasOneComponent($manyManySpec['through'], $manyManySpec['to']);
                if ($toClass === $parentClass) {
                    return $inverseComponentName;
                }
            }

Where you have $type = $type['through']; you'll actually need to do deep inspection on the through class to pull the type of the has_one property specified by $type['to'].

Hope that helps. :)

@sminnee
Copy link
Contributor

sminnee commented Jun 6, 2018

@silbinarywolf it'd be great if you could confirm that this is the issue so that we can close off silverstripe/silverstripe-versioned#155 as a misidentified error

@silbinarywolf
Copy link
Contributor

@tractorcow Ah yep. You're probably correct.
@sminnee I'll investigate this weekend and confirm if the issue is on my end.

silbinarywolf added a commit that referenced this issue Jun 9, 2018
…ugh' logic works - Github Issue #13

- Update documentation to include known limitations
silbinarywolf added a commit that referenced this issue Jun 9, 2018
…ugh' logic works - Github Issue #13

- Update documentation to include known limitations
silbinarywolf added a commit that referenced this issue Jun 9, 2018
…ugh' logic works - Github Issue #13

- Update documentation to include known limitations
@silbinarywolf
Copy link
Contributor

silbinarywolf commented Jun 9, 2018

@tractorcow You were correct.
cc @sminnee

My steps to confirm this were:

  • I made sure to pull down the latest Recipe CMS code
  • Checked for the existence of SiteTreeLink in the project
  • Run the following hacky-code to test the return type of LinkTracking()
<?php

use SilverStripe\CMS\Model\SiteTree;

class Page extends SiteTree
{
    private static $db = [];

    private static $has_one = [];

    public function getCMSFields() {
        $fields = parent::getCMSFields();
        foreach (Page::get() as $r) {
            $r->LinkTracking()->add($r);
            foreach ($r->LinkTracking() as $record) {
                var_dump(get_class($record)); exit;
            }
        }
        return $fields;
    }
}
  • The class type returned was Page and not SiteTreeLink.

This bug in my code has now been fixed, pushed up and tagged here:
silbinarywolf@d0feb04

Sorry for the false positive guys!
I'll be sure to test things more thoroughly before raising in the future.

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

3 participants