forwarded attribute reference with cycles (#311)

Code around registering variables was very fragile; sometimes it used a open generic type's name (i.t, List<> when registering
a variable for List<int>, some times the closed type name (i.e List<int> when processing List<int>), sometimes only the type simple name, etc...

This commit also normalizes both the 'name' and 'parent name' to only use respecitively 'ITypeSymbol.OriginalDefinition.ToDisplayString()'
and 'ITypeSymbol.ContainingSymbol.ToDisplayString()' and deviating from this only in cases that requires so.

Ideally we should investigate and document such deviations.

Cecilifier.Core.Tests/Tests/Integration/GenericsTestCase.cs

@@ -26,13 +26,13 @@ public void TestInstanceNonGenericMethodsOnGenericTypes()
         [Test]
         public void TestGenericInferredStaticMethods()
         {
-            AssertResourceTest(@"Generics/StaticInferredMethods");
+            AssertResourceTest("Generics/StaticInferredMethods");
         }
 
         [Test]
         public void TestGenericExplicitStaticMethods()
         {
-            AssertResourceTest(@"Generics/StaticExplicitMethods");
+            AssertResourceTest("Generics/StaticExplicitMethods");
         }
 
         [Test]

Cecilifier.Core.Tests/Tests/Unit/AttributesTest.cs

@@ -132,6 +132,38 @@ class MyGenericAttribute<T> : System.Attribute {}
                        \s+cls_foo_\d+.CustomAttributes.Add\(\k<attr>\);
                        """));
     }
+
+    [Test]
+    public void CyclicForwardReferenceToGenericAttributeWorks2()
+    {
+        var result = RunCecilifier("""
+                                        [MyGeneric<int>]
+                                        class Foo { }
+                                        
+                                        class MyGenericAttribute<T> : System.Attribute 
+                                        {
+                                            public Foo _foo;
+                                        }
+                                        """);
+
+        var cecilifiedCode = result.GeneratedCode.ReadToEnd();
+        Assert.That(
+            cecilifiedCode, 
+            Does.Match("""
+                       //Class : Foo
+                       \s+var (?<appliedTo>cls_foo_\d+) = new TypeDefinition\("", "Foo",.+\);
+                       \s+assembly.MainModule.Types.Add\(\k<appliedTo>\);
+                       
+                       \s+//Class : MyGenericAttribute
+                       \s+var (?<attrType>cls_myGenericAttribute_\d+) = new TypeDefinition\("", "MyGenericAttribute`1",.+ImportReference\(typeof\(System.Attribute\)\)\);
+                       \s+var gp_T_\d+ = new Mono.Cecil.GenericParameter\("T", \k<attrType>\);
+                       \s+\k<attrType>.GenericParameters.Add\(gp_T_\d+\);
+                       \s+assembly.MainModule.Types.Add\(\k<attrType>\);
+                       \s+var ctor_myGenericAttribute_4 = new MethodDefinition\(".ctor",.+TypeSystem.Void\);
+                       \s+var (?<attrInstance>attr_myGeneric_1_\d+) = new CustomAttribute\(new MethodReference\(ctor_myGenericAttribute_4.Name, ctor_myGenericAttribute_4.ReturnType\) {.+DeclaringType = \k<attrType>.MakeGenericInstanceType\(.+Int32\).+}\);
+                       \s+\k<appliedTo>.CustomAttributes.Add\(\k<attrInstance>\);
+                       """));
+    }
 
     [TestCase("LayoutKind.Auto, Pack=1, Size=12", 1, 12)]
     [TestCase("LayoutKind.Auto, Pack=1", 1, 0)]

Cecilifier.Core.Tests/Tests/Unit/Miscellaneous.cs

@@ -70,12 +70,13 @@ namespace NS { public struct FileStream { } }
                     class Foo 
                     {
                          public FileStream file; // System.IO.FileStream since NS.FileStream is not in scope here. 
-                         public NS.FileStream definedInFooBar; 
+                         public NS.FileStream definedInNS; 
                     }");
 
             var cecilifiedCode = result.GeneratedCode.ReadToEnd();
             Assert.That(cecilifiedCode.Contains("FieldDefinition(\"file\", FieldAttributes.Public, st_fileStream_0);"), Is.False, cecilifiedCode);
-            Assert.That(cecilifiedCode.Contains("FieldDefinition(\"definedInFooBar\", FieldAttributes.Public, st_fileStream_3);"), Is.True, cecilifiedCode);
+            Assert.That(cecilifiedCode,Does.Match("""var st_fileStream_0 = new TypeDefinition\("NS", "FileStream", .+\);"""));
+            Assert.That(cecilifiedCode.Contains("FieldDefinition(\"definedInNS\", FieldAttributes.Public, st_fileStream_0);"), Is.True, cecilifiedCode);
             Assert.That(cecilifiedCode.Contains("FieldDefinition(\"file\", FieldAttributes.Public, assembly.MainModule.ImportReference(typeof(System.IO.FileStream)));"), Is.True, cecilifiedCode);
         }
 

Cecilifier.Core.Tests/Tests/Unit/TypeResolverTests.cs

@@ -52,7 +52,7 @@ public void TypeParameterFromDeclaringType_Resolves_To_DeclaredVariable()
         var m1Symbol = context.SemanticModel.GetDeclaredSymbol(m1Syntax);
 
         // Simulates type parameter `T` being registered under type `Foo`
-        using var _ = context.DefinitionVariables.WithCurrent("Foo", "T", VariableMemberKind.TypeParameter, "TypeParameter_T_var");
+        using var _ = context.DefinitionVariables.WithCurrent("Foo<T>", "T", VariableMemberKind.TypeParameter, "TypeParameter_T_var");
         var resolved = context.TypeResolver.Resolve(m1Symbol.ReturnType, "fakeReference"); 
 
         Assert.That(resolved, Does.Match(@".+ImportReference\(typeof\(System.Func<>\)\)\.MakeGenericInstanceType\(TypeParameter_T_var\)"));
@@ -66,7 +66,7 @@ public void TypeParameterFromGenericMethod_Resolves_To_DeclaredVariable()
         var methodSymbol = context.SemanticModel.GetDeclaredSymbol(methodSyntax);
 
         // Simulates type parameter `T` being registered under method `M2`
-        using var _ = context.DefinitionVariables.WithCurrent("M2", "TM", VariableMemberKind.TypeParameter, "TypeParameter_TM_var");
+        using var _ = context.DefinitionVariables.WithCurrent("Foo<T>.M2<TM>()", "TM", VariableMemberKind.TypeParameter, "TypeParameter_TM_var");
         var resolved = context.TypeResolver.Resolve(methodSymbol.OriginalDefinition.ReturnType, "fakeReference"); 
 
         Assert.That(resolved, Does.Match(@".+ImportReference\(typeof\(System.Func<>\)\)\.MakeGenericInstanceType\(TypeParameter_TM_var\)"));
@@ -80,8 +80,8 @@ public void TypeParameterFromGenericMethodAndGenericType_Resolves_To_DeclaredVar
         var methodSymbol = context.SemanticModel.GetDeclaredSymbol(methodSyntax);
 
         // Simulates type parameters `T` & `TM` being registered under their respective members.
-        using var t = context.DefinitionVariables.WithCurrent("Foo", "T", VariableMemberKind.TypeParameter, "TypeParameter_Foo");
-        using var tm = context.DefinitionVariables.WithCurrent("M3", "TM", VariableMemberKind.TypeParameter, "TypeParameter_M3");
+        using var t = context.DefinitionVariables.WithCurrent("Foo<T>", "T", VariableMemberKind.TypeParameter, "TypeParameter_Foo");
+        using var tm = context.DefinitionVariables.WithCurrent("Foo<T>.M3<TM>()", "TM", VariableMemberKind.TypeParameter, "TypeParameter_M3");
         var resolved = context.TypeResolver.Resolve(methodSymbol.OriginalDefinition.ReturnType, "fakeReference"); 
 
         Assert.That(resolved, Does.Match(@".+ImportReference\(typeof\(System.Func<,>\)\)\.MakeGenericInstanceType\(TypeParameter_Foo, TypeParameter_M3\)"));
@@ -118,7 +118,7 @@ public void NestedTypes_WithTypeArguments_DefinedInCecilifiedCode(string methodN
         var methodSyntax = GetMethodSyntax(context, methodName);
         var methodSymbol = context.SemanticModel.GetDeclaredSymbol(methodSyntax).EnsureNotNull<ISymbol, IMethodSymbol>();
 
-        using var bar = context.DefinitionVariables.WithCurrent("", "Bar", VariableMemberKind.Type, "BarDefinition");
+        using var bar = context.DefinitionVariables.WithCurrent("<global namespace>", "Bar", VariableMemberKind.Type, "BarDefinition");
         var resolved = context.TypeResolver.Resolve(methodSymbol.OriginalDefinition.ReturnType, "methodReference");
         Assert.That(
             resolved, 

Cecilifier.Core/AST/ConstructorDeclarationVisitor.cs

@@ -81,26 +81,44 @@ public override void VisitConstructorInitializer(ConstructorInitializerSyntax no
 
         internal void DefaultCtorInjector(string typeDefVar, ClassDeclarationSyntax declaringClass, bool isStatic)
         {
-            DefaultCtorInjector(typeDefVar, declaringClass.Identifier.Text, DefaultCtorAccessibilityFor(declaringClass, isStatic), ResolveDefaultCtorFor(typeDefVar, declaringClass), isStatic, ctorBodyIL =>
+            var typeSymbol = Context.SemanticModel.GetDeclaredSymbol(declaringClass).EnsureNotNull();
+            DefaultCtorInjector(typeDefVar,  typeSymbol, DefaultCtorAccessibilityFor(declaringClass, isStatic), ResolveDefaultCtorFor(typeDefVar, declaringClass), isStatic, ctorBodyIL =>
             {
                 ProcessFieldInitialization(declaringClass, ctorBodyIL, isStatic);
             });
         }
 
+        internal void DefaultCtorInjector(string typeDefVar, INamedTypeSymbol type, string ctorAccessibility, string baseCtor, bool isStatic, Action<string> processInitializers)
+        {
+            DefaultCtorInjector(typeDefVar, type.Name, type.OriginalDefinition.ToDisplayString(), ctorAccessibility, baseCtor, isStatic, processInitializers);
+        }
+
         internal void DefaultCtorInjector(string typeDefVar, string typeName, string ctorAccessibility, string baseCtor, bool isStatic, Action<string> processInitializers)
+        {
+            DefaultCtorInjector(typeDefVar, typeName, typeName, ctorAccessibility, baseCtor, isStatic, processInitializers);
+        }
+
+        /// <param name="typeDefVar"></param>
+        /// <param name="normalizedTypeName">The type name without any symbols (such as angle brackets) considered invalid in variable names</param>
+        /// <param name="typeName">The symbol original definition name. For instance the generic type Gen&lt;T&gt; has a <paramref name="typeName"/> of Gen&lt;&gt;</param>
+        /// <param name="ctorAccessibility"></param>
+        /// <param name="baseCtor"></param>
+        /// <param name="isStatic"></param>
+        /// <param name="processInitializers">Action in charge of handling constructor initializers</param>
+        private void DefaultCtorInjector(string typeDefVar, string normalizedTypeName, string typeName, string ctorAccessibility, string baseCtor, bool isStatic, Action<string> processInitializers)
         {
             Context.WriteNewLine();
-            Context.WriteComment($"** Constructor: {typeName}() **");
+            Context.WriteComment($"** Constructor: {normalizedTypeName}() **");
 
             var ctorLocalVar = AddOrUpdateParameterlessCtorDefinition(
                 typeName,
                 ctorAccessibility,
                 isStatic,
-                Context.Naming.SyntheticVariable(typeName, isStatic ? ElementKind.StaticConstructor : ElementKind.Constructor));
+                Context.Naming.SyntheticVariable(normalizedTypeName, isStatic ? ElementKind.StaticConstructor : ElementKind.Constructor));
 
             AddCecilExpression($"{typeDefVar}.Methods.Add({ctorLocalVar});");
 
-            var ctorBodyIL = Context.Naming.ILProcessor($"ctor_{typeName}");
+            var ctorBodyIL = Context.Naming.ILProcessor($"ctor_{normalizedTypeName}");
             AddCecilExpression($"var {ctorBodyIL} = {ctorLocalVar}.Body.GetILProcessor();");
 
             processInitializers?.Invoke(ctorBodyIL);

Cecilifier.Core/AST/EnumDeclarationVisitor.cs

@@ -28,10 +28,11 @@ public override void VisitEnumDeclaration(EnumDeclarationSyntax node)
             var enumType = Context.Naming.Type(node);
             var enumSymbol = Context.SemanticModel.GetDeclaredSymbol(node).EnsureNotNull<ISymbol, INamedTypeSymbol>($"Something really bad happened. Roslyn failed to resolve the symbol for the enum {node.Identifier.Text}");
             var attrs = TypeModifiersToCecil(enumSymbol, node.Modifiers);
-            var typeDef = CecilDefinitionsFactory.Type(Context, enumType, enumSymbol.ContainingNamespace?.FullyQualifiedName(), enumSymbol.Name, attrs + " | TypeAttributes.Sealed", Context.TypeResolver.Bcl.System.Enum, enumSymbol.ContainingType?.Name,false, Array.Empty<ITypeSymbol>(), [], []);
+            var outerTypeVariable = Context.DefinitionVariables.GetVariable(enumSymbol.ContainingType?.ToDisplayString(), VariableMemberKind.Type, enumSymbol.ContainingType?.ContainingSymbol.ToDisplayString());
+            var typeDef = CecilDefinitionsFactory.Type(Context, enumType, enumSymbol.ContainingNamespace?.FullyQualifiedName(), enumSymbol.Name, attrs + " | TypeAttributes.Sealed", Context.TypeResolver.Bcl.System.Enum, outerTypeVariable,false, Array.Empty<ITypeSymbol>(), [], []);
             AddCecilExpressions(Context, typeDef);
 
-            var parentName = enumSymbol.ContainingSymbol.FullyQualifiedName();
+            var parentName = enumSymbol.ContainingSymbol.ToDisplayString();
             using (Context.DefinitionVariables.WithCurrent(parentName, enumSymbol.FullyQualifiedName(), VariableMemberKind.Type, enumType))
             {
                 //.class private auto ansi MyEnum

Cecilifier.Core/AST/GlobalStatementHandler.cs

@@ -1,5 +1,4 @@
 using System;
-using System.Collections.Generic;
 using System.Linq;
 using Cecilifier.Core.Extensions;
 using Cecilifier.Core.Misc;
@@ -28,7 +27,7 @@ internal GlobalStatementHandler(IVisitorContext context, GlobalStatementSyntax f
                 "Program",
                 typeModifiers,
                 context.TypeResolver.Bcl.System.Object,
-                null, // Top level type has no outer type.
+                DefinitionVariable.NotFound, // Top level type has no outer type.
                 false,
                 Array.Empty<ITypeSymbol>(), 
                 [], 
@@ -72,7 +71,7 @@ internal GlobalStatementHandler(IVisitorContext context, GlobalStatementSyntax f
 
         public bool HandleGlobalStatement(GlobalStatementSyntax node)
         {
-            using (context.DefinitionVariables.WithCurrent(string.Empty, "Program", VariableMemberKind.Type, typeVar))
+            using (context.DefinitionVariables.WithCurrent("<global namespace>", "Program", VariableMemberKind.Type, typeVar))
             using (context.DefinitionVariables.WithCurrentMethod("Program", "<Main>$", [], 0, methodVar))
             {
                 if (node.Statement.IsKind(SyntaxKind.LocalFunctionStatement))

Cecilifier.Core/AST/MethodDeclarationVisitor.cs

@@ -96,14 +96,14 @@ public override void VisitParameter(ParameterSyntax node)
             using var _ = LineInformationTracker.Track(Context, node);
 
             var containingSymbol = Context.SemanticModel.GetDeclaredSymbol(node).EnsureNotNull().ContainingSymbol;
-            var forwardedParamVar = Context.DefinitionVariables.GetVariable(node.Identifier.ValueText, VariableMemberKind.Parameter, containingSymbol.ToDisplayString());
+            var forwardedParamVar = Context.DefinitionVariables.GetVariable(node.Identifier.ValueText, VariableMemberKind.Parameter, containingSymbol.OriginalDefinition.ToDisplayString());
             if (forwardedParamVar.IsValid)
             {
                 paramVar = forwardedParamVar.VariableName;
             }
             else
             {
-                Context.DefinitionVariables.RegisterNonMethod(containingSymbol.ToDisplayString(), node.Identifier.ValueText, VariableMemberKind.Parameter, paramVar);
+                Context.DefinitionVariables.RegisterNonMethod(containingSymbol.OriginalDefinition.ToDisplayString(), node.Identifier.ValueText, VariableMemberKind.Parameter, paramVar);
                 var exps = CecilDefinitionsFactory.Parameter(Context, node, methodVar.VariableName, paramVar);
                 AddCecilExpressions(Context, exps);
             }
@@ -201,7 +201,7 @@ protected void ProcessMethodDeclaration<T>(T node, string variableName, string s
             var methodSymbol = Context.GetDeclaredSymbol(node);
             ProcessMethodDeclarationInternal(
                             node,
-                            methodSymbol.ContainingSymbol.Name,
+                            methodSymbol.ContainingSymbol.ToDisplayString(),
                             variableName,
                             methodSymbol,
                             node.Modifiers,

Cecilifier.Core/AST/PropertyDeclarationVisitor.cs

@@ -134,7 +134,7 @@ bool AttributeAlreadyAddedForAnotherMember()
 
             PropertyGenerator generator = new (Context);
             var propertyGenerationData = new PropertyGenerationData(
-                                    propertySymbol.ContainingType.Name,
+                                    propertySymbol.ContainingType.ToDisplayString(),
                                     propertyDeclaringTypeVar,
                                     propertySymbol.ContainingSymbol is INamedTypeSymbol { IsGenericType: true} && propertySymbol.IsDefinedInCurrentAssembly(Context),
                                     propDefVar,

Cecilifier.Core/AST/SyntaxWalkerBase.cs

@@ -739,9 +739,12 @@ protected static void HandleAttributesInTypeParameter(IVisitorContext context, I
             foreach (var typeParameter in typeParameters)
             {
                 var symbol = context.SemanticModel.GetDeclaredSymbol(typeParameter).EnsureNotNull();
-                var parentName = symbol.TypeParameterKind == TypeParameterKind.Method ? symbol.DeclaringMethod?.Name : symbol.DeclaringType?.Name;
-                var found = context.DefinitionVariables.GetVariable(typeParameter.Identifier.Text, VariableMemberKind.TypeParameter, parentName);
-                HandleAttributesInMemberDeclaration(context, typeParameter.AttributeLists, found.VariableName);
+                var parentName = symbol.TypeParameterKind == TypeParameterKind.Method ? symbol.DeclaringMethod?.OriginalDefinition.ToDisplayString() : symbol.DeclaringType?.OriginalDefinition.ToDisplayString();
+                var typeParamVariable = context.DefinitionVariables.GetVariable(typeParameter.Identifier.Text, VariableMemberKind.TypeParameter, parentName);
+                if (!typeParamVariable.IsValid)
+                    throw new Exception($"Failed to find variable for {symbol.ToDisplayString()}");
+
+                HandleAttributesInMemberDeclaration(context, typeParameter.AttributeLists, typeParamVariable.VariableName);
             }
         }
 
@@ -752,7 +755,7 @@ private static void HandleAttributesInMemberDeclaration(IVisitorContext context,
                 var attributeType = context.SemanticModel.GetSymbolInfo(attribute).Symbol.EnsureNotNull<ISymbol, IMethodSymbol>().ContainingType;
 
                 //https://github.com/adrianoc/cecilifier/issues/311
-                TypeDeclarationVisitor.EnsureForwardedTypeDefinition(context, attributeType, Array.Empty<TypeParameterSyntax>());
+                TypeDeclarationVisitor.EnsureForwardedTypeDefinition(context, attributeType, attributeType.OriginalDefinition.TypeParameters.SelectMany(tp => tp.DeclaringSyntaxReferences).Select(dsr => (TypeParameterSyntax) dsr.GetSyntax()));
 
                 var attrsExp = attributeType.AttributeKind() switch
                     {