Merge pull request #566 from icsharpcode/byref

Byref

Author: GrahamTheCoder

CHANGELOG.md

@@ -14,6 +14,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 * 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)
 
 ### C# -> VB
 

CodeConverter/CSharp/AdditionalAssignment.cs

@@ -0,0 +1,16 @@
+using Microsoft.CodeAnalysis.CSharp.Syntax;
+
+namespace ICSharpCode.CodeConverter.CSharp
+{
+    internal class AdditionalAssignment : IHoistedNode
+    {
+        public AdditionalAssignment(ExpressionSyntax lhs, ExpressionSyntax rhs)
+        {
+            RightHandSide = rhs ?? throw new System.ArgumentNullException(nameof(rhs));
+            LeftHandSide = lhs ?? throw new System.ArgumentNullException(nameof(lhs));
+        }
+
+        public ExpressionSyntax LeftHandSide { get; set; }
+        public ExpressionSyntax RightHandSide { get; }
+    }
+}

CodeConverter/CSharp/AdditionalLocal.cs …onverter/CSharp/AdditionalDeclaration.csCodeConverter/CSharp/AdditionalLocal.cs renamed to CodeConverter/CSharp/AdditionalDeclaration.cs CodeConverter/CSharp/AdditionalLocal.cs renamed to CodeConverter/CSharp/AdditionalDeclaration.cs

@@ -1,25 +1,27 @@
 using System;
+using System.Collections.Generic;
+using System.Linq;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CSharp;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
 
 namespace ICSharpCode.CodeConverter.CSharp
 {
-    public class AdditionalLocal
+    internal class AdditionalDeclaration : IHoistedNode
     {
         public string Prefix { get; }
         public string Id { get; }
         public ExpressionSyntax Initializer { get; }
         public TypeSyntax Type { get; }
 
-        public AdditionalLocal(string prefix, ExpressionSyntax initializer, TypeSyntax type)
+        public AdditionalDeclaration(string prefix, ExpressionSyntax initializer, TypeSyntax type)
         {
             Prefix = prefix;
             Id = $"ph{Guid.NewGuid().ToString("N")}";
             Initializer = initializer;
             Type = type;
         }
 
-        public IdentifierNameSyntax IdentifierName => SyntaxFactory.IdentifierName(Id).WithAdditionalAnnotations(AdditionalLocals.Annotation);
+        public IdentifierNameSyntax IdentifierName => SyntaxFactory.IdentifierName(Id).WithAdditionalAnnotations(HoistedNodeState.Annotation);
     }
 }

CodeConverter/CSharp/AdditionalLocals.cs

@@ -508,6 +508,11 @@ public static AttributeArgumentListSyntax CreateAttributeArgumentList(params Att
             return SyntaxFactory.AttributeArgumentList(SyntaxFactory.SeparatedList(attributeArgumentSyntaxs));
         }
 
+        public static CSSyntax.LocalDeclarationStatementSyntax CreateLocalVariableDeclarationAndAssignment(string variableName, ExpressionSyntax initValue)
+        {
+            return SyntaxFactory.LocalDeclarationStatement(CreateVariableDeclarationAndAssignment(variableName, initValue));
+        }
+
         public static VariableDeclarationSyntax CreateVariableDeclarationAndAssignment(string variableName,
             ExpressionSyntax initValue, TypeSyntax explicitType = null)
         {

CodeConverter/CSharp/CommonConversions.cs

@@ -29,15 +29,14 @@ internal class DeclarationNodeVisitor : VBasic.VisualBasicSyntaxVisitor<Task<CSh
         private static readonly Type DllImportType = typeof(DllImportAttribute);
         private static readonly Type CharSetType = typeof(CharSet);
         private static readonly SyntaxToken SemicolonToken = SyntaxFactory.Token(Microsoft.CodeAnalysis.CSharp.SyntaxKind.SemicolonToken);
-        private static readonly TypeSyntax VarType = SyntaxFactory.ParseTypeName("var");
         private readonly CSharpCompilation _csCompilation;
         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 AdditionalLocals _additionalLocals = new AdditionalLocals();
+        private readonly HoistedNodeState _additionalLocals = new HoistedNodeState();
         private uint _failedMemberConversionMarkerCount;
         private readonly HashSet<string> _extraUsingDirectives = new HashSet<string>();
         private readonly VisualBasicEqualityComparison _visualBasicEqualityComparison;
@@ -490,7 +489,7 @@ private IEnumerable<MemberDeclarationSyntax> CreateMemberDeclarations(IReadOnlyC
                     foreach (var f in fieldDecls) yield return f;
                 } else
                 {
-                    if (_additionalLocals.Count() > 0) {
+                    if (_additionalLocals.GetDeclarations().Count() > 0) {
                         foreach (var additionalDecl in CreateAdditionalLocalMembers(convertedModifiers, attributes, decl)) {
                             yield return additionalDecl;
                         }
@@ -547,26 +546,19 @@ private IEnumerable<MemberDeclarationSyntax> CreateAdditionalLocalMembers(Syntax
             var methodName = invocationExpressionSyntax.Expression
                 .ChildNodes().OfType<SimpleNameSyntax>().Last();
             var newMethodName = $"{methodName.Identifier.ValueText}_{v.Identifier.ValueText}";
-            var localVars = _additionalLocals.Select(l => l.Value)
-                .Select(al =>
-                    SyntaxFactory.LocalDeclarationStatement(
-                        CommonConversions.CreateVariableDeclarationAndAssignment(al.Prefix, al.Initializer)))
-                .Cast<StatementSyntax>().ToList();
-            var newInitializer = v.Initializer.Value.ReplaceNodes(
-                v.Initializer.Value.GetAnnotatedNodes(AdditionalLocals.Annotation), (an, _) => {
-                                // This should probably use a unique name like in MethodBodyVisitor - a collision is far less likely here
-                                var id = ((IdentifierNameSyntax)an).Identifier.ValueText;
-                    return SyntaxFactory.IdentifierName(_additionalLocals[id].Prefix);
-                });
-            var body = SyntaxFactory.Block(
-                localVars.Concat(SyntaxFactory.SingletonList(SyntaxFactory.ReturnStatement(newInitializer))));
-            var methodAttrs = SyntaxFactory.List<AttributeListSyntax>();
+            var declarationInfo = _additionalLocals.GetDeclarations();
+            
+            var localVars = declarationInfo
+                .Select(al => CommonConversions.CreateLocalVariableDeclarationAndAssignment(al.Prefix, al.Initializer))
+                .ToArray<StatementSyntax>();
+
+            // This should probably use a unique name like in MethodBodyVisitor - a collision is far less likely here
+            var newNames = declarationInfo.ToDictionary(l => l.Id, l => l.Prefix);
+            var newInitializer = HoistedNodeState.ReplaceNames(v.Initializer.Value, newNames);
+
+            var body = SyntaxFactory.Block(localVars.Concat(SyntaxFactory.ReturnStatement(newInitializer).Yield()));
             // Method calls in initializers must be static in C# - Supporting this is #281
-            var modifiers = SyntaxFactory.TokenList(SyntaxFactory.Token(Microsoft.CodeAnalysis.CSharp.SyntaxKind.StaticKeyword));
-            var typeConstraints = SyntaxFactory.List<TypeParameterConstraintClauseSyntax>();
-            var parameterList = SyntaxFactory.ParameterList();
-            var methodDecl = SyntaxFactory.MethodDeclaration(methodAttrs, modifiers, decl.Type, null,
-                SyntaxFactory.Identifier(newMethodName), null, parameterList, typeConstraints, body, null);
+            var methodDecl = CreateParameterlessMethod(newMethodName, decl.Type, body);
             yield return methodDecl;
 
             var newVar =
@@ -578,6 +570,17 @@ private IEnumerable<MemberDeclarationSyntax> CreateAdditionalLocalMembers(Syntax
             yield return SyntaxFactory.FieldDeclaration(SyntaxFactory.List(attributes), convertedModifiers, newVarDecl);
         }
 
+        private static MethodDeclarationSyntax CreateParameterlessMethod(string newMethodName, TypeSyntax type, BlockSyntax body)
+        {
+            var modifiers = SyntaxFactory.TokenList(SyntaxFactory.Token(Microsoft.CodeAnalysis.CSharp.SyntaxKind.StaticKeyword));
+            var typeConstraints = SyntaxFactory.List<TypeParameterConstraintClauseSyntax>();
+            var parameterList = SyntaxFactory.ParameterList();
+            var methodAttrs = SyntaxFactory.List<AttributeListSyntax>();
+            var methodDecl = SyntaxFactory.MethodDeclaration(methodAttrs, modifiers, type, null,
+                SyntaxFactory.Identifier(newMethodName), null, parameterList, typeConstraints, body, null);
+            return methodDecl;
+        }
+
         private List<MethodWithHandles> GetMethodWithHandles(VBSyntax.TypeBlockSyntax parentType)
         {
             if (parentType == null || !(this._semanticModel.GetDeclaredSymbol((global::Microsoft.CodeAnalysis.SyntaxNode)parentType) is ITypeSymbol containingType)) return new List<MethodWithHandles>();
@@ -1077,7 +1080,7 @@ public override async Task<CSharpSyntaxNode> VisitEventBlock(VBSyntax.EventBlock
             var attributes = await block.AttributeLists.SelectManyAsync(CommonConversions.ConvertAttribute);
             var modifiers = CommonConversions.ConvertModifiers(block, block.Modifiers, GetMemberContext(node));
 
-            var rawType = (TypeSyntax) await (block.AsClause?.Type).AcceptAsync(_triviaConvertingExpressionVisitor) ?? VarType;
+            var rawType = (TypeSyntax) await (block.AsClause?.Type).AcceptAsync(_triviaConvertingExpressionVisitor) ?? ValidSyntaxFactory.VarType;
 
             var convertedAccessors = await node.Accessors.SelectAsync(async a => await a.AcceptAsync(TriviaConvertingDeclarationVisitor));
             _additionalDeclarations.Add(node, convertedAccessors.OfType<MemberDeclarationSyntax>().ToArray());