Fix Cyclic Dependencies in Go

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
 

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"

src/Kiota.Builder/CodeDOM/CodeNamespace.cs

@@ -72,11 +72,11 @@ public CodeNamespace GetRootNamespace()
     public IEnumerable<CodeInterface> Interfaces => InnerChildElements.Values.OfType<CodeInterface>();
     public IEnumerable<CodeConstant> Constants => InnerChildElements.Values.OfType<CodeConstant>();
     public IEnumerable<CodeFile> Files => InnerChildElements.Values.OfType<CodeFile>();
-    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<CodeNamespace>(nsName, false);
+        var result = FindChildByName<CodeNamespace>(nsName, findInChildElements);
         if (result == null)
             foreach (var childNS in InnerChildElements.Values.OfType<CodeNamespace>())
             {

src/Kiota.Builder/Refiners/GoRefiner.cs

@@ -37,8 +37,8 @@
                 static x => x.ToFirstCharacterLowerCase(),
                 GenerationLanguage.Go);
             FlattenNestedHierarchy(generatedCode);
-            FlattenGoParamsFileNames(generatedCode);
-            FlattenGoFileNames(generatedCode);
+            FlattenParamsFileNames(generatedCode);
+            FlattenFileNames(generatedCode);
             NormalizeNamespaceNames(generatedCode);
             AddInnerClasses(
                 generatedCode,
@@ -46,7 +46,7 @@
                 string.Empty,
                 false,
                 MergeOverLappedStrings);
             if (_configuration.ExcludeBackwardCompatible) //TODO remove condition for v2
                 RemoveRequestConfigurationClasses(generatedCode,
                     new CodeUsing
                     {
@@ -190,12 +190,13 @@
             );
             AddParsableImplementsForModelClasses(
                 generatedCode,
                 "Parsable"
             );
             RenameInnerModelsToAppended(
                 generatedCode
             );
             cancellationToken.ThrowIfCancellationRequested();
+            CorrectCyclicReference(generatedCode);
             CopyModelClassesAsInterfaces(
                 generatedCode,
                 x => $"{x.Name}able"
@@ -218,6 +219,192 @@
         }, cancellationToken);
     }
 
+    private void CorrectCyclicReference(CodeElement currentElement)
+    {
+        var currentNameSpace = currentElement.GetImmediateParentOfType<CodeNamespace>();
+        var modelsNameSpace = findClientNameSpace(currentNameSpace)
+            ?.FindNamespaceByName(
+                $"{_configuration.ClientNamespaceName}.{GenerationConfiguration.ModelsNamespaceSegmentName}");
+
+        if (modelsNameSpace == null)
+            return;
+
+        var dependencies = new Dictionary<string, HashSet<string>>();
+        GetUsingsInModelsNameSpace(modelsNameSpace, modelsNameSpace, dependencies);
+
+        var migratedNamespaces = new Dictionary<string, string>();
+        var cycles = FindCycles(dependencies);
+        foreach (var cycle in cycles)
+        {
+            foreach (var cycleReference in cycle.Value)
+            {
+                var dupNS = cycleReference[^2]; // 2nd last element is target base namespace
+                var nameSpace = modelsNameSpace.FindNamespaceByName(dupNS, true);
+
+                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 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<string, HashSet<string>> dependencies)
+    {
+        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<CodeNamespace>())
+            .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 => GetUsingsInModelsNameSpace(modelsNameSpace, codeNameSpace, dependencies));
+    }
+
+    /// <summary>
+    /// 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
+    /// </summary>
+    /// <param name="dependencies"></param>
+    /// <returns></returns>
+    static Dictionary<string, List<List<string>>> FindCycles(Dictionary<string, HashSet<string>> dependencies)
+    {
+        var cycles = new Dictionary<string, List<List<string>>>();
+        var visited = new HashSet<string>();
+        var stack = new Stack<string>();
+
+        foreach (var node in dependencies.Keys)
+        {
+            if (!visited.Contains(node))
+            {
+                SearchCycles(node, node, dependencies, visited, stack, cycles);
+            }
+        }
+
+        return cycles;
+    }
+
+
+    /// <summary>
+    /// Performs a DFS search to find cycles in the graph. Method will stop at the first cycle found in each node
+    /// </summary>
+    static void SearchCycles(string parentNode, string node, Dictionary<string, HashSet<string>> dependencies, HashSet<string> visited, Stack<string> stack, Dictionary<string, List<List<string>>> 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().Concat(new[] { neighbor }).ToList();
+                    if (!cycles.ContainsKey(parentNode))
+                        cycles[parentNode] = new List<List<string>>();
+
+                    cycles[parentNode].Add(cycle);
+                }
+                else if (!visited.Contains(neighbor))
+                {
+                    SearchCycles(parentNode, neighbor, dependencies, visited, stack, cycles);
+                }
+            }
+        }
+
+        stack.Pop();
+    }
+
+    private void MigrateNameSpace(CodeNamespace currentNameSpace, CodeNamespace targetNameSpace)
+    {
+        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 => MigrateNameSpace(x, targetNameSpace));
+    }
+    private void CorrectReferencesToMigratedModels(CodeElement currentElement, Dictionary<string, string> migratedNamespaces)
+    {
+        if (currentElement is CodeNamespace cn)
+        {
+            var usings = cn.GetChildElements()
+                .SelectMany(static x => x.GetChildElements(false))
+                .OfType<ProprietableBlockDeclaration>()
+                .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<CodeNamespace>().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)
@@ -234,7 +421,7 @@
         CrawlTree(currentElement, GenerateCodeFiles);
     }
 
     private string MergeOverLappedStrings(string start, string end)
     {
         var search = "RequestBuilder";
         start = start.ToFirstCharacterUpperCase();
@@ -247,7 +434,7 @@
         return $"{start}{end}";
     }
 
     private void CorrectBackingStoreTypes(CodeElement currentElement)
     {
         if (!_configuration.UsesBackingStore)
             return;
@@ -343,11 +530,9 @@
             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);
             }
