Add Validation for disconnected dependencies

mikhail-dcl · Oct 23, 2023 · aac53ea · aac53ea
1 parent 06e66cc
commit aac53ea
Show file tree

Hide file tree

Showing 18 changed files with 346 additions and 39 deletions.
diff --git a/Arch.SystemGroups.SourceGenerator/CommonUtils.cs b/Arch.SystemGroups.SourceGenerator/CommonUtils.cs
@@ -1,4 +1,5 @@
-using System.Collections.Generic;
+using System;
+using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Linq;
 using System.Text;
@@ -63,6 +64,17 @@ public static string GetTypeReferenceInGlobalNotation(ITypeSymbol typeSymbol)
 
         if (IsGenericArgument(typeSymbol))
             return typeSymbol.ToString();
+
+        // Append each generic argument within typeSymbol with "global::" prefix
+        if (typeSymbol is INamedTypeSymbol { IsGenericType: true, TypeArguments.Length: > 0 } namedTypeSymbol)
+        {
+            var genericArguments = GetGenericArguments(typeSymbol, true);
+
+            var originalDefinitionName = namedTypeSymbol.OriginalDefinition.ToString(); // Ends with '<TName,...TNameN>'
+            var genericStartIndex = originalDefinitionName.IndexOf('<');
+
+            return "global::" + originalDefinitionName.Remove(genericStartIndex) + genericArguments;
+        }
 
         return "global::" + typeSymbol;
     }
@@ -87,7 +99,7 @@ public static bool IsGenericArgument(ISymbol symbol)
     public static bool IsGenericType(this ITypeSymbol typeSymbol) => 
         typeSymbol is INamedTypeSymbol { IsGenericType: true, TypeArguments.Length: > 0 };
 
