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

bugfix/composed type member naming #3340

Merged
merged 10 commits into from
Sep 22, 2023
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Changed

- Localhost based descriptions are not cached anymore to facilitate development workflows. [#3316](https://github.com/microsoft/kiota/issues/3316)
- Changed parameter order in with_url method body to match the signature of RequestBuilder constructor in Python. [#3328](https://github.com/microsoft/kiota/issues/3328
- Fixed a bug where inline composed types for components schemas would have the wrong name. [#3067](https://github.com/microsoft/kiota/issues/3067)
- Changed parameter order in with_url method body to match the signature of RequestBuilder constructor in Python. [#3328](https://github.com/microsoft/kiota/issues/3328)
- Removed redundant undefined qualifier in TypeScript for properties. [#3244](https://github.com/microsoft/kiota/issues/3244)
- The default status code response is now used as 4XX and 5XX when those class responses are not provided in the description. [#3245](https://github.com/microsoft/kiota/issues/3245)
- Adds codes files in typescript to reduce number of generated files. [#2116](https://github.com/microsoft/kiota/issues/2116)
Expand Down
28 changes: 12 additions & 16 deletions it/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -259,32 +259,32 @@
],
"IdempotencySuppressions": [
{
"Language": "typescript",
"Rationale": "https://github.com/microsoft/kiota/issues/1812"
"Language": "csharp",
"Rationale": "https://github.com/microsoft/kiota/issues/2952"
},
{
"Language": "go",
"Rationale": "https://github.com/microsoft/kiota/issues/3067"
"Rationale": "https://github.com/microsoft/kiota/issues/2834"
},
{
"Language": "php",
"Rationale": "https://github.com/microsoft/kiota/issues/3067"
"Rationale": "https://github.com/microsoft/kiota/issues/2964"
},
{
"Language": "java",
"Rationale": "https://github.com/microsoft/kiota/issues/3067"
"Rationale": "https://github.com/microsoft/kiota/issues/2842"
},
{
"Language": "csharp",
"Rationale": "https://github.com/microsoft/kiota/issues/3067"
"Language": "python",
"Rationale": "https://github.com/microsoft/kiota/issues/2842"
},
{
"Language": "ruby",
"Rationale": "https://github.com/microsoft/kiota/issues/1816"
"Language": "typescript",
"Rationale": "https://github.com/microsoft/kiota/issues/1812"
},
{
"Language": "python",
"Rationale": "https://github.com/microsoft/kiota/issues/3067"
"Language": "ruby",
"Rationale": "https://github.com/microsoft/kiota/issues/1816"
}
]
},
Expand Down Expand Up @@ -324,14 +324,10 @@
}
],
"IdempotencySuppressions": [
{
"Language": "php",
"Rationale": "https://github.com/microsoft/kiota/issues/3067"
},
{
"Language": "typescript",
"Rationale": "https://github.com/microsoft/kiota/issues/1812"
}
]
}
}
}
15 changes: 6 additions & 9 deletions src/Kiota.Builder/BaseCodeParameterOrderComparer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,12 @@ protected virtual int GetKindOrderHint(CodeParameterKind kind)
CodeParameterKind.Path => 4,
CodeParameterKind.RequestConfiguration => 5,
CodeParameterKind.RequestBody => 6,
#pragma warning disable CS0618
CodeParameterKind.ResponseHandler => 7,
#pragma warning restore CS0618
CodeParameterKind.Serializer => 8,
CodeParameterKind.BackingStore => 9,
CodeParameterKind.SetterValue => 10,
CodeParameterKind.ParseNode => 11,
CodeParameterKind.Custom => 12,
_ => 13,
CodeParameterKind.Serializer => 7,
CodeParameterKind.BackingStore => 8,
CodeParameterKind.SetterValue => 9,
CodeParameterKind.ParseNode => 10,
CodeParameterKind.Custom => 11,
_ => 12,
};
}
private const int OptionalWeight = 10000;
Expand Down
12 changes: 12 additions & 0 deletions src/Kiota.Builder/CodeDOM/CodeBlock.cs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ private T HandleDuplicatedExceptions<T>(T element, CodeElement returnedValue) wh
if (returnedValue == element)
return element;
if (element is CodeMethod currentMethod)
{
if (currentMethod.IsOfKind(CodeMethodKind.IndexerBackwardCompatibility) &&
returnedValue is CodeProperty cProp &&
cProp.IsOfKind(CodePropertyKind.RequestBuilder) &&
Expand All @@ -99,6 +100,17 @@ returnedValue is CodeProperty cProp &&
return result2;
}
}
}
else if (element is CodeProperty currentProperty &&
currentProperty.Kind is CodePropertyKind.Custom &&
returnedValue is CodeClass returnedClass && returnedClass.Kind is CodeClassKind.Model &&
InnerChildElements.TryAdd($"{element.Name}-property", currentProperty))
return element; // inline type property: transforming union type to wrapper class
else if (element is CodeClass currentClass &&
currentClass.Kind is CodeClassKind.Model &&
returnedValue is CodeProperty returnedProperty && returnedProperty.Kind is CodePropertyKind.Custom &&
InnerChildElements.TryAdd($"{element.Name}-model", currentClass))
return element; // inline type property: transforming wrapper class to union type

