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

Fix serialization and deserialization of composed types in TS #5461

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all 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: 0 additions & 4 deletions it/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -229,10 +229,6 @@
},
"apisguru::stripe.com": {
"Suppressions": [
{
"Language": "typescript",
"Rationale": "https://github.com/microsoft/kiota/issues/5256"
},
{
"Language": "java",
"Rationale": "https://github.com/microsoft/kiota/issues/2842"
Expand Down
23 changes: 1 addition & 22 deletions src/Kiota.Builder/Refiners/TypeScriptRefiner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -169,31 +169,10 @@
GroupReusableModelsInSingleFile(modelsNamespace);
RemoveSelfReferencingUsings(generatedCode);
AddAliasToCodeFileUsings(generatedCode);
CorrectSerializerParameters(generatedCode);
cancellationToken.ThrowIfCancellationRequested();
}, cancellationToken);
}

private static void CorrectSerializerParameters(CodeElement currentElement)
{
if (currentElement is CodeFunction currentFunction &&
currentFunction.OriginalLocalMethod.Kind is CodeMethodKind.Serializer)
{
foreach (var parameter in currentFunction.OriginalLocalMethod.Parameters
.Where(p => GetOriginalComposedType(p.Type) is CodeComposedTypeBase composedType &&
composedType.IsComposedOfObjectsAndPrimitives(IsPrimitiveType)))
{
var composedType = GetOriginalComposedType(parameter.Type)!;
var newType = (CodeComposedTypeBase)composedType.Clone();
var nonPrimitiveTypes = composedType.Types.Where(x => !IsPrimitiveType(x, composedType)).ToArray();
newType.SetTypes(nonPrimitiveTypes);
parameter.Type = newType;
}
}

CrawlTree(currentElement, CorrectSerializerParameters);
}

