Merge pull request #1209 from icsharpcode/delegates

Delegates

Author: GrahamTheCoder

CodeConverter.sln

@@ -1,5 +1,6 @@
-using ICSharpCode.CodeConverter.Util.FromRoslyn;
-using Microsoft.CodeAnalysis.VisualBasic;
+using Microsoft.CodeAnalysis.VisualBasic;
+using static Microsoft.CodeAnalysis.VisualBasic.VisualBasicExtensions;
+using ICSharpCode.CodeConverter.Util.FromRoslyn;
 
 namespace ICSharpCode.CodeConverter.CSharp;
 
@@ -35,7 +36,9 @@ public async Task<CSharpSyntaxNode> ConvertPropertyStatementAsync(VBSyntax.Prope
         var convertibleModifiers = node.Modifiers.Where(m => !m.IsKind(VBasic.SyntaxKind.ReadOnlyKeyword, VBasic.SyntaxKind.WriteOnlyKeyword, VBasic.SyntaxKind.DefaultKeyword));
         var modifiers = CommonConversions.ConvertModifiers(node, convertibleModifiers.ToList(), node.GetMemberContext());
         var isIndexer = CommonConversions.IsDefaultIndexer(node);
-        var propSymbol = ModelExtensions.GetDeclaredSymbol(_semanticModel, node) as IPropertySymbol;
+        IPropertySymbol propSymbol = node.Parent is VBSyntax.PropertyBlockSyntax pb
+            ? _semanticModel.GetDeclaredSymbol(pb)
+            : _semanticModel.GetDeclaredSymbol(node);
         var accessedThroughMyClass = IsAccessedThroughMyClass(node, node.Identifier, propSymbol);
 
         var directlyConvertedCsIdentifier = CommonConversions.CsEscapedIdentifier(node.Identifier.Value as string);
@@ -444,15 +447,28 @@ public CSSyntax.BlockSyntax WithImplicitReturnStatements(VBSyntax.MethodBlockBas
         CSSyntax.IdentifierNameSyntax csReturnVariableOrNull)
     {
         if (!node.MustReturn()) return convertedStatements;
-        if (_semanticModel.GetDeclaredSymbol(node) is { } ms && ms.ReturnsVoidOrAsyncTask()) {
+        var methodSymbol = node switch {
+            VBSyntax.MethodBlockSyntax mb => _semanticModel.GetDeclaredSymbol(mb.SubOrFunctionStatement),
+            VBSyntax.AccessorBlockSyntax ab => _semanticModel.GetDeclaredSymbol(ab.AccessorStatement),
+            _ => _semanticModel.GetDeclaredSymbol(node)
+        } as IMethodSymbol;
+        if (methodSymbol?.ReturnsVoidOrAsyncTask() == true) {
             return convertedStatements;
         }
 
 
         var preBodyStatements = new List<CSSyntax.StatementSyntax>();
         var postBodyStatements = new List<CSSyntax.StatementSyntax>();
 
-        var functionSym = ModelExtensions.GetDeclaredSymbol(_semanticModel, node);
+        var symbol = node switch {
+            VBSyntax.MethodBlockSyntax mb => _semanticModel.GetDeclaredSymbol(mb.SubOrFunctionStatement),
+            VBSyntax.AccessorBlockSyntax ab => _semanticModel.GetDeclaredSymbol(ab.AccessorStatement),
+            _ => null
+        };
+        var functionSym = symbol switch {
+            IMethodSymbol ms => ms,
+            _ => _semanticModel.GetDeclaredSymbol(node) as IMethodSymbol
+        };
         if (functionSym != null) {
             var returnType = CommonConversions.GetTypeSyntax(functionSym.GetReturnType());
 

CodeConverter/CSharp/AccessorDeclarationNodeConverter.cs

@@ -90,8 +90,8 @@ private async Task<ExpressionSyntax> ConvertMyGroupCollectionPropertyGetWithUnde
             switch (operation) {
                 case IConversionOperation co:
                     return await ConvertMyGroupCollectionPropertyGetWithUnderlyingFieldAsync(co.Operand.Syntax);
-                case IPropertyReferenceOperation pro when pro.Property.IsMyGroupCollectionProperty():
-                    var associatedField = pro.Property.GetAssociatedField();
+                case IPropertyReferenceOperation pro when pro.Property.IsMyGroupCollectionProperty:
+                    var associatedField = pro.Property.AssociatedField;
                     var propertyReferenceSyntax = (VisualBasicSyntaxNode)((IPropertyReferenceOperation)pro.Instance).Syntax;
                     var qualification = await propertyReferenceSyntax.AcceptAsync<ExpressionSyntax>(_triviaConvertingVisitor);
                     return SyntaxFactory.MemberAccessExpression(SyntaxKind.SimpleMemberAccessExpression, qualification, ValidSyntaxFactory.IdentifierName(associatedField.Name));

CodeConverter/CSharp/BuiltInVisualBasicOperatorSubstitutions.cs

@@ -0,0 +1,38 @@
+using System.Diagnostics;
+
+namespace ICSharpCode.CodeConverter.CSharp;
+
+internal class CachedReflectedDelegate<TArg, TResult>
+{
+    private Func<TArg, TResult> _cachedDelegate;
+    private readonly string _propertyName;
+
+    public CachedReflectedDelegate(string propertyName)
+    {
+        _propertyName = propertyName;
+    }
+
+    public TResult GetValue(TArg instance)
+    {
+        if (_cachedDelegate != null) return _cachedDelegate(instance);
+
+        var getDelegate = instance.ReflectedPropertyGetter(_propertyName)
+            ?.CreateOpenInstanceDelegateForcingType<TArg, TResult>();
+        if (getDelegate == null) {
+            Debug.Fail($"Delegate not found for {instance.GetType()}");
+            return default;
+        }
+
+        _cachedDelegate = getDelegate;
+        return _cachedDelegate(instance);
+    }
+
+    public TResult GetValueOrDefault(TArg instance, TResult defaultValue = default)
+    {
+        try {
+            return GetValue(instance);
+        } catch (Exception) {
+            return defaultValue;
+        }
+    }
+}

CodeConverter/CSharp/CachedReflectedDelegate.cs

@@ -1,67 +1,51 @@
 using System.Collections;
-using System.Diagnostics;
 
 namespace ICSharpCode.CodeConverter.CSharp;
 
 /// <summary>
 /// Use and add to this class with great care. There is great potential for issues in different VS versions if something private/internal got renamed.
 /// </summary>
 /// <remarks>
-/// Lots of cases need a method and a backing field in a way that's awkward to wrap into a single object, so I've put them adjacent for easy reference.
+/// Each extension block groups extension methods by type, with the cached delegates stored in the containing class.
 /// I've opted for using the minimal amount of hardcoded internal names (instead getting the runtime type of the argument) in the hope of being resilient against a simple rename.
 /// However, if extra subclasses are created and there's no common base type containing the property, or the property itself is renamed, this will obviously break.
 /// It'd be great to run a CI build configuration that updates all nuget packages to the latest prerelease and runs all the tests (this would also catch other breaking API changes before they hit).
 /// </remarks>
 internal static class CachedReflectedDelegates
 {
-    public static bool IsMyGroupCollectionProperty(this IPropertySymbol declaredSymbol) =>
-        GetCachedReflectedPropertyDelegate(declaredSymbol, "IsMyGroupCollectionProperty", ref _isMyGroupCollectionProperty);
-    private static Func<ISymbol, bool> _isMyGroupCollectionProperty;
+    private static CachedReflectedDelegate<ISymbol, bool> IsMyGroupCollectionPropertyDelegate { get; } =
+        new CachedReflectedDelegate<ISymbol, bool>("IsMyGroupCollectionProperty");
 
-    public static ISymbol GetAssociatedField(this IPropertySymbol declaredSymbol) =>
-        GetCachedReflectedPropertyDelegate(declaredSymbol, "AssociatedField", ref _associatedField);
-    private static Func<ISymbol, ISymbol> _associatedField;
+    private static CachedReflectedDelegate<ISymbol, ISymbol> AssociatedFieldDelegate { get; } =
+        new CachedReflectedDelegate<ISymbol, ISymbol>("AssociatedField");
 
-    public static SyntaxTree GetEmbeddedSyntaxTree(this Location loc) =>
-        GetCachedReflectedPropertyDelegate(loc, "PossiblyEmbeddedOrMySourceTree", ref _possiblyEmbeddedOrMySourceTree);
-    private static Func<Location, SyntaxTree> _possiblyEmbeddedOrMySourceTree;
-    
-    public static bool GetIsUsing(this ILocalSymbol l)
+    extension(IPropertySymbol declaredSymbol)
     {
-        try {
-            return GetCachedReflectedPropertyDelegate(l, "IsUsing", ref _isUsing);
-        } catch (Exception) {
-            return false;
-        }
-    }
-
-    private static Func<ILocalSymbol, bool> _isUsing;
-
+        public bool IsMyGroupCollectionProperty => IsMyGroupCollectionPropertyDelegate.GetValue(declaredSymbol);
 
+        public ISymbol AssociatedField => AssociatedFieldDelegate.GetValue(declaredSymbol);
+    }
 
-    /// <remarks>Unfortunately the roslyn UnassignedVariablesWalker and all useful collections created from it are internal only
-    /// Other attempts using DataFlowsIn on each reference showed that "DataFlowsIn" even from an uninitialized variable (at least in the case of ints)
-    /// https://github.com/dotnet/roslyn/blob/007022c37c6d21ee100728954bd75113e0dfe4bd/src/Compilers/VisualBasic/Portable/Analysis/FlowAnalysis/UnassignedVariablesWalker.vb#L15
-    /// It'd be possible to see the result of the diagnostic analysis, but that would miss out value types, which don't cause a warning in VB
-    /// PERF: Assume we'll only be passed one type of data flow analysis (VisualBasicDataFlowAnalysis)
-    /// </remarks>
-    public static IEnumerable<ISymbol> GetVbUnassignedVariables(this DataFlowAnalysis methodFlow) =>
-        GetCachedReflectedPropertyDelegate(methodFlow, "UnassignedVariables", ref _vbUnassignedVariables).Cast<ISymbol>();
-    private static Func<DataFlowAnalysis, IEnumerable> _vbUnassignedVariables;
+    private static CachedReflectedDelegate<Location, SyntaxTree> PossiblyEmbeddedOrMySourceTreeDelegate { get; } =
+        new CachedReflectedDelegate<Location, SyntaxTree>("PossiblyEmbeddedOrMySourceTree");
 
-    private static TDesiredTarget GetCachedReflectedPropertyDelegate<TDesiredArg, TDesiredTarget>(TDesiredArg instance, string propertyToAccess,
-        ref Func<TDesiredArg, TDesiredTarget> cachedDelegate)
+    extension(Location loc)
     {
-        if (cachedDelegate != null) return cachedDelegate(instance);
+        public SyntaxTree EmbeddedSyntaxTree => PossiblyEmbeddedOrMySourceTreeDelegate.GetValue(loc);
+    }
 
-        var getDelegate = instance.ReflectedPropertyGetter(propertyToAccess)
-            ?.CreateOpenInstanceDelegateForcingType<TDesiredArg, TDesiredTarget>();
-        if (getDelegate == null) {
-            Debug.Fail($"Delegate not found for {instance.GetType()}");
-            return default;
-        }
+    private static CachedReflectedDelegate<DataFlowAnalysis, IEnumerable<ISymbol>> VbUnassignedVariablesDelegate { get; } =
+        new CachedReflectedDelegate<DataFlowAnalysis, IEnumerable<ISymbol>>("UnassignedVariables");
 
-        cachedDelegate = getDelegate;
-        return cachedDelegate(instance);
+    extension(DataFlowAnalysis methodFlow)
+    {
+        /// <remarks>Unfortunately the roslyn UnassignedVariablesWalker and all useful collections created from it are internal only
+        /// Other attempts using DataFlowsIn on each reference showed that "DataFlowsIn" even from an uninitialized variable (at least in the case of ints)
+        /// https://github.com/dotnet/roslyn/blob/007022c37c6d21ee100728954bd75113e0dfe4bd/src/Compilers/VisualBasic/Portable/Analysis/FlowAnalysis/UnassignedVariablesWalker.vb#L15
+        /// It'd be possible to see the result of the diagnostic analysis, but that would miss out value types, which don't cause a warning in VB
+        /// PERF: Assume we'll only be passed one type of data flow analysis (VisualBasicDataFlowAnalysis)
+        /// </remarks>
+        public IEnumerable<ISymbol> VbUnassignedVariables =>
+            VbUnassignedVariablesDelegate.GetValue(methodFlow);
     }
-}
+}

CodeConverter/CSharp/CachedReflectedDelegates.cs

@@ -8,6 +8,7 @@
 using Microsoft.CodeAnalysis.FindSymbols;
 using Microsoft.CodeAnalysis.Operations;
 using Microsoft.CodeAnalysis.Simplification;
+using static Microsoft.CodeAnalysis.VisualBasic.VisualBasicExtensions;
 using ArgumentListSyntax = Microsoft.CodeAnalysis.VisualBasic.Syntax.ArgumentListSyntax;
 using ArrayRankSpecifierSyntax = Microsoft.CodeAnalysis.CSharp.Syntax.ArrayRankSpecifierSyntax;
 using ArrayTypeSyntax = Microsoft.CodeAnalysis.CSharp.Syntax.ArrayTypeSyntax;
@@ -598,7 +599,12 @@ private async Task<ExpressionSyntax> GetParameterizedSetterArgAsync(IOperation o
     public CSSyntax.IdentifierNameSyntax GetRetVariableNameOrNull(VBSyntax.MethodBlockBaseSyntax node)
     {
         if (!node.MustReturn()) return null;
-        if (SemanticModel.GetDeclaredSymbol(node) is IMethodSymbol ms && ms.ReturnsVoidOrAsyncTask()) {
+        var methodSymbol = node switch {
+            VBSyntax.MethodBlockSyntax mb => SemanticModel.GetDeclaredSymbol(mb.SubOrFunctionStatement),
+            VBSyntax.AccessorBlockSyntax ab => SemanticModel.GetDeclaredSymbol(ab.AccessorStatement),
+            _ => SemanticModel.GetDeclaredSymbol(node)
+        } as IMethodSymbol;
+        if (methodSymbol?.ReturnsVoidOrAsyncTask() == true) {
             return null;
         }