Merge pull request #5843 from microsoft/fix/php-integration-tests

Fixes failing Php integration tests because of wrong parameter types
microsoft · Dec 5, 2024 · eb93ca9 · eb93ca9
2 parents 0c13a88 + 66c738c
commit eb93ca9
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 27 deletions.
diff --git a/it/config.json b/it/config.json
@@ -32,10 +32,6 @@
       {
         "Language": "ruby",
         "Rationale": "https://github.com/microsoft/kiota/issues/1816"
-      },
-      {
-        "Language": "php",
-        "Rationale": "https://github.com/microsoft/kiota/issues/5779"
       }
     ],
     "ExcludePatterns": [

diff --git a/it/php/phpstan.neon b/it/php/phpstan.neon
@@ -7,4 +7,4 @@ parameters:
     - src
   ignoreErrors:
     - '#Parameter [\w\W]+ \$errorMappings of method [\w\\]+RequestAdapter::send[\w]+\(\) expects [\w\W\s]+ given\.#'
-    - '#getFieldDeserializers\(\) should return array\<string, callable\(Microsoft\\Kiota\\Abstractions\\Serialization\\ParseNode\)\: void\> but returns array\{[\d]+\: Closure\(Microsoft\\Kiota\\Abstractions\\Serialization\\ParseNode\)\: void.+#'
+    - '#getFieldDeserializers\(\) should return array\<string, callable\(Microsoft\\Kiota\\Abstractions\\Serialization\\ParseNode\)\: void\> but returns array\{.+\: Closure\(Microsoft\\Kiota\\Abstractions\\Serialization\\ParseNode\)\: void.+#'
diff --git a/src/Kiota.Builder/Writers/Php/CodeMethodWriter.cs b/src/Kiota.Builder/Writers/Php/CodeMethodWriter.cs
@@ -475,35 +475,42 @@ _ when conventions.PrimitiveTypes.Contains(lowerCaseProp) => $"write{lowerCasePr
     }
 
     private const string ParseNodeVarName = "$parseNode";
-    private string GetDeserializationMethodName(CodeTypeBase propType, CodeMethod method)
+    private (string, string) GetDeserializationMethodName(CodeTypeBase propType, CodeMethod method)
     {
         var isCollection = propType.CollectionKind != CodeTypeBase.CodeTypeCollectionKind.None;
         var propertyType = conventions.GetTypeString(propType, method, false);
-        var parseNodeMethod = string.Empty;
+        var parseNodeMethod = (string.Empty, string.Empty);
         if (propType is CodeType currentType)
         {
             if (isCollection)
                 parseNodeMethod = currentType.TypeDefinition switch
                 {
-                    CodeEnum enumType => $"getCollectionOfEnumValues({enumType.Name.ToFirstCharacterUpperCase()}::class)",
-                    _ => $"getCollectionOfObjectValues([{conventions.TranslateType(propType)}::class, '{CreateDiscriminatorMethodName}'])"
+                    CodeEnum enumType => (string.Empty, $"getCollectionOfEnumValues({enumType.Name.ToFirstCharacterUpperCase()}::class)"),
+                    CodeClass => (string.Empty, $"getCollectionOfObjectValues([{conventions.TranslateType(propType)}::class, '{CreateDiscriminatorMethodName}'])"),
+                    _ => (conventions.TranslateType(propType), $"getCollectionOfPrimitiveValues('{conventions.TranslateType(propType)}')")
                 };
             else if (currentType.TypeDefinition is CodeEnum)
-                parseNodeMethod = $"getEnumValue({propertyType.ToFirstCharacterUpperCase()}::class)";
+                parseNodeMethod = (string.Empty, $"getEnumValue({propertyType.ToFirstCharacterUpperCase()}::class)");
         }
 
         var lowerCaseType = propertyType.ToLowerInvariant();
-        return string.IsNullOrEmpty(parseNodeMethod) ? lowerCaseType switch
-        {
-            "int" => "getIntegerValue()",
-            "bool" => "getBooleanValue()",
-            "number" => "getIntegerValue()",
-            "decimal" or "double" => "getFloatValue()",
-            "streaminterface" => "getBinaryContent()",
-            "byte" => "getByteValue()",
-            _ when conventions.PrimitiveTypes.Contains(lowerCaseType) => $"get{propertyType.ToFirstCharacterUpperCase()}Value()",
-            _ => $"getObjectValue([{propertyType.ToFirstCharacterUpperCase()}::class, '{CreateDiscriminatorMethodName}'])",
-        } : parseNodeMethod;
+        var res = parseNodeMethod;
+        if (string.IsNullOrEmpty(parseNodeMethod.Item2))
+            res = (string.Empty, lowerCaseType switch
+            {
+                "int" => "getIntegerValue()",
+                "bool" => "getBooleanValue()",
+                "number" => "getIntegerValue()",
+                "decimal" or "double" => "getFloatValue()",
+                "streaminterface" => "getBinaryContent()",
+                "byte" => "getByteValue()",
+                _ when conventions.PrimitiveTypes.Contains(lowerCaseType) =>
+                    $"get{propertyType.ToFirstCharacterUpperCase()}Value()",
+                _ =>
+                    $"getObjectValue([{propertyType.ToFirstCharacterUpperCase()}::class, '{CreateDiscriminatorMethodName}'])",
+            });
+
+        return res;
     }
 
     private void WriteSetterBody(LanguageWriter writer, CodeMethod codeElement, CodeClass parentClass)
@@ -672,7 +679,7 @@ private void WriteDeserializerPropertyCallback(CodeProperty property, CodeMethod
             writer.WriteLine("},");
             return;
         }
-        writer.WriteLine($"'{property.WireName}' => fn(ParseNode $n) => $o->{property.Setter!.Name.ToFirstCharacterLowerCase()}($n->{GetDeserializationMethodName(property.Type, method)}),");
+        writer.WriteLine($"'{property.WireName}' => fn(ParseNode $n) => $o->{property.Setter!.Name.ToFirstCharacterLowerCase()}($n->{GetDeserializationMethodName(property.Type, method).Item2}),");
     }
 
     private static void WriteDeserializerBodyForIntersectionModel(CodeClass parentClass, LanguageWriter writer)
@@ -903,7 +910,8 @@ private void WriteFactoryMethodBodyForIntersectionModel(CodeMethod codeElement,
         {
             if (property.Type is CodeType propertyType)
             {
-                var deserializationMethodName = $"{ParseNodeVarName}->{GetDeserializationMethodName(propertyType, codeElement)}";
+                var methodName = GetDeserializationMethodName(propertyType, codeElement);
+                var deserializationMethodName = $"{ParseNodeVarName}->{methodName.Item2}";
                 writer.StartBlock($"{(includeElse ? "} else " : string.Empty)}if ({deserializationMethodName} !== null) {{");
                 writer.WriteLine($"{ResultVarName}->{property.Setter!.Name.ToFirstCharacterLowerCase()}({deserializationMethodName});");
                 writer.DecreaseIndent();
@@ -973,9 +981,16 @@ private void WriteFactoryMethodBodyForUnionModelForUnDiscriminatedTypes(CodeMeth
             .ToArray();
         foreach (var property in otherProps)
         {
-            var serializationMethodName = $"{ParseNodeVarName}->{GetDeserializationMethodName(property.Type, currentElement)}";
+            var methodName = GetDeserializationMethodName(property.Type, currentElement);
+            var serializationMethodName = $"{ParseNodeVarName}->{methodName.Item2}";
+            const string finalValueName = "$finalValue";
             writer.StartBlock($"{(includeElse ? "} else " : string.Empty)}if ({serializationMethodName} !== null) {{");
-            writer.WriteLine($"{ResultVarName}->{property.Setter!.Name.ToFirstCharacterLowerCase()}({serializationMethodName});");
+            if (!string.IsNullOrEmpty(methodName.Item1))
+            {
+                writer.WriteLine($"/** @var array<{methodName.Item1}> {finalValueName} */");
+            }
+            writer.WriteLine($"{finalValueName} = {serializationMethodName};");
+            writer.WriteLine($"{ResultVarName}->{property.Setter!.Name.ToFirstCharacterLowerCase()}({finalValueName});");
             writer.DecreaseIndent();
             if (!includeElse)
                 includeElse = true;

diff --git a/tests/Kiota.Builder.Tests/Writers/Php/CodeMethodWriterTests.cs b/tests/Kiota.Builder.Tests/Writers/Php/CodeMethodWriterTests.cs
@@ -1474,9 +1474,11 @@ public void WritesModelFactoryBodyForUnionModels()
         Assert.Contains("if ('#kiota.complexType1' === $mappingValue) {", result);
         Assert.Contains("$result->setComplexType1Value(new ComplexType1())", result);
         Assert.Contains("if ($parseNode->getStringValue() !== null) {", result);
-        Assert.Contains("$result->setStringValue($parseNode->getStringValue())", result);
+        Assert.Contains("$finalValue = $parseNode->getStringValue()", result);
+        Assert.Contains("$result->setStringValue($finalValue)", result);
         Assert.Contains("else if ($parseNode->getCollectionOfObjectValues([ComplexType2::class, 'createFromDiscriminatorValue']) !== null) {", result);
-        Assert.Contains("$result->setComplexType2Value($parseNode->getCollectionOfObjectValues([ComplexType2::class, 'createFromDiscriminatorValue']))", result);
+        Assert.Contains("$finalValue = $parseNode->getCollectionOfObjectValues([ComplexType2::class, 'createFromDiscriminatorValue'])", result);
+        Assert.Contains("$result->setComplexType2Value($finalValue)", result);
         Assert.Contains("return $result", result);
         Assert.DoesNotContain("return new UnionTypeWrapper()", result);
         AssertExtensions.Before("$parseNode->getStringValue()", "getCollectionOfObjectValues([ComplexType2::class, 'createFromDiscriminatorValue'])", result);