-    public static string GetGenericArguments(ITypeSymbol typeSymbol)
+    public static string GetGenericArguments(ITypeSymbol typeSymbol, bool globalNotation)
     {
         if (!(typeSymbol is INamedTypeSymbol { IsGenericType: true } namedTypeSymbol) || namedTypeSymbol.TypeArguments.Length == 0)
             return string.Empty;
@@ -97,7 +109,7 @@ public static string GetGenericArguments(ITypeSymbol typeSymbol)
         for (int i = 0; i < namedTypeSymbol.TypeArguments.Length; i++)
         {
             ITypeSymbol typeArgument = namedTypeSymbol.TypeArguments[i];
-            genericArgumentsBuilder.Append(typeArgument.Name);
+            genericArgumentsBuilder.Append(globalNotation ? GetTypeReferenceInGlobalNotation(typeArgument) : typeArgument.Name);
 
             if (i < namedTypeSymbol.TypeArguments.Length - 1)
             {

diff --git a/Arch.SystemGroups.SourceGenerator/CreateGroupGenerator.cs b/Arch.SystemGroups.SourceGenerator/CreateGroupGenerator.cs
@@ -5,7 +5,7 @@ public static class CreateGroupGenerator
     public static string GetTryGetCreateGroup(in GroupInfo groupInfo)
     {
         return groupInfo.Behaviour is { IsCustom: true, HasParameterlessConstructor: false } 
-            ? $"worldBuilder.TryRegisterGroup<{groupInfo.ClassName}>(typeof({groupInfo.UpdateInGroup}), {EdgesGenerator.AddEdgesCachedFieldName}, {groupInfo.ThrottlingEnabled.ToString().ToLower()});" 
-            : $"worldBuilder.TryCreateGroup<{groupInfo.ClassName}>(typeof({groupInfo.UpdateInGroup}), {EdgesGenerator.AddEdgesCachedFieldName}, {groupInfo.ThrottlingEnabled.ToString().ToLower()});";
+            ? $"worldBuilder.TryRegisterGroup<{groupInfo.ClassName}>(typeof({groupInfo.UpdateInGroup}), {EdgesGenerator.AddEdgesCachedFieldName}, {EdgesGenerator.ValidateEdgesCachedFieldName}, {groupInfo.ThrottlingEnabled.ToString().ToLower()});" 
+            : $"worldBuilder.TryCreateGroup<{groupInfo.ClassName}>(typeof({groupInfo.UpdateInGroup}), {EdgesGenerator.AddEdgesCachedFieldName}, {EdgesGenerator.ValidateEdgesCachedFieldName}, {groupInfo.ThrottlingEnabled.ToString().ToLower()});";
     }
 }
diff --git a/Arch.SystemGroups.SourceGenerator/EdgesGenerator.cs b/Arch.SystemGroups.SourceGenerator/EdgesGenerator.cs
@@ -1,4 +1,5 @@
 using System.Collections.Generic;
+using System.Linq;
 using System.Text;
 using Microsoft.CodeAnalysis;
 
@@ -7,6 +8,36 @@ namespace Arch.SystemGroups.SourceGenerator
     public static class EdgesGenerator
     {
         public const string AddEdgesCachedFieldName = "_addEdgesCached";
+        public const string ValidateEdgesCachedFieldName = "_validateEdgesCached";
+        public const string ValidateEdgesMethodName = "ValidateEdges";
+        public const string DisconnectedDependenciesFieldName = "disconnectedDependencies";
+
+        public static string GetValidateEdgesCachedField() =>
+            $"private static readonly Action<List<DisconnectedDependenciesInfo.WrongTypeBinding>> {ValidateEdgesCachedFieldName} = {ValidateEdgesMethodName};";
+
+        public static string GetValidateEdgesBody(IList<ITypeSymbol> updateBefore, IList<ITypeSymbol> updateAfter, 
+            ITypeSymbol group, string className, ITypeSymbol thisType, string typeGenericArguments)
+        {
+            var builder = new StringBuilder();
+
+            foreach (var dependency in updateBefore)
+                InsertValidateEdge(dependency);
+
+            foreach (var dependency in updateAfter)
+                InsertValidateEdge(dependency);
+
+            return builder.ToString();
+
+            void InsertValidateEdge(ITypeSymbol dependency)
+            {
+                // Filter out references to self
+                if (dependency.Equals(thisType, SymbolEqualityComparer.Default))
+                    return;
+
+                builder.AppendFormat(
+                    $"ArchSystemsSorter.ValidateEdge({DisconnectedDependenciesFieldName}, typeof({className}{typeGenericArguments}), typeof({group}), typeof({dependency}), {dependency}.Metadata.UpdateInGroup);");
+            }
+        }
 
         public static string GetAddEdgesCachedField() =>
             $"private static readonly Action<Dictionary<Type, List<Type>>> {AddEdgesCachedFieldName} = AddEdges;";

diff --git a/Arch.SystemGroups.SourceGenerator/InjectToWorldGenerator.cs b/Arch.SystemGroups.SourceGenerator/InjectToWorldGenerator.cs
@@ -57,6 +57,8 @@ public static StringBuilder GetAddToGroup(in SystemInfo systemInfo, string typeG
             sb.Append($"typeof({systemInfo.ClassName}{typeGenericArguments}), ");
             sb.Append(EdgesGenerator.AddEdgesCachedFieldName);
             sb.Append(", ");
+            sb.Append(EdgesGenerator.ValidateEdgesCachedFieldName);
+            sb.Append(", ");
             sb.Append(systemInfo.ThrottlingEnabled.ToString().ToLower());
             sb.Append(");");
 

diff --git a/Arch.SystemGroups.SourceGenerator/PartialClassGenerator.cs b/Arch.SystemGroups.SourceGenerator/PartialClassGenerator.cs
@@ -198,11 +198,15 @@ private static string AccessibilityToString(Accessibility accessibility)
 
     private static string GetSystemPartialClass(in SystemInfo systemInfo, WorldInfo worldInfo)
     {
-        var typeGenericArguments = CommonUtils.GetGenericArguments(systemInfo.This);
+        var typeGenericArguments = CommonUtils.GetGenericArguments(systemInfo.This, false);
         var typeGenericArgumentsWhere = CommonUtils.GetGenericConstraintsString(systemInfo.This);
         var addEdgesFieldDeclaration = EdgesGenerator.GetAddEdgesCachedField();
         var addEdgesBody = EdgesGenerator.GetAddEdgesBody(systemInfo.UpdateBefore, systemInfo.UpdateAfter, systemInfo.ClassName, systemInfo.This, typeGenericArguments);
 
+        var validateEdgesFieldDeclaration = EdgesGenerator.GetValidateEdgesCachedField();
+        var validateEdgesBody = EdgesGenerator.GetValidateEdgesBody(systemInfo.UpdateBefore, systemInfo.UpdateAfter,
+            systemInfo.UpdateInGroup, systemInfo.ClassName, systemInfo.This, typeGenericArguments);
+
         var injectToWorldMethodParams = InjectToWorldGenerator.GetMethodArgumentsWithoutWorld(in systemInfo).ToString();
         var passArguments = InjectToWorldGenerator.GetPassArguments(in systemInfo).ToString();
         var systemInstantiation = InjectToWorldGenerator.GetSystemInstantiation(in systemInfo, typeGenericArguments);
@@ -241,6 +245,8 @@ private static string GetSystemPartialClass(in SystemInfo systemInfo, WorldInfo
                 {{systemInfo.AccessModifier}} partial class {{systemInfo.ClassName}}{{typeGenericArguments}}{
                     
                 {{addEdgesFieldDeclaration}}
+                
+                {{validateEdgesFieldDeclaration}}
 
                 public static {{systemInfo.ClassName}}{{typeGenericArguments}} InjectToWorld(ref ArchSystemsWorldBuilder<{{worldType}}> worldBuilder{{injectToWorldMethodParams}})
                 {
@@ -254,6 +260,11 @@ private static void AddEdges(Dictionary<Type, List<Type>> edgesMap)
                 {
                     {{addEdgesBody}}
                 }
+                
+                private static void {{EdgesGenerator.ValidateEdgesMethodName}}(List<DisconnectedDependenciesInfo.WrongTypeBinding> {{EdgesGenerator.DisconnectedDependenciesFieldName}})
+                {
+                    {{validateEdgesBody}}
+                }
 
                 {{metadata}}
 
@@ -271,6 +282,10 @@ public static string GetGroupPartialClass(in GroupInfo groupInfo)
         var addEdgesFieldDeclaration = EdgesGenerator.GetAddEdgesCachedField();
         var addEdgesBody = EdgesGenerator.GetAddEdgesBody(groupInfo.UpdateBefore, groupInfo.UpdateAfter, groupInfo.ClassName, groupInfo.This, string.Empty);
 
+        var validateEdgesFieldDeclaration = EdgesGenerator.GetValidateEdgesCachedField();
+        var validateEdgesBody = EdgesGenerator.GetValidateEdgesBody(groupInfo.UpdateBefore, groupInfo.UpdateAfter,
+            groupInfo.UpdateInGroup, groupInfo.ClassName, groupInfo.This, string.Empty);
+
         // If there is no system directly attached to the group we need to create the tree of groups, otherwise the leaf system will be detached
         var groupInjection = InjectToWorldGenerator.GetGroupInjectionInvocation(groupInfo.UpdateInGroup);
         var createGroup = CreateGroupGenerator.GetTryGetCreateGroup(in groupInfo);
@@ -290,6 +305,8 @@ public static string GetGroupPartialClass(in GroupInfo groupInfo)
                 {{groupInfo.AccessModifier}} partial class {{groupInfo.ClassName}}{{(customBehaviour ? string.Empty : ": DefaultGroup<float>")}} {
                     
                 {{addEdgesFieldDeclaration}}
+                
+                {{validateEdgesFieldDeclaration}}
 
                 public static void TryCreateGroup<T>(ref ArchSystemsWorldBuilder<T> worldBuilder)
                 {
@@ -301,6 +318,11 @@ private static void AddEdges(Dictionary<Type, List<Type>> edgesMap)
                 {
                     {{addEdgesBody}}
                 }
+                
+                private static void {{EdgesGenerator.ValidateEdgesMethodName}}(List<DisconnectedDependenciesInfo.WrongTypeBinding> {{EdgesGenerator.DisconnectedDependenciesFieldName}})
+                {
+                    {{validateEdgesBody}}
+                }
 
                 {{metadata}}
 

diff --git a/Arch.SystemGroups.Tests/DisconnectedDependencies/DisconnectedDependenciesTests.cs b/Arch.SystemGroups.Tests/DisconnectedDependencies/DisconnectedDependenciesTests.cs
@@ -0,0 +1,33 @@
+using Arch.SystemGroups.Tests.TestSetup1;
+using NSubstitute;
+
+namespace Arch.SystemGroups.Tests.DisconnectedDependencies;
+
+public class DisconnectedDependenciesTests
+{
+    private ArchSystemsWorldBuilder<TestWorld> _worldBuilder;
+
+    [SetUp]
+    public void SetUp()
+    {
+        _worldBuilder = new ArchSystemsWorldBuilder<TestWorld>(new TestWorld(), Substitute.For<IPlayerLoop>());
+    }
+
+    [Test]
+    public void ThrowsOnSystems()
+    {
+        DDSystem1.InjectToWorld(ref _worldBuilder);
+        DDSystem2.InjectToWorld(ref _worldBuilder);
+
+        Assert.Throws<DisconnectedDependenciesFoundException>(() => _worldBuilder.Finish());
+    }
+
+    [Test]
+    public void ThrowsOnGroups()
+    {
+        DDSystem1Gr1.InjectToWorld(ref _worldBuilder);
+        DDSystem1Gr2.InjectToWorld(ref _worldBuilder);
+
+        Assert.Throws<DisconnectedDependenciesFoundException>(() => _worldBuilder.Finish());
+    }
+}
diff --git a/Arch.SystemGroups.Tests/DisconnectedDependencies/Groups.cs b/Arch.SystemGroups.Tests/DisconnectedDependencies/Groups.cs
@@ -0,0 +1,16 @@
+using Arch.SystemGroups.DefaultSystemGroups;
+
+namespace Arch.SystemGroups.Tests.DisconnectedDependencies;
+
+[UpdateInGroup(typeof(PresentationSystemGroup))]
+public partial class DDGroup1
+{
+
+}
+
+[UpdateInGroup(typeof(SimulationSystemGroup))]
+[UpdateBefore(typeof(DDGroup1))]
+public partial class DDGroup2
+{
+
+}
diff --git a/Arch.SystemGroups.Tests/DisconnectedDependencies/Systems.cs b/Arch.SystemGroups.Tests/DisconnectedDependencies/Systems.cs
@@ -0,0 +1,38 @@
+using Arch.System;
+using Arch.SystemGroups.DefaultSystemGroups;
+using Arch.SystemGroups.Tests.TestSetup1;
+
+namespace Arch.SystemGroups.Tests.DisconnectedDependencies;
+
+[UpdateInGroup(typeof(PostRenderingSystemGroup))]
+[UpdateAfter(typeof(DDSystem2))] // exception
+public partial class DDSystem1 : BaseSystem<TestWorld, float>
+{
+    public DDSystem1(TestWorld world) : base(world)
+    {
+    }
+}
+
+[UpdateInGroup(typeof(SimulationSystemGroup))]
+public partial class DDSystem2 : BaseSystem<TestWorld, float>
+{
+    public DDSystem2(TestWorld world) : base(world)
+    {
+    }
+}
+
+[UpdateInGroup(typeof(DDGroup1))]
+public partial class DDSystem1Gr1 : BaseSystem<TestWorld, float>
+{
+    public DDSystem1Gr1(TestWorld world) : base(world)
+    {
+    }
+}
+
+[UpdateInGroup(typeof(DDGroup2))]
+public partial class DDSystem1Gr2 : BaseSystem<TestWorld, float>
+{
+    public DDSystem1Gr2(TestWorld world) : base(world)
+    {
+    }
+}
diff --git a/Arch.SystemGroups.Tests/GenericSetup/NestedArgument.cs b/Arch.SystemGroups.Tests/GenericSetup/NestedArgument.cs
@@ -0,0 +1,29 @@
+using Arch.System;
+using Arch.SystemGroups.DefaultSystemGroups;
+using Arch.SystemGroups.Tests.TestSetup1;
+
+namespace Arch.SystemGroups.Tests.GenericSetup;
+
+public class ParentClass<T>
+{
+    public class NestedType
+    {
+        public T Value;
+    }
+}
+
+public struct ParamGen
+{
+    public bool Value;
+}
+
+[UpdateInGroup(typeof(SimulationSystemGroup))]
+public partial class SystemWithGenericNestedArgument : BaseSystem<TestWorld, float>
+{
+    private readonly ParentClass<ParamGen>.NestedType _param;
+
+    internal SystemWithGenericNestedArgument(TestWorld world, ParentClass<ParamGen>.NestedType param) : base(world)
+    {
+        _param = param;
+    }
+}
diff --git a/Arch.SystemGroups.Tests/TestSetup2/Groups/GroupsB.cs b/Arch.SystemGroups.Tests/TestSetup2/Groups/GroupsB.cs
@@ -15,17 +15,17 @@ public partial class GroupBB
 
 [UpdateInGroup(typeof(GroupBA))]
 // This dependency should not be taken into consideration even as
-// this group does belong to the GroupBB hierarchy.
-[UpdateAfter(typeof(GroupBB))]
+// this group does not belong to the GroupBB hierarchy.
+// [UpdateAfter(typeof(GroupBB))] no tolerance
 public partial class GroupBAA
 {
 }
 
 [UpdateInGroup(typeof(GroupBA))]
 [UpdateAfter(typeof(GroupBAA))]
 // This dependency should not be taken into consideration even as
-// this group does belong to the A hierarchy.
-[UpdateBefore(typeof(GroupAA))] 
+// this group does not belong to the A hierarchy.
+// [UpdateBefore(typeof(GroupAA))] no tolerance
 public partial class GroupBAB
 {
 }

diff --git a/Arch.SystemGroups.Tests/TestSetup2/Systems/SystemsA.cs b/Arch.SystemGroups.Tests/TestSetup2/Systems/SystemsA.cs
@@ -33,7 +33,7 @@ public SystemCGroupAA(TestWorld2 world) : base(world)
 // Group AB
 
 [UpdateInGroup(typeof(GroupAB))]
-[UpdateBefore(typeof(SystemCGroupAA))] // < -- ignore
+// [UpdateBefore(typeof(SystemCGroupAA))] // no tolerance
 [UpdateBefore(typeof(SystemCGroupAB))]
 public partial class SystemAGroupAB : BaseSystem<TestWorld2, float>
 {

diff --git a/Arch.SystemGroups.Tests/TestSetup2/Systems/SystemsB.cs b/Arch.SystemGroups.Tests/TestSetup2/Systems/SystemsB.cs
@@ -49,10 +49,10 @@ public SystemDGroupBA(TestWorld2 world) : base(world)
 [UpdateInGroup(typeof(GroupBB))]
 [UpdateBefore(typeof(SystemBGroupBB))]
 // The following edges will be ignored as belong to a different group
-[UpdateAfter(typeof(SystemCGroupBA))]
-[UpdateAfter(typeof(SystemBGroupBA))]
-[UpdateAfter(typeof(SystemAGroupBA))]
-[UpdateAfter(typeof(SystemDGroupBA))]
+// [UpdateAfter(typeof(SystemCGroupBA))] no tolerance
+// [UpdateAfter(typeof(SystemBGroupBA))]
+// [UpdateAfter(typeof(SystemAGroupBA))]
+// [UpdateAfter(typeof(SystemDGroupBA))]
 public partial class SystemAGroupBB : BaseSystem<TestWorld2, float>
 {
     public SystemAGroupBB(TestWorld2 world) : base(world)

diff --git a/Arch.SystemGroups/Arch.SystemGroups.csproj.DotSettings b/Arch.SystemGroups/Arch.SystemGroups.csproj.DotSettings
@@ -2,4 +2,5 @@
 	<s:Boolean x:Key="/Default/CodeInspection/NamespaceProvider/NamespaceFoldersToSkip/=builder/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/CodeInspection/NamespaceProvider/NamespaceFoldersToSkip/=groups/@EntryIndexedValue">True</s:Boolean>
 	<s:Boolean x:Key="/Default/CodeInspection/NamespaceProvider/NamespaceFoldersToSkip/=playerloophelper/@EntryIndexedValue">True</s:Boolean>
-	<s:Boolean x:Key="/Default/CodeInspection/NamespaceProvider/NamespaceFoldersToSkip/=playerloophelper_005Caggregation/@EntryIndexedValue">True</s:Boolean></wpf:ResourceDictionary>
+	<s:Boolean x:Key="/Default/CodeInspection/NamespaceProvider/NamespaceFoldersToSkip/=playerloophelper_005Caggregation/@EntryIndexedValue">True</s:Boolean>
+	<s:Boolean x:Key="/Default/CodeInspection/NamespaceProvider/NamespaceFoldersToSkip/=systemsdependencies/@EntryIndexedValue">True</s:Boolean></wpf:ResourceDictionary>