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

extension_loaded breaks MixedAssignment #6457

Closed
serbancatalin opened this issue Sep 8, 2021 · 8 comments
Closed

extension_loaded breaks MixedAssignment #6457

serbancatalin opened this issue Sep 8, 2021 · 8 comments

Comments

@serbancatalin
Copy link

Hello,

https://psalm.dev/r/8cfb4b0115

If you comment if (extension_loaded('invalid')) it will know that $referenceDate is DateTime

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/8cfb4b0115
<?php

class TestCommand
{
    public function execute(): void
    {
        if (extension_loaded('invalid')) {
        }

        $referenceDate = $this->getReferenceDate();
        
        echo $referenceDate->format('Ymd');
    }

    /**
     * @return \DateTime
     */
    public function getReferenceDate(): \DateTime
    {
        return new \DateTime();
    }
}
Psalm output (using commit e10b809):

INFO: MixedAssignment - 10:9 - Unable to determine the type that $referenceDate is being assigned to

INFO: MixedArgument - 12:14 - Argument 1 of echo cannot be mixed, expecting string

@orklah
Copy link
Collaborator

orklah commented Sep 8, 2021

Can you describe your use case a little?

When Psalm encounters a extension_loaded function, it will check its content to see if the extension is loaded in the environment that runs Psalm.

If it's not, the flag will help skiping part of the analysis to avoid emitting too much error just because the extension is not there.

'invalid' is not detected so Psalm tries its best. There may be a bug lying here because there is no specific code that Psalm should be afraid of, but I'd like to understand what you need this for before diving into the code for possibly hours :p

(Note that if you just need a quick solution, you can do that: https://psalm.dev/r/0fc2dc3293)

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/0fc2dc3293
<?php

class TestCommand
{
    public function execute(): void
    {
        $invalid = 'invalid';
        if (extension_loaded($invalid)) {
        }

        $referenceDate = $this->getReferenceDate();
        
        echo $referenceDate->format('Ymd');
    }

    /**
     * @return \DateTime
     */
    public function getReferenceDate(): \DateTime
    {
        return new \DateTime();
    }
}
Psalm output (using commit e10b809):

No issues!

@serbancatalin
Copy link
Author

serbancatalin commented Sep 8, 2021

I am using newrelic extension to name and start/stop a transaction.
This extension is not loaded in the environment that runs Psalm.

Something like

if (\extension_loaded('newrelic')) {
    newrelic_end_transaction();
    newrelic_ignore_transaction();
    newrelic_start_transaction('test');
    newrelic_name_transaction('test');
    newrelic_background_job(false);
}

Your solution is good.
I can also move this code into another method.

Thank you

@orklah
Copy link
Collaborator

orklah commented Sep 8, 2021

you should maybe consider loading the extension on Psalm's environment. it may be able to find more bugs when the extension is there.

I'll close this as it seems a pretty edge case and you're satisfied with the quick solution. If I see another similar issue, we'll think about it more.

Thanks for your feedback anyway!

@orklah orklah closed this as completed Sep 8, 2021
@nicolashohm
Copy link

nicolashohm commented Nov 15, 2023

Just wanted to let you know that I encountered the same or similar issue.

I have a PHP file as entry point which does check for NewRelic extension at the beginning, but errors after the extension check are ignored by psalm.

Environment: Psalm 5.15.0@5c774aca4746caf3d239d9c8cadb9f882ca29352
Target PHP version: 8.1 (inferred from composer.json) Enabled extensions: dom, pdo, redis.

Here is a simplified example of my use case:

entrypoint.php:

<?php 
require_once __DIR__ . 'vendor/autoload.php';

if (extension_loaded('newrelic')) {
    newrelic_name_transaction('Test');
}

$factory = new Factory();
$factory->createBarz();

Factory.php

<?php

class Factory {
    public function createFoo() { }
}

Psalm runs and reports no errors even though the factory method doesn't exist.

As soon as I extract extension_loaded('newrelic') into a method or even just put the extension name in a variable as suggested by @orklah :

$extension = 'newrelic';
if (extension_loaded($extension)) {

it works, and psalm reports that createBarz does not exist in the Factory.

I tried to reproduce it with psalm.dev but there it reports the issues just fine: https://psalm.dev/r/b3e4a13ad6

Anyway, I'm fine with the solution to extract the extension_loaded check into a method.

@danog
Copy link
Collaborator

danog commented Nov 15, 2023

Hmm could you try reproducing on psalm.dev? This should have been fixed by #10295...

@orklah
Copy link
Collaborator

orklah commented Nov 15, 2023

There was no release since then so it makes sense the issue is still here on 5.15

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants