Merge pull request #1737 from microsoft/philip/php/failing-generation

PHP: Multi-definition trimming and property defaults
microsoft · Jul 15, 2022 · 5050e5d · 5050e5d
2 parents 69ce14e + 9b067ec
commit 5050e5d
Show file tree

Hide file tree

Showing 8 changed files with 39 additions and 21 deletions.
diff --git a/.vscode/launch.json b/.vscode/launch.json
@@ -84,9 +84,9 @@
                   "--language",
                   "php",
                   "-o",
-                  "${workspaceFolder}/samples/msgraph-mail/php/src/Generated/Mail",
+                  "${workspaceFolder}/samples/msgraph-mail/php/src/",
                   "-n",
-                  "GraphPhpV4"
+                  "Microsoft\\Graph"
             ],
             "cwd": "${workspaceFolder}/src/kiota",
             "console": "internalConsole",

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - Fixed a bug where collections types would generate invalid return types in CSharp.
 - Fixed a bug where a nullable entry in anyOf schemas would create unnecessary composed types.
+- Removed duplicate properties defined in base types from model serialization and deserialization methods and initialise property defaults in constructor. [#1737](https://github.com/microsoft/kiota/pull/1737)
 
 ## [0.3.0] - 2022-07-08
 

diff --git a/README.md b/README.md
@@ -24,7 +24,7 @@ The following table provides an overview of the languages supported by Kiota and
 | CSharp | [✔](https://github.com/microsoft/kiota/projects/5) | [✔](https://github.com/microsoft/kiota-abstractions-dotnet)     | [JSON](https://github.com/microsoft/kiota-serialization-json-dotnet), [TEXT](https://github.com/microsoft/kiota-serialization-text-dotnet)                             | [Anonymous](https://github.com/microsoft/kiota-abstractions-dotnet/blob/main/src/authentication/AnonymousAuthenticationProvider.cs), [Azure](https://github.com/microsoft/kiota-authentication-azure-dotnet) | [✔](https://github.com/microsoft/kiota-http-dotnet) | [link](https://microsoft.github.io/kiota/get-started/dotnet) |
 | Go | [✔](https://github.com/microsoft/kiota/projects/8) | [✔](https://github.com/microsoft/kiota-abstractions-go)         | [JSON](https://github.com/microsoft/kiota-serialization-json-go), [TEXT](https://github.com/microsoft/kiota-serialization-text-go)                                 | [Anonymous](https://github.com/microsoft/kiota-abstractions-go/blob/main/authentication/anonymous_authentication_provider.go), [Azure](https://github.com/microsoft/kiota-authentication-azure-go/) | [✔](https://github.com/microsoft/kiota-http-go/) | [link](https://microsoft.github.io/kiota/get-started/go) |
 | Java | [✔](https://github.com/microsoft/kiota/projects/7) | [✔](https://github.com/microsoft/kiota-java/tree/main/components/abstractions)       | [JSON](https://github.com/microsoft/kiota-java/tree/main/components/serialization/json), [TEXT](https://github.com/microsoft/kiota-java/tree/main/components/serialization/text)                               | [Anonymous](https://github.com/microsoft/kiota-java/blob/main/components/abstractions/src/main/java/com/microsoft/kiota/authentication/AnonymousAuthenticationProvider.java), [Azure](https://github.com/microsoft/kiota-java/tree/main/components/authentication/azure) | [✔](https://github.com/microsoft/kiota-java/tree/main/components/http/okHttp) | [link](https://microsoft.github.io/kiota/get-started/java) |
-| PHP | [✔](https://github.com/microsoft/kiota/projects/4) | [✔](https://github.com/microsoft/kiota-abstractions-php)          | [JSON](https://github.com/microsoft/kiota-serialization-json-php), [TEXT](https://github.com/microsoft/kiota-serialization-text-php)                                | [Anonymous](https://github.com/microsoft/kiota-abstractions-php/blob/main/src/Authentication/AnonymousAuthenticationProvider.php), [✔️ PHP League](https://github.com/microsoft/kiota-authentication-phpleague-php) | [✔](https://github.com/microsoft/kiota-http-guzzle-php) |  |
+| PHP | [✔](https://github.com/microsoft/kiota/projects/4) | [✔](https://github.com/microsoft/kiota-abstractions-php)          | [JSON](https://github.com/microsoft/kiota-serialization-json-php), [TEXT](https://github.com/microsoft/kiota-serialization-text-php)                                | [Anonymous](https://github.com/microsoft/kiota-abstractions-php/blob/main/src/Authentication/AnonymousAuthenticationProvider.php), [✔️ PHP League](https://github.com/microsoft/kiota-authentication-phpleague-php) | [✔](https://github.com/microsoft/kiota-http-guzzle-php) | [link](https://microsoft.github.io/kiota/get-started/php) |
 | Python | [▶](https://github.com/microsoft/kiota/projects/3) | [✔](./abstractions/python)  | [JSON](./serialization/python/json), [❌ TEXT](https://github.com/microsoft/kiota/issues/1406) | [Anonymous](./abstractions/python/kiota/abstractions/authentication/anonymous_authentication_provider.py), [Azure](./authentication/python/azure) | [✔](./http/python/requests) |  |
 | Ruby | [✔](https://github.com/microsoft/kiota/projects/6) | [✔](./abstractions/ruby)       | [JSON](./serialization/ruby/json/microsoft_kiota_serialization), [❌ TEXT](https://github.com/microsoft/kiota/issues/1049) | [Anonymous](./abstractions/ruby/microsoft_kiota_abstractions/lib/microsoft_kiota_abstractions/authentication/anonymous_authentication_provider.rb), [❌ Azure](https://github.com/microsoft/kiota/issues/421) | [✔](./http/ruby/nethttp/microsoft_kiota_nethttplibrary)| [link](https://microsoft.github.io/kiota/get-started/ruby)  |
 | TypeScript/JavaScript | [✔](https://github.com/microsoft/kiota/projects/2) | [✔](https://github.com/microsoft/kiota-typescript/tree/main/packages/abstractions) | [JSON](https://github.com/microsoft/kiota-typescript/tree/main/packages/serialization/json), [TEXT](https://github.com/microsoft/kiota-typescript/tree/main/packages/serialization/text)                         | [Anonymous](https://github.com/microsoft/kiota-typescript/blob/main/packages/abstractions/src/authentication/anonymousAuthenticationProvider.ts), [Azure](https://github.com/microsoft/kiota-typescript/tree/main/packages/authentication/azure) | [✔](https://github.com/microsoft/kiota-typescript/tree/main/packages/http/fetch) | [link](https://microsoft.github.io/kiota/get-started/typescript) |

diff --git a/src/Kiota.Builder/GenerationConfiguration.cs b/src/Kiota.Builder/GenerationConfiguration.cs
@@ -11,7 +11,6 @@ public class GenerationConfiguration {
         public string ModelsNamespaceName { get => $"{ClientNamespaceName}{NamespaceNameSeparator}models"; }
         public GenerationLanguage Language { get; set; } = GenerationLanguage.CSharp;
         public string ApiRootUrl { get; set; }
-        public string[] PropertiesPrefixToStrip { get; set; } = new string[] { "@odata."};
         public bool UsesBackingStore { get; set; }
         public HashSet<string> Serializers { get; set; } = new(StringComparer.OrdinalIgnoreCase){
             "Microsoft.Kiota.Serialization.Json.JsonSerializationWriterFactory",

diff --git a/src/Kiota.Builder/Writers/Php/CodeMethodWriter.cs b/src/Kiota.Builder/Writers/Php/CodeMethodWriter.cs
@@ -71,21 +71,22 @@ private static void WriteConstructorBody(CodeClass parentClass, CodeMethod curre
             if(inherits)
                 writer.WriteLine("parent::__construct();");
             foreach(var propWithDefault in parentClass.GetPropertiesOfKind(
-                                                                            CodePropertyKind.BackingStore,
-                                                                            CodePropertyKind.RequestBuilder,
-                                                                            CodePropertyKind.UrlTemplate,
-                                                                            CodePropertyKind.PathParameters)
-                                            .Where(x => !string.IsNullOrEmpty(x.DefaultValue))
-                                            .OrderByDescending(x => x.Kind)
-                                            .ThenBy(x => x.Name))
+                    CodePropertyKind.BackingStore,
+                    CodePropertyKind.RequestBuilder,
+                    CodePropertyKind.UrlTemplate,
+                    CodePropertyKind.PathParameters)
+                .Where(x => !string.IsNullOrEmpty(x.DefaultValue))
+                .OrderByDescending(x => x.Kind)
+                .ThenBy(x => x.Name))
             {
                 var isPathSegment = propWithDefault.IsOfKind(CodePropertyKind.PathParameters);
-                writer.WriteLine($"$this->{propWithDefault.NamePrefix}{propWithDefault.Name.ToFirstCharacterLowerCase()} = {(isPathSegment ? "[]" :propWithDefault.DefaultValue.ReplaceDoubleQuoteWithSingleQuote())};");
+                writer.WriteLine($"$this->{propWithDefault.Name.ToFirstCharacterLowerCase()} = {(isPathSegment ? "[]" :propWithDefault.DefaultValue.ReplaceDoubleQuoteWithSingleQuote())};");
             }
-            foreach(var propWithDefault in parentClass.GetPropertiesOfKind(CodePropertyKind.AdditionalData) //additional data and backing Store rely on accessors
+            foreach(var propWithDefault in parentClass.GetPropertiesOfKind(CodePropertyKind.AdditionalData, CodePropertyKind.Custom) //additional data and custom properties rely on accessors
                 .Where(x => !string.IsNullOrEmpty(x.DefaultValue))
                 .OrderBy(x => x.Name)) {
-                writer.WriteLine($"$this->{propWithDefault.Name.ToFirstCharacterLowerCase()} = {propWithDefault.DefaultValue};");
+                var setterName = propWithDefault.SetterFromCurrentOrBaseType?.Name.ToFirstCharacterLowerCase() ?? $"set{propWithDefault.SymbolName.ToFirstCharacterUpperCase()}";
+                writer.WriteLine($"$this->{setterName}({propWithDefault.DefaultValue.ReplaceDoubleQuoteWithSingleQuote()});");
             }
             if(currentMethod.IsOfKind(CodeMethodKind.Constructor, CodeMethodKind.ClientConstructor)) {
                 AssignPropertyFromParameter(parentClass, currentMethod, CodeParameterKind.RequestAdapter, CodePropertyKind.RequestAdapter, writer);
@@ -238,7 +239,7 @@ private void WriteSerializerBody(CodeMethod codeMethod, CodeClass parentClass, L
             if(inherits && implementsParsable)
                 writer.WriteLine($"parent::serialize({writerParameterName});");
             var customProperties = parentClass.GetPropertiesOfKind(CodePropertyKind.Custom);
-            foreach(var otherProp in customProperties) {
+            foreach(var otherProp in customProperties.Where(x => !x.ExistsInBaseType)) {
                 writer.WriteLine($"{writerParameterName}->{GetSerializationMethodName(otherProp.Type)}('{otherProp.SerializationName ?? otherProp.Name.ToFirstCharacterLowerCase()}', $this->{otherProp.Name.ToFirstCharacterLowerCase()});");
             }
             if(additionalDataProperty != null)
@@ -401,6 +402,7 @@ private void WriteDeserializerBody(CodeClass parentClass, LanguageWriter writer,
             if(fieldToSerialize.Any()) {
                 writer.IncreaseIndent();
                 fieldToSerialize
+                    .Where(x => !x.ExistsInBaseType)
                     .OrderBy(x => x.Name)
                     .Select(x => 
                         $"'{x.SerializationName ?? x.Name.ToFirstCharacterLowerCase()}' => function (ParseNode $n) use ({currentObjectName}) {{ {currentObjectName}->{x.Setter.Name.ToFirstCharacterLowerCase()}({GetDeserializationMethodName(x.Type, method)}); }},")

diff --git a/src/Kiota.Builder/Writers/Php/CodePropertyWriter.cs b/src/Kiota.Builder/Writers/Php/CodePropertyWriter.cs
@@ -39,7 +39,7 @@ private void WritePropertyDocComment(CodeProperty codeProperty, LanguageWriter w
             {
                 writer.WriteLine($"{conventions.DocCommentPrefix}@QueryParameter(\"{codeProperty.SerializationName}\")");
             }
-            writer.WriteLine($"{conventions.DocCommentPrefix}@var {typeString}{(codeProperty.Type.IsNullable ? "|null" : string.Empty)} ${codeProperty.Name} " +
+            writer.WriteLine($"{conventions.DocCommentPrefix}@var {typeString}{(codeProperty.Type.IsNullable ? "|null" : string.Empty)} ${codeProperty.Name.ToFirstCharacterLowerCase()} " +
                              $"{(hasDescription ? propertyDescription : string.Empty)}");
             writer.WriteLine(conventions.DocCommentEnd);
         }

diff --git a/tests/Kiota.Builder.Tests/Writers/Php/CodeMethodWriterTests.cs b/tests/Kiota.Builder.Tests/Writers/Php/CodeMethodWriterTests.cs
@@ -203,6 +203,7 @@ public void WriteRequestExecutor()
             new object[] { new CodeProperty { Name = "dateValue", Type = new CodeType { Name = "DateTime" }, Access = AccessModifier.Private}, "$writer->writeDateTimeValue('dateValue', $this->dateValue);" },
             new object[] { new CodeProperty { Name = "duration", Type = new CodeType { Name = "duration" }, Access = AccessModifier.Private}, "$writer->writeDateIntervalValue('duration', $this->duration);" },
             new object[] { new CodeProperty { Name = "stream", Type = new CodeType { Name = "binary" }, Access = AccessModifier.Private}, "$writer->writeBinaryContent('stream', $this->stream);" },
+            new object[] { new CodeProperty { Name = "definedInParent", Type = new CodeType { Name = "string"}, OriginalPropertyFromBaseType = new CodeProperty{}}, "$write->writeStringValue('definedInParent', $this->definedInParent);"}
         };
 
         [Theory]
@@ -233,7 +234,10 @@ public void WriteSerializer(CodeProperty property, string expected)
             _codeMethodWriter.WriteCodeElement(codeMethod, languageWriter);
             var result = stringWriter.ToString();
             Assert.Contains("public function serialize(SerializationWriter $writer)", result);
-            Assert.Contains(expected, stringWriter.ToString());
+            if (property.ExistsInBaseType)
+                Assert.DoesNotContain(expected, result);
+            else
+                Assert.Contains(expected, stringWriter.ToString());
         }
 
         [Fact]
@@ -479,6 +483,7 @@ public void WriteIndexerBody()
             new object[] { new CodeProperty { Name = "years", Type = new CodeType { Name = "int", CollectionKind = CodeTypeBase.CodeTypeCollectionKind.Array }, Access = AccessModifier.Private},
                 "'years' => function (ParseNode $n) use ($o) { $o->setYears($n->getCollectionOfPrimitiveValues())"
             },
+            new object[] { new CodeProperty{ Name = "definedInParent", Type = new CodeType { Name = "string"}, OriginalPropertyFromBaseType = new CodeProperty() }, "'definedInParent' => function (ParseNode $n) use ($o) { $o->setDefinedInParent($n->getStringValue())"}
         };
         private static CodeClass GetParentClassInStaticContext()
         {
@@ -509,7 +514,10 @@ public void WriteDeserializer(CodeProperty property, string expected)
             parentClass.AddProperty(property);
             _refiner.Refine(parentClass.Parent as CodeNamespace);
             languageWriter.Write(deserializerMethod);
-            Assert.Contains(expected, stringWriter.ToString());
+            if (property.ExistsInBaseType)
+                Assert.DoesNotContain(expected, stringWriter.ToString());
+            else
+                Assert.Contains(expected, stringWriter.ToString());
         }
 
         [Fact]
@@ -571,13 +579,21 @@ public void WriteConstructorBody()
                 ReturnType = new CodeType() {Name = "void"},
                 Kind = CodeMethodKind.Constructor
             };
-            var closingClass = parentClass;
             parentClass.AddMethod(constructor);
-
+
+            var propWithDefaultValue = new CodeProperty()
+            {
+                Name = "type",
+                DefaultValue = "\"#microsoft.graph.entity\"",
+                Kind = CodePropertyKind.Custom
+            };
+            parentClass.AddProperty(propWithDefaultValue);
+
             _codeMethodWriter.WriteCodeElement(constructor, languageWriter);
             var result = stringWriter.ToString();
 
             Assert.Contains("public function __construct", result);
+            Assert.Contains("$this->setType('#microsoft.graph.entity')", result);
         }
 
         [Fact]

diff --git a/tests/Kiota.Builder.Tests/Writers/Php/CodePropertyWriterTests.cs b/tests/Kiota.Builder.Tests/Writers/Php/CodePropertyWriterTests.cs
@@ -34,7 +34,7 @@ public void WritePropertyDocs()
         {
             var property = new CodeProperty()
             {
-                Name = "email",
+                Name = "Email",
                 Access = AccessModifier.Private,
                 Type = new CodeType()
                 {