From 6ce49f8a92fb633f4741be1aa94e37d111a4610c Mon Sep 17 00:00:00 2001 From: rkodev <43806892+rkodev@users.noreply.github.com> Date: Wed, 25 Sep 2024 17:41:48 +0300 Subject: [PATCH 1/8] detect cyclic refrences --- src/Kiota.Builder/Refiners/GoRefiner.cs | 89 +++++++++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/src/Kiota.Builder/Refiners/GoRefiner.cs b/src/Kiota.Builder/Refiners/GoRefiner.cs index ca5480c6cb..c0593601d4 100644 --- a/src/Kiota.Builder/Refiners/GoRefiner.cs +++ b/src/Kiota.Builder/Refiners/GoRefiner.cs @@ -201,6 +201,7 @@ public override Task RefineAsync(CodeNamespace generatedCode, CancellationToken x => $"{x.Name}able" ); AddContextParameterToGeneratorMethods(generatedCode); + CorrectCyclicReference(generatedCode); CorrectTypes(generatedCode); CorrectCoreTypesForBackingStore(generatedCode, $"{conventions.StoreHash}.BackingStoreFactoryInstance()", false); CorrectBackingStoreTypes(generatedCode); @@ -218,6 +219,94 @@ public override Task RefineAsync(CodeNamespace generatedCode, CancellationToken }, cancellationToken); } + private void CorrectCyclicReference(CodeElement currentElement) + { + var currentNameSpace = currentElement.GetImmediateParentOfType(); + var modelsNameSpace = findClientNameSpace(currentNameSpace) + ?.FindNamespaceByName($"{_configuration.ClientNamespaceName}.{GenerationConfiguration.ModelsNamespaceSegmentName}"); + + if (modelsNameSpace == null) + throw new InvalidOperationException($"missing models namespace"); + + var dependencies = new Dictionary>(); + GetUsingsInModelsNameSpace(modelsNameSpace, modelsNameSpace, dependencies); + + // check cycles + var cycles = FindCycles(dependencies); + foreach (var cycle in cycles) + { + //Console.WriteLine(string.Join(" -> ", cycle)); + // TODO move the classes to the root namespace + //FlattenNameSpace(cycle, modelsNameSpace); + } + } + + private void FlattenNameSpace(CodeNamespace currentNameSpace, CodeNamespace targetNameSpace) + { + + } + + private void GetUsingsInModelsNameSpace(CodeNamespace modelsNameSpace, CodeNamespace currentNameSpace, Dictionary> dependencies) + { + if (!modelsNameSpace.Name.Equals(currentNameSpace.Name, StringComparison.OrdinalIgnoreCase) && !currentNameSpace.IsChildOf(modelsNameSpace)) + return; + + currentNameSpace.Namespaces + .Where(codeNameSpace => !dependencies.ContainsKey(codeNameSpace.Name)) + .ToList() + .ForEach(codeNameSpace => + { + dependencies[codeNameSpace.Name] = codeNameSpace.Classes + .SelectMany(codeClass => codeClass.Usings) + .Select(usingStatement => usingStatement.Name) + .ToHashSet(); + + GetUsingsInModelsNameSpace(modelsNameSpace, codeNameSpace, dependencies); + }); + } + + static Dictionary> FindCycles(Dictionary> dependencies) + { + var cycles = new Dictionary>(); + var visited = new HashSet(); + var stack = new Stack(); + + foreach (var node in dependencies.Keys) + { + if (!visited.Contains(node)) + { + Dfs(node, dependencies, visited, stack, cycles); + } + } + + return cycles; + } + + static void Dfs(string node, Dictionary> dependencies, HashSet visited, Stack stack, Dictionary> cycles) + { + visited.Add(node); + stack.Push(node); + + if (dependencies.TryGetValue(node, out var value)) + { + foreach (var neighbor in value) + { + if (stack.Contains(neighbor)) + { + var cycle = stack.Reverse().TakeWhile(n => n != neighbor).Reverse().Concat(new[] { neighbor }) + .ToList(); + cycles[node] = cycle; + } + else if (!visited.Contains(neighbor)) + { + Dfs(neighbor, dependencies, visited, stack, cycles); + } + } + } + + stack.Pop(); + } + private void GenerateCodeFiles(CodeElement currentElement) { if (currentElement is CodeInterface codeInterface && currentElement.Parent is CodeNamespace codeNamespace) From ecf8ebfa4e49271719d2e74bd8a388297325f798 Mon Sep 17 00:00:00 2001 From: rkodev <43806892+rkodev@users.noreply.github.com> Date: Wed, 25 Sep 2024 19:14:38 +0300 Subject: [PATCH 2/8] move all elements to the root dependency --- src/Kiota.Builder/CodeDOM/CodeNamespace.cs | 4 +- src/Kiota.Builder/Refiners/GoRefiner.cs | 99 +++++++++++++++++----- 2 files changed, 80 insertions(+), 23 deletions(-) diff --git a/src/Kiota.Builder/CodeDOM/CodeNamespace.cs b/src/Kiota.Builder/CodeDOM/CodeNamespace.cs index e6556a0c75..846fc67ed8 100644 --- a/src/Kiota.Builder/CodeDOM/CodeNamespace.cs +++ b/src/Kiota.Builder/CodeDOM/CodeNamespace.cs @@ -72,11 +72,11 @@ public CodeNamespace GetRootNamespace() public IEnumerable Interfaces => InnerChildElements.Values.OfType(); public IEnumerable Constants => InnerChildElements.Values.OfType(); public IEnumerable Files => InnerChildElements.Values.OfType(); - public CodeNamespace? FindNamespaceByName(string nsName) + public CodeNamespace? FindNamespaceByName(string nsName, bool findInChildElements = false) { ArgumentException.ThrowIfNullOrEmpty(nsName); if (nsName.Equals(Name, StringComparison.OrdinalIgnoreCase)) return this; - var result = FindChildByName(nsName, false); + var result = FindChildByName(nsName, findInChildElements); if (result == null) foreach (var childNS in InnerChildElements.Values.OfType()) { diff --git a/src/Kiota.Builder/Refiners/GoRefiner.cs b/src/Kiota.Builder/Refiners/GoRefiner.cs index c0593601d4..1fd378fa68 100644 --- a/src/Kiota.Builder/Refiners/GoRefiner.cs +++ b/src/Kiota.Builder/Refiners/GoRefiner.cs @@ -226,24 +226,79 @@ private void CorrectCyclicReference(CodeElement currentElement) ?.FindNamespaceByName($"{_configuration.ClientNamespaceName}.{GenerationConfiguration.ModelsNamespaceSegmentName}"); if (modelsNameSpace == null) - throw new InvalidOperationException($"missing models namespace"); + throw new InvalidOperationException("missing models namespace"); var dependencies = new Dictionary>(); GetUsingsInModelsNameSpace(modelsNameSpace, modelsNameSpace, dependencies); // check cycles + var migratedNamespaces = new Dictionary(); var cycles = FindCycles(dependencies); foreach (var cycle in cycles) { - //Console.WriteLine(string.Join(" -> ", cycle)); - // TODO move the classes to the root namespace - //FlattenNameSpace(cycle, modelsNameSpace); + foreach (var duplicate in cycle.Value) + { + var dupNS = duplicate[^2]; // 2nd last element is the duplicate namespace + var nameSpace = modelsNameSpace.FindNamespaceByName(dupNS, true); + var rootNameSpace = cycle.Key.Equals(modelsNameSpace.Name, StringComparison.OrdinalIgnoreCase) ? modelsNameSpace : modelsNameSpace.FindNamespaceByName(cycle.Key, true); + migratedNamespaces[dupNS] = cycle.Key; + FlattenNameSpace(nameSpace!, rootNameSpace!); + } + } + + // update all elements that have a migrated namespace + foreach (var migrated in migratedNamespaces) + { + // TODO + // update all elements that have a migrated namespace } } private void FlattenNameSpace(CodeNamespace currentNameSpace, CodeNamespace targetNameSpace) { + Func GetComposedName = codeClass => + { + var classNameList = getPathsName(codeClass, codeClass.Name); + return string.Join(string.Empty, classNameList.Count > 1 ? classNameList.Skip(1) : classNameList); + }; + currentNameSpace.Classes.ToList().ForEach(x => + { + currentNameSpace.RemoveChildElement(x); + x.Name = GetComposedName(x); + x.Parent = targetNameSpace; + targetNameSpace.AddClass(x); + }); + currentNameSpace.Enums.ToList().ForEach(x => + { + currentNameSpace.RemoveChildElement(x); + x.Name = GetComposedName(x); + x.Parent = targetNameSpace; + targetNameSpace.AddEnum(x); + }); + currentNameSpace.Interfaces.ToList().ForEach(x => + { + currentNameSpace.RemoveChildElement(x); + x.Name = GetComposedName(x); + x.Parent = targetNameSpace; + targetNameSpace.AddInterface(x); + }); + currentNameSpace.Functions.ToList().ForEach(x => + { + currentNameSpace.RemoveChildElement(x); + x.Name = GetComposedName(x); + x.Parent = targetNameSpace; + targetNameSpace.AddFunction(x); + }); + currentNameSpace.Constants.ToList().ForEach(x => + { + currentNameSpace.RemoveChildElement(x); + x.Name = GetComposedName(x); + x.Parent = targetNameSpace; + targetNameSpace.AddConstant(x); + }); + + currentNameSpace.Namespaces.ToList().ForEach(x => FlattenNameSpace(x, targetNameSpace)); } private void GetUsingsInModelsNameSpace(CodeNamespace modelsNameSpace, CodeNamespace currentNameSpace, Dictionary> dependencies) @@ -251,23 +306,23 @@ private void GetUsingsInModelsNameSpace(CodeNamespace modelsNameSpace, CodeNames if (!modelsNameSpace.Name.Equals(currentNameSpace.Name, StringComparison.OrdinalIgnoreCase) && !currentNameSpace.IsChildOf(modelsNameSpace)) return; + dependencies[currentNameSpace.Name] = currentNameSpace.Classes + .SelectMany(codeClass => codeClass.Usings) + .Where(x => x.Declaration != null && !x.Declaration.IsExternal) + .Select(static x => x.Declaration?.TypeDefinition?.GetImmediateParentOfType()) + .Where(ns => ns != null && ns.Name != currentNameSpace.Name) + .Select(static ns => ns!.Name) + .ToHashSet(); + currentNameSpace.Namespaces .Where(codeNameSpace => !dependencies.ContainsKey(codeNameSpace.Name)) .ToList() - .ForEach(codeNameSpace => - { - dependencies[codeNameSpace.Name] = codeNameSpace.Classes - .SelectMany(codeClass => codeClass.Usings) - .Select(usingStatement => usingStatement.Name) - .ToHashSet(); - - GetUsingsInModelsNameSpace(modelsNameSpace, codeNameSpace, dependencies); - }); + .ForEach(codeNameSpace => GetUsingsInModelsNameSpace(modelsNameSpace, codeNameSpace, dependencies)); } - static Dictionary> FindCycles(Dictionary> dependencies) + static Dictionary>> FindCycles(Dictionary> dependencies) { - var cycles = new Dictionary>(); + var cycles = new Dictionary>>(); var visited = new HashSet(); var stack = new Stack(); @@ -275,14 +330,14 @@ static Dictionary> FindCycles(Dictionary> dependencies, HashSet visited, Stack stack, Dictionary> cycles) + static void Dfs(string parentNode, string node, Dictionary> dependencies, HashSet visited, Stack stack, Dictionary>> cycles) { visited.Add(node); stack.Push(node); @@ -293,13 +348,15 @@ static void Dfs(string node, Dictionary> dependencies, H { if (stack.Contains(neighbor)) { - var cycle = stack.Reverse().TakeWhile(n => n != neighbor).Reverse().Concat(new[] { neighbor }) - .ToList(); - cycles[node] = cycle; + var cycle = stack.Reverse().Concat(new[] { neighbor }).ToList(); + if (!cycles.ContainsKey(parentNode)) + cycles[parentNode] = new List>(); + + cycles[parentNode].Add(cycle); } else if (!visited.Contains(neighbor)) { - Dfs(neighbor, dependencies, visited, stack, cycles); + Dfs(parentNode, neighbor, dependencies, visited, stack, cycles); } } } From 689c94d9b01bb61ed90e60e77ffe960bdf675c29 Mon Sep 17 00:00:00 2001 From: rkodev <43806892+rkodev@users.noreply.github.com> Date: Wed, 25 Sep 2024 22:33:01 +0300 Subject: [PATCH 3/8] enable tests --- it/config.json | 12 ----------- src/Kiota.Builder/Refiners/GoRefiner.cs | 28 +++++++++++-------------- 2 files changed, 12 insertions(+), 28 deletions(-) diff --git a/it/config.json b/it/config.json index a85fc9c22b..f6e1a2ae70 100644 --- a/it/config.json +++ b/it/config.json @@ -148,10 +148,6 @@ } ], "Suppressions": [ - { - "Language": "go", - "Rationale": "https://github.com/microsoft/kiota/issues/3436" - }, { "Language": "ruby", "Rationale": "https://github.com/microsoft/kiota/issues/2484" @@ -237,10 +233,6 @@ "Language": "typescript", "Rationale": "https://github.com/microsoft/kiota/issues/5256" }, - { - "Language": "go", - "Rationale": "https://github.com/microsoft/kiota/issues/2834" - }, { "Language": "java", "Rationale": "https://github.com/microsoft/kiota/issues/2842" @@ -259,10 +251,6 @@ } ], "IdempotencySuppressions": [ - { - "Language": "go", - "Rationale": "https://github.com/microsoft/kiota/issues/2834" - }, { "Language": "java", "Rationale": "https://github.com/microsoft/kiota/issues/2842" diff --git a/src/Kiota.Builder/Refiners/GoRefiner.cs b/src/Kiota.Builder/Refiners/GoRefiner.cs index 1fd378fa68..8a67af1352 100644 --- a/src/Kiota.Builder/Refiners/GoRefiner.cs +++ b/src/Kiota.Builder/Refiners/GoRefiner.cs @@ -223,7 +223,8 @@ private void CorrectCyclicReference(CodeElement currentElement) { var currentNameSpace = currentElement.GetImmediateParentOfType(); var modelsNameSpace = findClientNameSpace(currentNameSpace) - ?.FindNamespaceByName($"{_configuration.ClientNamespaceName}.{GenerationConfiguration.ModelsNamespaceSegmentName}"); + ?.FindNamespaceByName( + $"{_configuration.ClientNamespaceName}.{GenerationConfiguration.ModelsNamespaceSegmentName}"); if (modelsNameSpace == null) throw new InvalidOperationException("missing models namespace"); @@ -231,7 +232,6 @@ private void CorrectCyclicReference(CodeElement currentElement) var dependencies = new Dictionary>(); GetUsingsInModelsNameSpace(modelsNameSpace, modelsNameSpace, dependencies); - // check cycles var migratedNamespaces = new Dictionary(); var cycles = FindCycles(dependencies); foreach (var cycle in cycles) @@ -240,28 +240,17 @@ private void CorrectCyclicReference(CodeElement currentElement) { var dupNS = duplicate[^2]; // 2nd last element is the duplicate namespace var nameSpace = modelsNameSpace.FindNamespaceByName(dupNS, true); - var rootNameSpace = cycle.Key.Equals(modelsNameSpace.Name, StringComparison.OrdinalIgnoreCase) ? modelsNameSpace : modelsNameSpace.FindNamespaceByName(cycle.Key, true); + var rootNameSpace = cycle.Key.Equals(modelsNameSpace.Name, StringComparison.OrdinalIgnoreCase) + ? modelsNameSpace + : modelsNameSpace.FindNamespaceByName(cycle.Key, true); migratedNamespaces[dupNS] = cycle.Key; FlattenNameSpace(nameSpace!, rootNameSpace!); } } - - // update all elements that have a migrated namespace - foreach (var migrated in migratedNamespaces) - { - // TODO - // update all elements that have a migrated namespace - } } private void FlattenNameSpace(CodeNamespace currentNameSpace, CodeNamespace targetNameSpace) { - Func GetComposedName = codeClass => - { - var classNameList = getPathsName(codeClass, codeClass.Name); - return string.Join(string.Empty, classNameList.Count > 1 ? classNameList.Skip(1) : classNameList); - }; - currentNameSpace.Classes.ToList().ForEach(x => { currentNameSpace.RemoveChildElement(x); @@ -299,6 +288,13 @@ private void FlattenNameSpace(CodeNamespace currentNameSpace, CodeNamespace targ }); currentNameSpace.Namespaces.ToList().ForEach(x => FlattenNameSpace(x, targetNameSpace)); + return; + + string GetComposedName(CodeElement codeClass) + { + var classNameList = getPathsName(codeClass, codeClass.Name); + return string.Join(string.Empty, classNameList.Count > 1 ? classNameList.Skip(1) : classNameList); + } } private void GetUsingsInModelsNameSpace(CodeNamespace modelsNameSpace, CodeNamespace currentNameSpace, Dictionary> dependencies) From a7c858c5da973811347936e8cd020c6b2f5eeffc Mon Sep 17 00:00:00 2001 From: rkodev <43806892+rkodev@users.noreply.github.com> Date: Thu, 26 Sep 2024 08:55:44 +0300 Subject: [PATCH 4/8] Adds unit tests --- CHANGELOG.md | 1 + src/Kiota.Builder/Refiners/GoRefiner.cs | 175 +++++++++++------- .../Refiners/GoLanguageRefinerTests.cs | 47 +++++ 3 files changed, 157 insertions(+), 66 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e9e44af4f6..a00ec3cb5f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fixed a bug where python constructor parameters are being cast to strings leading to bugs as the types is unknown on graph call. [microsoftgraph/msgraph-sdk-python#165](https://github.com/microsoftgraph/msgraph-sdk-python/issues/165) - Fixed a bug where child path segment from single parameter path segment would be incorrectly escaped. [#5433](https://github.com/microsoft/kiota/issues/5433) - Fixed inconsistent typing information generated for `ParsableFactory` and stream return types in python [kiota-abstractions-python#533](https://github.com/microsoft/kiota-abstractions-python/issues/333) +- Fixed cyclic depencies in generated Go code. [#2834](https://github.com/microsoft/kiota/issues/2834) ## [1.18.0] - 2024-09-05 diff --git a/src/Kiota.Builder/Refiners/GoRefiner.cs b/src/Kiota.Builder/Refiners/GoRefiner.cs index 8a67af1352..f4fbfe1e4b 100644 --- a/src/Kiota.Builder/Refiners/GoRefiner.cs +++ b/src/Kiota.Builder/Refiners/GoRefiner.cs @@ -37,8 +37,8 @@ public override Task RefineAsync(CodeNamespace generatedCode, CancellationToken static x => x.ToFirstCharacterLowerCase(), GenerationLanguage.Go); FlattenNestedHierarchy(generatedCode); - FlattenGoParamsFileNames(generatedCode); - FlattenGoFileNames(generatedCode); + FlattenParamsFileNames(generatedCode); + FlattenFileNames(generatedCode); NormalizeNamespaceNames(generatedCode); AddInnerClasses( generatedCode, @@ -196,12 +196,12 @@ public override Task RefineAsync(CodeNamespace generatedCode, CancellationToken generatedCode ); cancellationToken.ThrowIfCancellationRequested(); + CorrectCyclicReference(generatedCode); CopyModelClassesAsInterfaces( generatedCode, x => $"{x.Name}able" ); AddContextParameterToGeneratorMethods(generatedCode); - CorrectCyclicReference(generatedCode); CorrectTypes(generatedCode); CorrectCoreTypesForBackingStore(generatedCode, $"{conventions.StoreHash}.BackingStoreFactoryInstance()", false); CorrectBackingStoreTypes(generatedCode); @@ -227,7 +227,7 @@ private void CorrectCyclicReference(CodeElement currentElement) $"{_configuration.ClientNamespaceName}.{GenerationConfiguration.ModelsNamespaceSegmentName}"); if (modelsNameSpace == null) - throw new InvalidOperationException("missing models namespace"); + return; var dependencies = new Dictionary>(); GetUsingsInModelsNameSpace(modelsNameSpace, modelsNameSpace, dependencies); @@ -236,65 +236,29 @@ private void CorrectCyclicReference(CodeElement currentElement) var cycles = FindCycles(dependencies); foreach (var cycle in cycles) { - foreach (var duplicate in cycle.Value) + foreach (var cycleReference in cycle.Value) { - var dupNS = duplicate[^2]; // 2nd last element is the duplicate namespace + var dupNS = cycleReference[^2]; // 2nd last element is target base namespace var nameSpace = modelsNameSpace.FindNamespaceByName(dupNS, true); - var rootNameSpace = cycle.Key.Equals(modelsNameSpace.Name, StringComparison.OrdinalIgnoreCase) - ? modelsNameSpace - : modelsNameSpace.FindNamespaceByName(cycle.Key, true); - migratedNamespaces[dupNS] = cycle.Key; - FlattenNameSpace(nameSpace!, rootNameSpace!); + + migratedNamespaces[dupNS] = modelsNameSpace.Name; + MigrateNameSpace(nameSpace!, modelsNameSpace); + + if (!cycle.Key.Equals(modelsNameSpace.Name, StringComparison.OrdinalIgnoreCase) && !migratedNamespaces.ContainsKey(cycle.Key)) + { + migratedNamespaces[cycle.Key] = modelsNameSpace.Name; + MigrateNameSpace(modelsNameSpace.FindNamespaceByName(cycle.Key, true)!, modelsNameSpace); + } } } + + CorrectReferencesToMigratedModels(currentElement, migratedNamespaces); } - private void FlattenNameSpace(CodeNamespace currentNameSpace, CodeNamespace targetNameSpace) + private string GetComposedName(CodeElement codeClass) { - currentNameSpace.Classes.ToList().ForEach(x => - { - currentNameSpace.RemoveChildElement(x); - x.Name = GetComposedName(x); - x.Parent = targetNameSpace; - targetNameSpace.AddClass(x); - }); - currentNameSpace.Enums.ToList().ForEach(x => - { - currentNameSpace.RemoveChildElement(x); - x.Name = GetComposedName(x); - x.Parent = targetNameSpace; - targetNameSpace.AddEnum(x); - }); - currentNameSpace.Interfaces.ToList().ForEach(x => - { - currentNameSpace.RemoveChildElement(x); - x.Name = GetComposedName(x); - x.Parent = targetNameSpace; - targetNameSpace.AddInterface(x); - }); - currentNameSpace.Functions.ToList().ForEach(x => - { - currentNameSpace.RemoveChildElement(x); - x.Name = GetComposedName(x); - x.Parent = targetNameSpace; - targetNameSpace.AddFunction(x); - }); - currentNameSpace.Constants.ToList().ForEach(x => - { - currentNameSpace.RemoveChildElement(x); - x.Name = GetComposedName(x); - x.Parent = targetNameSpace; - targetNameSpace.AddConstant(x); - }); - - currentNameSpace.Namespaces.ToList().ForEach(x => FlattenNameSpace(x, targetNameSpace)); - return; - - string GetComposedName(CodeElement codeClass) - { - var classNameList = getPathsName(codeClass, codeClass.Name); - return string.Join(string.Empty, classNameList.Count > 1 ? classNameList.Skip(1) : classNameList); - } + var classNameList = getPathsName(codeClass, codeClass.Name); + return string.Join(string.Empty, classNameList.Count > 1 ? classNameList.Skip(1) : classNameList); } private void GetUsingsInModelsNameSpace(CodeNamespace modelsNameSpace, CodeNamespace currentNameSpace, Dictionary> dependencies) @@ -316,6 +280,12 @@ private void GetUsingsInModelsNameSpace(CodeNamespace modelsNameSpace, CodeNames .ForEach(codeNameSpace => GetUsingsInModelsNameSpace(modelsNameSpace, codeNameSpace, dependencies)); } + /// + /// Returns a dictionary of cycles in the graph with the key being the base namespace and the values being the path to the cycle + /// In GoLang, any self referencing namespace in a tree is a cycle, therefore the whole namespace is moved to the root + /// + /// + /// static Dictionary>> FindCycles(Dictionary> dependencies) { var cycles = new Dictionary>>(); @@ -326,14 +296,18 @@ static Dictionary>> FindCycles(Dictionary> dependencies, HashSet visited, Stack stack, Dictionary>> cycles) + + /// + /// Performs a DFS search to find cycles in the graph. Method will stop at the first cycle found in each node + /// + static void SearchCycles(string parentNode, string node, Dictionary> dependencies, HashSet visited, Stack stack, Dictionary>> cycles) { visited.Add(node); stack.Push(node); @@ -352,7 +326,7 @@ static void Dfs(string parentNode, string node, Dictionary + { + currentNameSpace.RemoveChildElement(x); + x.Name = GetComposedName(x); + x.Parent = targetNameSpace; + targetNameSpace.AddClass(x); + }); + currentNameSpace.Enums.ToList().ForEach(x => + { + currentNameSpace.RemoveChildElement(x); + x.Name = GetComposedName(x); + x.Parent = targetNameSpace; + targetNameSpace.AddEnum(x); + }); + currentNameSpace.Interfaces.ToList().ForEach(x => + { + currentNameSpace.RemoveChildElement(x); + x.Name = GetComposedName(x); + x.Parent = targetNameSpace; + targetNameSpace.AddInterface(x); + }); + currentNameSpace.Functions.ToList().ForEach(x => + { + currentNameSpace.RemoveChildElement(x); + x.Name = GetComposedName(x); + x.Parent = targetNameSpace; + targetNameSpace.AddFunction(x); + }); + currentNameSpace.Constants.ToList().ForEach(x => + { + currentNameSpace.RemoveChildElement(x); + x.Name = GetComposedName(x); + x.Parent = targetNameSpace; + targetNameSpace.AddConstant(x); + }); + + currentNameSpace.Namespaces.ToList().ForEach(x => MigrateNameSpace(x, targetNameSpace)); + } + private void CorrectReferencesToMigratedModels(CodeElement currentElement, Dictionary migratedNamespaces) + { + if (currentElement is CodeNamespace cn) + { + var usings = cn.GetChildElements() + .SelectMany(static x => x.GetChildElements(false)) + .OfType() + .SelectMany(static x => x.Usings) + .Where(x => migratedNamespaces.ContainsKey(x.Name)) + .ToArray(); + + foreach (var codeUsing in usings) + { + if (codeUsing.Parent is not ProprietableBlockDeclaration blockDeclaration || + codeUsing.Declaration?.TypeDefinition?.GetImmediateParentOfType().Name != + migratedNamespaces[codeUsing.Name]) + { + continue; + } + + blockDeclaration.RemoveUsings(codeUsing); + blockDeclaration.AddUsings(new CodeUsing + { + Name = migratedNamespaces[codeUsing.Name], + Declaration = codeUsing.Declaration + }); + } + } + + CrawlTree(currentElement, x => CorrectReferencesToMigratedModels(x, migratedNamespaces)); + } private void GenerateCodeFiles(CodeElement currentElement) { if (currentElement is CodeInterface codeInterface && currentElement.Parent is CodeNamespace codeNamespace) @@ -485,11 +530,9 @@ private void FlattenNestedHierarchy(CodeElement currentElement) var packageRootNameSpace = findNameSpaceAtLevel(parentNameSpace, currentNamespace, 1); if (!packageRootNameSpace.Name.Equals(currentNamespace.Name, StringComparison.Ordinal) && modelNameSpace != null && !currentNamespace.IsChildOf(modelNameSpace)) { - var classNameList = getPathsName(codeClass, codeClass.Name); - var newClassName = string.Join(string.Empty, classNameList.Count > 1 ? classNameList.Skip(1) : classNameList); currentNamespace.RemoveChildElement(codeClass); - codeClass.Name = newClassName; + codeClass.Name = GetComposedName(codeClass); codeClass.Parent = packageRootNameSpace; packageRootNameSpace.AddClass(codeClass); } @@ -498,7 +541,7 @@ private void FlattenNestedHierarchy(CodeElement currentElement) CrawlTree(currentElement, FlattenNestedHierarchy); } - private void FlattenGoParamsFileNames(CodeElement currentElement) + private void FlattenParamsFileNames(CodeElement currentElement) { if (currentElement is CodeMethod codeMethod && codeMethod.IsOfKind(CodeMethodKind.RequestGenerator, CodeMethodKind.RequestExecutor)) @@ -513,7 +556,7 @@ private void FlattenGoParamsFileNames(CodeElement currentElement) } - CrawlTree(currentElement, FlattenGoParamsFileNames); + CrawlTree(currentElement, FlattenParamsFileNames); } private List getPathsName(CodeElement codeClass, string fileName, bool removeDuplicate = true) @@ -551,7 +594,7 @@ private static CodeNamespace findNameSpaceAtLevel(CodeNamespace rootNameSpace, C return namespaceList[^level]; } - private void FlattenGoFileNames(CodeElement currentElement) + private void FlattenFileNames(CodeElement currentElement) { // add the namespace to the name of the code element and the file name if (currentElement is CodeClass codeClass @@ -571,7 +614,7 @@ private void FlattenGoFileNames(CodeElement currentElement) nextNameSpace.AddClass(codeClass); } - CrawlTree(currentElement, FlattenGoFileNames); + CrawlTree(currentElement, FlattenFileNames); } private static void FixConstructorClashes(CodeElement currentElement, Func nameCorrection) diff --git a/tests/Kiota.Builder.Tests/Refiners/GoLanguageRefinerTests.cs b/tests/Kiota.Builder.Tests/Refiners/GoLanguageRefinerTests.cs index 8994eb193b..995384b774 100644 --- a/tests/Kiota.Builder.Tests/Refiners/GoLanguageRefinerTests.cs +++ b/tests/Kiota.Builder.Tests/Refiners/GoLanguageRefinerTests.cs @@ -841,6 +841,53 @@ public async Task ReplacesDurationByNativeTypeAsync() Assert.NotEmpty(model.StartBlock.Usings); Assert.Equal("ISODuration", method.ReturnType.Name); } + [Fact] + public async Task CorrectesCircularDependencies() + { + var modelsNS = root.AddNamespace("ApiSdk.models"); + + var subANamespace = modelsNS.AddNamespace($"{modelsNS.Name}.suba"); + var modelA = subANamespace.AddClass(new CodeClass + { + Kind = CodeClassKind.Model, + Name = "ModelA", + }).First(); + + var subBNamespace = modelsNS.AddNamespace($"{modelsNS.Name}.subb"); + var modelB = subBNamespace.AddClass(new CodeClass + { + Kind = CodeClassKind.Model, + Name = "ModelB", + }).First(); + + modelA.StartBlock.AddUsings(new CodeUsing + { + Name = subBNamespace.Name, + Declaration = new() + { + Name = modelB.Name, + TypeDefinition = modelB, + IsExternal = false + } + }); + + modelB.StartBlock.AddUsings(new CodeUsing + { + Name = subANamespace.Name, + Declaration = new() + { + Name = modelA.Name, + TypeDefinition = modelA, + IsExternal = false + } + }); + + await ILanguageRefiner.RefineAsync(new GenerationConfiguration { Language = GenerationLanguage.Go }, root); + Assert.Equal("ApiSdk.models", modelB.GetImmediateParentOfType().Name); // migrated to root namespace + Assert.Equal("ApiSdk.models", modelA.GetImmediateParentOfType().Name); // migrated to root namespace + Assert.Equal("SubaModelA", modelA.Name); // renamed to avoid conflict + Assert.Equal("SubbModelB", modelB.Name); // renamed to avoid conflict + } #endregion #region GoRefinerTests From 4a88ed90294f79ff3d2fd79d6e8b71de0a7e52bb Mon Sep 17 00:00:00 2001 From: rkodev <43806892+rkodev@users.noreply.github.com> Date: Thu, 3 Oct 2024 12:12:02 +0300 Subject: [PATCH 5/8] fix: optimize code --- src/Kiota.Builder/Refiners/GoRefiner.cs | 66 ++++++++++++++----------- 1 file changed, 37 insertions(+), 29 deletions(-) diff --git a/src/Kiota.Builder/Refiners/GoRefiner.cs b/src/Kiota.Builder/Refiners/GoRefiner.cs index f4fbfe1e4b..a9ffa5a3d5 100644 --- a/src/Kiota.Builder/Refiners/GoRefiner.cs +++ b/src/Kiota.Builder/Refiners/GoRefiner.cs @@ -267,17 +267,17 @@ private void GetUsingsInModelsNameSpace(CodeNamespace modelsNameSpace, CodeNames return; dependencies[currentNameSpace.Name] = currentNameSpace.Classes - .SelectMany(codeClass => codeClass.Usings) - .Where(x => x.Declaration != null && !x.Declaration.IsExternal) + .SelectMany(static codeClass => codeClass.Usings) + .Where(static x => x.Declaration != null && !x.Declaration.IsExternal) .Select(static x => x.Declaration?.TypeDefinition?.GetImmediateParentOfType()) - .Where(ns => ns != null && ns.Name != currentNameSpace.Name) + .Where(ns => ns != null && !ns.Name.Equals(currentNameSpace.Name, StringComparison.OrdinalIgnoreCase)) .Select(static ns => ns!.Name) .ToHashSet(); - currentNameSpace.Namespaces - .Where(codeNameSpace => !dependencies.ContainsKey(codeNameSpace.Name)) - .ToList() - .ForEach(codeNameSpace => GetUsingsInModelsNameSpace(modelsNameSpace, codeNameSpace, dependencies)); + foreach (var codeNameSpace in currentNameSpace.Namespaces.Where(codeNameSpace => !dependencies.ContainsKey(codeNameSpace.Name))) + { + GetUsingsInModelsNameSpace(modelsNameSpace, codeNameSpace, dependencies); + } } /// @@ -286,7 +286,7 @@ private void GetUsingsInModelsNameSpace(CodeNamespace modelsNameSpace, CodeNames /// /// /// - static Dictionary>> FindCycles(Dictionary> dependencies) + private static Dictionary>> FindCycles(Dictionary> dependencies) { var cycles = new Dictionary>>(); var visited = new HashSet(); @@ -307,18 +307,19 @@ static Dictionary>> FindCycles(Dictionary /// Performs a DFS search to find cycles in the graph. Method will stop at the first cycle found in each node /// - static void SearchCycles(string parentNode, string node, Dictionary> dependencies, HashSet visited, Stack stack, Dictionary>> cycles) + private static void SearchCycles(string parentNode, string node, Dictionary> dependencies, HashSet visited, Stack stack, Dictionary>> cycles) { visited.Add(node); stack.Push(node); if (dependencies.TryGetValue(node, out var value)) { + var stackSet = new HashSet(stack); foreach (var neighbor in value) { - if (stack.Contains(neighbor)) + if (stackSet.Contains(neighbor)) { - var cycle = stack.Reverse().Concat(new[] { neighbor }).ToList(); + var cycle = stack.Reverse().Concat([neighbor]).ToList(); if (!cycles.ContainsKey(parentNode)) cycles[parentNode] = new List>(); @@ -336,50 +337,57 @@ static void SearchCycles(string parentNode, string node, Dictionary + foreach (var codeClass in currentNameSpace.Classes) { - currentNameSpace.RemoveChildElement(x); - x.Name = GetComposedName(x); - x.Parent = targetNameSpace; - targetNameSpace.AddClass(x); - }); - currentNameSpace.Enums.ToList().ForEach(x => + currentNameSpace.RemoveChildElement(codeClass); + codeClass.Name = GetComposedName(codeClass); + codeClass.Parent = targetNameSpace; + targetNameSpace.AddClass(codeClass); + } + + foreach (var x in currentNameSpace.Enums) { currentNameSpace.RemoveChildElement(x); x.Name = GetComposedName(x); x.Parent = targetNameSpace; targetNameSpace.AddEnum(x); - }); - currentNameSpace.Interfaces.ToList().ForEach(x => + } + + foreach (var x in currentNameSpace.Interfaces) { currentNameSpace.RemoveChildElement(x); x.Name = GetComposedName(x); x.Parent = targetNameSpace; targetNameSpace.AddInterface(x); - }); - currentNameSpace.Functions.ToList().ForEach(x => + } + + foreach (var x in currentNameSpace.Functions) { currentNameSpace.RemoveChildElement(x); x.Name = GetComposedName(x); x.Parent = targetNameSpace; targetNameSpace.AddFunction(x); - }); - currentNameSpace.Constants.ToList().ForEach(x => + } + + foreach (var x in currentNameSpace.Constants) { currentNameSpace.RemoveChildElement(x); x.Name = GetComposedName(x); x.Parent = targetNameSpace; targetNameSpace.AddConstant(x); - }); + } - currentNameSpace.Namespaces.ToList().ForEach(x => MigrateNameSpace(x, targetNameSpace)); + foreach (var ns in currentNameSpace.Namespaces) + { + MigrateNameSpace(ns, targetNameSpace); + } } private void CorrectReferencesToMigratedModels(CodeElement currentElement, Dictionary migratedNamespaces) { if (currentElement is CodeNamespace cn) { var usings = cn.GetChildElements() - .SelectMany(static x => x.GetChildElements(false)) + .SelectMany(static x => x.GetChildElements()) .OfType() .SelectMany(static x => x.Usings) .Where(x => migratedNamespaces.ContainsKey(x.Name)) @@ -388,8 +396,8 @@ private void CorrectReferencesToMigratedModels(CodeElement currentElement, Dicti foreach (var codeUsing in usings) { if (codeUsing.Parent is not ProprietableBlockDeclaration blockDeclaration || - codeUsing.Declaration?.TypeDefinition?.GetImmediateParentOfType().Name != - migratedNamespaces[codeUsing.Name]) + codeUsing.Declaration?.TypeDefinition?.GetImmediateParentOfType().Name.Equals(migratedNamespaces[codeUsing.Name], StringComparison.OrdinalIgnoreCase) == false + ) { continue; } From 6d029762f397d7f6c6aee55730f8747e9e865377 Mon Sep 17 00:00:00 2001 From: rkodev <43806892+rkodev@users.noreply.github.com> Date: Thu, 3 Oct 2024 13:06:01 +0300 Subject: [PATCH 6/8] Increase coverage --- .../Refiners/GoLanguageRefinerTests.cs | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/Kiota.Builder.Tests/Refiners/GoLanguageRefinerTests.cs b/tests/Kiota.Builder.Tests/Refiners/GoLanguageRefinerTests.cs index 995384b774..19fcdca3b0 100644 --- a/tests/Kiota.Builder.Tests/Refiners/GoLanguageRefinerTests.cs +++ b/tests/Kiota.Builder.Tests/Refiners/GoLanguageRefinerTests.cs @@ -852,6 +852,24 @@ public async Task CorrectesCircularDependencies() Kind = CodeClassKind.Model, Name = "ModelA", }).First(); + subANamespace.AddEnum(new CodeEnum + { + Name = "ModelAEnum", + }); + subANamespace.AddInterface(new CodeInterface + { + Name = "ModelAInterface", + OriginalClass = modelA, + }); + subANamespace.AddFunction(new CodeFunction( + new CodeMethod + { + Name = "ModelAFunction", + IsStatic = true, + Parent = modelA, + ReturnType = new CodeType() + }) + ); var subBNamespace = modelsNS.AddNamespace($"{modelsNS.Name}.subb"); var modelB = subBNamespace.AddClass(new CodeClass @@ -887,6 +905,8 @@ public async Task CorrectesCircularDependencies() Assert.Equal("ApiSdk.models", modelA.GetImmediateParentOfType().Name); // migrated to root namespace Assert.Equal("SubaModelA", modelA.Name); // renamed to avoid conflict Assert.Equal("SubbModelB", modelB.Name); // renamed to avoid conflict + + Assert.Empty(subANamespace.GetChildElements(true)); } #endregion From 950acdfa25c30f4604c091970d296374c8e5b0d0 Mon Sep 17 00:00:00 2001 From: rkodev <43806892+rkodev@users.noreply.github.com> Date: Thu, 3 Oct 2024 16:13:36 +0300 Subject: [PATCH 7/8] fix code clean --- src/Kiota.Builder/Refiners/GoRefiner.cs | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/Kiota.Builder/Refiners/GoRefiner.cs b/src/Kiota.Builder/Refiners/GoRefiner.cs index a9ffa5a3d5..3bf3129daa 100644 --- a/src/Kiota.Builder/Refiners/GoRefiner.cs +++ b/src/Kiota.Builder/Refiners/GoRefiner.cs @@ -238,16 +238,18 @@ private void CorrectCyclicReference(CodeElement currentElement) { foreach (var cycleReference in cycle.Value) { - var dupNS = cycleReference[^2]; // 2nd last element is target base namespace - var nameSpace = modelsNameSpace.FindNamespaceByName(dupNS, true); + var dupNs = cycleReference[^2]; // 2nd last element is target base namespace + var nameSpace = modelsNameSpace.FindNamespaceByName(dupNs, true); - migratedNamespaces[dupNS] = modelsNameSpace.Name; + migratedNamespaces[dupNs] = modelsNameSpace.Name; MigrateNameSpace(nameSpace!, modelsNameSpace); - if (!cycle.Key.Equals(modelsNameSpace.Name, StringComparison.OrdinalIgnoreCase) && !migratedNamespaces.ContainsKey(cycle.Key)) + if (!cycle.Key.Equals(modelsNameSpace.Name, StringComparison.OrdinalIgnoreCase) + && !migratedNamespaces.ContainsKey(cycle.Key) + && modelsNameSpace.FindNamespaceByName(cycle.Key, true) is { } currentNs) { migratedNamespaces[cycle.Key] = modelsNameSpace.Name; - MigrateNameSpace(modelsNameSpace.FindNamespaceByName(cycle.Key, true)!, modelsNameSpace); + MigrateNameSpace(currentNs, modelsNameSpace); } } } @@ -270,8 +272,9 @@ private void GetUsingsInModelsNameSpace(CodeNamespace modelsNameSpace, CodeNames .SelectMany(static codeClass => codeClass.Usings) .Where(static x => x.Declaration != null && !x.Declaration.IsExternal) .Select(static x => x.Declaration?.TypeDefinition?.GetImmediateParentOfType()) - .Where(ns => ns != null && !ns.Name.Equals(currentNameSpace.Name, StringComparison.OrdinalIgnoreCase)) - .Select(static ns => ns!.Name) + .OfType() + .Where(ns => !ns.Name.Equals(currentNameSpace.Name, StringComparison.OrdinalIgnoreCase)) + .Select(static ns => ns.Name) .ToHashSet(); foreach (var codeNameSpace in currentNameSpace.Namespaces.Where(codeNameSpace => !dependencies.ContainsKey(codeNameSpace.Name))) From 078a6b17931ed5b4c11725b29f309de235c43c0c Mon Sep 17 00:00:00 2001 From: rkodev <43806892+rkodev@users.noreply.github.com> Date: Mon, 7 Oct 2024 11:21:26 +0300 Subject: [PATCH 8/8] minor fixes --- src/Kiota.Builder/Refiners/GoRefiner.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Kiota.Builder/Refiners/GoRefiner.cs b/src/Kiota.Builder/Refiners/GoRefiner.cs index 3bf3129daa..2754d7e818 100644 --- a/src/Kiota.Builder/Refiners/GoRefiner.cs +++ b/src/Kiota.Builder/Refiners/GoRefiner.cs @@ -263,7 +263,7 @@ private string GetComposedName(CodeElement codeClass) return string.Join(string.Empty, classNameList.Count > 1 ? classNameList.Skip(1) : classNameList); } - private void GetUsingsInModelsNameSpace(CodeNamespace modelsNameSpace, CodeNamespace currentNameSpace, Dictionary> dependencies) + private static void GetUsingsInModelsNameSpace(CodeNamespace modelsNameSpace, CodeNamespace currentNameSpace, Dictionary> dependencies) { if (!modelsNameSpace.Name.Equals(currentNameSpace.Name, StringComparison.OrdinalIgnoreCase) && !currentNameSpace.IsChildOf(modelsNameSpace)) return; @@ -275,7 +275,7 @@ private void GetUsingsInModelsNameSpace(CodeNamespace modelsNameSpace, CodeNames .OfType() .Where(ns => !ns.Name.Equals(currentNameSpace.Name, StringComparison.OrdinalIgnoreCase)) .Select(static ns => ns.Name) - .ToHashSet(); + .ToHashSet(StringComparer.OrdinalIgnoreCase); foreach (var codeNameSpace in currentNameSpace.Namespaces.Where(codeNameSpace => !dependencies.ContainsKey(codeNameSpace.Name))) { @@ -385,7 +385,7 @@ private void MigrateNameSpace(CodeNamespace currentNameSpace, CodeNamespace targ MigrateNameSpace(ns, targetNameSpace); } } - private void CorrectReferencesToMigratedModels(CodeElement currentElement, Dictionary migratedNamespaces) + private static void CorrectReferencesToMigratedModels(CodeElement currentElement, Dictionary migratedNamespaces) { if (currentElement is CodeNamespace cn) {