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 with anonymous functions in Generic_Sniffs_WhiteSpace_ScopeIndentSniff #337

Closed
oliverklee opened this issue Dec 5, 2014 · 8 comments

Comments

@oliverklee
Copy link
Contributor

The gist in https://gist.github.com/oliverklee/d5d80c176685de0874ea produces the following false positive:

class Testcase {
    private $subject;

    /**
     * @test
     */
    public function recordLogin_changes_lastLogin_in_Db()
    {
        $this->mockedDatabase
            ->expects($this->once())
            ->method('update')
            ->with(
                'user',
                $this->callback(
                    function ()
                    {
                        return;
                    }
                )
            );

        $this->subject->recordLogin();
    }
}

Line indented incorrectly, expected at least 16 spaces, found 8

This is similar to #298 - I don't know whether this is a regression of that bug or just similar.

@oliverklee oliverklee changed the title False with anonymous functions positive in Generic_Sniffs_WhiteSpace_ScopeIndentSniff False positive with anonymous functions positive in Generic_Sniffs_WhiteSpace_ScopeIndentSniff Dec 5, 2014
@oliverklee oliverklee changed the title False positive with anonymous functions positive in Generic_Sniffs_WhiteSpace_ScopeIndentSniff False positive with anonymous functions in Generic_Sniffs_WhiteSpace_ScopeIndentSniff Dec 5, 2014
gsherwood added a commit that referenced this issue Dec 8, 2014
@gsherwood
Copy link
Member

Thanks for the report. I've added a fix for this case.

@oliverklee
Copy link
Contributor Author

Thanks! ❤️

@MaartenStaa
Copy link

Looks like this may only be partially fixed. This example function is still highlighted with three errors, starting at if (static::$instanceCache[$key] === null) {:

function getInstance(Quest $quest, UserCharacter $character)
{
    $key = $quest->id . '_' . $character->id;

    if (empty(static::$instanceCache[$key])) {
        static::$instanceCache[$key] =
            static::where('questID', '=', $quest->id)
                ->where('userCharacterID', '=', $character->id)
                ->where(
                    function ($query) {
                        $query->whereNull('completed');
                        $query->orWhere('completed', '=', 0);
                    }
                )
                ->first();

        if (static::$instanceCache[$key] === null) {
            static::$instanceCache[$key] = new static(
                array(
                    'userCharacterID' => $character->id,
                    'questID' => $quest->id,
                )
            );
        }
    }

    return static::$instanceCache[$key];
}

gsherwood added a commit that referenced this issue Dec 8, 2014
@gsherwood
Copy link
Member

Thanks for the extra failing code. I found the problem with that block and have committed a fix for it.

@MaartenStaa
Copy link

Thanks, that solved most of my issues. However, I have identified two additional failing cases.

$foo->load(
    array(
        'bar' => function ($baz) {
            $baz->call();
        }
    )
);

hello();

And:

$foo = array_unique(
    array_map(
        function ($entry) {
            return $entry * 2;
        },
        array()
    )
);
bar($foo);

@aik099
Copy link
Contributor

aik099 commented Dec 9, 2014

Now I'm glad, that I'm not using any closures in PHP 😉

@gsherwood
Copy link
Member

@Aistina I like you 😄

These 2 new issues were actually caused by the parenthesis and not the closure, which is nice at least. I didn't quite take yesterday's fix far enough.

@MaartenStaa
Copy link

Why, thank you ^_^

After that fix some new issues popped up though. I posted them in the new issue #358.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants