Merge pull request #2049 from microsoft/php/fix-generation-issue-v2

Php/fix generation issues

CHANGELOG.md

@@ -14,7 +14,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Added support for multi-valued headers in CSharp, TypeScript, Go, and Java. [#2032](https://github.com/microsoft/kiota/issues/2032)
 
 ### Changed
-
+- Fixed issue with wrong imports for PHP. [#2049](https://github.com/microsoft/kiota/pull/2049)
+- Fix issue where discriminator types were never getting imported for PHP. [#2049](https://github.com/microsoft/kiota/pull/2049)
+- Fix issue where class aliasing was never working as expected for PHP. [#2049](https://github.com/microsoft/kiota/pull/2049)
 - Fixed colliding imports for factory methods in TypeScript. [#2009](https://github.com/microsoft/kiota/issues/2009)
 - Switched to lazy loading module imports in Python. [#2007](https://github.com/microsoft/kiota/issues/2007)
 - Caters for type names being used from System namespace in CSharp generation [#2021](https://github.com/microsoft/kiota/issues/2021)

src/Kiota.Builder/PathSegmenters/PHPPathSegmenter.cs

@@ -1,4 +1,6 @@
-using Kiota.Builder.CodeDOM;
+using System;
+using System.Linq;
+using Kiota.Builder.CodeDOM;
 using Kiota.Builder.Extensions;
 
 namespace Kiota.Builder.PathSegmenters
@@ -8,6 +10,7 @@ public class PhpPathSegmenter : CommonPathSegmenter
         public PhpPathSegmenter(string rootPath, string clientNamespaceName) : base(rootPath, clientNamespaceName) { }
         public override string FileSuffix => ".php";
         public override string NormalizeNamespaceSegment(string segmentName) => segmentName.ToFirstCharacterUpperCase();
+        protected static new string GetLastFileNameSegment(CodeElement currentElement) => currentElement.Name.Split(new char[] {'.', '\\'}, StringSplitOptions.RemoveEmptyEntries).Last();
         public override string NormalizeFileName(CodeElement currentElement) => GetLastFileNameSegment(currentElement).ToFirstCharacterUpperCase();
     }
 }

src/Kiota.Builder/Refiners/CommonLanguageRefiner.cs

@@ -710,35 +710,37 @@ protected static void AddParentClassToErrorClasses(CodeElement currentElement, s
     protected static void AddDiscriminatorMappingsUsingsToParentClasses(CodeElement currentElement, string parseNodeInterfaceName, bool addFactoryMethodImport = false, bool addUsings = true) {
         if(currentElement is CodeMethod currentMethod &&
             currentMethod.Parent is CodeClass parentClass &&
-            parentClass.StartBlock is ClassDeclaration declaration) {
+            parentClass.StartBlock is ClassDeclaration declaration) { 
+            var parentClassNamespace = parentClass.GetImmediateParentOfType<CodeNamespace>();
                 if(currentMethod.IsOfKind(CodeMethodKind.Factory) &&
                     (parentClass.DiscriminatorInformation?.HasBasicDiscriminatorInformation ?? false)) {
-                        if(addUsings)
-                            declaration.AddUsings(parentClass.DiscriminatorInformation.DiscriminatorMappings
-                                .Select(static x => x.Value)
-                                .OfType<CodeType>()
-                                .Where(static x => x.TypeDefinition != null)
-                                .Select(x => new CodeUsing {
-                                    Name = x.TypeDefinition.GetImmediateParentOfType<CodeNamespace>().Name,
-                                    Declaration = new CodeType {
-                                        Name = x.TypeDefinition.Name,
-                                        TypeDefinition = x.TypeDefinition,
-                                    },
+                    if(addUsings)
+                        declaration.AddUsings(parentClass.DiscriminatorInformation.DiscriminatorMappings
+                            .Select(static x => x.Value)
+                            .OfType<CodeType>()
+                            .Where(static x => x.TypeDefinition != null)
+                            .Where(x => x.TypeDefinition.GetImmediateParentOfType<CodeNamespace>() != parentClassNamespace)
+                            .Select(x => new CodeUsing {
+                                Name = x.TypeDefinition.GetImmediateParentOfType<CodeNamespace>().Name,
+                                Declaration = new CodeType {
+                                    Name = x.TypeDefinition.Name,
+                                    TypeDefinition = x.TypeDefinition,
+                                },
                             }).ToArray());
-                        if (currentMethod.Parameters.OfKind(CodeParameterKind.ParseNode, out var parameter))
-                            parameter.Type.Name = parseNodeInterfaceName;
+                    if (currentMethod.Parameters.OfKind(CodeParameterKind.ParseNode, out var parameter))
+                        parameter.Type.Name = parseNodeInterfaceName;
                 } else if (addFactoryMethodImport &&
                     currentMethod.IsOfKind(CodeMethodKind.RequestExecutor) &&
                     currentMethod.ReturnType is CodeType type &&
                     type.TypeDefinition is CodeClass modelClass &&
                     modelClass.GetChildElements(true).OfType<CodeMethod>().FirstOrDefault(static x => x.IsOfKind(CodeMethodKind.Factory)) is CodeMethod factoryMethod) {
-                        declaration.AddUsings(new CodeUsing {
-                            Name = modelClass.GetImmediateParentOfType<CodeNamespace>().Name,
-                            Declaration = new CodeType {
-                                Name = factoryMethod.Name,
-                                TypeDefinition = factoryMethod,
-                            }
-                        });
+                    declaration.AddUsings(new CodeUsing {
+                        Name = modelClass.GetImmediateParentOfType<CodeNamespace>().Name,
+                        Declaration = new CodeType {
+                            Name = factoryMethod.Name,
+                            TypeDefinition = factoryMethod,
+                        }
+                    });
                 }
         }
         CrawlTree(currentElement, x => AddDiscriminatorMappingsUsingsToParentClasses(x, parseNodeInterfaceName, addFactoryMethodImport, addUsings));

src/Kiota.Builder/Refiners/PhpRefiner.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Linq;
 using System.Threading;
@@ -18,6 +19,7 @@ public override Task Refine(CodeNamespace generatedCode, CancellationToken cance
     {
         return Task.Run(() => {
             cancellationToken.ThrowIfCancellationRequested();
+            // Imports should be done before adding getters and setters since AddGetterAndSetterMethods can remove properties from classes when backing store is enabled
             ReplaceReservedNames(generatedCode, new PhpReservedNamesProvider(), reservedWord => $"Escaped{reservedWord.ToFirstCharacterUpperCase()}");
             AddParentClassToErrorClasses(
                 generatedCode,
@@ -30,17 +32,17 @@ public override Task Refine(CodeNamespace generatedCode, CancellationToken cance
             ConvertUnionTypesToWrapper(generatedCode, 
                 _configuration.UsesBackingStore,
                 false);
-            AddDiscriminatorMappingsUsingsToParentClasses(
-                generatedCode,
-                "ParseNode",
-                addUsings: false
-            );
             cancellationToken.ThrowIfCancellationRequested();
             CorrectParameterType(generatedCode);
             MakeModelPropertiesNullable(generatedCode);
             ReplaceIndexersByMethodsWithParameter(generatedCode, generatedCode, false, "ById");
             cancellationToken.ThrowIfCancellationRequested();
-            AddPropertiesAndMethodTypesImports(generatedCode, true, false, true);
+            MoveClassesWithNamespaceNamesUnderNamespace(generatedCode);
+            AddDiscriminatorMappingsUsingsToParentClasses(
+                generatedCode,
+                "ParseNode",
+                addUsings: true
+            );
             var defaultConfiguration = new GenerationConfiguration();
             ReplaceDefaultSerializationModules(generatedCode,
                 defaultConfiguration.Serializers,
@@ -55,15 +57,13 @@ public override Task Refine(CodeNamespace generatedCode, CancellationToken cance
                     "Microsoft\\Kiota\\Serialization\\Text\\TextParseNodeFactory"}
             );
             cancellationToken.ThrowIfCancellationRequested();
-            AliasUsingWithSameSymbol(generatedCode);
             AddSerializationModulesImport(generatedCode, new []{"Microsoft\\Kiota\\Abstractions\\ApiClientBuilder"}, null, '\\');
             CorrectCoreType(generatedCode, CorrectMethodType, CorrectPropertyType, CorrectImplements);
             cancellationToken.ThrowIfCancellationRequested();
             AddInnerClasses(generatedCode,
                 true,
                 string.Empty,
                 true);
-            // Imports should be done before adding getters and setters since AddGetterAndSetterMethods can remove properties from classes when backing store is enabled
             AddDefaultImports(generatedCode, defaultUsingEvaluators);
             AddGetterAndSetterMethods(generatedCode,
                 new() {
@@ -77,10 +77,11 @@ public override Task Refine(CodeNamespace generatedCode, CancellationToken cance
                 "set");
             AddParsableImplementsForModelClasses(generatedCode, "Parsable");
             ReplaceBinaryByNativeType(generatedCode, "StreamInterface", "Psr\\Http\\Message", true, _configuration.UsesBackingStore);
-            cancellationToken.ThrowIfCancellationRequested();
-            MoveClassesWithNamespaceNamesUnderNamespace(generatedCode);
             CorrectCoreTypesForBackingStore(generatedCode, "BackingStoreFactorySingleton::getInstance()->createBackingStore()");
             CorrectBackingStoreSetterParam(generatedCode);
+            AddPropertiesAndMethodTypesImports(generatedCode, true, false, true);
+            AliasUsingWithSameSymbol(generatedCode);
+            cancellationToken.ThrowIfCancellationRequested();
         }, cancellationToken);
     }
     private static readonly Dictionary<string, (string, CodeUsing)> DateTypesReplacements = new(StringComparer.OrdinalIgnoreCase)
@@ -137,7 +138,7 @@ public override Task Refine(CodeNamespace generatedCode, CancellationToken cance
             "Microsoft\\Kiota\\Abstractions", "HttpMethod", "RequestInformation", "RequestOption"),
         new (x => x is CodeMethod method && method.IsOfKind(CodeMethodKind.RequestExecutor),
             "Microsoft\\Kiota\\Abstractions", "ResponseHandler"),
-        new (x => x is CodeClass @class && @class.IsOfKind(CodeClassKind.Model) && @class.Properties.Any(x => x.IsOfKind(CodePropertyKind.AdditionalData)),
+        new (static x => x is CodeClass @class && @class.IsOfKind(CodeClassKind.Model) && @class.Properties.Any(static y => y.IsOfKind(CodePropertyKind.AdditionalData)),
             "Microsoft\\Kiota\\Abstractions\\Serialization", "AdditionalDataHolder"),
         new (x => x is CodeMethod method && method.IsOfKind(CodeMethodKind.Serializer),
             "Microsoft\\Kiota\\Abstractions\\Serialization", "SerializationWriter"),
@@ -155,8 +156,8 @@ public override Task Refine(CodeNamespace generatedCode, CancellationToken cance
         new (x => x is CodeMethod method && method.IsOfKind(CodeMethodKind.RequestExecutor), "Http\\Promise", "Promise", "RejectedPromise"),
         new (x => x is CodeMethod method && method.IsOfKind(CodeMethodKind.RequestExecutor), "", "Exception"),
         new (x => x is CodeEnum, "Microsoft\\Kiota\\Abstractions\\", "Enum"),
-        new(x => x is CodeProperty {Type.Name: {}} property && property.Type.Name.Equals("DateTime", StringComparison.OrdinalIgnoreCase), "", "DateTime"),
-        new(x => x is CodeProperty {Type.Name: {}} property && property.Type.Name.Equals("DateTimeOffset", StringComparison.OrdinalIgnoreCase), "", "DateTime"),
+        new(static x => x is CodeProperty {Type.Name: {}} property && property.Type.Name.Equals("DateTime", StringComparison.OrdinalIgnoreCase), "", "\\DateTime"),
+        new(static x => x is CodeProperty {Type.Name: {}} property && property.Type.Name.Equals("DateTimeOffset", StringComparison.OrdinalIgnoreCase), "", "\\DateTime"),
         new(x => x is CodeMethod method && method.IsOfKind(CodeMethodKind.ClientConstructor), "Microsoft\\Kiota\\Abstractions", "ApiClientBuilder"),
         new(x => x is CodeProperty property && property.IsOfKind(CodePropertyKind.QueryParameter) && !string.IsNullOrEmpty(property.SerializationName), "Microsoft\\Kiota\\Abstractions", "QueryParameter"),
         new(x => x is CodeClass codeClass && codeClass.IsOfKind(CodeClassKind.RequestConfiguration), "Microsoft\\Kiota\\Abstractions", "RequestOption")
@@ -240,15 +241,17 @@ private static void AliasUsingWithSameSymbol(CodeElement currentElement) {
                     .Where(x => x.Declaration
                         .Name
                         .Equals(currentClass.Name, StringComparison.OrdinalIgnoreCase)));
-            foreach (var usingElement in duplicatedSymbolsUsings)
+            var symbolsUsing = duplicatedSymbolsUsings as CodeUsing[] ?? duplicatedSymbolsUsings.ToArray();
+            foreach (var usingElement in symbolsUsing)
             {
                 var declaration = usingElement.Declaration.TypeDefinition?.Name;
                 if (string.IsNullOrEmpty(declaration)) continue;
-                var replacement = string.Join(string.Empty, usingElement.Declaration.TypeDefinition.GetImmediateParentOfType<CodeNamespace>().Name
+                var replacement = string.Join("\\", usingElement.Declaration.TypeDefinition.GetImmediateParentOfType<CodeNamespace>().Name
                     .Split(new[]{'\\', '.'}, StringSplitOptions.RemoveEmptyEntries)
                     .Select(x => x.ToFirstCharacterUpperCase())
                     .ToArray());
-                usingElement.Alias = $"{replacement}{declaration.ToFirstCharacterUpperCase()}";
+                usingElement.Alias = $"{(string.IsNullOrEmpty(replacement) ? string.Empty : $"\\{replacement}")}\\{declaration.ToFirstCharacterUpperCase()}";
+                usingElement.Declaration.Name = usingElement.Alias;
             }
         }
         CrawlTree(currentElement, AliasUsingWithSameSymbol);

src/Kiota.Builder/Writers/Php/CodeMethodWriter.cs

@@ -535,7 +535,7 @@ protected string GetSendRequestMethodName(bool isVoid, bool isStream, bool isCol
             return "sendAsync";
         }
 
-        private static void WriteFactoryMethodBody(CodeMethod codeElement, CodeClass parentClass, LanguageWriter writer){
+        private void WriteFactoryMethodBody(CodeMethod codeElement, CodeClass parentClass, LanguageWriter writer){
             var parseNodeParameter = codeElement.Parameters.OfKind(CodeParameterKind.ParseNode);
             if(parentClass.DiscriminatorInformation.ShouldWriteDiscriminatorForInheritedType && parseNodeParameter != null) {
                 writer.WriteLines($"$mappingValueNode = ${parseNodeParameter.Name.ToFirstCharacterLowerCase()}->getChildNode(\"{parentClass.DiscriminatorInformation.DiscriminatorPropertyName}\");",
@@ -545,7 +545,7 @@ private static void WriteFactoryMethodBody(CodeMethod codeElement, CodeClass par
                 writer.WriteLine("switch ($mappingValue) {");
                 writer.IncreaseIndent();
                 foreach(var mappedType in parentClass.DiscriminatorInformation.DiscriminatorMappings) {
-                    writer.WriteLine($"case '{mappedType.Key}': return new {mappedType.Value.AllTypes.First().Name.ToFirstCharacterUpperCase()}();");
+                    writer.WriteLine($"case '{mappedType.Key}': return new {conventions.GetTypeString(mappedType.Value.AllTypes.First(), parentClass)}();");
                 }
                 writer.CloseBlock();
                 writer.CloseBlock();

src/Kiota.Builder/Writers/Php/PhpConventionService.cs

@@ -50,7 +50,7 @@ public override string GetTypeString(CodeTypeBase code, CodeElement targetElemen
                 var typeName = TranslateType(currentType);
                 if (!currentType.IsExternal && IsSymbolDuplicated(typeName, targetElement))
                 {
-                    return $"{MakeNamespaceAliasVariable(currentType.TypeDefinition.GetImmediateParentOfType<CodeNamespace>().Name.ToFirstCharacterUpperCase())}{typeName.ToFirstCharacterUpperCase()}";
+                    return $"\\{currentType.TypeDefinition.GetImmediateParentOfType<CodeNamespace>().Name.ReplaceDotsWithSlashInNamespaces()}\\{typeName.ToFirstCharacterUpperCase()}";
                 }
             }
             return code is {IsCollection: true} ? "array" : TranslateType(code);
@@ -63,7 +63,7 @@ public override string TranslateType(CodeType type)
             {
                 "boolean" => "bool",
                 "double" => "float",
-                "decimal" or "byte" => "string",
+                "decimal" or "byte" or "guid" => "string",
                 "integer" or "int32" or "int64" or "sbyte" => "int",
                 "object" or "string" or "array" or "float" or "void" => typeName.ToLowerInvariant(),
                 "binary" => "StreamInterface",
@@ -172,18 +172,19 @@ public void WriteNamespaceAndImports(ClassDeclaration codeElement, LanguageWrite
                 codeElement.Usings?
                     .Where(x => x.Declaration != null && (x.Declaration.IsExternal ||
                                 !x.Declaration.Name.Equals(codeElement.Name, StringComparison.OrdinalIgnoreCase)))
+                    .Where(static x => string.IsNullOrEmpty(x.Alias))
                     .Select(x =>
                     {
                         string namespaceValue;
                         if (x.Declaration is {IsExternal: true})
                         {
                             namespaceValue = string.IsNullOrEmpty(x.Declaration.Name) ? string.Empty : $"{x.Declaration.Name.ReplaceDotsWithSlashInNamespaces()}\\";
                             return
-                                $"use {namespaceValue}{x.Name.ReplaceDotsWithSlashInNamespaces()}{(!string.IsNullOrEmpty(x.Alias) ? $" as {x.Alias}" : string.Empty)};";
+                                $"use {namespaceValue}{x.Name.ReplaceDotsWithSlashInNamespaces()};";
                         }
                         namespaceValue = string.IsNullOrEmpty(x.Name) ? string.Empty : $"{x.Name.ReplaceDotsWithSlashInNamespaces()}\\";
                             return
-                                $"use {namespaceValue}{x.Declaration.Name.ReplaceDotsWithSlashInNamespaces()}{(!string.IsNullOrEmpty(x.Alias) ? $" as {x.Alias}" : string.Empty)};";
+                                $"use {namespaceValue}{x.Declaration.Name.ReplaceDotsWithSlashInNamespaces()};";
                     })
                         .Distinct()
                     .OrderBy(x => x)
@@ -217,20 +218,14 @@ internal void AddParametersAssignment(LanguageWriter writer, CodeTypeBase pathPa
         }
 
         private static bool IsSymbolDuplicated(string symbol, CodeElement targetElement) {
-            var targetClass = targetElement as CodeClass ?? targetElement.GetImmediateParentOfType<CodeClass>();
-            if (targetClass.Parent is CodeClass parentClass) 
+            var targetClass = targetElement as CodeClass ?? targetElement?.GetImmediateParentOfType<CodeClass>();
+            if (targetClass?.Parent is CodeClass parentClass) 
                 targetClass = parentClass;
-            return targetClass.StartBlock
+            return targetClass?.StartBlock
                 ?.Usings
                 ?.Where(x => !x.IsExternal && symbol.Equals(x.Declaration.TypeDefinition.Name, StringComparison.OrdinalIgnoreCase))
                 ?.Distinct(_usingDeclarationNameComparer)
                 ?.Count() > 1;
         }
-
-        private static string MakeNamespaceAliasVariable(string name)
-        {
-            var parts = name.Split(new[]{'\\', '.'}, StringSplitOptions.RemoveEmptyEntries);
-            return string.Join(string.Empty, parts.Select(x => x.ToFirstCharacterUpperCase()).ToArray());
-        }
     }
 }