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

Task/python imports refactor #2379

Merged
merged 44 commits into from
Mar 22, 2023
Merged
Show file tree
Hide file tree
Changes from 42 commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
032e5d0
Add python relative import manager
samwelkanda Mar 8, 2023
7423f0d
Add a codeusingwriter for python
samwelkanda Mar 9, 2023
5fe8d47
Add option to include parent namespace when adding discriminator mapp…
samwelkanda Mar 9, 2023
a8ac925
Call method to add discriminator mapping usings to parent classes
samwelkanda Mar 9, 2023
4b705b7
Add type checking import
samwelkanda Mar 9, 2023
c69c961
Remove lazy import using
samwelkanda Mar 9, 2023
bc7fa36
Update class declaration writer
samwelkanda Mar 9, 2023
4a8eb37
Update property writer to add local imports for request builders with…
samwelkanda Mar 9, 2023
d02f6b6
Add logic to write discriminator method body
samwelkanda Mar 9, 2023
fb1868a
Write local imports in indexers and request builders with parameters
samwelkanda Mar 9, 2023
6134aa4
Add deferred imports for request executors and deserializers
samwelkanda Mar 9, 2023
bcde39f
Use snake case to match variable name passed to request builder
samwelkanda Mar 9, 2023
504668b
Update python writer with missing parameters
samwelkanda Mar 9, 2023
68788b5
Add incluparentnamespace parameter when crawling tree
samwelkanda Mar 9, 2023
a7f9e86
Add code class declaration writer tests
samwelkanda Mar 9, 2023
2860b65
Add property writer tests
samwelkanda Mar 9, 2023
0fe2f5f
Add codemethodwriter tests
samwelkanda Mar 9, 2023
3cc823b
Merge branch 'main' into task/python-imports-refactor
samwelkanda Mar 10, 2023
b858fa4
Fix formatting issues
samwelkanda Mar 10, 2023
ef29354
Update method writer tests
samwelkanda Mar 10, 2023
cf7427e
Add codeusing writer tests
samwelkanda Mar 10, 2023
0f13120
Fix formatting
samwelkanda Mar 10, 2023
07f5d0c
Add changelog entry
samwelkanda Mar 10, 2023
7a2d47c
Add type annotation for discriminator fields variable
samwelkanda Mar 13, 2023
e3ac57a
Update test
samwelkanda Mar 13, 2023
d02fd21
Fix fields type hint
samwelkanda Mar 13, 2023
fc9ba34
Merge branch 'main' into task/python-imports-refactor
samwelkanda Mar 16, 2023
9e2cc79
Bump test coverage in classdeclarationwriter
samwelkanda Mar 16, 2023
55b79a0
Merge branch 'main' into task/python-imports-refactor
samwelkanda Mar 20, 2023
7cec2a2
Add python code element order comparer
samwelkanda Mar 21, 2023
17078da
Add code renderer for python
samwelkanda Mar 21, 2023
b3d0f1b
Write methods before properties
samwelkanda Mar 21, 2023
71bf775
Add python element comparer tests
samwelkanda Mar 21, 2023
d652177
Merge branch 'main' into task/python-imports-refactor
samwelkanda Mar 21, 2023
3b40157
Fix formatting
samwelkanda Mar 21, 2023
5cf3d83
Update CHANGELOG
samwelkanda Mar 21, 2023
a6c5301
Remove suppressions for passing it tests
samwelkanda Mar 21, 2023
ef4bbed
Merge branch 'main' into task/python-imports-refactor
samwelkanda Mar 22, 2023
75d51f3
Merge branch 'main' into task/python-imports-refactor
samwelkanda Mar 22, 2023
362cab5
Merge branch 'main' into task/python-imports-refactor
baywet Mar 22, 2023
49e7fc7
Apply suggestions from code review
baywet Mar 22, 2023
de51794
Update src/Kiota.Builder/Writers/Python/CodeUsingWriter.cs
baywet Mar 22, 2023
ecf2e07
- fixes failing unit test
baywet Mar 22, 2023
f882f33
Merge branch 'main' into task/python-imports-refactor
baywet Mar 22, 2023
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
5 changes: 4 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
### Changed

- 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 +27,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