Skip to content

Commit

Permalink
Task/python imports refactor (#2379)
Browse files Browse the repository at this point in the history
* Add python relative import manager

* Add a codeusingwriter for python

* Add option to include parent namespace when adding discriminator mapping usings

* Call method to add discriminator mapping usings to parent classes

* Add type checking import

* Remove lazy import using

* Update class declaration writer

* Update property writer to add local imports for request builders with no parameters

* Add logic to write discriminator method body

* Write local imports in indexers and request builders with parameters

* Add deferred imports for request executors and deserializers

* Use snake case to match variable name passed to request builder

* Update python writer with missing parameters

* Add incluparentnamespace parameter when crawling tree

* Add code class declaration writer tests

* Add property writer tests

* Add codemethodwriter tests

* Fix formatting issues

* Update method writer tests

* Add codeusing writer tests

* Fix formatting

* Add changelog entry

* Add type annotation for discriminator fields variable

* Update test

* Fix fields type hint

* Bump test coverage in classdeclarationwriter

* Add python code element order comparer

* Add code renderer for python

* Write methods before properties

* Add python element comparer tests

* Fix formatting

* Update CHANGELOG

* Remove suppressions for passing it tests

* Apply suggestions from code review

---------

Co-authored-by: Vincent Biret <[email protected]>
  • Loading branch information
samwelkanda and baywet authored Mar 22, 2023
1 parent 0119d15 commit a9a0dd1
Show file tree
Hide file tree
Showing 20 changed files with 792 additions and 106 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- Fixed a bug where lookup of reference ids failed for AllOf more than one level up.
- Fixed a bug where a CLI client would not set the content types for requests. (Shell)
- Fixed linting errors by re-ordering methods and properties in Python.
- Changed python import mechanism to facilitate code completion. [#2380](https://github.com/microsoft/kiota/issues/2380)
- Fixed a bug where discriminator methods were missing possible types in Python [#2381](https://github.com/microsoft/kiota/issues/2381)
- Fixed a bug where boolean or number enums would be mapped to enums instead of primitive types. [#2367](https://github.com/microsoft/kiota/issues/2367)
- Fixed a bug where CSharp inherited constructor name was incorrect. [#2351](https://github.com/microsoft/kiota/issues/2351)
- Fixed a bug where java refiner would emit method's parameters types without normalizing the name.
Expand All @@ -25,6 +28,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Fixed a bug where go refiner would emit incorrect code when inlining error parents
- Fixed a bug in PHP where the base URL path parameter key didn't match the URI template.


## [1.0.1] - 2023-03-11

- Fixed a bug where double would not be mapped properly.
Expand Down
12 changes: 0 additions & 12 deletions it/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -74,19 +74,11 @@
{
"Language": "php",
"Rationale": "https://github.com/microsoft/kiota/issues/2351"
},
{
"Language": "python",
"Rationale": "https://github.com/microsoft/kiota/issues/2351"
}
]
},
"./tests/Kiota.Builder.IntegrationTests/NoUnderscoresInModel.yaml": {
"Suppressions": [
{
"Language": "python",
"Rationale": "https://github.com/microsoft/kiota/issues/2361"
},
{
"Language": "ruby",
"Rationale": "https://github.com/microsoft/kiota/issues/2374"
Expand Down Expand Up @@ -124,10 +116,6 @@
{
"Language": "php",
"Rationale": "https://github.com/microsoft/kiota/issues/2378"
},
{
"Language": "python",
"Rationale": "https://github.com/microsoft/kiota/issues/2381"
}
]
},
Expand Down
6 changes: 4 additions & 2 deletions src/Kiota.Builder/CodeElementOrderComparer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,10 @@ protected virtual int GetTypeFactor(CodeElement element)
_ => 0,
};
}
private static readonly int methodKindWeight = 10;
protected static int GetMethodKindFactor(CodeElement element)

protected virtual int methodKindWeight { get; } = 10;

protected virtual int GetMethodKindFactor(CodeElement element)
{
if (element is CodeMethod method)
return method.Kind switch
Expand Down
34 changes: 34 additions & 0 deletions src/Kiota.Builder/CodeElementOrderComparerPython.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
using Kiota.Builder.CodeDOM;

namespace Kiota.Builder;
public class CodeElementOrderComparerPython : CodeElementOrderComparer
{
protected override int GetTypeFactor(CodeElement element)
{
return element switch
{
CodeUsing => 1,
ClassDeclaration => 2,
InterfaceDeclaration => 3,
CodeMethod => 4,
CodeIndexer => 5,
CodeProperty => 6,
CodeClass => 7,
BlockEnd => 8,
_ => 0,
};
}
protected override int methodKindWeight { get; } = 200;
protected override int GetMethodKindFactor(CodeElement element)
{
if (element is CodeMethod method)
return method.Kind switch
{
CodeMethodKind.ClientConstructor => 1,
CodeMethodKind.Constructor => 0,
CodeMethodKind.RawUrlConstructor => 3,
_ => 2,
};
return 0;
}
}
5 changes: 3 additions & 2 deletions src/Kiota.Builder/CodeRenderers/CodeRenderer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@ namespace Kiota.Builder.CodeRenderers;
/// </summary>
public class CodeRenderer
{
public CodeRenderer(GenerationConfiguration configuration)
public CodeRenderer(GenerationConfiguration configuration, CodeElementOrderComparer? elementComparer = null)
{
ArgumentNullException.ThrowIfNull(configuration);
_configuration = configuration;
_rendererElementComparer = configuration.ShouldRenderMethodsOutsideOfClasses ? new CodeElementOrderComparerWithExternalMethods() : new CodeElementOrderComparer();
_rendererElementComparer = elementComparer ?? (configuration.ShouldRenderMethodsOutsideOfClasses ? new CodeElementOrderComparerWithExternalMethods() : new CodeElementOrderComparer());
}
public async Task RenderCodeNamespaceToSingleFileAsync(LanguageWriter writer, CodeElement codeElement, string outputFile, CancellationToken cancellationToken)
{
Expand Down Expand Up @@ -91,6 +91,7 @@ public static CodeRenderer GetCodeRender(GenerationConfiguration config) =>
config.Language switch
{
GenerationLanguage.TypeScript => new TypeScriptCodeRenderer(config),
GenerationLanguage.Python => new PythonCodeRenderer(config),
_ => new CodeRenderer(config),
};

Expand Down
9 changes: 9 additions & 0 deletions src/Kiota.Builder/CodeRenderers/PythonCodeRenderer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
using System.Linq;
using Kiota.Builder.CodeDOM;
using Kiota.Builder.Configuration;

namespace Kiota.Builder.CodeRenderers;
public class PythonCodeRenderer : CodeRenderer
{
public PythonCodeRenderer(GenerationConfiguration configuration) : base(configuration, new CodeElementOrderComparerPython()) { }
}
20 changes: 17 additions & 3 deletions src/Kiota.Builder/Refiners/CommonLanguageRefiner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -994,7 +994,7 @@ protected static void AddParentClassToErrorClasses(CodeElement currentElement, s
}
CrawlTree(currentElement, x => AddParentClassToErrorClasses(x, parentClassName, parentClassNamespace, addNamespaceToInheritDeclaration));
}
protected static void AddDiscriminatorMappingsUsingsToParentClasses(CodeElement currentElement, string parseNodeInterfaceName, bool addFactoryMethodImport = false, bool addUsings = true)
protected static void AddDiscriminatorMappingsUsingsToParentClasses(CodeElement currentElement, string parseNodeInterfaceName, bool addFactoryMethodImport = false, bool addUsings = true, bool includeParentNamespace = false)
{
if (currentElement is CodeMethod currentMethod &&
currentMethod.Parent is CodeClass parentClass &&
Expand All @@ -1004,7 +1004,21 @@ currentMethod.Parent is CodeClass parentClass &&
(parentClass.DiscriminatorInformation?.HasBasicDiscriminatorInformation ?? false) &&
parentClass.GetImmediateParentOfType<CodeNamespace>() is CodeNamespace parentClassNamespace)
{
if (addUsings)
if (addUsings && includeParentNamespace)
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,
},
}).ToArray());
else if (addUsings && !includeParentNamespace)
declaration.AddUsings(parentClass.DiscriminatorInformation.DiscriminatorMappings
.Select(static x => x.Value)
.OfType<CodeType>()
Expand Down Expand Up @@ -1039,7 +1053,7 @@ type.TypeDefinition is CodeClass modelClass &&
});
}
}
CrawlTree(currentElement, x => AddDiscriminatorMappingsUsingsToParentClasses(x, parseNodeInterfaceName, addFactoryMethodImport, addUsings));
CrawlTree(currentElement, x => AddDiscriminatorMappingsUsingsToParentClasses(x, parseNodeInterfaceName, addFactoryMethodImport, addUsings, includeParentNamespace));
}
protected static void ReplaceLocalMethodsByGlobalFunctions(CodeElement currentElement, Func<CodeMethod, string> nameUpdateCallback, Func<CodeMethod, CodeUsing[]>? usingsCallback, params CodeMethodKind[] kindsToReplace)
{
Expand Down
9 changes: 7 additions & 2 deletions src/Kiota.Builder/Refiners/PythonRefiner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -90,15 +90,20 @@ public override Task Refine(CodeNamespace generatedCode, CancellationToken cance
AddQueryParameterMapperMethod(
generatedCode
);
AddDiscriminatorMappingsUsingsToParentClasses(
generatedCode,
"ParseNode",
addUsings: true,
includeParentNamespace: true
);
RemoveHandlerFromRequestBuilder(generatedCode);
}, cancellationToken);
}

