Merge branch 'main' into 5635-php---doc-comments-with--lead-to-invali…

…d-code
microsoft · Nov 13, 2024 · 9c17019 · 9c17019
2 parents d472aeb + d32a432
commit 9c17019
Show file tree

Hide file tree

Showing 11 changed files with 106 additions and 38 deletions.
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
 
 ### Changed
 
+- Fixed python generation in scenarios with opening/closing tags for code comments. [#5636](https://github.com/microsoft/kiota/issues/5636)
 - Fixed Python error when a class inherits from a base class and implements an interface. [5637](https://github.com/microsoft/kiota/issues/5637)
 - Fix anyOf/oneOf generation in TypeScript. [5353](https://github.com/microsoft/kiota/issues/5353)
 - Fixed invalid code in Php caused by "*/*/" in property description. [5635](https://github.com/microsoft/kiota/issues/5635)

diff --git a/it/config.json b/it/config.json
@@ -36,10 +36,6 @@
       {
         "Language": "ruby",
         "Rationale": "https://github.com/microsoft/kiota/issues/1816"
-      },
-      {
-        "Language": "python",
-        "Rationale": "https://github.com/microsoft/kiota/issues/5636"
       }
     ],
     "ExcludePatterns": [
@@ -243,10 +239,6 @@
       {
         "Language": "ruby",
         "Rationale": "https://github.com/microsoft/kiota/issues/2484"
-      },
-      {
-        "Language": "python",
-        "Rationale": "https://github.com/microsoft/kiota/issues/5744"
       }
     ]
   },

diff --git a/src/Kiota.Builder/Refiners/CommonLanguageRefiner.cs b/src/Kiota.Builder/Refiners/CommonLanguageRefiner.cs
@@ -1475,15 +1475,28 @@ protected static void RemoveRequestConfigurationClassesCommonProperties(CodeElem
 
         CrawlTree(currentElement, x => RemoveRequestConfigurationClassesCommonProperties(x, baseTypeUsing));
     }
-    protected static void RemoveUntypedNodePropertyValues(CodeElement currentElement)
+    protected static void RemoveUntypedNodeTypeValues(CodeElement currentElement)
     {
-        if (currentElement is CodeProperty currentProperty
-            && currentElement.Parent is CodeClass parentClass
-            && currentProperty.Type.Name.Equals(KiotaBuilder.UntypedNodeName, StringComparison.OrdinalIgnoreCase))
+        switch (currentElement)
         {
-            parentClass.RemoveChildElement(currentProperty);
+            case CodeProperty currentProperty when currentElement.Parent is CodeClass parentClass && currentProperty.Type.Name.Equals(KiotaBuilder.UntypedNodeName, StringComparison.OrdinalIgnoreCase):
+                parentClass.RemoveChildElement(currentProperty);
+                break;
+            case CodeMethod currentMethod when currentMethod.IsOfKind(CodeMethodKind.RequestExecutor):
+                if (currentMethod.ReturnType.Name.Equals(KiotaBuilder.UntypedNodeName, StringComparison.OrdinalIgnoreCase))
+                {
+                    currentMethod.ReturnType = new CodeType { Name = "binary", IsExternal = true };
+                }
+                if (currentMethod.Parameters.Where(x => x.Kind is CodeParameterKind.RequestBody && x.Type.Name.Equals(KiotaBuilder.UntypedNodeName, StringComparison.OrdinalIgnoreCase)).ToList() is { Count: > 0 } parameters)
+                {
+                    foreach (var parameter in parameters)
+                    {
+                        parameter.Type = new CodeType { Name = "binary", IsExternal = true };
+                    }
+                }
+                break;
         }
-        CrawlTree(currentElement, RemoveUntypedNodePropertyValues);
+        CrawlTree(currentElement, RemoveUntypedNodeTypeValues);
     }
     protected static void RemoveRequestConfigurationClasses(CodeElement currentElement, CodeUsing? configurationParameterTypeUsing = null, CodeType? defaultValueForGenericTypeParam = null, bool keepRequestConfigurationClass = false, bool addDeprecation = false, CodeUsing? usingForDefaultGenericParameter = null)
     {

diff --git a/src/Kiota.Builder/Refiners/PhpRefiner.cs b/src/Kiota.Builder/Refiners/PhpRefiner.cs
@@ -57,7 +57,7 @@ public override Task RefineAsync(CodeNamespace generatedCode, CancellationToken
             AddParsableImplementsForModelClasses(generatedCode, "Parsable");
             AddRequestConfigurationConstructors(generatedCode);
             AddDefaultImports(generatedCode, defaultUsingEvaluators);
-            RemoveUntypedNodePropertyValues(generatedCode);
+            RemoveUntypedNodeTypeValues(generatedCode);
             AddCollectionValidationUtilImportToModels(generatedCode);
             cancellationToken.ThrowIfCancellationRequested();
             AddGetterAndSetterMethods(generatedCode,

diff --git a/src/Kiota.Builder/Refiners/PythonRefiner.cs b/src/Kiota.Builder/Refiners/PythonRefiner.cs
@@ -26,7 +26,7 @@ public override Task RefineAsync(CodeNamespace generatedCode, CancellationToken
             );
             CorrectCommonNames(generatedCode);
             RemoveMethodByKind(generatedCode, CodeMethodKind.RawUrlConstructor);
-            RemoveUntypedNodePropertyValues(generatedCode);
+            RemoveUntypedNodeTypeValues(generatedCode);
             DisableActionOf(generatedCode,
             CodeParameterKind.RequestConfiguration);
             MoveRequestBuilderPropertiesToBaseType(generatedCode,

diff --git a/src/Kiota.Builder/Refiners/RubyRefiner.cs b/src/Kiota.Builder/Refiners/RubyRefiner.cs
@@ -63,7 +63,7 @@ public override Task RefineAsync(CodeNamespace generatedCode, CancellationToken
             cancellationToken.ThrowIfCancellationRequested();
             AddParsableImplementsForModelClasses(generatedCode, "MicrosoftKiotaAbstractions::Parsable");
             AddDefaultImports(generatedCode, defaultUsingEvaluators);
-            RemoveUntypedNodePropertyValues(generatedCode);
+            RemoveUntypedNodeTypeValues(generatedCode);
             CorrectCoreType(generatedCode, CorrectMethodType, CorrectPropertyType, CorrectImplements);
             cancellationToken.ThrowIfCancellationRequested();
             ReplacePropertyNames(generatedCode,

diff --git a/src/Kiota.Builder/Refiners/SwiftRefiner.cs b/src/Kiota.Builder/Refiners/SwiftRefiner.cs
@@ -41,10 +41,10 @@ public override Task RefineAsync(CodeNamespace generatedCode, CancellationToken
                 true,
                 false,
                 true);
+            RemoveUntypedNodeTypeValues(generatedCode);
             AddDefaultImports(
                 generatedCode,
                 defaultUsingEvaluators);
-            RemoveUntypedNodePropertyValues(generatedCode);
             cancellationToken.ThrowIfCancellationRequested();
             CorrectCoreType(
                 generatedCode,

diff --git a/src/Kiota.Builder/Writers/Python/CodeMethodWriter.cs b/src/Kiota.Builder/Writers/Python/CodeMethodWriter.cs
@@ -384,10 +384,15 @@ private void WriteDirectAccessProperties(CodeClass parentClass, LanguageWriter w
         {
             var returnType = conventions.GetTypeString(propWithDefault.Type, propWithDefault, true, writer);
             var defaultValue = propWithDefault.DefaultValue;
-            if (propWithDefault.Type is CodeType propertyType && propertyType.TypeDefinition is CodeEnum enumDefinition)
+            switch (propWithDefault.Type)
             {
-                _codeUsingWriter.WriteDeferredImport(parentClass, enumDefinition.Name, writer);
-                defaultValue = $"{enumDefinition.Name}({defaultValue})";
+                case CodeType { TypeDefinition: CodeEnum enumDefinition }:
+                    _codeUsingWriter.WriteDeferredImport(parentClass, enumDefinition.Name, writer);
+                    defaultValue = $"{enumDefinition.Name}({defaultValue})";
+                    break;
+                case CodeType propType when propType.Name.Equals("boolean", StringComparison.OrdinalIgnoreCase):
+                    defaultValue = defaultValue.TrimQuotes().ToFirstCharacterUpperCase();// python booleans start in uppercase
+                    break;
             }
             conventions.WriteInLineDescription(propWithDefault, writer);
             if (parentClass.IsOfKind(CodeClassKind.Model))
@@ -412,10 +417,15 @@ private void WriteSetterAccessProperties(CodeClass parentClass, LanguageWriter w
                                         .ThenBy(static x => x.Name))
         {
             var defaultValue = propWithDefault.DefaultValue;
-            if (propWithDefault.Type is CodeType propertyType && propertyType.TypeDefinition is CodeEnum enumDefinition)
+            switch (propWithDefault.Type)
             {
-                _codeUsingWriter.WriteDeferredImport(parentClass, enumDefinition.Name, writer);
-                defaultValue = $"{enumDefinition.Name}({defaultValue})";
+                case CodeType { TypeDefinition: CodeEnum enumDefinition }:
+                    _codeUsingWriter.WriteDeferredImport(parentClass, enumDefinition.Name, writer);
+                    defaultValue = $"{enumDefinition.Name}({defaultValue})";
+                    break;
+                case CodeType propType when propType.Name.Equals("boolean", StringComparison.OrdinalIgnoreCase):
+                    defaultValue = defaultValue.TrimQuotes().ToFirstCharacterUpperCase();// python booleans start in uppercase
+                    break;
             }
             var returnType = conventions.GetTypeString(propWithDefault.Type, propWithDefault, true, writer);
             conventions.WriteInLineDescription(propWithDefault, writer);
@@ -583,7 +593,7 @@ private void WriteRequestExecutorBody(CodeMethod codeElement, RequestParams requ
         var isStream = conventions.StreamTypeName.Equals(returnType, StringComparison.OrdinalIgnoreCase);
         var returnTypeWithoutCollectionSymbol = GetReturnTypeWithoutCollectionSymbol(codeElement, returnType);
         var genericTypeForSendMethod = GetSendRequestMethodName(isVoid, isStream, codeElement.ReturnType.IsCollection, returnTypeWithoutCollectionSymbol, isEnum);
-        var newFactoryParameter = GetTypeFactory(isVoid, isStream, isEnum, returnTypeWithoutCollectionSymbol);
+        var newFactoryParameter = GetTypeFactory(isVoid, isStream, isEnum, returnTypeWithoutCollectionSymbol, codeElement.ReturnType.IsCollection);
         var errorMappingVarName = NoneKeyword;
         if (codeElement.ErrorMappings.Any())
         {
@@ -809,11 +819,11 @@ private string GetSerializationMethodName(CodeTypeBase propType)
             _ => "write_object_value",
         };
     }
-    internal string GetTypeFactory(bool isVoid, bool isStream, bool isEnum, string returnType)
+    internal string GetTypeFactory(bool isVoid, bool isStream, bool isEnum, string returnType, bool isCollection)
     {
         if (isVoid) return string.Empty;
         if (isStream || isEnum) return $" \"{returnType}\",";
-        if (conventions.IsPrimitiveType(returnType)) return $" {returnType},";
+        if (conventions.IsPrimitiveType(returnType)) return isCollection ? $" {returnType}," : $" \"{returnType}\",";
         return $" {returnType},";
     }
     private string GetSendRequestMethodName(bool isVoid, bool isStream, bool isCollection, string returnType,

diff --git a/src/Kiota.Builder/Writers/Python/PythonConventionService.cs b/src/Kiota.Builder/Writers/Python/PythonConventionService.cs
@@ -95,7 +95,7 @@ public override string GetTypeString(CodeTypeBase code, CodeElement targetElemen
         throw new InvalidOperationException($"type of type {code.GetType()} is unknown");
     }
 #pragma warning restore CA1822 // Method should be static
-    internal static string RemoveInvalidDescriptionCharacters(string originalDescription) => originalDescription.Replace("\\", "/", StringComparison.OrdinalIgnoreCase);
+    internal static string RemoveInvalidDescriptionCharacters(string originalDescription) => originalDescription.Replace("\\", "/", StringComparison.OrdinalIgnoreCase).Replace("\"\"\"", "\\\"\\\"\\\"", StringComparison.OrdinalIgnoreCase);
     public override string TranslateType(CodeType type)
     {
         ArgumentNullException.ThrowIfNull(type);
@@ -130,7 +130,7 @@ public bool IsPrimitiveType(string typeName)
     {
         return typeName switch
         {
-            "int" or "float" or "str" or "bool" or "None" => true,
+            "int" or "float" or "str" or "bool" or "None" or "datetime.datetime" or "datetime.timedelta" or "datetime.date" or "datetime.time" => true,
             _ => false,
         };
     }

diff --git a/tests/Kiota.Builder.Tests/Refiners/PythonLanguageRefinerTests.cs b/tests/Kiota.Builder.Tests/Refiners/PythonLanguageRefinerTests.cs
@@ -584,5 +584,34 @@ public async Task AddsPropertiesAndMethodTypesImportsPythonAsync()
         Assert.Single(requestBuilder.Methods, x => x.IsOfKind(CodeMethodKind.RequestExecutor));
         Assert.DoesNotContain("QueryParameters", declaration.Usings.Select(x => x.Name));
     }
+    [Fact]
+    public async Task ReplacesUntypedNodeInMethodParameterAndReturnTypeAsync()
+    {
+        var requestBuilderClass = root.AddClass(new CodeClass() { Name = "NodeRequestBuilder" }).First();
+        var method = new CodeMethod
+        {
+            Name = "getAsync",
+            ReturnType = new CodeType()
+            {
+                Name = KiotaBuilder.UntypedNodeName,//Returns untyped node
+                IsExternal = true
+            },
+            Kind = CodeMethodKind.RequestExecutor
+        };
+        method.AddParameter(new CodeParameter()
+        {
+            Name = "jsonData",
+            Type = new CodeType()
+            {
+                Name = KiotaBuilder.UntypedNodeName, //Has untyped node parameter
+                IsExternal = true
+            },
+            Kind = CodeParameterKind.RequestBody
+        });
+        requestBuilderClass.AddMethod(method);
+        await ILanguageRefiner.RefineAsync(new GenerationConfiguration { Language = GenerationLanguage.Python }, root);
+        Assert.Equal("bytes", method.Parameters.First().Type.Name);// type is renamed to use the stream type
+        Assert.Equal("bytes", method.ReturnType.Name);// return type is renamed to use the stream type
+    }
     #endregion
 }
diff --git a/tests/Kiota.Builder.Tests/Writers/Python/CodeMethodWriterTests.cs b/tests/Kiota.Builder.Tests/Writers/Python/CodeMethodWriterTests.cs
@@ -820,12 +820,13 @@ public void WritesUnionDeSerializerBody()
         Assert.Contains("return {}", result);
     }
     [Theory]
-    [InlineData(true, false, false, "string", "")]
-    [InlineData(false, true, false, "Stream", " \"Stream\",")]
-    [InlineData(false, false, true, "SomeEnum", " \"SomeEnum\",")]
-    [InlineData(false, false, false, "int", " int,")]
-    [InlineData(false, false, false, "CustomType", " CustomType,")]
-    public void GetTypeFactory_ReturnsCorrectString(bool isVoid, bool isStream, bool isEnum, string returnType, string expected)
+    [InlineData(true, false, false, false, "string", "")]
+    [InlineData(false, true, false, false, "Stream", " \"Stream\",")]
+    [InlineData(false, false, true, false, "SomeEnum", " \"SomeEnum\",")]
+    [InlineData(false, false, false, true, "int", " int,")]
+    [InlineData(false, false, false, false, "int", " \"int\",")]
+    [InlineData(false, false, false, false, "CustomType", " CustomType,")]
+    public void GetTypeFactory_ReturnsCorrectString(bool isVoid, bool isStream, bool isEnum, bool isCollection, string returnType, string expected)
     {
         var mockConventionService = new Mock<PythonConventionService>();
 
@@ -835,9 +836,7 @@ public void GetTypeFactory_ReturnsCorrectString(bool isVoid, bool isStream, bool
             false // usesBackingStore
         );
 
-        var result = codeMethodWriter.GetTypeFactory(isVoid, isStream, isEnum, returnType);
-
-
+        var result = codeMethodWriter.GetTypeFactory(isVoid, isStream, isEnum, returnType, isCollection);
         Assert.Equal(expected, result);
     }
     [Fact]
@@ -1592,6 +1591,30 @@ public void WritesConstructor()
         Assert.DoesNotContain("get_path_parameters(", result);
     }
     [Fact]
+    public void EscapesCommentCharactersInDescription()
+    {
+        setup();
+        method.Kind = CodeMethodKind.Constructor;
+        method.IsAsync = false;
+        parentClass.Kind = CodeClassKind.Custom;
+        parentClass.AddProperty(new CodeProperty
+        {
+            Name = "prop_without_default_value",
+            Kind = CodePropertyKind.Custom,
+            Documentation = new()
+            {
+                DescriptionTemplate = "This property has a description with comments \"\"\".",
+            },
+            Type = new CodeType
+            {
+                Name = "string"
+            }
+        });
+        writer.Write(method);
+        var result = tw.ToString();
+        Assert.Contains("This property has a description with comments \\\"\\\"\\\".", result);
+    }
+    [Fact]
     public void WritesWithUrl()
     {
         setup();