Skip to content

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

Merged
danielkerr merged 1 commit intoopencart:3.0.x.xfrom
AJenbo:3phpstan1
Feb 11, 2024
Merged

[3.0.x.x] Make identically identified functions have the same signature#13665
danielkerr merged 1 commit intoopencart:3.0.x.xfrom
AJenbo:3phpstan1

Conversation

@AJenbo
Copy link
Copy Markdown
Contributor

@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
Copy Markdown
Contributor

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
Copy Markdown
Contributor

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
Copy Markdown
Contributor 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
Copy Markdown
Contributor

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
@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