diff --git a/CHANGELOG.md b/CHANGELOG.md index 40aab1df90..5d3939d0db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- Fixed cyclic depencies in generated Go code. [#2834](https://github.com/microsoft/kiota/issues/2834) + ## [1.19.0] - 2024-10-03 ### Added 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/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 ca5480c6cb..2754d7e818 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,6 +196,7 @@ public override Task RefineAsync(CodeNamespace generatedCode, CancellationToken generatedCode ); cancellationToken.ThrowIfCancellationRequested(); + CorrectCyclicReference(generatedCode); CopyModelClassesAsInterfaces( generatedCode, x => $"{x.Name}able" @@ -218,6 +219,203 @@ 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) + return; + + var dependencies = new Dictionary>(); + GetUsingsInModelsNameSpace(modelsNameSpace, modelsNameSpace, dependencies); + + var migratedNamespaces = new Dictionary(); + 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) + && modelsNameSpace.FindNamespaceByName(cycle.Key, true) is { } currentNs) + { + migratedNamespaces[cycle.Key] = modelsNameSpace.Name; + MigrateNameSpace(currentNs, 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 static void GetUsingsInModelsNameSpace(CodeNamespace modelsNameSpace, CodeNamespace currentNameSpace, Dictionary> dependencies) + { + if (!modelsNameSpace.Name.Equals(currentNameSpace.Name, StringComparison.OrdinalIgnoreCase) && !currentNameSpace.IsChildOf(modelsNameSpace)) + return; + + dependencies[currentNameSpace.Name] = currentNameSpace.Classes + .SelectMany(static codeClass => codeClass.Usings) + .Where(static x => x.Declaration != null && !x.Declaration.IsExternal) + .Select(static x => x.Declaration?.TypeDefinition?.GetImmediateParentOfType()) + .OfType() + .Where(ns => !ns.Name.Equals(currentNameSpace.Name, StringComparison.OrdinalIgnoreCase)) + .Select(static ns => ns.Name) + .ToHashSet(StringComparer.OrdinalIgnoreCase); + + foreach (var codeNameSpace in currentNameSpace.Namespaces.Where(codeNameSpace => !dependencies.ContainsKey(codeNameSpace.Name))) + { + 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 + /// + /// + /// + private 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)) + { + SearchCycles(node, node, dependencies, visited, stack, cycles); + } + } + + return cycles; + } + + + /// + /// Performs a DFS search to find cycles in the graph. Method will stop at the first cycle found in each node + /// + 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 (stackSet.Contains(neighbor)) + { + var cycle = stack.Reverse().Concat([neighbor]).ToList(); + if (!cycles.ContainsKey(parentNode)) + cycles[parentNode] = new List>(); + + 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) + { + foreach (var codeClass in currentNameSpace.Classes) + { + 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); + } + + foreach (var x in currentNameSpace.Interfaces) + { + currentNameSpace.RemoveChildElement(x); + x.Name = GetComposedName(x); + x.Parent = targetNameSpace; + targetNameSpace.AddInterface(x); + } + + foreach (var x in currentNameSpace.Functions) + { + currentNameSpace.RemoveChildElement(x); + x.Name = GetComposedName(x); + x.Parent = targetNameSpace; + targetNameSpace.AddFunction(x); + } + + foreach (var x in currentNameSpace.Constants) + { + currentNameSpace.RemoveChildElement(x); + x.Name = GetComposedName(x); + x.Parent = targetNameSpace; + targetNameSpace.AddConstant(x); + } + + foreach (var ns in currentNameSpace.Namespaces) + { + MigrateNameSpace(ns, targetNameSpace); + } + } + private static void CorrectReferencesToMigratedModels(CodeElement currentElement, Dictionary migratedNamespaces) + { + if (currentElement is CodeNamespace cn) + { + var usings = cn.GetChildElements() + .SelectMany(static x => x.GetChildElements()) + .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.Equals(migratedNamespaces[codeUsing.Name], StringComparison.OrdinalIgnoreCase) == false + ) + { + 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) @@ -343,11 +541,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); } @@ -356,7 +552,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)) @@ -371,7 +567,7 @@ private void FlattenGoParamsFileNames(CodeElement currentElement) } - CrawlTree(currentElement, FlattenGoParamsFileNames); + CrawlTree(currentElement, FlattenParamsFileNames); } private List getPathsName(CodeElement codeClass, string fileName, bool removeDuplicate = true) @@ -409,7 +605,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 @@ -429,7 +625,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..19fcdca3b0 100644 --- a/tests/Kiota.Builder.Tests/Refiners/GoLanguageRefinerTests.cs +++ b/tests/Kiota.Builder.Tests/Refiners/GoLanguageRefinerTests.cs @@ -841,6 +841,73 @@ 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(); + 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 + { + 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 + + Assert.Empty(subANamespace.GetChildElements(true)); + } #endregion #region GoRefinerTests