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

Php/fix generation issues #2049

Merged
merged 16 commits into from
Dec 13, 2022
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 3 additions & 1 deletion src/Kiota.Builder/PathSegmenters/PHPPathSegmenter.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Kiota.Builder.CodeDOM;
using System.Linq;
baywet marked this conversation as resolved.
Show resolved Hide resolved
using Kiota.Builder.CodeDOM;
using Kiota.Builder.Extensions;

namespace Kiota.Builder.PathSegmenters
Expand All @@ -8,6 +9,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();
}
}
44 changes: 23 additions & 21 deletions src/Kiota.Builder/Refiners/CommonLanguageRefiner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
35 changes: 19 additions & 16 deletions src/Kiota.Builder/Refiners/PhpRefiner.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Linq;
using System.Threading;
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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() {
Expand All @@ -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)
Expand Down Expand Up @@ -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"),
Expand All @@ -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")
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions src/Kiota.Builder/Writers/Php/CodeMethodWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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}\");",
Expand All @@ -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();
Expand Down
21 changes: 8 additions & 13 deletions src/Kiota.Builder/Writers/Php/PhpConventionService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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",
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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());
}
}
}
Loading