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

False positive (OneObjectOperatorPerLine) for property access within a class #53

Conversation

UFOMelkor
Copy link
Contributor

When I have something like this:

<?php
class Group
{
    public $id;
}

class User
{
    private $group;

    public function getGroupId()
    {
        return $this->group->id;
    }
}

Then I expect no error from the OneObjectOperatorPerLineSniff to occur,
But I got Only one object operator per line. at line 13 (return $this->group->id;).

The error is triggered in Line 113. It occurs because $this-> is counted as first Object Operator. In handleTwoObjectOperators this case is excluded by the $isOwnCall which tracks whether the starting point is $this.

I would suggest to do the same in handleExcludedFluentInterfaces. I adjusted at a little bit (because we can have more than 1 callerTokens).
For explanation let's have a look at three examples: $this->group->id (which should raise no error), $user->group->id (which should raise an error) and $this->user->group->id (which should also raise an error):

$callerTokens (simplified) count($callerTokens) $isOwnCall Early exit?
$this->group->id ['group'] 1 true Yes
$user->group->id ['group'] 1 false No
$this->user->group->id ['user', 'group'] 2 true No

This could be handled by the following check:

if ((count($this->callerTokens) - (int) $isOwnCall) === 0) {
    return;
}

@TomasVotruba
Copy link
Contributor

I love the very clear example and description. Thank you.

I came across this myself few times, so I'm glad you bring the solution.

@TomasVotruba TomasVotruba merged commit 213f2e6 into object-calisthenics:master Mar 13, 2017
@UFOMelkor UFOMelkor deleted the hotfix/single-property-access branch March 13, 2017 11:50
@UFOMelkor
Copy link
Contributor Author

Thank you for the fast response :-)

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.

None yet

2 participants