PHP_VERSION_ID incorrect value

[momala454]

Describe the bug
the plugin doesn't verify that parameter types of mb_strtolower are correct. Maybe there are more functions with the same problem.

To Reproduce

echo mb_strtolower(new stdClass());
echo strtolower(new stdClass());
mysqli_connect(new stdClass());
die(new stdClass());

Expected behavior
an error should be displayed on each of those lines, but it is shown only on strtolower and mysqli_connect.
Btw, no tooltip is shown on "die"

Screenshots

Platform and version
windows
1.8.2

[tianyiw2013]

echo mb_strtolower(new stdClass());

My vscode has an error message.

Maybe it's because your project file is contaminated by other ide-helper or polyfill files. You can see it in your screenshot.

For example a file like this https://github.com/symfony/polyfill/blob/main/src/Mbstring/bootstrap.php

die(new stdClass());

die is a language construct. In fact, It accepts the mixed type, PHP will cast it to a string.

class test
{
    function __toString()
    {
        return __CLASS__;
    }
}

die(new test());

# see the error message of PHP
die(new stdClass); // Uncaught Error: Object of class stdClass could not be converted to string
trim(new stdClass); // Uncaught TypeError: trim(): Argument #1 ($string) must be of type string, stdClass given

[momala454]

Thanks, you're right, that was symfony. Strangely it was loading the PHP<8 and the PHP>=8 file.
I suppose intelephense could detect this and ignore the file

if (\PHP_VERSION_ID >= 80000) {
    return require __DIR__.'/bootstrap80.php';
}

Why does it show 50306 if i have configured php 8 ?
running the script shows 80102 for 8.1.2

[KapitanOczywisty]

Why does it show 50306 if i have configured php 8 ?

It's statically defined in stubs. You shouldn't rely on constant values showed in tooltips.

[momala454]

it makes sense. But do you think it's a good idea to change it to the configured value so the symfony functions for PHP of a lower version are ignored ?

So like in this following code, if i'm on php8, the signature of the function is the one with int $a

if (\PHP_VERSION_ID < 80000) {
    function helloWorldTestVersion(string $a) {

    }
}
else {
    function helloWorldTestVersion(int $a) {

    }
}

helloWorldTestVersion('a');

currently intelephense show both of them

[jfcherng]

That's irrelevant. Even you use

if (false) {
    function helloWorldTestVersion(string $a) {

    }
}
else {
    function helloWorldTestVersion(int $a) {

    }
}

Both signatures are still provided by intelephense.

[momala454]

what should symfony or intelephense do to prevent us to see the function definitions we don't want to see then ?

what is your opinion to change the way intelphense handle method definitions to ignore them if they are never executed (like in your example) ?

[tianyiw2013]

The idea is good, but I think this feature is difficult to implement.

[KapitanOczywisty]

Dead code detection: #1718

I think this is a bad idea to have different function signature in different php versions (unless you're dealing with resource/object migration). Better is to keep version logic inside a function:

function helloWorldTestVersion(string $a) {
  if (\PHP_VERSION_ID < 80000) {
  } else {
  }
}

If you need to make some polyfills, place them into separate file e.g. polyfills/php-80.php. Then file can be excluded in intelephense.references.exclude or intelephense.files.exclude. Currently @ignore is not working, but it'd be another option.

There are some existing polyfills, which should remove this whole problem: https://packagist.org/explore/?query=symfony%2Fpolyfill-php

[bmewburn]

Closing as out of scope. It is not feasible to provide the level of analysis needed to solve this edge case.

[momala454]

@bmewburn and just for the constant PHP_VERSION_ID, could the extension just use the version defined in settings ?

[bmewburn]

@momala454 I think this again falls into the edge case category that is very low priority. However, I did remember just now that there is an existing way to better control what versions of a function is indexed by intelephense. You can add annotations above the definition like so:

/**
 * @removed 8.1.0
 */
 function myFunction(string $string) { }

/**
 * @since 8.1.0 
 */
   function myFunction(object $object) { }