if (element.GetType() == returnedValue.GetType())
return (T)returnedValue;
Expand Down
2 changes: 1 addition & 1 deletion src/Kiota.Builder/CodeDOM/CodeClass.cs
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ public bool ContainsPropertyWithWireName(string wireName)
}
public IEnumerable<CodeClass> AddInnerClass(params CodeClass[] codeClasses)
{
if (codeClasses == null || codeClasses.Any(x => x == null))
if (codeClasses == null || codeClasses.Any(static x => x == null))
throw new ArgumentNullException(nameof(codeClasses));
if (!codeClasses.Any())
throw new ArgumentOutOfRangeException(nameof(codeClasses));
Expand Down
27 changes: 22 additions & 5 deletions src/Kiota.Builder/CodeDOM/CodeComposedTypeBase.cs
Original file line number Diff line number Diff line change
@@ -1,23 +1,34 @@
using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Linq;

namespace Kiota.Builder.CodeDOM;
/// <summary>
/// The base class for composed types like union or exclusion.
/// </summary>
public abstract class CodeComposedTypeBase : CodeTypeBase, IDiscriminatorInformationHolder
public abstract class CodeComposedTypeBase : CodeTypeBase, IDiscriminatorInformationHolder, IDeprecableElement
{
private static string NormalizeKey(CodeType codeType) => $"{codeType.Name}_{codeType.CollectionKind}";
public void AddType(params CodeType[] codeTypes)
{
ArgumentNullException.ThrowIfNull(codeTypes);
if (Array.Exists(codeTypes, static x => x == null))
throw new ArgumentNullException(nameof(codeTypes), "One of the provided types was null");
EnsureElementsAreChildren(codeTypes);
foreach (var codeType in codeTypes.Where(x => x != null && !Types.Contains(x)))
types.Add(codeType);
foreach (var codeType in codeTypes)
if (!types.TryAdd(NormalizeKey(codeType), codeType))
throw new InvalidOperationException($"The type {codeType.Name} was already added");
}
private readonly List<CodeType> types = new();
public bool ContainsType(CodeType codeType)
{
ArgumentNullException.ThrowIfNull(codeType);
return types.ContainsKey(NormalizeKey(codeType));
}
private readonly ConcurrentDictionary<string, CodeType> types = new(StringComparer.OrdinalIgnoreCase);
public IEnumerable<CodeType> Types
{
get => types;
get => types.Values.OrderBy(NormalizeKey, StringComparer.OrdinalIgnoreCase);
}
private DiscriminatorInformation? _discriminatorInformation;
/// <inheritdoc />
Expand Down Expand Up @@ -45,6 +56,7 @@ protected override TChildType BaseClone<TChildType>(CodeTypeBase source, bool cl
if (sourceComposed.Types?.Any() ?? false)
AddType(sourceComposed.Types.ToArray());
DiscriminatorInformation = (DiscriminatorInformation)sourceComposed.DiscriminatorInformation.Clone();
Deprecation = sourceComposed.Deprecation;
return this is TChildType casted ? casted : throw new InvalidCastException($"Cannot cast {GetType().Name} to {typeof(TChildType).Name}");
}
/// <summary>
Expand All @@ -54,4 +66,9 @@ public CodeNamespace? TargetNamespace
{
get; set;
}
public DeprecationInformation? Deprecation
{
get;
set;
}
}
11 changes: 2 additions & 9 deletions src/Kiota.Builder/CodeDOM/CodeIntersectionType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,10 @@ namespace Kiota.Builder.CodeDOM;
/// <summary>
/// The base class for exclusion types. (one of the properties at a time)
/// </summary>
public class CodeIntersectionType : CodeComposedTypeBase, ICloneable, IDeprecableElement
public class CodeIntersectionType : CodeComposedTypeBase, ICloneable
{
public DeprecationInformation? Deprecation
{
get; set;
}

public override object Clone()
{
var value = new CodeIntersectionType().BaseClone<CodeIntersectionType>(this);
value.Deprecation = Deprecation;
return value;
return new CodeIntersectionType().BaseClone<CodeIntersectionType>(this);
}
}
2 changes: 0 additions & 2 deletions src/Kiota.Builder/CodeDOM/CodeParameter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@ public enum CodeParameterKind
/// The request headers when used as a executor/generator parameter. Most languages use the intermediate RequestConfiguration wrapper class.
///</summary>
Headers,
[Obsolete("The parameter kind is replaced by a request option instead")]
ResponseHandler,
RequestBody,
SetterValue,
RequestAdapter,
Expand Down
10 changes: 2 additions & 8 deletions src/Kiota.Builder/CodeDOM/CodeUnionType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,10 @@ namespace Kiota.Builder.CodeDOM;
/// <summary>
/// The base class for union types. (anyOf multiple properties at a time)
/// </summary>
public class CodeUnionType : CodeComposedTypeBase, ICloneable, IDeprecableElement
public class CodeUnionType : CodeComposedTypeBase, ICloneable
{
public DeprecationInformation? Deprecation
{
get; set;
}
public override object Clone()
{
var value = new CodeUnionType().BaseClone<CodeUnionType>(this);
value.Deprecation = Deprecation;
return value;
return new CodeUnionType().BaseClone<CodeUnionType>(this);
}
}
37 changes: 11 additions & 26 deletions src/Kiota.Builder/KiotaBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1114,7 +1114,7 @@

if (!"string".Equals(parameter.Type.Name, StringComparison.OrdinalIgnoreCase))
{ // adding a second indexer for the string version of the parameter so we keep backward compatibility
//TODO remove for v2

Check warning on line 1117 in src/Kiota.Builder/KiotaBuilder.cs

View workflow job for this annotation

GitHub Actions / Build

Complete the task associated to this 'TODO' comment. (https://rules.sonarsource.com/csharp/RSPEC-1135)
var backCompatibleValue = (CodeIndexer)result[0].Clone();
backCompatibleValue.Name += "-string";
backCompatibleValue.IndexParameter.Type = DefaultIndexerParameterType;
Expand Down Expand Up @@ -1334,21 +1334,6 @@
AddRequestBuilderMethodParameters(currentNode, operationType, operation, requestConfigClass, executorMethod);
parentClass.AddMethod(executorMethod);

#pragma warning disable CS0618
var handlerParam = new CodeParameter
{
Name = "responseHandler",
Optional = true,
Kind = CodeParameterKind.ResponseHandler,
Documentation = new()
{
Description = "Response handler to use in place of the default response handling provided by the core service",
},
Type = new CodeType { Name = "IResponseHandler", IsExternal = true },
};
executorMethod.AddParameter(handlerParam);// Add response handler parameter
#pragma warning restore CS0618

var cancellationParam = new CodeParameter
{
Name = "cancellationToken",
Expand Down Expand Up @@ -1602,9 +1587,9 @@
if (parentSchema.Items?.Reference?.Id?.EndsWith(title, StringComparison.OrdinalIgnoreCase) ?? false) return parentSchema.Items.Reference.Id;
return parentSchema.GetSchemaReferenceIds().FirstOrDefault(refId => refId.EndsWith(title, StringComparison.OrdinalIgnoreCase));
}
private CodeTypeBase CreateComposedModelDeclaration(OpenApiUrlTreeNode currentNode, OpenApiSchema schema, OpenApiOperation? operation, string suffixForInlineSchema, CodeNamespace codeNamespace, bool isRequestBody)
private CodeTypeBase CreateComposedModelDeclaration(OpenApiUrlTreeNode currentNode, OpenApiSchema schema, OpenApiOperation? operation, string suffixForInlineSchema, CodeNamespace codeNamespace, bool isRequestBody, string typeNameForInlineSchema)
{
var typeName = currentNode.GetClassName(config.StructuredMimeTypes, operation: operation, suffix: suffixForInlineSchema, schema: schema, requestBody: isRequestBody).CleanupSymbolName();
var typeName = string.IsNullOrEmpty(typeNameForInlineSchema) ? currentNode.GetClassName(config.StructuredMimeTypes, operation: operation, suffix: suffixForInlineSchema, schema: schema, requestBody: isRequestBody).CleanupSymbolName() : typeNameForInlineSchema;
var typesCount = schema.AnyOf?.Count ?? schema.OneOf?.Count ?? 0;
if (typesCount == 1 && schema.Nullable && schema.IsInclusiveUnion() || // nullable on the root schema outside of anyOf
typesCount == 2 && (schema.AnyOf?.Any(static x => // nullable on a schema in the anyOf
Expand Down Expand Up @@ -1654,16 +1639,18 @@
if (string.IsNullOrEmpty(className))
if (GetPrimitiveType(currentSchema) is CodeType primitiveType && !string.IsNullOrEmpty(primitiveType.Name))
{
unionType.AddType(primitiveType);
if (!unionType.ContainsType(primitiveType))
unionType.AddType(primitiveType);
continue;
}
else
className = $"{unionType.Name}Member{++membersWithNoName}";
var codeDeclaration = AddModelDeclarationIfDoesntExist(currentNode, currentSchema, className, shortestNamespace);
unionType.AddType(new CodeType
var declarationType = new CodeType
{
TypeDefinition = codeDeclaration,
});
TypeDefinition = AddModelDeclarationIfDoesntExist(currentNode, currentSchema, className, shortestNamespace),
};
if (!unionType.ContainsType(declarationType))
unionType.AddType(declarationType);
}
return unionType;
}
Expand Down Expand Up @@ -1696,7 +1683,7 @@
if ((schema.IsInclusiveUnion() || schema.IsExclusiveUnion()) && string.IsNullOrEmpty(schema.Format)
&& !schema.IsODataPrimitiveType())
{ // OData types are oneOf string, type + format, enum
return CreateComposedModelDeclaration(currentNode, schema, operation, suffix, codeNamespace, isRequestBody);
return CreateComposedModelDeclaration(currentNode, schema, operation, suffix, codeNamespace, isRequestBody, typeNameForInlineSchema);
}

if (schema.IsObject() || schema.Properties.Any() || schema.IsEnum() || !string.IsNullOrEmpty(schema.AdditionalProperties?.Type))
Expand Down Expand Up @@ -2067,9 +2054,7 @@
.Select(x =>
{
var propertySchema = x.Value;
var className = propertySchema.GetSchemaName().CleanupSymbolName();
if (string.IsNullOrEmpty(className))
className = $"{model.Name}_{x.Key.CleanupSymbolName()}";
var className = $"{model.Name}_{x.Key.CleanupSymbolName()}";
var shortestNamespaceName = GetModelsNamespaceNameFromReferenceId(propertySchema.Reference?.Id);
var targetNamespace = string.IsNullOrEmpty(shortestNamespaceName) ? ns :
rootNamespace?.FindOrAddNamespace(shortestNamespaceName) ?? ns;
Expand Down
1 change: 0 additions & 1 deletion src/Kiota.Builder/Refiners/CSharpRefiner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@
IsExternal = true
}
});
//TODO uncomment on the next major version

Check warning on line 34 in src/Kiota.Builder/Refiners/CSharpRefiner.cs

View workflow job for this annotation

GitHub Actions / Build

Complete the task associated to this 'TODO' comment. (https://rules.sonarsource.com/csharp/RSPEC-1135)
// RemoveRequestConfigurationClasses(generatedCode,
// new CodeUsing
// {

Check warning on line 37 in src/Kiota.Builder/Refiners/CSharpRefiner.cs

View workflow job for this annotation

GitHub Actions / Build

Remove this commented out code. (https://rules.sonarsource.com/csharp/RSPEC-125)
// Name = "RequestConfiguration",
// Declaration = new CodeType
// {
Expand Down Expand Up @@ -107,7 +107,6 @@
generatedCode,
"IParseNode"
);
RemoveHandlerFromRequestBuilder(generatedCode);
}, cancellationToken);
}
protected static void DisambiguatePropertiesWithClassNames(CodeElement currentElement)
Expand Down
16 changes: 1 addition & 15 deletions src/Kiota.Builder/Refiners/CommonLanguageRefiner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -486,6 +486,7 @@
{
Description = description,
},
Deprecation = codeComposedType.Deprecation,
}).Last();
}
else if (codeComposedType.TargetNamespace is CodeNamespace targetNamespace)
Expand Down Expand Up @@ -608,9 +609,9 @@
if (currentElement is CodeIndexer currentIndexer &&
currentElement.Parent is CodeClass indexerParentClass)
{
if (indexerParentClass.ContainsMember(currentElement.Name)) // TODO remove condition for v2 necessary because of the second case of Go block

Check warning on line 612 in src/Kiota.Builder/Refiners/CommonLanguageRefiner.cs

View workflow job for this annotation

GitHub Actions / Build

Complete the task associated to this 'TODO' comment. (https://rules.sonarsource.com/csharp/RSPEC-1135)
indexerParentClass.RemoveChildElement(currentElement);
//TODO remove whole block except for last else if body for v2

Check warning on line 614 in src/Kiota.Builder/Refiners/CommonLanguageRefiner.cs

View workflow job for this annotation

GitHub Actions / Build

Complete the task associated to this 'TODO' comment. (https://rules.sonarsource.com/csharp/RSPEC-1135)
if (language == GenerationLanguage.Go)
{
if (currentIndexer.IsLegacyIndexer)
Expand Down Expand Up @@ -1377,21 +1378,6 @@
}
CrawlTree(currentElement, RemoveDiscriminatorMappingsTargetingSubNamespaces);
}
protected void RemoveHandlerFromRequestBuilder(CodeElement currentElement)
{
if (currentElement is CodeClass currentClass && currentClass.IsOfKind(CodeClassKind.RequestBuilder))
{
var codeMethods = currentClass.Methods.Where(x => x.Kind == CodeMethodKind.RequestExecutor);
foreach (var codeMethod in codeMethods)
{
#pragma warning disable CS0618
codeMethod.RemoveParametersByKind(CodeParameterKind.ResponseHandler);
#pragma warning restore CS0618
}
}

CrawlTree(currentElement, RemoveHandlerFromRequestBuilder);
}
protected static void MoveRequestBuilderPropertiesToBaseType(CodeElement currentElement, CodeUsing baseTypeUsing, AccessModifier? accessModifier = null)
{
ArgumentNullException.ThrowIfNull(baseTypeUsing);
Expand Down
1 change: 0 additions & 1 deletion src/Kiota.Builder/Refiners/GoRefiner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,6 @@ public override Task Refine(CodeNamespace generatedCode, CancellationToken cance
generatedCode,
x => $"{x.Name}able"
);
RemoveHandlerFromRequestBuilder(generatedCode);
AddContextParameterToGeneratorMethods(generatedCode);
CorrectTypes(generatedCode);
CorrectCoreTypesForBackingStore(generatedCode, $"{conventions.StoreHash}.BackingStoreFactoryInstance()", false);
Expand Down
1 change: 0 additions & 1 deletion src/Kiota.Builder/Refiners/JavaRefiner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,6 @@ public override Task Refine(CodeNamespace generatedCode, CancellationToken cance
"ParseNode",
addUsings: true
);
RemoveHandlerFromRequestBuilder(generatedCode);
SplitLongDiscriminatorMethods(generatedCode);
AddPrimaryErrorMessage(generatedCode,
"getMessage",
Expand Down
1 change: 0 additions & 1 deletion src/Kiota.Builder/Refiners/PhpRefiner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,6 @@ public override Task Refine(CodeNamespace generatedCode, CancellationToken cance
AddPropertiesAndMethodTypesImports(generatedCode, true, false, true);
CorrectBackingStoreSetterParam(generatedCode);
CorrectCoreTypesForBackingStore(generatedCode, "BackingStoreFactorySingleton::getInstance()->createBackingStore()");
RemoveHandlerFromRequestBuilder(generatedCode);
cancellationToken.ThrowIfCancellationRequested();
AliasUsingWithSameSymbol(generatedCode);
RemoveRequestConfigurationClassesCommonProperties(generatedCode,
Expand Down
1 change: 0 additions & 1 deletion src/Kiota.Builder/Refiners/PythonRefiner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,6 @@ public override Task Refine(CodeNamespace generatedCode, CancellationToken cance
addUsings: true,
includeParentNamespace: true
);
RemoveHandlerFromRequestBuilder(generatedCode);
}, cancellationToken);
}

Expand Down
1 change: 0 additions & 1 deletion src/Kiota.Builder/Refiners/RubyRefiner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,6 @@ public override Task Refine(CodeNamespace generatedCode, CancellationToken cance
"ParseNode",
addUsings: true
);
RemoveHandlerFromRequestBuilder(generatedCode);
}, cancellationToken);
}
private static void ShortenLongNamespaceNames(CodeElement currentElement)
Expand Down
1 change: 0 additions & 1 deletion src/Kiota.Builder/Refiners/TypeScriptRefiner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ public override Task Refine(CodeNamespace generatedCode, CancellationToken cance
{
cancellationToken.ThrowIfCancellationRequested();
RemoveMethodByKind(generatedCode, CodeMethodKind.RawUrlConstructor);
RemoveHandlerFromRequestBuilder(generatedCode);
ReplaceReservedNames(generatedCode, new TypeScriptReservedNamesProvider(), static x => $"{x}Escaped");
ReplaceReservedExceptionPropertyNames(generatedCode, new TypeScriptExceptionsReservedNamesProvider(), static x => $"{x}Escaped");
MoveRequestBuilderPropertiesToBaseType(generatedCode,
Expand Down
Loading
Loading