PHPCompatibility icon indicating copy to clipboard operation
PHPCompatibility copied to clipboard

Is it possible to doesn't check small piece of code based on PHP_VERSION_ID and create annotation about it

Open stodorovic opened this issue 9 years ago • 5 comments

It's an idea which can offer to developers to use old PHP functions without errors, notices,... Basically, php has constant PHP_VERSION_ID ( http://php.net/manual/en/reserved.constants.php#reserved.constants.core ). It's integer and it's very helpful for conditional which depends on php version.

Example:

if ( PHP_VERSION_ID < 50400 && ini_get('safe_mode') ) {
      // here is old code which works on PHP 5.2 and 5.3 if it's safe_mode
}

or second example

if ( PHP_VERSION_ID < 50400 && $quotes ) {             
    ini_set('magic_quotes_runtime', 0);  
}

It offers chance to we can create efficient code (which passes all tests) with PHP backwards compatibility.

stodorovic avatar Nov 13 '16 13:11 stodorovic

The only way to do that would be to add a docblock annotation above and below the section of code. Using if structures is not the right way to go here, since it would be extremely difficult (in fact impossible) to ensure we interpret everything correctly using static analysis.

wimg avatar Nov 13 '16 13:11 wimg

It's great to create annotation that section isn't checked. Basically, they are simple functions (as ini_set) or settings of some variables. Even it's possible to create warning if this section has complex code and more than few lines. I have some my code which already use something similar and I'll be happy if it's possible to it passes all tests. Also, a lot of wp plugins use simple ini_get/ini_set (almost a line of code) and php compatibility checker reports a lot of false warnings/errors. I want to help related to coding/testing if I can somehow.

stodorovic avatar Nov 13 '16 14:11 stodorovic

The main concern I have is that, by adding this, people will start using it on pieces of code that should be updated, but because they disable the check, it will never show up anymore. Then at some point they upgrade to PHP 7.x and everything breaks...

wimg avatar Nov 13 '16 14:11 wimg

PHPCS already has build in annotations which could be used here, though I agree with @wimg that it's not a good idea to start using these all over the place, as they might cause you to miss out on messages which you should receive.

Also, and this might be better, once #291 has been merged, every sniff will have modular error codes, so instead you could then add something along the lines of the following to your ruleset:

<rule ref="PHPCompatibility.PHP.DeprecatedIniDirectives.safe_modeRemoved">
    <exclude-pattern>*/my-file-which-uses-version-check.php</exclude-pattern>
</rule>

This will ignore just and only that particular error for that particular file.

Once #291 is merged, run PHPCS with the -s option to see the exact sniff codes which you can use.

jrfnl avatar Nov 13 '16 15:11 jrfnl

FYI: as of PHPCS 3.2.0 - which just came out -, you can very selectively turn off just one sniff for a piece of code using the phpcs:disable PHPCompatibility.PHP.DeprecatedIniDirectives.safe_modeRemoved and phpcs:enable annotations.

For more information, see the updated wiki page and the changelog for PHPCS 3.2.0:

  • https://github.com/squizlabs/PHP_CodeSniffer/wiki/Advanced-Usage#ignoring-parts-of-a-file
  • https://github.com/squizlabs/PHP_CodeSniffer/releases/tag/3.2.0

As that means that you would not be missing out on results from other sniffs as you'd only selectively and locally disable the one errorcode, you're basically also documenting for other developers that that code has been verified and confirmed as ok compatibility-wise.

Does that help ?

jrfnl avatar Dec 14 '17 01:12 jrfnl