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

Squiz.Commenting.FunctionComment false positive when function contains closure #1550

Closed
josephzidell opened this issue Jul 7, 2017 · 6 comments
Milestone

Comments

@josephzidell
Copy link
Contributor

There is a false positive in src\Standards\Squiz\Sniffs\Commenting\FunctionCommentSniff.php

This code throws error: Function return type is not void, but function is returning void here

/**
 * Test function
 *
 * @return array
 */
public function test()
{
	function () {
		return;
	};

	return [];
}
@michalbundyra
Copy link
Contributor

The same issue we have also in case:

function test()
{
    new class() {
        public function test() {
            return;
        }
    };

    return [];
}

@josephzidell Here is my PR with fix for both cases: #1551

@josephzidell
Copy link
Contributor Author

Thanks for coming up with a fix so quickly

@josephzidell
Copy link
Contributor Author

@webimpress Found another, related issue

Code:

/**
 * Test function
 *
 * @return void
 */
public function test()
{
	function () {
		return 4;
	};
}

Error: Function return type is void, but function contains return statement

@michalbundyra
Copy link
Contributor

@josephzidell I've added more tests 4a28555
and all of them pass:
https://travis-ci.org/squizlabs/PHP_CodeSniffer/builds/252693542?utm_source=github_status&utm_medium=notification

It seems that my previous solution also covers these cases.

@josephzidell
Copy link
Contributor Author

👍

@gsherwood gsherwood added this to the 3.1.0 milestone Jul 20, 2017
@gsherwood gsherwood changed the title Return type false positive Squiz.Commenting.FunctionComment false positive when function contains closure Aug 22, 2017
gsherwood added a commit that referenced this issue Aug 22, 2017
gsherwood added a commit that referenced this issue Aug 22, 2017
@gsherwood
Copy link
Member

This has now been fixed. Thanks a lot for reporting this.

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

3 participants