Raise Squiz.PHP.NonExecutableCode to error

[gmponos]

Not sure if this was on purpose.

This rules is intent to throw a warning and not an error.

[greg0ire]

This rules is intent to throw a warning and not an error.

Are you saying that the intent is wrong? If yes, why not report that upstream instead?

[gmponos]

No no... I think the intent of Squiz.PHP.NonExecutableCode is what it is..

Not sure if doctrine org intentions was to have it as a warning or if you just missed that.. If this rule is added to doctrine standard as a warning on purpose then we can close this..

[SenseException]

I don't know if this was intended to be a warning, but I like to have an error here for NonExecutableCode. It had a few false positives in the past and maybe that's why it's a warning, but if this isn't an issue anymore, then an error would be fine.

[greg0ire]

Not sure about the target branch… is this really a bugfix?

[Ocramius]

IMO requires major anyway, still LGTM

[derrabus]

Maybe we should consider removing the rule completely. It produces false positives for throw expressions in PHP 8 and PHPStan/Psalm should have a better dead code detection.

see squizlabs/PHP_CodeSniffer#2857

[Ocramius]

Works too 👍

[jrfnl]

Maybe we should consider removing the rule completely. It produces false positives for throw expressions in PHP 8 and PHPStan/Psalm should have a better dead code detection.

see squizlabs/PHP_CodeSniffer#2857

FYI: The above mentioned issues have been fixed and the fixes will be included in PHPCS 3.8.0.