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

[3.0.x.x] Make identically identified functions have the same signature #13665

Merged
merged 1 commit into from
Feb 11, 2024

Conversation

AJenbo
Copy link
Collaborator

@AJenbo AJenbo commented Feb 10, 2024

@mhcwebdesign nice progress on the PHPStan issues. This PR should make the branch pass the check fully.

The reason it's failing is that there are multiple classes with the same name and namespace, coatning method with the same name but different signatures.

This is just a proposed solution, idk if you prefer to solve it by renaming them or some other way.

@AJenbo AJenbo changed the title Resolve PHPStan level 1 errors caused by signature collision [3.0.x.x] Resolve PHPStan level 1 errors caused by signature collision Feb 10, 2024
@mhcwebdesign
Copy link
Collaborator

This doesn't explain the phpstan bugs, see #13663

For a given set of PHP files, phpstan should always produce the same results, it doesn't! Until then, I suggest we disable phpstan, or come up with a proper fix!

@mhcwebdesign
Copy link
Collaborator

Besides, your change to the model/extension/payment/opayo.php for the sendCurl is wrong! phpstan needs to be fixed if it can't cope with functions using optional parameters.

@AJenbo
Copy link
Collaborator Author

AJenbo commented Feb 11, 2024

This has noting to do with optional parameters and isn't a bug in PHPStan. It's caused by there being two classes with the same name that both have a method called sendCurl(). One has 3 arguments the other 2. So it depends on which one is processed first and there for will give different results on different computers.
This issue was solved in OC4 by using namespace.

Wrong. In the case of the sendCurl methods, they are defined in differently named classes (16 of them in OC 3.0.x.x), and can have therefore different method signatures.

Please undo your changes!

@AJenbo AJenbo changed the title [3.0.x.x] Resolve PHPStan level 1 errors caused by signature collision [3.0.x.x] Make identically identified functions have the same signature Feb 11, 2024
@mhcwebdesign
Copy link
Collaborator

Sorry, you are wrong. Different classes in PHP, using the same name for a method, can use different method signatures for this. These are not bugs in OC 3.0.x.x! And developers shouldn't have to worry about how they name class methods, with what signatures, as long the methods are correctly used and called upon.

I reported this bug here: phpstan/phpstan#10560 on the phpstan repository, though I don't have time to write more sample codes for this on their repo. Busy making OC 3.0.x.x stable again!

@danielkerr danielkerr merged commit d938af8 into opencart:3.0.x.x Feb 11, 2024
6 checks passed
@AJenbo AJenbo deleted the 3phpstan1 branch February 11, 2024 16:35
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

Successfully merging this pull request may close these issues.

3 participants