private const string AbstractionsPackageName = "kiota_abstractions";
private static readonly AdditionalUsingEvaluator[] defaultUsingEvaluators = {
new (static x => x is CodeClass, "__future__", "annotations"),
new (static x => x is CodeClass, "typing", "Any, Callable, Dict, List, Optional, Union"),
new (static x => x is CodeClass, $"{AbstractionsPackageName}.utils", "lazy_import"),
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),
$"{AbstractionsPackageName}.request_adapter", "RequestAdapter"),
new (static x => x is CodeMethod method && method.IsOfKind(CodeMethodKind.RequestGenerator),
Expand Down
87 changes: 21 additions & 66 deletions src/Kiota.Builder/Writers/Python/CodeClassDeclarationWriter.cs
Original file line number Diff line number Diff line change
@@ -1,95 +1,50 @@
using System;
using System.Linq;

using Kiota.Builder.CodeDOM;
using Kiota.Builder.Extensions;

namespace Kiota.Builder.Writers.Python;
public class CodeClassDeclarationWriter : BaseElementWriter<ClassDeclaration, PythonConventionService>
{

public CodeClassDeclarationWriter(PythonConventionService conventionService) : base(conventionService)
private readonly CodeUsingWriter _codeUsingWriter;
public CodeClassDeclarationWriter(PythonConventionService conventionService, string clientNamespaceName) : base(conventionService)
{
_codeUsingWriter = new(clientNamespaceName);
}
public override void WriteCodeElement(ClassDeclaration codeElement, LanguageWriter writer)
{
ArgumentNullException.ThrowIfNull(codeElement);
ArgumentNullException.ThrowIfNull(writer);
WriteExternalImports(codeElement, writer); // external imports before internal imports
WriteInternalImports(codeElement, writer);
var parentNamespace = codeElement.GetImmediateParentOfType<CodeNamespace>();
_codeUsingWriter.WriteExternalImports(codeElement, writer); // external imports before internal imports
_codeUsingWriter.WriteConditionalInternalImports(codeElement, writer, parentNamespace);

if (codeElement.Parent is CodeClass parentClass)
{
if (codeElement.Inherits != null)
_codeUsingWriter.WriteDeferredImport(parentClass, codeElement.Inherits.Name, writer);
foreach (var implement in codeElement.Implements)
_codeUsingWriter.WriteDeferredImport(parentClass, implement.Name, writer);

}


var abcClass = !codeElement.Implements.Any() ? string.Empty : $"{codeElement.Implements.Select(static x => x.Name.ToFirstCharacterUpperCase()).Aggregate((x, y) => x + ", " + y)}";
var derivation = codeElement.Inherits is CodeType inheritType &&
conventions.GetTypeString(inheritType, codeElement) is string inheritSymbol &&
!string.IsNullOrEmpty(inheritSymbol) ?
inheritSymbol :
abcClass;



if (codeElement.Parent?.Parent is CodeClass)
{
writer.WriteLine("@dataclass");
}
writer.WriteLine($"class {codeElement.Name.ToFirstCharacterUpperCase()}({derivation}):");
writer.IncreaseIndent();
if (codeElement.Parent is CodeClass parentClass)
conventions.WriteShortDescription(parentClass.Documentation.Description, writer);
}

private static void WriteExternalImports(ClassDeclaration codeElement, LanguageWriter writer)
{
var externalImportSymbolsAndPaths = codeElement.Usings
.Where(static x => x.IsExternal)
.Select(x => (x.Name, string.Empty, x.Declaration?.Name))
.GroupBy(x => x.Item3)
.OrderBy(x => x.Key);
if (externalImportSymbolsAndPaths.Any())
{
foreach (var codeUsing in externalImportSymbolsAndPaths)
if (!string.IsNullOrWhiteSpace(codeUsing.Key))
{
if (codeUsing.Key == "-")
writer.WriteLine($"import {codeUsing.Select(x => GetAliasedSymbol(x.Item1, x.Item2)).Distinct().OrderBy(x => x).Aggregate((x, y) => x + ", " + y)}");
else
writer.WriteLine($"from {codeUsing.Key.ToSnakeCase()} import {codeUsing.Select(x => GetAliasedSymbol(x.Item1, x.Item2)).Distinct().OrderBy(x => x).Aggregate((x, y) => x + ", " + y)}");
}
writer.WriteLine();
}
}

private static void WriteInternalImports(ClassDeclaration codeElement, LanguageWriter writer)
{
var internalImportSymbolsAndPaths = codeElement.Usings
.Where(x => !x.IsExternal)
.Select(static x => GetImportPathForUsing(x))
.GroupBy(x => x.Item3)
.Where(x => !string.IsNullOrEmpty(x.Key))
.OrderBy(x => x.Key);
if (internalImportSymbolsAndPaths.Any())
{
foreach (var codeUsing in internalImportSymbolsAndPaths)
foreach (var symbol in codeUsing.Select(static x => GetAliasedSymbol(x.Item1, x.Item2)).Distinct().OrderBy(static x => x))
writer.WriteLine($"{symbol} = lazy_import('{codeUsing.Key.ToSnakeCase().Replace("._", ".")}.{symbol}')");
writer.WriteLine();
}
}
private static string GetAliasedSymbol(string symbol, string alias)
{
return string.IsNullOrEmpty(alias) ? symbol : $"{symbol} as {alias}";
}

/// <summary>
/// Returns the import path for the given using and import context namespace.
/// </summary>
/// <param name="codeUsing">The using to import into the current namespace context</param>
/// <returns>The import symbol, it's alias if any and the import path</returns>
private static (string, string, string) GetImportPathForUsing(CodeUsing codeUsing)
{
var typeDef = codeUsing.Declaration?.TypeDefinition;
if (typeDef == null)
return (codeUsing.Name, codeUsing.Alias, ""); // it's relative to the folder, with no declaration or type definition (default failsafe)

var importSymbol = typeDef.Name.ToSnakeCase();

var importPath = typeDef.GetImmediateParentOfType<CodeNamespace>().Name;
return (importSymbol, codeUsing.Alias, importPath);
if (codeElement.Parent is CodeClass parent)
conventions.WriteShortDescription(parent.Documentation.Description, writer);
}
}
Loading

0 comments on commit a9a0dd1

Please sign in to comment.