@@ -356,7 +541,7 @@
         CrawlTree(currentElement, FlattenNestedHierarchy);
     }
 
-    private void FlattenGoParamsFileNames(CodeElement currentElement)
+    private void FlattenParamsFileNames(CodeElement currentElement)
     {
         if (currentElement is CodeMethod codeMethod
             && codeMethod.IsOfKind(CodeMethodKind.RequestGenerator, CodeMethodKind.RequestExecutor))
@@ -371,7 +556,7 @@
 
         }
 
-        CrawlTree(currentElement, FlattenGoParamsFileNames);
+        CrawlTree(currentElement, FlattenParamsFileNames);
     }
 
     private List<string> getPathsName(CodeElement codeClass, string fileName, bool removeDuplicate = true)
@@ -388,7 +573,7 @@
             .ToList();
 
         // check if the last element contains current name and remove it
         if (namespacePathSegments.Count > 0 && removeDuplicate && fileName.ToFirstCharacterUpperCase().Contains(namespacePathSegments.Last(), StringComparison.Ordinal))
             namespacePathSegments.RemoveAt(namespacePathSegments.Count - 1);
 
         namespacePathSegments.Add(fileName.ToFirstCharacterUpperCase());
@@ -409,7 +594,7 @@
         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
@@ -429,7 +614,7 @@
             nextNameSpace.AddClass(codeClass);
         }
 
-        CrawlTree(currentElement, FlattenGoFileNames);
+        CrawlTree(currentElement, FlattenFileNames);
     }
 
     private static void FixConstructorClashes(CodeElement currentElement, Func<string, string> nameCorrection)
@@ -552,7 +737,7 @@
         "UUID",
         "Guid"
     };
     private static readonly AdditionalUsingEvaluator[] defaultUsingEvaluators = {
         new (static x => x is CodeProperty prop && prop.IsOfKind(CodePropertyKind.RequestAdapter),
             AbstractionsNamespaceName, "RequestAdapter"),
         new (static x => x is CodeMethod method && method.IsOfKind(CodeMethodKind.RequestGenerator),
@@ -608,12 +793,12 @@
     private const string SerializationNamespaceName = "github.com/microsoft/kiota-abstractions-go/serialization";
     internal const string UntypedNodeName = "UntypedNodeable";
 
     private void CorrectImplements(ProprietableBlockDeclaration block)
     {
         block.ReplaceImplementByName(KiotaBuilder.AdditionalHolderInterface, "AdditionalDataHolder");
         block.ReplaceImplementByName(KiotaBuilder.BackedModelInterface, "BackedModel");
     }
     private static void CorrectMethodType(CodeMethod currentMethod)
     {
         var parentClass = currentMethod.Parent as CodeClass;
         if (currentMethod.IsOfKind(CodeMethodKind.RequestGenerator, CodeMethodKind.RequestExecutor))

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<CodeNamespace>().Name); // migrated to root namespace
+        Assert.Equal("ApiSdk.models", modelA.GetImmediateParentOfType<CodeNamespace>().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