Use stack so nested types get correct initializers

GrahamTheCoder · GrahamTheCoder · commit 1a8c5c862293 · 2020-05-02T13:59:50.000+01:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,12 +10,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 
 ### VB -> C#
 
-* Improve multi-declaration field conversion for arrrays - [#559](https://github.com/icsharpcode/CodeConverter/issues/559)
+* Improve multi-declaration field conversion for arrays - [#559](https://github.com/icsharpcode/CodeConverter/issues/559)
 * Add parentheses around ternary statement - [#565](https://github.com/icsharpcode/CodeConverter/issues/565)
 * When converting ForEach loop, avoid duplicate variable compilation issue [#558](https://github.com/icsharpcode/CodeConverter/issues/558)
 * Improvements to for loop with missing semantic info - [#482](https://github.com/icsharpcode/CodeConverter/issues/482)
 * Fix logic issue when converting property passed byref - [#324](https://github.com/icsharpcode/CodeConverter/issues/324)
 * Fix logic issue when converting expression passed in byref within conditional expression - [#310](https://github.com/icsharpcode/CodeConverter/issues/310)
+* Added constructors now only added to the relevant type - not other types in the same file
 
 ### C# -> VB
 
diff --git a/CodeConverter/CSharp/DeclarationNodeVisitor.cs b/CodeConverter/CSharp/DeclarationNodeVisitor.cs
@@ -19,6 +19,7 @@
 
 namespace ICSharpCode.CodeConverter.CSharp
 {
+
     /// <summary>
     /// Declaration nodes, and nodes only used directly in that declaration (i.e. never within an expression)
     /// e.g. Class, Enum, TypeConstraint
@@ -33,18 +34,15 @@ internal class DeclarationNodeVisitor : VBasic.VisualBasicSyntaxVisitor<Task<CSh
         private readonly SyntaxGenerator _csSyntaxGenerator;
         private readonly Compilation _compilation;
         private readonly SemanticModel _semanticModel;
-        private readonly MethodsWithHandles _methodsWithHandles = new MethodsWithHandles();
         private readonly Dictionary<VBSyntax.StatementSyntax, MemberDeclarationSyntax[]> _additionalDeclarations = new Dictionary<VBSyntax.StatementSyntax, MemberDeclarationSyntax[]>();
-        private readonly AdditionalInitializers _additionalInitializers;
+        private readonly TypeContext _typeContext = new TypeContext();
         private readonly HoistedNodeState _additionalLocals = new HoistedNodeState();
         private uint _failedMemberConversionMarkerCount;
         private readonly HashSet<string> _extraUsingDirectives = new HashSet<string>();
         private readonly VisualBasicEqualityComparison _visualBasicEqualityComparison;
         private HashSet<string> _accessedThroughMyClass;
         public CommentConvertingVisitorWrapper TriviaConvertingDeclarationVisitor { get; }
         private readonly CommentConvertingVisitorWrapper _triviaConvertingExpressionVisitor;
-
-
         private string _topAncestorNamespace;
 
         private CommonConversions CommonConversions { get; }
@@ -62,8 +60,7 @@ public DeclarationNodeVisitor(Document document, Compilation compilation, Semant
             var expressionEvaluator = new ExpressionEvaluator(semanticModel, _visualBasicEqualityComparison);
             var typeConversionAnalyzer = new TypeConversionAnalyzer(semanticModel, csCompilation, _extraUsingDirectives, _csSyntaxGenerator, expressionEvaluator);
             CommonConversions = new CommonConversions(document, semanticModel, typeConversionAnalyzer, csSyntaxGenerator, csCompilation);
-            _additionalInitializers = new AdditionalInitializers();
-            var expressionNodeVisitor = new ExpressionNodeVisitor(semanticModel, _visualBasicEqualityComparison, _additionalLocals, csCompilation, _methodsWithHandles, CommonConversions, _extraUsingDirectives);
+            var expressionNodeVisitor = new ExpressionNodeVisitor(semanticModel, _visualBasicEqualityComparison, _additionalLocals, csCompilation, _typeContext, CommonConversions, _extraUsingDirectives);
             _triviaConvertingExpressionVisitor = expressionNodeVisitor.TriviaConvertingExpressionVisitor;
             _createMethodBodyVisitorAsync = expressionNodeVisitor.CreateMethodBodyVisitor;
             CommonConversions.TriviaConvertingExpressionVisitor = _triviaConvertingExpressionVisitor;
@@ -196,15 +193,12 @@ private async Task<string> WithDeclarationCasing(VBSyntax.NamespaceBlockSyntax n
         private async Task<IEnumerable<MemberDeclarationSyntax>> ConvertMembers(VBSyntax.TypeBlockSyntax parentType)
         {
             var members = parentType.Members;
+            var additionalInitializers = new AdditionalInitializers();
+            var methodsWithHandles = MethodsWithHandles.Create(GetMethodWithHandles(parentType));
 
-            //TODO: Store these per-type so nested classes aren't affected
-            _methodsWithHandles.Initialize(GetMethodWithHandles(parentType));
-            _additionalInitializers.AdditionalInstanceInitializers.Clear();
-            _additionalInitializers.AdditionalInstanceInitializers.Clear();
+            if (methodsWithHandles.Any()) _extraUsingDirectives.Add("System.Runtime.CompilerServices");//For MethodImplOptions.Synchronized
 
-            if (_methodsWithHandles.Any()) _extraUsingDirectives.Add("System.Runtime.CompilerServices");//For MethodImplOptions.Synchronized
-
-            var directlyConvertedMembers = await GetDirectlyConvertMembers();
+            IEnumerable<MemberDeclarationSyntax> directlyConvertedMembers = await GetDirectlyConvertedMembers(additionalInitializers, methodsWithHandles);
 
             var namedTypeSymbol = _semanticModel.GetDeclaredSymbol(parentType);
             bool shouldAddTypeWideInitToThisPart = ShouldAddTypeWideInitToThisPart(parentType, namedTypeSymbol);
@@ -213,19 +207,24 @@ private async Task<IEnumerable<MemberDeclarationSyntax>> ConvertMembers(VBSyntax
             if (shouldAddTypeWideInitToThisPart) {
                 if (requiresInitializeComponent) {
                     // Constructor event handlers not required since they'll be inside InitializeComponent
-                    directlyConvertedMembers = directlyConvertedMembers.Concat(_methodsWithHandles.CreateDelegatingMethodsRequiredByInitializeComponent());
+                    directlyConvertedMembers = directlyConvertedMembers.Concat(methodsWithHandles.CreateDelegatingMethodsRequiredByInitializeComponent());
                 } else {
-                    _additionalInitializers.AdditionalInstanceInitializers.AddRange(_methodsWithHandles.GetConstructorEventHandlers());
+                    additionalInitializers.AdditionalInstanceInitializers.AddRange(methodsWithHandles.GetConstructorEventHandlers());
                 }
             }
 
-            return _additionalInitializers.WithAdditionalInitializers(namedTypeSymbol, directlyConvertedMembers.ToList(), CommonConversions.ConvertIdentifier(parentType.BlockStatement.Identifier), shouldAddTypeWideInitToThisPart, requiresInitializeComponent);
+            return additionalInitializers.WithAdditionalInitializers(namedTypeSymbol, directlyConvertedMembers.ToList(), CommonConversions.ConvertIdentifier(parentType.BlockStatement.Identifier), shouldAddTypeWideInitToThisPart, requiresInitializeComponent);
 
-            async Task<IEnumerable<MemberDeclarationSyntax>> GetDirectlyConvertMembers()
+            async Task<MemberDeclarationSyntax[]> GetDirectlyConvertedMembers(AdditionalInitializers additionalInitializers, MethodsWithHandles methodsWithHandles)
             {
-                return await members.SelectManyAsync(async member =>
-                    new[]{await ConvertMember(member)}.Concat(GetAdditionalDeclarations(member)));
-
+                _typeContext.Push(methodsWithHandles, additionalInitializers);
+                try {
+                    var convertedMembers = await members.SelectManyAsync(async member =>
+                        new[] { await ConvertMember(member) }.Concat(GetAdditionalDeclarations(member)));
+                    return convertedMembers.ToArray();
+                } finally {
+                    _typeContext.Pop();
+                }
             }
         }
 
@@ -515,14 +514,15 @@ private IEnumerable<MemberDeclarationSyntax> CreateWithEventsMembers(SyntaxToken
                 .Where(a => a.Initializer != null)
                 .ToDictionary(v => v.Identifier.Text, v => v.Initializer);
             var fieldDecl = decl.RemoveNodes(initializers.Values, SyntaxRemoveOptions.KeepNoTrivia);
+            var initializerState = _typeContext.Initializers;
             var initializerCollection = convertedModifiers.Any(m => m.IsKind(Microsoft.CodeAnalysis.CSharp.SyntaxKind.StaticKeyword))
-                ? _additionalInitializers.AdditionalStaticInitializers
-                : _additionalInitializers.AdditionalInstanceInitializers;
+                ? initializerState.AdditionalStaticInitializers
+                : initializerState.AdditionalInstanceInitializers;
             foreach (var initializer in initializers) {
                 initializerCollection.Add((SyntaxFactory.IdentifierName(initializer.Key), Microsoft.CodeAnalysis.CSharp.SyntaxKind.SimpleAssignmentExpression, initializer.Value.Value));
             }
 
-            var fieldDecls = _methodsWithHandles.GetDeclarationsForFieldBackedProperty(fieldDecl,
+            var fieldDecls = _typeContext.MethodsWithHandles.GetDeclarationsForFieldBackedProperty(fieldDecl,
                 convertedModifiers, SyntaxFactory.List(attributes));
             return fieldDecls;
         }
@@ -897,7 +897,7 @@ public override async Task<CSharpSyntaxNode> VisitMethodBlock(VBSyntax.MethodBlo
 
             if (node.SubOrFunctionStatement.Identifier.Text == "InitializeComponent" && node.SubOrFunctionStatement.IsKind(VBasic.SyntaxKind.SubStatement) && declaredSymbol.ContainingType.IsDesignerGeneratedTypeWithInitializeComponent(_compilation)) {
                 var firstResumeLayout = convertedStatements.Statements.FirstOrDefault(IsThisResumeLayoutInvocation) ?? convertedStatements.Statements.Last();
-                convertedStatements = convertedStatements.InsertNodesBefore(firstResumeLayout, _methodsWithHandles.GetInitializeComponentClassEventHandlers());
+                convertedStatements = convertedStatements.InsertNodesBefore(firstResumeLayout, _typeContext.MethodsWithHandles.GetInitializeComponentClassEventHandlers());
             }
 
             var body = WithImplicitReturnStatements(node, convertedStatements, csReturnVariableOrNull);
diff --git a/CodeConverter/CSharp/ExpressionNodeVisitor.cs b/CodeConverter/CSharp/ExpressionNodeVisitor.cs
@@ -36,7 +36,7 @@ internal class ExpressionNodeVisitor : VBasic.VisualBasicSyntaxVisitor<Task<CSha
         private readonly VisualBasicEqualityComparison _visualBasicEqualityComparison;
         private readonly Stack<ExpressionSyntax> _withBlockLhs = new Stack<ExpressionSyntax>();
         private readonly HoistedNodeState _additionalLocals;
-        private readonly MethodsWithHandles _methodsWithHandles;
+        private readonly TypeContext _typeContext;
         private readonly QueryConverter _queryConverter;
         private readonly Lazy<IDictionary<ITypeSymbol, string>> _convertMethodsLookupByReturnType;
         private readonly Compilation _csCompilation;
@@ -45,7 +45,7 @@ internal class ExpressionNodeVisitor : VBasic.VisualBasicSyntaxVisitor<Task<CSha
 
         public ExpressionNodeVisitor(SemanticModel semanticModel,
             VisualBasicEqualityComparison visualBasicEqualityComparison, HoistedNodeState additionalLocals,
-            Compilation csCompilation, MethodsWithHandles methodsWithHandles, CommonConversions commonConversions,
+            Compilation csCompilation, TypeContext typeContext, CommonConversions commonConversions,
             HashSet<string> extraUsingDirectives)
         {
             CommonConversions = commonConversions;
@@ -56,7 +56,7 @@ public ExpressionNodeVisitor(SemanticModel semanticModel,
             TriviaConvertingExpressionVisitor = new CommentConvertingVisitorWrapper(this, _semanticModel.SyntaxTree);
             _queryConverter = new QueryConverter(commonConversions, TriviaConvertingExpressionVisitor);
             _csCompilation = csCompilation;
-            _methodsWithHandles = methodsWithHandles;
+            _typeContext = typeContext;
             _extraUsingDirectives = extraUsingDirectives;
 
             // If this isn't needed, the assembly with Conversions may not be referenced, so this must be done lazily
@@ -1303,7 +1303,7 @@ public override async Task<CSharpSyntaxNode> VisitTypeArgumentList(VBasic.Syntax
 
         public async Task<VBasic.VisualBasicSyntaxVisitor<Task<SyntaxList<StatementSyntax>>>> CreateMethodBodyVisitor(VBasic.VisualBasicSyntaxNode node, bool isIterator = false, IdentifierNameSyntax csReturnVariable = null)
         {
-            var methodBodyVisitor = await MethodBodyExecutableStatementVisitor.CreateAsync(node, _semanticModel, TriviaConvertingExpressionVisitor, CommonConversions, _withBlockLhs, _extraUsingDirectives, _additionalLocals, _methodsWithHandles, isIterator, csReturnVariable);
+            var methodBodyVisitor = await MethodBodyExecutableStatementVisitor.CreateAsync(node, _semanticModel, TriviaConvertingExpressionVisitor, CommonConversions, _withBlockLhs, _extraUsingDirectives, _additionalLocals, _typeContext, isIterator, csReturnVariable);
             return methodBodyVisitor.CommentConvertingVisitor;
         }
 
diff --git a/CodeConverter/CSharp/ITypeContext.cs b/CodeConverter/CSharp/ITypeContext.cs
@@ -0,0 +1,8 @@
+﻿namespace ICSharpCode.CodeConverter.CSharp
+{
+    internal interface ITypeContext
+    {
+        AdditionalInitializers Initializers { get; }
+        MethodsWithHandles MethodsWithHandles { get; }
+    }
+}
diff --git a/CodeConverter/CSharp/MethodBodyExecutableStatementVisitor.cs b/CodeConverter/CSharp/MethodBodyExecutableStatementVisitor.cs
@@ -41,11 +41,11 @@ internal class MethodBodyExecutableStatementVisitor : VBasic.VisualBasicSyntaxVi
 
         private CommonConversions CommonConversions { get; }
 
-        public static async Task<MethodBodyExecutableStatementVisitor> CreateAsync(VBasic.VisualBasicSyntaxNode node, SemanticModel semanticModel, CommentConvertingVisitorWrapper triviaConvertingExpressionVisitor, CommonConversions commonConversions, Stack<ExpressionSyntax> withBlockLhs, HashSet<string> extraUsingDirectives, HoistedNodeState additionalLocals, MethodsWithHandles methodsWithHandles, bool isIterator, IdentifierNameSyntax csReturnVariable)
+        public static async Task<MethodBodyExecutableStatementVisitor> CreateAsync(VBasic.VisualBasicSyntaxNode node, SemanticModel semanticModel, CommentConvertingVisitorWrapper triviaConvertingExpressionVisitor, CommonConversions commonConversions, Stack<ExpressionSyntax> withBlockLhs, HashSet<string> extraUsingDirectives, HoistedNodeState additionalLocals, TypeContext typeContext, bool isIterator, IdentifierNameSyntax csReturnVariable)
         {
             var solution = commonConversions.Document.Project.Solution;
             var declarationsToInlineInLoop = await solution.GetDescendantsToInlineInLoopAsync(semanticModel, node);
-            return new MethodBodyExecutableStatementVisitor(node, semanticModel, triviaConvertingExpressionVisitor, commonConversions, withBlockLhs, extraUsingDirectives, additionalLocals, methodsWithHandles, declarationsToInlineInLoop) {
+            return new MethodBodyExecutableStatementVisitor(node, semanticModel, triviaConvertingExpressionVisitor, commonConversions, withBlockLhs, extraUsingDirectives, additionalLocals, typeContext, declarationsToInlineInLoop) {
                 IsIterator = isIterator,
                 ReturnVariable = csReturnVariable,
             };
@@ -54,15 +54,15 @@ public static async Task<MethodBodyExecutableStatementVisitor> CreateAsync(VBasi
         private MethodBodyExecutableStatementVisitor(VBasic.VisualBasicSyntaxNode methodNode, SemanticModel semanticModel,
             CommentConvertingVisitorWrapper expressionVisitor, CommonConversions commonConversions,
             Stack<ExpressionSyntax> withBlockLhs, HashSet<string> extraUsingDirectives,
-            HoistedNodeState additionalLocals, MethodsWithHandles methodsWithHandles, HashSet<ILocalSymbol> localsToInlineInLoop)
+            HoistedNodeState additionalLocals, TypeContext typeContext, HashSet<ILocalSymbol> localsToInlineInLoop)
         {
             _methodNode = methodNode;
             _semanticModel = semanticModel;
             _expressionVisitor = expressionVisitor;
             CommonConversions = commonConversions;
             _withBlockLhs = withBlockLhs;
             _extraUsingDirectives = extraUsingDirectives;
-            _methodsWithHandles = methodsWithHandles;
+            _methodsWithHandles = typeContext.MethodsWithHandles;
             var byRefParameterVisitor = new HoistedNodeStateVisitor(this, additionalLocals, semanticModel, _generatedNames);
             CommentConvertingVisitor = new CommentConvertingMethodBodyVisitor(byRefParameterVisitor);
             _vbBooleanTypeSymbol = _semanticModel.Compilation.GetTypeByMetadataName("System.Boolean");
diff --git a/CodeConverter/CSharp/MethodsWithHandles.cs b/CodeConverter/CSharp/MethodsWithHandles.cs
@@ -15,12 +15,19 @@ internal class MethodsWithHandles
         private List<MethodWithHandles> _methodWithHandleses;
         private ILookup<string, MethodWithHandles> _handledMethodsFromPropertyWithEventName;
 
-        public void Initialize(List<MethodWithHandles> methodWithHandleses)
+
+        public MethodsWithHandles(List<MethodWithHandles> methodWithHandleses, ILookup<string, MethodWithHandles> handledMethodsFromPropertyWithEventName)
         {
             _methodWithHandleses = methodWithHandleses;
-            _handledMethodsFromPropertyWithEventName = methodWithHandleses
+            _handledMethodsFromPropertyWithEventName = handledMethodsFromPropertyWithEventName;
+        }
+
+        public static MethodsWithHandles Create(List<MethodWithHandles> methodWithHandleses)
+        {
+            var handledMethodsFromPropertyWithEventName = methodWithHandleses
                 .SelectMany(m => m.HandledPropertyEventCSharpIds.Select(h => (EventPropertyName: h.Item1.Text, MethodWithHandles: m)))
                 .ToLookup(m => m.EventPropertyName, m => m.MethodWithHandles);
+            return new MethodsWithHandles(methodWithHandleses, handledMethodsFromPropertyWithEventName);
         }
 
         public bool Any()
diff --git a/CodeConverter/CSharp/TypeContext.cs b/CodeConverter/CSharp/TypeContext.cs
@@ -0,0 +1,19 @@
+﻿using System.Collections.Generic;
+
+namespace ICSharpCode.CodeConverter.CSharp
+{
+    internal class TypeContext : ITypeContext
+    {
+        private readonly Stack<(AdditionalInitializers Initializers, MethodsWithHandles Methods)> _contextStack = new Stack<(AdditionalInitializers Initializers, MethodsWithHandles Methods)>();
+
+        public AdditionalInitializers Initializers => _contextStack.Peek().Initializers;
+        public MethodsWithHandles MethodsWithHandles => _contextStack.Peek().Methods;
+
+        public void Push(MethodsWithHandles methodWithHandles, AdditionalInitializers additionalInitializers)
+        {
+            _contextStack.Push((additionalInitializers, methodWithHandles));
+        }
+
+        public void Pop() => _contextStack.Pop();
+    }
+}
diff --git a/Tests/CSharp/MemberTests/EventMemberTests.cs b/Tests/CSharp/MemberTests/EventMemberTests.cs
@@ -240,6 +240,9 @@ End Sub
 
     Sub PrintTestMessage3() Handles NonSharedEventClassInstance.TestEvent
     End Sub
+
+    Public Class NestedShouldNotGainConstructor
+    End Class
 End Class
 
 Public Class ShouldNotGainConstructor
@@ -339,20 +342,19 @@ public static void PrintTestMessage2()
     public void PrintTestMessage3()
     {
     }
+
+    public partial class NestedShouldNotGainConstructor
+    {
+    }
 }
 
 public partial class ShouldNotGainConstructor
 {
-    static ShouldNotGainConstructor()
-    {
-        SharedEventClassInstance = new MyEventClass();
-    }
 }
 1 source compilation errors:
 BC30516: Overload resolution failed because no accessible 'New' accepts this number of arguments.
-2 target compilation errors:
-CS1547: Keyword 'void' cannot be used in this context
-CS0103: The name 'SharedEventClassInstance' does not exist in the current context", hasLineCommentConversionIssue: true);//TODO: Improve comment mapping for events
+1 target compilation errors:
+CS1547: Keyword 'void' cannot be used in this context", hasLineCommentConversionIssue: true);//TODO: Improve comment mapping for events
         }
 
         [Fact]

Original file line number	Diff line number	Diff line change
`@@ -240,6 +240,9 @@ End Sub`
`240`	`240`
`241`	`241`	`Sub PrintTestMessage3() Handles NonSharedEventClassInstance.TestEvent`
`242`	`242`	`End Sub`
	`243`	`+`
	`244`	`+ Public Class NestedShouldNotGainConstructor`
	`245`	`+ End Class`
`243`	`246`	`End Class`
`244`	`247`
`245`	`248`	`Public Class ShouldNotGainConstructor`
`@@ -339,20 +342,19 @@ public static void PrintTestMessage2()`
`339`	`342`	`public void PrintTestMessage3()`
`340`	`343`	`{`
`341`	`344`	`}`
	`345`	`+`
	`346`	`+ public partial class NestedShouldNotGainConstructor`
	`347`	`+ {`
	`348`	`+ }`
`342`	`349`	`}`
`343`	`350`
`344`	`351`	`public partial class ShouldNotGainConstructor`
`345`	`352`	`{`
`346`		`- static ShouldNotGainConstructor()`
`347`		`- {`
`348`		`- SharedEventClassInstance = new MyEventClass();`
`349`		`- }`
`350`	`353`	`}`
`351`	`354`	`1 source compilation errors:`
`352`	`355`	`BC30516: Overload resolution failed because no accessible 'New' accepts this number of arguments.`
`353`		`-2 target compilation errors:`
`354`		`-CS1547: Keyword 'void' cannot be used in this context`
`355`		`-CS0103: The name 'SharedEventClassInstance' does not exist in the current context", hasLineCommentConversionIssue: true);//TODO: Improve comment mapping for events`
	`356`	`+1 target compilation errors:`
	`357`	`+CS1547: Keyword 'void' cannot be used in this context", hasLineCommentConversionIssue: true);//TODO: Improve comment mapping for events`
`356`	`358`	`}`
`357`	`359`
`358`	`360`	`[Fact]`