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

Autoload is maybe wrong: Cannot redeclare Safe\array_combine() #741

Closed
nepda opened this issue Dec 21, 2020 · 12 comments
Closed

Autoload is maybe wrong: Cannot redeclare Safe\array_combine() #741

nepda opened this issue Dec 21, 2020 · 12 comments

Comments

@nepda
Copy link

nepda commented Dec 21, 2020

  • Larastan Version: v0.6.11
  • --level used: 8

Description

When running phpstan with larastan extension on specific functions this error happened. I created a repository to reproduce this:

https://github.com/nepda/phpstan-larastan-bug

Without larastan, it will run perfectly.

Laravel code where the issue was found

<?php declare(strict_types=1);

namespace App;

use function Safe\dns_get_record;

class Test2
{
    private function test(): void
    {
        dns_get_record('example.com');
    }
}
php ./vendor/bin/phpstan analyse app/Test2.php                                                                                                                                                                                                                     [11:59:21]
Note: Using configuration file /var/www/html/phpstan.neon.
PHP Fatal error:  Cannot redeclare Safe\array_combine() (previously declared in /var/www/html/vendor/thecodingmachine/safe/generated/array.php:20) in /var/www/html/vendor/thecodingmachine/safe/generated/array.php on line 20
Fatal error: Cannot redeclare Safe\array_combine() (previously declared in /var/www/html/vendor/thecodingmachine/safe/generated/array.php:20) in /var/www/html/vendor/thecodingmachine/safe/generated/array.php on line 20

In array.php line 20:
                                                                                                                                    
  Cannot redeclare Safe\array_combine() (previously declared in /var/www/html/vendor/thecodingmachine/safe/generated/array.php:20)  
                                                                                                                                    

Failed to execute command php ./vendor/bin/phpstan analyse app/Test2.php: exit status 255
FAIL

If you remove those lines:

includes:
    - vendor/nunomaduro/larastan/extension.neon

it will run without errors.
This is related to: thecodingmachine/safe#253

@szepeviktor
Copy link
Collaborator

szepeviktor commented Dec 21, 2020

Try adding , false, see the above PR.

Actually you need two false-es in vendor/nunomaduro/larastan/src/Types/GenericEloquentBuilderTypeNodeResolverExtension.php for both is_subclass_of

@szepeviktor
Copy link
Collaborator

Want to see stange things?

Replace the contents of vendor/thecodingmachine/safe/generated/array.php with

error_log('-----------------------------------------------------------------------------------------------');
var_export(debug_backtrace(0, 6));

@nepda
Copy link
Author

nepda commented Dec 21, 2020

Okay, the tests seems to complain about your changes...

Will there be a new version?

@szepeviktor
Copy link
Collaborator

@canvural Could you chime in?

@asbiin
Copy link

asbiin commented Dec 28, 2020

I've made a fix in thecodingmachine/safe, which you will find in this branch: https://github.com/asbiin/safe/tree/enclose-functions

@szepeviktor
Copy link
Collaborator

It seems like the autoloading problem is in our side, in Larastan.

@canvural
Copy link
Collaborator

Hi,

From the example repo you created, file Test3 passes successfully. So I think this error should be fixed in thecodingmachine/safe not here. If it was an issue with Larastan's autolading stuff, json_encode also would cause problems.

@j3j5
Copy link
Contributor

j3j5 commented Mar 9, 2022

I'm sorry to comment back on a closed issue, but I believe the issue is on larastan and this issue shouldn't be closed.

I've been debugging it and the problem come from these lines, larastan is checking whether Safe\array is a subclass of Model or Builder using is_subclass_of, unfortunately, the is_subclass_of call with Safe\array, finds the Safe\Array class is not loaded (because it isn't a class, although the file itself with the functions is already loaded) so it triggers the autoloader again, loading the array.php file again and hence causing the error Cannot redeclare Safe\array....

I'm a bit lost on the larastan code so I'm not sure why is trying to check whether Safe\array is a subclass of Model, or how to avoid it, but I do believe the issue is on larastan side. #742 went on the right direction, but I'm not sure what check (if any) can be added to make sure that the string returned by $nameScope->resolveStringName($innerTypeNode->name) is indeed a class, but I know it is being loaded the 2nd time because of that code in larastan.

Maybe the solution comes from finding out why larastan is trying to check Safe\array as a class in the first place, but I'm not that familiar with the code yet. Hope this helps somebody with more knowledge to find a solution.

@canvural
Copy link
Collaborator

canvural commented Mar 14, 2022

@j3j5 I added a commit that not uses is_subclass_of Can you try dev-master now?

If it still happens it means it's an issue in either PHPStan or Safe.

@mariusjp
Copy link

@j3j5 I added a commit that not uses is_subclass_of Can you try dev-master now?

If it still happens it means it's an issue in either PHPStan or Safe.

By updating my test (thecodingmachine/phpstan-safe-rule#29 (comment)) with composer require nunomaduro/larastan:dev-master -W I can confirm I'm not getting the array_flip error anymore (which was the same type of error as mentioned in this issue)

@j3j5
Copy link
Contributor

j3j5 commented Mar 14, 2022

I'm afraid the issue is still present on packages :( So I guess you are right @canvural and the source wasn't (only) the use of is_subclassof. This proving to be a really sneaky bug, haha.

@MJTheOne I just tested and the latest stable version of all the packages doesn't cause the issue on your setup either. For my tests I'm trying an updated version of the repo posted by OP.

I'll try to dig further later in the day. Thanks for trying @canvural !!

@j3j5
Copy link
Contributor

j3j5 commented Mar 16, 2022

Ok, good news, I've found the issue, haha, and it turns out it's either on Safe's side or PHPStan's. It turns out that the PHPDocs on the functions defined on the Safe library use array to specify the param type or return type, and then, PHPStan (or rather, its PHPDocParser) tries to resolve array within the Safe namespace as Safe\array, and since there's a file called array.php that matches the PSR4 spec for a class with that name, it tries to autoload it to see if that's the correct type, but the file is already loaded so the error of "redeclaring" the function gets thrown. That explains why Test.php (which uses openssl_pkcs12_read) and Test2.php (using dns_get_record()) throw the error but Test3.php (with json_encode()) doesn't, because its PHPDoc does not contain any conflicting type.

Now, using \array as the type for @param or @return on the Safe library fixes this issue, but I have to wonder whether this is a bug on PHPDocParser (unless explicitly defined, it should go for native types first) or not. I can't find anything specific on it but it never occurred to me to use the global namespace for native types on PHPDocs on my own code, but maybe it is the right thing to do. I'm going to suggest that solution on the Safe repo.

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