private static void AddAliasToCodeFileUsings(CodeElement currentElement)
{
if (currentElement is CodeFile codeFile)
Expand Down Expand Up @@ -704,7 +683,7 @@
m.IsOfKind(CodeMethodKind.RequestExecutor, CodeMethodKind.RequestGenerator) &&
m.Parameters.Any(IsMultipartBody);
// for Kiota abstraction library if the code is not required for runtime purposes e.g. interfaces then the IsErasable flag is set to true
private static readonly AdditionalUsingEvaluator[] defaultUsingEvaluators = {

Check warning on line 686 in src/Kiota.Builder/Refiners/TypeScriptRefiner.cs

View workflow job for this annotation

GitHub Actions / Build

Refactor this field to reduce its Cognitive Complexity from 21 to the 15 allowed. (https://rules.sonarsource.com/csharp/RSPEC-3776)

Check warning on line 686 in src/Kiota.Builder/Refiners/TypeScriptRefiner.cs

View workflow job for this annotation

GitHub Actions / Build

Refactor this field to reduce its Cognitive Complexity from 21 to the 15 allowed. (https://rules.sonarsource.com/csharp/RSPEC-3776)
new (static x => x is CodeMethod method && method.Kind is CodeMethodKind.ClientConstructor,
AbstractionsPackageName, true, "RequestAdapter"),
new (static x => x is CodeMethod method && method.Kind is CodeMethodKind.RequestGenerator,
Expand Down Expand Up @@ -1483,7 +1462,7 @@

function.AddUsing(new CodeUsing
{
Name = modelDeserializerFunction.Parent.Name,
Name = modelDeserializerFunction.Name,
Declaration = new CodeType
{
Name = modelDeserializerFunction.Name,
Expand Down
28 changes: 19 additions & 9 deletions src/Kiota.Builder/Writers/TypeScript/CodeFunctionWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,11 @@
if (GetOriginalComposedType(composedParam) is not { } composedType) return;

writer.StartBlock("return {");
if (composedType.Types.Any(x => IsPrimitiveType(x, composedType, false)))
{
var expression = string.Join(" ?? ", composedType.Types.Where(x => IsPrimitiveType(x, composedType, false)).Select(codeType => $"n.{conventions.GetDeserializationMethodName(codeType, codeElement.OriginalLocalMethod, composedType.IsCollection)}"));
writer.WriteLine($"\"\" : n => {{ {composedParam.Name.ToFirstCharacterLowerCase()} = {expression}}},");
}
foreach (var mappedType in composedType.Types.Where(x => !IsPrimitiveType(x, composedType, false)))
{
var functionName = GetDeserializerFunctionName(codeElement, mappedType);
Expand All @@ -90,10 +95,10 @@
{
var paramName = composedParam.Name.ToFirstCharacterLowerCase();
writer.WriteLine($"if ({paramName} === undefined || {paramName} === null) return;");
writer.StartBlock($"switch (typeof {paramName}) {{");
writer.StartBlock($"switch (true) {{");
foreach (var type in composedType.Types.Where(x => IsPrimitiveType(x, composedType, false)))
{
WriteCaseStatementForPrimitiveTypeSerialization(type, "key", paramName, codeElement, writer);
WriteCaseStatementForPrimitiveTypeSerialization(type, "undefined", paramName, codeElement, writer);
}
writer.CloseBlock();
return;
Expand All @@ -110,7 +115,6 @@

private void WriteSerializationFunctionForCodeIntersectionType(CodeComposedTypeBase composedType, CodeParameter composedParam, CodeFunction method, LanguageWriter writer)
{
// Serialization/Deserialization functions can be called for object types only
foreach (var mappedType in composedType.Types.Where(x => !IsPrimitiveType(x, composedType)))
{
var functionName = GetSerializerFunctionName(method, mappedType);
Expand Down Expand Up @@ -166,22 +170,25 @@
{
writer.StartBlock($"case \"{mappedType.Key}\":");
writer.WriteLine($"{GetSerializerFunctionName(codeElement, mappedType.Value)}(writer, {paramName} as {mappedType.Value.AllTypes.First().Name.ToFirstCharacterUpperCase()});");
writer.WriteLine("break;");

Check warning on line 173 in src/Kiota.Builder/Writers/TypeScript/CodeFunctionWriter.cs

View workflow job for this annotation

GitHub Actions / Build

Define a constant instead of using this literal 'break;' 4 times. (https://rules.sonarsource.com/csharp/RSPEC-1192)

Check warning on line 173 in src/Kiota.Builder/Writers/TypeScript/CodeFunctionWriter.cs

View workflow job for this annotation

GitHub Actions / Build

Define a constant instead of using this literal 'break;' 4 times. (https://rules.sonarsource.com/csharp/RSPEC-1192)
writer.DecreaseIndent();
}

writer.CloseBlock();
}

private void WriteCaseStatementForPrimitiveTypeSerialization(CodeTypeBase type, string key, string value, CodeFunction method, LanguageWriter writer)
private void WriteCaseStatementForPrimitiveTypeSerialization(CodeTypeBase type, string key, string modelParamName, CodeFunction method, LanguageWriter writer)
{
var nodeType = conventions.GetTypeString(type, method, false);
var serializationName = GetSerializationMethodName(type, method.OriginalLocalMethod);
if (string.IsNullOrEmpty(serializationName) || string.IsNullOrEmpty(nodeType)) return;

writer.StartBlock($"case \"{nodeType}\":");
writer.WriteLine($"writer.{serializationName}({key}, {value});");
writer.WriteLine($"break;");
writer.StartBlock(type.IsCollection
? $"case Array.isArray({modelParamName}) && ({modelParamName}).every(item => typeof item === '{nodeType}') :"
: $"case typeof {modelParamName} === \"{nodeType}\":");

writer.WriteLine($"writer.{serializationName}({key}, {modelParamName} as {conventions.GetTypeString(type, method)});");
writer.WriteLine("break;");
writer.DecreaseIndent();
}

Expand Down Expand Up @@ -329,7 +336,10 @@
var codeFunction = Array.Find(codeFunctions,
func => func.GetImmediateParentOfType<CodeNamespace>().Name == myNamespace.Name) ??
throw new InvalidOperationException($"Function {functionName} not found in namespace {myNamespace.Name}");
return conventions.GetTypeString(new CodeType { TypeDefinition = codeFunction }, codeElement, false);

var targetElement = codeElement.GetImmediateParentOfType<CodeFile>();

return GetTypescriptTypeString(new CodeType { TypeDefinition = codeFunction }, targetElement, includeCollectionInformation: false);
}

private void WriteSerializerFunction(CodeFunction codeElement, LanguageWriter writer)
Expand Down Expand Up @@ -432,7 +442,7 @@
if (string.IsNullOrEmpty(serializationName) || string.IsNullOrEmpty(nodeType)) return;

writer.StartBlock(type.IsCollection
? $"Array.isArray({modelParamName}.{codePropertyName}) && ({modelParamName}.{codePropertyName}).every(item => typeof item === '{nodeType}') :"
? $"case Array.isArray({modelParamName}.{codePropertyName}) && ({modelParamName}.{codePropertyName}).every(item => typeof item === '{nodeType}') :"
: $"case typeof {modelParamName}.{codePropertyName} === \"{nodeType}\":");

writer.WriteLine($"writer.{serializationName}(\"{codeProperty.WireName}\", {spreadOperator}{modelParamName}.{codePropertyName}{defaultValueSuffix} as {nodeType});");
Expand Down
6 changes: 4 additions & 2 deletions src/Kiota.Builder/Writers/TypeScript/CodeMethodWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
ArgumentNullException.ThrowIfNull(writer);
if (codeElement.Parent is CodeFunction) return;

var returnType = GetTypescriptTypeString(codeElement.ReturnType, codeElement, inlineComposedTypeString: true);
var codeFile = codeElement.GetImmediateParentOfType<CodeFile>();
var returnType = GetTypescriptTypeString(codeElement.ReturnType, codeFile, inlineComposedTypeString: true);
var isVoid = "void".EqualsIgnoreCase(returnType);
WriteMethodDocumentation(codeElement, writer, isVoid);
WriteMethodPrototype(codeElement, writer, returnType, isVoid);
Expand All @@ -30,6 +31,7 @@
}
internal static void WriteMethodDocumentationInternal(CodeMethod code, LanguageWriter writer, bool isVoid, TypeScriptConventionService typeScriptConventionService)
{
var codeFile = code.GetImmediateParentOfType<CodeFile>();
var returnRemark = (isVoid, code.IsAsync) switch
{
(true, _) => string.Empty,
Expand All @@ -41,7 +43,7 @@
code.Parameters
.Where(static x => x.Documentation.DescriptionAvailable)
.OrderBy(static x => x.Name)
.Select(x => $"@param {x.Name} {x.Documentation.GetDescription(type => GetTypescriptTypeString(type, code, inlineComposedTypeString: true), ReferenceTypePrefix, ReferenceTypeSuffix, RemoveInvalidDescriptionCharacters)}")
.Select(x => $"@param {x.Name} {x.Documentation.GetDescription(type => GetTypescriptTypeString(type, codeFile, inlineComposedTypeString: true), ReferenceTypePrefix, ReferenceTypeSuffix, RemoveInvalidDescriptionCharacters)}")
.Union([returnRemark])
.Union(GetThrownExceptionsRemarks(code)));
}
Expand All @@ -64,7 +66,7 @@
{
WriteMethodPrototypeInternal(code, writer, returnType, isVoid, conventions, false);
}
internal static void WriteMethodPrototypeInternal(CodeMethod code, LanguageWriter writer, string returnType, bool isVoid, TypeScriptConventionService pConventions, bool isFunction)

Check warning on line 69 in src/Kiota.Builder/Writers/TypeScript/CodeMethodWriter.cs

View workflow job for this annotation

GitHub Actions / Build

Refactor this method to reduce its Cognitive Complexity from 18 to the 15 allowed. (https://rules.sonarsource.com/csharp/RSPEC-3776)
{
var accessModifier = isFunction || code.Parent is CodeInterface ? string.Empty : pConventions.GetAccessModifier(code.Access);
var isConstructor = code.IsOfKind(CodeMethodKind.Constructor, CodeMethodKind.ClientConstructor);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,13 @@
public const string TYPE_DURATION = "Duration";
#pragma warning restore CA1707 // Remove the underscores

internal void WriteAutoGeneratedStart(LanguageWriter writer)

Check warning on line 43 in src/Kiota.Builder/Writers/TypeScript/TypeScriptConventionService.cs

View workflow job for this annotation

GitHub Actions / Build

Make 'WriteAutoGeneratedStart' a static method. (https://rules.sonarsource.com/csharp/RSPEC-2325)

Check warning on line 43 in src/Kiota.Builder/Writers/TypeScript/TypeScriptConventionService.cs

View workflow job for this annotation

GitHub Actions / Build

Make 'WriteAutoGeneratedStart' a static method. (https://rules.sonarsource.com/csharp/RSPEC-2325)
{
writer.WriteLine("/* tslint:disable */");
writer.WriteLine("/* eslint-disable */");
writer.WriteLine("// Generated by Microsoft Kiota");
}
internal void WriteAutoGeneratedEnd(LanguageWriter writer)

Check warning on line 49 in src/Kiota.Builder/Writers/TypeScript/TypeScriptConventionService.cs

View workflow job for this annotation

GitHub Actions / Build

Make 'WriteAutoGeneratedEnd' a static method. (https://rules.sonarsource.com/csharp/RSPEC-2325)
{
writer.WriteLine("/* tslint:enable */");
writer.WriteLine("/* eslint-enable */");
Expand Down Expand Up @@ -81,19 +81,32 @@
};
}

private static bool ShouldIncludeCollectionInformationForParameter(CodeParameter parameter)
{
return !(GetOriginalComposedType(parameter) is not null
&& parameter.Parent is CodeMethod codeMethod
&& (codeMethod.IsOfKind(CodeMethodKind.Serializer) || codeMethod.IsOfKind(CodeMethodKind.Deserializer)));
}

public override string GetParameterSignature(CodeParameter parameter, CodeElement targetElement, LanguageWriter? writer = null)
{
ArgumentNullException.ThrowIfNull(parameter);
var includeCollectionInformation = ShouldIncludeCollectionInformationForParameter(parameter);
var paramType = GetTypescriptTypeString(parameter.Type, targetElement, includeCollectionInformation: includeCollectionInformation, inlineComposedTypeString: true);
var isComposedOfPrimitives = GetOriginalComposedType(parameter.Type) is CodeComposedTypeBase composedType && composedType.IsComposedOfPrimitives(IsPrimitiveType);
var composedType = GetOriginalComposedType(parameter.Type);
var paramType = GetTypescriptTypeString(parameter.Type, targetElement, includeCollectionInformation: true, inlineComposedTypeString: true);

if (composedType != null && parameter.Parent is CodeMethod cm && cm.IsOfKind(CodeMethodKind.Serializer))
{
// eliminate primitive types from serializers with composed type signature
var newType = (CodeComposedTypeBase)composedType.Clone();
var nonPrimitiveTypes = composedType.Types.Where(x => !IsPrimitiveTypeOrPrimitiveCollection(x, composedType)).ToArray();
newType.SetTypes(nonPrimitiveTypes);
paramType = GetTypescriptTypeString(newType, targetElement, includeCollectionInformation: true, inlineComposedTypeString: true);
}
var isComposedOfPrimitives = composedType != null && composedType.IsComposedOfPrimitives(IsPrimitiveType);

// add a 'Parsable' type to the parameter if it is composed of non-Parsable types
var parsableTypes = (
composedType != null,
parameter.Parent is CodeMethod method && (method.IsOfKind(CodeMethodKind.Deserializer) || method.IsOfKind(CodeMethodKind.Serializer))
) switch
{
(true, true) => "Parsable | ",
_ => string.Empty,
};

var defaultValueSuffix = (string.IsNullOrEmpty(parameter.DefaultValue), parameter.Kind, isComposedOfPrimitives) switch
{
(false, CodeParameterKind.DeserializationTarget, false) when parameter.Parent is CodeMethod codeMethod && codeMethod.Kind is CodeMethodKind.Serializer
Expand All @@ -107,7 +120,7 @@
(false, CodeParameterKind.DeserializationTarget) => ("Partial<", ">"),
_ => (string.Empty, string.Empty),
};
return $"{parameter.Name.ToFirstCharacterLowerCase()}{(parameter.Optional && parameter.Type.IsNullable ? "?" : string.Empty)}: {partialPrefix}{paramType}{partialSuffix}{(parameter.Type.IsNullable ? " | undefined" : string.Empty)}{defaultValueSuffix}";
return $"{parameter.Name.ToFirstCharacterLowerCase()}{(parameter.Optional && parameter.Type.IsNullable ? "?" : string.Empty)}: {partialPrefix}{parsableTypes}{paramType}{partialSuffix}{(parameter.Type.IsNullable ? " | undefined" : string.Empty)}{defaultValueSuffix}";
}

public override string GetTypeString(CodeTypeBase code, CodeElement targetElement, bool includeCollectionInformation = true, LanguageWriter? writer = null)
Expand Down Expand Up @@ -212,7 +225,7 @@
return (!string.IsNullOrEmpty(codeType.TypeDefinition?.Name) ? codeType.TypeDefinition.Name : codeType.Name).ToFirstCharacterUpperCase();
}

public static bool IsPrimitiveType(string typeName)

Check warning on line 228 in src/Kiota.Builder/Writers/TypeScript/TypeScriptConventionService.cs

View workflow job for this annotation

GitHub Actions / Build

All 'IsPrimitiveType' method overloads should be adjacent. (https://rules.sonarsource.com/csharp/RSPEC-4136)

Check warning on line 228 in src/Kiota.Builder/Writers/TypeScript/TypeScriptConventionService.cs

View workflow job for this annotation

GitHub Actions / Build

All 'IsPrimitiveType' method overloads should be adjacent. (https://rules.sonarsource.com/csharp/RSPEC-4136)
{
return typeName switch
{
Expand All @@ -227,6 +240,8 @@

public static bool IsPrimitiveType(CodeType codeType, CodeComposedTypeBase codeComposedTypeBase) => IsPrimitiveType(codeType, codeComposedTypeBase, true);

public static bool IsPrimitiveTypeOrPrimitiveCollection(CodeType codeType, CodeComposedTypeBase codeComposedTypeBase) => IsPrimitiveType(codeType, codeComposedTypeBase, false);

public static bool IsPrimitiveType(CodeType codeType, CodeComposedTypeBase codeComposedTypeBase, bool includeCollectionInformation) => IsPrimitiveType(GetTypescriptTypeString(codeType, codeComposedTypeBase, includeCollectionInformation));

internal static string RemoveInvalidDescriptionCharacters(string originalDescription) => originalDescription?.Replace("\\", "/", StringComparison.OrdinalIgnoreCase) ?? string.Empty;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1252,9 +1252,9 @@ public async Task Writes_UnionOfPrimitiveValues_SerializerFunctionAsync()
writer.Write(serializerFunction);
var serializerFunctionStr = tw.ToString();
Assert.Contains("return", serializerFunctionStr);
Assert.Contains("switch", serializerFunctionStr);
Assert.Contains("case \"number\":", serializerFunctionStr);
Assert.Contains("case \"string\":", serializerFunctionStr);
Assert.Contains("switch (true)", serializerFunctionStr);
Assert.Contains("case typeof primitives === \"number\":", serializerFunctionStr);
Assert.Contains("case typeof primitives === \"string\":", serializerFunctionStr);
Assert.Contains("break", serializerFunctionStr);
AssertExtensions.CurlyBracesAreClosed(serializerFunctionStr, 1);
}
Expand Down
Loading