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

Two properties (one with an underscore _ one without) results in having only one getter method #502

Closed
sippsolutions opened this issue Feb 14, 2024 · 2 comments

Comments

@sippsolutions
Copy link
Contributor

sippsolutions commented Feb 14, 2024

Bug Report

Q A
BC Break yes
Version 3.1.1

Summary

Two properties having the same name but one is with underscores results in one of them having no getter method.

Current behavior

When generating type classes the \Phpro\SoapClient\CodeGenerator\Util\Normalizer::normalizeMethodName removes underscores.
\Phpro\SoapClient\CodeGenerator\Assembler\GetterAssembler::assemble then removes the generated (generatePropertyMethod) method (via removeMethod).
When you have two properties named TEST_KEY and TESTKEY both of them are transformed into a class property, but only one of them will get a "getter" method as the underscores get removed and the previously generated method for TEST_KEY (getTESTKEY) will be removed as TESTKEY gets assigned the same getter name (getTESTKEY).
Unfortunately I cannot change the WSDL as it comes from a third party API.

\Phpro\SoapClient\CodeGenerator\Util\Normalizer::generatePropertyMethod should be changed from regex ^a-z0-9]+ to ^a-z0-9]_+.
OR, maybe a workaround: If two properties have the same getter method name, the first one should not be removed with removeMethod but the second one should get a new getter name (like first one having getTESTKEY, second one getting getTESTKEY__2) - this would result in a non-breaking change as existing method names are not affected.

How to reproduce

Generate type classes with two properties named the same but one property having an additional underscore inside of the name.

Expected behavior

Both properties have their corresponding getter method.


Working example for the workaround (should also be applied for Setters, ImmutableSetters, ...):

--- src/Phpro/SoapClient/CodeGenerator/Assembler/GetterAssembler.php	2024-02-14 15:53:30
+++ src/Phpro/SoapClient/CodeGenerator/Assembler/GetterAssembler.php	2024-02-14 15:54:33
@@ -2,13 +2,13 @@
 
 namespace Phpro\SoapClient\CodeGenerator\Assembler;
 
+use Laminas\Code\Generator\MethodGenerator;
 use Phpro\SoapClient\CodeGenerator\Context\ContextInterface;
 use Phpro\SoapClient\CodeGenerator\Context\PropertyContext;
+use Phpro\SoapClient\CodeGenerator\LaminasCodeFactory\DocBlockGeneratorFactory;
 use Phpro\SoapClient\CodeGenerator\Model\Property;
 use Phpro\SoapClient\CodeGenerator\Util\Normalizer;
-use Phpro\SoapClient\CodeGenerator\LaminasCodeFactory\DocBlockGeneratorFactory;
 use Phpro\SoapClient\Exception\AssemblerException;
-use Laminas\Code\Generator\MethodGenerator;
 use Soap\Engine\Metadata\Model\TypeMeta;
 
 /**
@@ -59,6 +59,13 @@
         try {
             $prefix = $this->getPrefix($property);
             $methodName = Normalizer::generatePropertyMethod($prefix, $property->getName());
+            if ($class->hasMethod($methodName)) {
+                $i = 2;
+                while ($class->hasMethod($methodName . '__' . $i)) {
+                    $i++;
+                }
+                $methodName = $methodName . '__' . $i;
+            }
             $class->removeMethod($methodName);
 
             $methodGenerator = new MethodGenerator($methodName);
@veewee
Copy link
Contributor

veewee commented Feb 18, 2024

Hello,

Thanks for reporting!
Having 2 fields with the same name only variated by a underscore sounds - besides very confusing from an implementor side - also very uncommon.

If we want to fix this, I'dd say allowing the generatePropertyMethod to contain underscores sounds like the way to go.
This might be a subtle change in this library, but will result in broken code in packages that use this library : the public API of the generated types changes which will result in broken code. This is probably something we want to do in next major version which won't be ready anyday soon.

I'll put it to the list of tasks to take into consideration. You could solve this with a custom assembler from your end ATM.

@veewee
Copy link
Contributor

veewee commented Jul 5, 2024

Hello there @sippsolutions,

I wanted to get back to this issue:
We are finishing up v4 of the soap-client package which uses a new encoding system internally.
With this new system, you should be able to get around this issue more easily.

We are eager to receive some early feedback on this new release.
If you got some time, feel free to play around with the alpha version:

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

2 participants