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.

Sign up for GitHub

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: missing properties in python when no additional properties #5041

Merged
merged 2 commits into from
Aug 1, 2024
Merged

fix: missing properties in python when no additional properties #5041

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bc1b471
fix: missing properties in python when no additional properties
baywet Jul 30, 2024
a4f05cd
fix: regression for defaulted properties
baywet Aug 1, 2024
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Changed

- Fixed a bug where properties would be missing in Python models if they didn't have additional properties. [#5037](https://github.com/microsoft/kiota/issues/5037)
- Fixed a bug in dotnet where CS1587 warnings are generated in generated enums with descriptions [#4957](https://github.com/microsoft/kiota/issues/4957)
- Fixed a bug where the copilot teams toolkit integration would serialize empty declarative copilots. [#4974](https://github.com/microsoft/kiota/issues/4974)
- Fixed a bug for the docker image where the volume path would not match the expected configuration for the description.
Expand Down
4 changes: 4 additions & 0 deletions src/Kiota.Builder/Refiners/PythonRefiner.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@
private const string AbstractionsPackageName = "kiota_abstractions";
private const string SerializationModuleName = $"{AbstractionsPackageName}.serialization";
private const string StoreModuleName = $"{AbstractionsPackageName}.store";
private static readonly AdditionalUsingEvaluator[] defaultUsingEvaluators = {

Check warning on line 163 in src/Kiota.Builder/Refiners/PythonRefiner.cs

View workflow job for this annotation

GitHub Actions / Build 

Refactor this field to reduce its Cognitive Complexity from 19 to the 15 allowed. (https://rules.sonarsource.com/csharp/RSPEC-3776)
new (static x => x is CodeClass, "__future__", "annotations"),
new (static x => x is CodeClass, "typing", "Any, Callable, Dict, List, Optional, TYPE_CHECKING, Union"),
new (static x => x is CodeProperty prop && prop.IsOfKind(CodePropertyKind.RequestAdapter),
Expand Down Expand Up @@ -199,7 +199,7 @@
"warnings", "warn"),
};

private static void CorrectCommonNames(CodeElement currentElement)

Check warning on line 202 in src/Kiota.Builder/Refiners/PythonRefiner.cs

View workflow job for this annotation

GitHub Actions / Build 

Refactor this method to reduce its Cognitive Complexity from 34 to the 15 allowed. (https://rules.sonarsource.com/csharp/RSPEC-3776)
{
if (currentElement is CodeMethod m &&
currentElement.Parent is CodeClass parentClassM)
Expand Down Expand Up @@ -288,6 +288,10 @@
if (!string.IsNullOrEmpty(currentProperty.DefaultValue))
currentProperty.DefaultValue = "{}";
}
else if (currentProperty.Kind is CodePropertyKind.Custom && currentProperty.Type.IsNullable && string.IsNullOrEmpty(currentProperty.DefaultValue))
{
currentProperty.DefaultValue = "None";
}
currentProperty.Type.Name = currentProperty.Type.Name.ToFirstCharacterUpperCase();
CorrectCoreTypes(currentProperty.Parent as CodeClass, DateTypesReplacements, currentProperty.Type);
}
Expand Down
31 changes: 16 additions & 15 deletions src/Kiota.Builder/Writers/Python/CodeMethodWriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
if (codeElement.Parent is not CodeClass parentClass) throw new InvalidOperationException("the parent of a method should be a class");

var returnType = conventions.GetTypeString(codeElement.ReturnType, codeElement, true, writer);
var isVoid = "None".Equals(returnType, StringComparison.OrdinalIgnoreCase);
var isVoid = NoneKeyword.Equals(returnType, StringComparison.OrdinalIgnoreCase);
if (parentClass.IsOfKind(CodeClassKind.Model) && (codeElement.IsOfKind(CodeMethodKind.Setter) || codeElement.IsOfKind(CodeMethodKind.Getter) || codeElement.IsOfKind(CodeMethodKind.Constructor)))
{
writer.IncreaseIndent();
Expand All @@ -40,7 +40,7 @@
var requestParams = new RequestParams(requestBodyParam, requestConfigParam, requestContentType);
if (!codeElement.IsOfKind(CodeMethodKind.Setter) &&
!(codeElement.IsOfKind(CodeMethodKind.Constructor) && parentClass.IsOfKind(CodeClassKind.RequestBuilder)))
foreach (var parameter in codeElement.Parameters.Where(static x => !x.Optional).OrderBy(static x => x.Name))

Check warning on line 43 in src/Kiota.Builder/Writers/Python/CodeMethodWriter.cs

View workflow job for this annotation

GitHub Actions / Build 

Loop should be simplified by calling Select(parameter => parameter.Name)) (https://rules.sonarsource.com/csharp/RSPEC-3267)
{
var parameterName = parameter.Name;
writer.StartBlock($"if not {parameterName}:");
Expand Down Expand Up @@ -135,7 +135,7 @@
writer.WriteLine($"return {parentClass.Name}()");
}
private const string ResultVarName = "result";
private void WriteFactoryMethodBodyForUnionModel(CodeMethod codeElement, CodeClass parentClass, CodeParameter parseNodeParameter, LanguageWriter writer)

Check warning on line 138 in src/Kiota.Builder/Writers/Python/CodeMethodWriter.cs

View workflow job for this annotation

GitHub Actions / Build 

Refactor this method to reduce its Cognitive Complexity from 20 to the 15 allowed. (https://rules.sonarsource.com/csharp/RSPEC-3776)
{
writer.WriteLine($"{ResultVarName} = {parentClass.Name}()");
var includeElse = false;
Expand Down Expand Up @@ -164,7 +164,7 @@
}
writer.WriteLine($"return {ResultVarName}");
}
private void WriteFactoryMethodBodyForIntersectionModel(CodeMethod codeElement, CodeClass parentClass, CodeParameter parseNodeParameter, LanguageWriter writer)

Check warning on line 167 in src/Kiota.Builder/Writers/Python/CodeMethodWriter.cs

View workflow job for this annotation

GitHub Actions / Build 

Refactor this method to reduce its Cognitive Complexity from 17 to the 15 allowed. (https://rules.sonarsource.com/csharp/RSPEC-3776)
{
writer.WriteLine($"{ResultVarName} = {parentClass.Name}()");
var includeElse = false;
Expand Down Expand Up @@ -215,7 +215,7 @@
writer.WriteLine($"{DiscriminatorMappingVarName} = {parseNodeParameter.Name}.get_child_node(\"{parentClass.DiscriminatorInformation.DiscriminatorPropertyName}\").get_str_value()");
writer.DecreaseIndent();
writer.StartBlock($"except AttributeError:");
writer.WriteLine($"{DiscriminatorMappingVarName} = None");
writer.WriteLine($"{DiscriminatorMappingVarName} = {NoneKeyword}");
writer.DecreaseIndent();
}
if (parentClass.DiscriminatorInformation.ShouldWriteDiscriminatorForInheritedType)
Expand Down Expand Up @@ -324,7 +324,7 @@
return _SetterAccessProperties;
}
}
private void WriteConstructorBody(CodeClass parentClass, CodeMethod currentMethod, LanguageWriter writer, bool inherits)

Check warning on line 327 in src/Kiota.Builder/Writers/Python/CodeMethodWriter.cs

View workflow job for this annotation

GitHub Actions / Build 

Refactor this method to reduce its Cognitive Complexity from 28 to the 15 allowed. (https://rules.sonarsource.com/csharp/RSPEC-3776)
{
if (inherits && !parentClass.IsOfKind(CodeClassKind.Model))
{
Expand All @@ -351,7 +351,7 @@
writer.WriteLine($"super().__init__({requestAdapterParameter.Name}, {urlTemplateProperty.DefaultValue ?? ""}, {pathParametersParameter.Name})");
}
else
writer.WriteLine($"super().__init__({requestAdapterParameter.Name}, {urlTemplateProperty.DefaultValue ?? ""}, None)");
writer.WriteLine($"super().__init__({requestAdapterParameter.Name}, {urlTemplateProperty.DefaultValue ?? ""}, {NoneKeyword})");
}
else
writer.WriteLine("super().__init__()");
Expand All @@ -373,10 +373,10 @@
writer.IncreaseIndent();
}
}
private void WriteDirectAccessProperties(CodeClass parentClass, LanguageWriter writer)

Check warning on line 376 in src/Kiota.Builder/Writers/Python/CodeMethodWriter.cs

View workflow job for this annotation

GitHub Actions / Build 

Refactor this method to reduce its Cognitive Complexity from 20 to the 15 allowed. (https://rules.sonarsource.com/csharp/RSPEC-3776)
{
foreach (var propWithDefault in parentClass.GetPropertiesOfKind(DirectAccessProperties)
.Where(static x => !string.IsNullOrEmpty(x.DefaultValue))
.Where(static x => !string.IsNullOrEmpty(x.DefaultValue) && !NoneKeyword.Equals(x.DefaultValue, StringComparison.Ordinal))
.OrderByDescending(static x => x.Kind)
.ThenBy(static x => x.Name))
{
Expand All @@ -390,7 +390,7 @@
conventions.WriteInLineDescription(propWithDefault, writer);
if (parentClass.IsOfKind(CodeClassKind.Model))
{
writer.WriteLine($"{propWithDefault.Name}: {(propWithDefault.Type.IsNullable ? "Optional[" : string.Empty)}{returnType}{(propWithDefault.Type.IsNullable ? "]" : string.Empty)} = {defaultValue}");

Check warning on line 393 in src/Kiota.Builder/Writers/Python/CodeMethodWriter.cs

View workflow job for this annotation

GitHub Actions / Build 

Define a constant instead of using this literal 'Optional[' 7 times. (https://rules.sonarsource.com/csharp/RSPEC-1192)
writer.WriteLine();
}
else
Expand All @@ -403,7 +403,7 @@
private void WriteSetterAccessProperties(CodeClass parentClass, LanguageWriter writer)
{
foreach (var propWithDefault in parentClass.GetPropertiesOfKind(SetterAccessProperties)
.Where(static x => !string.IsNullOrEmpty(x.DefaultValue))
.Where(static x => !string.IsNullOrEmpty(x.DefaultValue) && !NoneKeyword.Equals(x.DefaultValue, StringComparison.Ordinal))
// do not apply the default value if the type is composed as the default value may not necessarily which type to use
.Where(static x => x.Type is not CodeType propType || propType.TypeDefinition is not CodeClass propertyClass || propertyClass.OriginalComposedType is null)
.OrderByDescending(static x => x.Kind)
Expand All @@ -426,19 +426,20 @@
writer.WriteLine($"self.{setterString}");
}
}
private const string NoneKeyword = "None";
private void WriteSetterAccessPropertiesWithoutDefaults(CodeClass parentClass, LanguageWriter writer)

Check warning on line 430 in src/Kiota.Builder/Writers/Python/CodeMethodWriter.cs

View workflow job for this annotation

GitHub Actions / Build 

Refactor this method to reduce its Cognitive Complexity from 17 to the 15 allowed. (https://rules.sonarsource.com/csharp/RSPEC-3776)
{
foreach (var propWithoutDefault in parentClass.GetPropertiesOfKind(SetterAccessProperties)
.Where(static x => string.IsNullOrEmpty(x.DefaultValue))
.Where(static x => string.IsNullOrEmpty(x.DefaultValue) || NoneKeyword.Equals(x.DefaultValue, StringComparison.Ordinal))
.OrderByDescending(static x => x.Kind)
.ThenBy(static x => x.Name))
{
var returnType = conventions.GetTypeString(propWithoutDefault.Type, propWithoutDefault, true, writer);
conventions.WriteInLineDescription(propWithoutDefault, writer);
if (parentClass.IsOfKind(CodeClassKind.Model))
writer.WriteLine($"{propWithoutDefault.Name}: {(propWithoutDefault.Type.IsNullable ? "Optional[" : string.Empty)}{returnType}{(propWithoutDefault.Type.IsNullable ? "]" : string.Empty)} = None");
writer.WriteLine($"{propWithoutDefault.Name}: {(propWithoutDefault.Type.IsNullable ? "Optional[" : string.Empty)}{returnType}{(propWithoutDefault.Type.IsNullable ? "]" : string.Empty)} = {NoneKeyword}");
else
writer.WriteLine($"self.{conventions.GetAccessModifier(propWithoutDefault.Access)}{propWithoutDefault.NamePrefix}{propWithoutDefault.Name}: {(propWithoutDefault.Type.IsNullable ? "Optional[" : string.Empty)}{returnType}{(propWithoutDefault.Type.IsNullable ? "]" : string.Empty)} = None");
writer.WriteLine($"self.{conventions.GetAccessModifier(propWithoutDefault.Access)}{propWithoutDefault.NamePrefix}{propWithoutDefault.Name}: {(propWithoutDefault.Type.IsNullable ? "Optional[" : string.Empty)}{returnType}{(propWithoutDefault.Type.IsNullable ? "]" : string.Empty)} = {NoneKeyword}");
}
}
private static void WriteSetterBody(CodeMethod codeElement, LanguageWriter writer, CodeClass parentClass)
Expand Down Expand Up @@ -539,7 +540,7 @@
private void WriteDeserializerBodyForInheritedModel(bool inherits, CodeMethod codeElement, CodeClass parentClass, LanguageWriter writer)
{
_codeUsingWriter.WriteInternalImports(parentClass, writer);
writer.StartBlock("fields: Dict[str, Callable[[Any], None]] = {");
writer.StartBlock($"fields: Dict[str, Callable[[Any], {NoneKeyword}]] = {{");
foreach (var otherProp in parentClass
.GetPropertiesOfKind(CodePropertyKind.Custom)
.Where(static x => !x.ExistsInBaseType)
Expand Down Expand Up @@ -579,7 +580,7 @@
var returnTypeWithoutCollectionSymbol = GetReturnTypeWithoutCollectionSymbol(codeElement, returnType);
var genericTypeForSendMethod = GetSendRequestMethodName(isVoid, isStream, codeElement.ReturnType.IsCollection, returnTypeWithoutCollectionSymbol);
var newFactoryParameter = GetTypeFactory(isVoid, isStream, returnTypeWithoutCollectionSymbol);
var errorMappingVarName = "None";
var errorMappingVarName = NoneKeyword;
if (codeElement.ErrorMappings.Any())
{
_codeUsingWriter.WriteInternalErrorMappingImports(parentClass, writer);
Expand Down Expand Up @@ -660,7 +661,7 @@
.ThenBy(static x => x.Name))
{
writer.StartBlock($"{(includeElse ? "el" : string.Empty)}if self.{otherProp.Name}:");
writer.WriteLine($"writer.{GetSerializationMethodName(otherProp.Type)}(None, self.{otherProp.Name})");
writer.WriteLine($"writer.{GetSerializationMethodName(otherProp.Type)}({NoneKeyword}, self.{otherProp.Name})");
writer.DecreaseIndent();
if (!includeElse)
includeElse = true;
Expand All @@ -677,7 +678,7 @@
.ThenBy(static x => x.Name))
{
writer.StartBlock($"{(includeElse ? "el" : string.Empty)}if self.{otherProp.Name}:");
writer.WriteLine($"writer.{GetSerializationMethodName(otherProp.Type)}(None, self.{otherProp.Name})");
writer.WriteLine($"writer.{GetSerializationMethodName(otherProp.Type)}({NoneKeyword}, self.{otherProp.Name})");
writer.DecreaseIndent();
if (!includeElse)
includeElse = true;
Expand All @@ -695,7 +696,7 @@
.Select(static x => x.Name)
.OrderBy(static x => x, StringComparer.OrdinalIgnoreCase)
.Aggregate(static (x, y) => $"self.{x}, self.{y}");
writer.WriteLine($"writer.{GetSerializationMethodName(complexProperties[0].Type)}(None, {propertiesNames})");
writer.WriteLine($"writer.{GetSerializationMethodName(complexProperties[0].Type)}({NoneKeyword}, {propertiesNames})");
if (includeElse)
{
writer.DecreaseIndent();
Expand All @@ -706,7 +707,7 @@
{
var nullablePrefix = code.ReturnType.IsNullable && !isVoid ? "Optional[" : string.Empty;
var nullableSuffix = code.ReturnType.IsNullable && !isVoid ? "]" : string.Empty;
var returnRemark = isVoid ? "Returns: None" : $"Returns: {nullablePrefix}{returnType}{nullableSuffix}";
var returnRemark = isVoid ? $"Returns: {NoneKeyword}" : $"Returns: {nullablePrefix}{returnType}{nullableSuffix}";
conventions.WriteLongDescription(code,
writer,
code.Parameters
Expand Down Expand Up @@ -744,12 +745,12 @@
_ => string.Empty
};
var nullReturnTypeSuffix = !isVoid && !isConstructor;
var returnTypeSuffix = nullReturnTypeSuffix ? $"{nullablePrefix}{returnType}{nullableSuffix}" : "None";
var returnTypeSuffix = nullReturnTypeSuffix ? $"{nullablePrefix}{returnType}{nullableSuffix}" : NoneKeyword;
if (!string.IsNullOrEmpty(propertyDecorator))
writer.WriteLine($"{propertyDecorator}");
writer.WriteLine($"{asyncPrefix}def {accessModifier}{methodName}({instanceReference}{parameters}) -> {returnTypeSuffix}:");
}
private string GetDeserializationMethodName(CodeTypeBase propType, CodeMethod codeElement, CodeClass parentClass)

Check warning on line 753 in src/Kiota.Builder/Writers/Python/CodeMethodWriter.cs

View workflow job for this annotation

GitHub Actions / Build 

Refactor this method to reduce its Cognitive Complexity from 16 to the 15 allowed. (https://rules.sonarsource.com/csharp/RSPEC-3776)
{
var isCollection = propType.CollectionKind != CodeTypeBase.CodeTypeCollectionKind.None;
var propertyType = conventions.TranslateType(propType);
Expand Down
Loading