Skip to content

Commit

Permalink
Fix Cyclic Dependencies in Go (#5467)
Browse files Browse the repository at this point in the history 
* detect cyclic refrences

* move all elements to the root dependency

* enable tests

* Adds unit tests

* fix: optimize code

* Increase coverage

* fix code clean

---------

Co-authored-by: Vincent Biret <[email protected]>
  • Loading branch information
@rkodev @baywet
rkodev and baywet authored Oct 7, 2024
1 parent f2b7472 commit 01710c8
Show file tree
Hide file tree
Showing 5 changed files with 276 additions and 23 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 0 additions & 12 deletions it/config.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand All @@ -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"
Expand Down
4 changes: 2 additions & 2 deletions src/Kiota.Builder/CodeDOM/CodeNamespace.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>())
{
Expand Down
214 changes: 205 additions & 9 deletions src/Kiota.Builder/Refiners/GoRefiner.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -196,6 +196,7 @@ public override Task RefineAsync(CodeNamespace generatedCode, CancellationToken
generatedCode
);
cancellationToken.ThrowIfCancellationRequested();
CorrectCyclicReference(generatedCode);
CopyModelClassesAsInterfaces(
generatedCode,
x => $"{x.Name}able"
Expand All @@ -218,6 +219,203 @@ public override Task RefineAsync(CodeNamespace generatedCode, CancellationToken
}, 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)
&& 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<string, HashSet<string>> 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<CodeNamespace>())
.OfType<CodeNamespace>()
.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);
}
}

/// <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>
private 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>
private 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))
{
var stackSet = new HashSet<string>(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<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)
{
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<string, string> migratedNamespaces)
{
if (currentElement is CodeNamespace cn)
{
var usings = cn.GetChildElements()
.SelectMany(static x => x.GetChildElements())
.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.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)
Expand Down Expand Up @@ -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);
}
Expand All @@ -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))
Expand All @@ -371,7 +567,7 @@ private void FlattenGoParamsFileNames(CodeElement currentElement)

}

CrawlTree(currentElement, FlattenGoParamsFileNames);
CrawlTree(currentElement, FlattenParamsFileNames);
}

private List<string> getPathsName(CodeElement codeClass, string fileName, bool removeDuplicate = true)
Expand Down Expand Up @@ -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
Expand All @@ -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<string, string> nameCorrection)
Expand Down
Loading

0 comments on commit 01710c8

Please sign in to comment.