ConstructorNameSniff false negative on PHP 8+

[gsteel]

PHP4 style constructors are no longer possible on PHP 8+

This sniff therefore yields false negative for a class such as:

class Something {
   public function something(): void
   {
   }
}

PHP_CodeSniffer/src/Standards/Generic/Sniffs/NamingConventions/ConstructorNameSniff.php

Lines 82 to 90 in a26c071

	if ($methodName === $className) {
	if (in_array('__construct', $this->functionList, true) === false) {
	$error = 'PHP4 style constructors are not allowed; use "__construct()" instead';
	$phpcsFile->addError($error, $stackPtr, 'OldStyle');
	}
	} else if ($methodName !== '__construct') {
	// Not a constructor.
	return;
	}

[jrfnl]

When I run PHPCS over that code I see the following error:

 3 | ERROR | PHP4 style constructors are not allowed; use "__construct()" instead
   |       | (Generic.NamingConventions.ConstructorName.OldStyle)

In other words, it is unclear what you are reporting (there is no false negative) and what you expect to be done about it.

Please clarify what behaviour you expected.

[gsteel]

Sorry @jrfnl
I got my true/false negative/positive all messed up.

On PHP 8, I'd expect no errors. On 7.x and below the error would remain present.

[jrfnl]

@gsteel Thanks for getting back to me, but I'm still not sure what you expect.

The PHP version on which PHPCS is being run does not always have a direct correlation to the PHP version the code under scan will run on.

And as that sniff has only one function - "ban PHP4 style constructors" -, if the code under scan is intended to be run on PHP 8.x, why are you including this sniff ?

This seems to me more like a configuration issue which can be solved via the project ruleset:

<exclude name="Generic.NamingConventions.ConstructorName"/>

[gsteel]

Yes, I've disabled the sniff in the relevant project.

I guess I was expecting some magic here and thought that it might be worth implementing; Assuming this kind of version detection logic is not appropriate for phpcs then please consider this a non-issue!

Thanks for your time today :)

[jrfnl]

I guess I was expecting some magic here and thought that it might be worth implementing

Well, there is some magic which could be implemented, but the feature that "magic" relies on is very rarely used, so I'm not sure how much effect it would have and if it wouldn't lead to more reports about actual false negatives, because the sniff suddenly doesn't always report issues anymore.

[gsteel]

TIL! If you think it's worth pursuing, I'd be happy to submit a patch if you could point me at a sniff you know of that uses a version check.

[jrfnl]

I'm honestly not sure how much grief it will cause with support requests.

However, you can find an example of it being used here: https://github.com/squizlabs/PHP_CodeSniffer/blob/master/src/Standards/Generic/Sniffs/PHP/DisallowAlternativePHPTagsSniff.php - though in this case, I would personally not presume that if no php_version is passed, the PHP version on which PHPCS is run should be used.