Analyze unsupported ref element arguments (#1914)

Co-authored-by: Alvaro <amjarag@gmail.com>
Co-authored-by: Shargon <shargon@gmail.com>

Author: 3 people

src/Neo.SmartContract.Analyzer/AnalyzerReleases.Unshipped.md

@@ -9,3 +9,9 @@ NC4060  | Method   | Error    | BitOperationsUsageAnalyzer
 Rule ID | Category | Severity | Notes
 --------|----------|----------|------------------------------------------------
 NC4032  | Usage    | Error    | Superseded by NC4036 for unsupported query expressions
+
+### Changed Rules
+
+Rule ID | New Category | New Severity | Old Category | Old Severity | Notes
+--------|--------------|--------------|--------------|--------------|------------------------------------------------
+NC4010  | Usage        | Error        | Usage        | Warning      | All reported by-reference forms are unsupported by the compiler

src/Neo.SmartContract.Analyzer/RefKeywordUsageAnalyzer.cs

@@ -22,16 +22,16 @@ public class RefKeywordUsageAnalyzer : DiagnosticAnalyzer
     {
         public const string DiagnosticId = "NC4010";
         private static readonly string Title = "Ref keyword usage";
-        private static readonly string MessageFormat = "'ref' keyword used in {0}";
-        private static readonly string Description = "Checks for the usage of 'ref' keywords.";
+        private static readonly string MessageFormat = "Unsupported by-reference usage in {0}";
+        private static readonly string Description = "Reports by-reference invocation forms that the Neo compiler cannot lower.";
         private const string Category = "Usage";
 
         private static readonly DiagnosticDescriptor Rule = new(
             DiagnosticId,
             Title,
             MessageFormat,
             Category,
-            DiagnosticSeverity.Warning,
+            DiagnosticSeverity.Error,
             isEnabledByDefault: true,
             description: Description);
 
@@ -49,6 +49,18 @@ private static void AnalyzeInvocation(SyntaxNodeAnalysisContext context)
             var invocationExpression = (InvocationExpressionSyntax)context.Node;
             foreach (var argument in invocationExpression.ArgumentList.Arguments)
             {
+                if ((argument.RefOrOutKeyword.IsKind(SyntaxKind.RefKeyword) ||
+                     argument.RefOrOutKeyword.IsKind(SyntaxKind.OutKeyword)) &&
+                    argument.Expression is ElementAccessExpressionSyntax elementAccess &&
+                    GetElementContainerKind(context, elementAccess.Expression) is { } containerKind)
+                {
+                    context.ReportDiagnostic(Diagnostic.Create(
+                        Rule,
+                        argument.GetLocation(),
+                        $"{containerKind} element argument; bind it to a ref local before forwarding"));
+                    continue;
+                }
+
                 // Report only on unsupported 'in' arguments (regular ref/out are supported)
                 if (argument.RefOrOutKeyword.IsKind(SyntaxKind.InKeyword) &&
                     !IsDelegateInvocation(context, invocationExpression))
@@ -59,6 +71,26 @@ private static void AnalyzeInvocation(SyntaxNodeAnalysisContext context)
             }
         }
 
+        private static string? GetElementContainerKind(
+            SyntaxNodeAnalysisContext context,
+            ExpressionSyntax expression)
+        {
+            var type = context.SemanticModel.GetTypeInfo(expression, context.CancellationToken).Type;
+            if (type is IArrayTypeSymbol)
+            {
+                return "array";
+            }
+
+            if (type is INamedTypeSymbol namedType &&
+                namedType.ContainingNamespace.ToDisplayString() == "System" &&
+                namedType.Name == "Span")
+            {
+                return "Span";
+            }
+
+            return null;
+        }
+
         private static bool IsDelegateInvocation(SyntaxNodeAnalysisContext context, InvocationExpressionSyntax invocationExpression)
         {
             return context.SemanticModel.GetSymbolInfo(invocationExpression, context.CancellationToken).Symbol is IMethodSymbol

tests/Neo.SmartContract.Analyzer.UnitTests/RefKeywordUsageAnalyzerUnitTest.cs

@@ -103,6 +103,87 @@ public void TestMethod(ref int value)
             await VerifyCS.VerifyAnalyzerAsync(test);
         }
 
+        [TestMethod]
+        public async Task ArrayElementRefArgument_ShouldReportDiagnostic()
+        {
+            var test = @"
+class TestClass
+{
+    private static void Increment(ref int value) => value++;
+
+    public static void Caller(int[] values)
+    {
+        Increment({|#0:ref values[0]|});
+    }
+}";
+
+            var expected = VerifyCS.Diagnostic(RefKeywordUsageAnalyzer.DiagnosticId)
+                .WithLocation(0)
+                .WithArguments("array element argument; bind it to a ref local before forwarding");
+            await VerifyCS.VerifyAnalyzerAsync(test, expected);
+        }
+
+        [TestMethod]
+        public async Task SpanElementRefArgument_ShouldReportDiagnostic()
+        {
+            var test = @"
+using System;
+
+class TestClass
+{
+    private static void Increment(ref int value) => value++;
+
+    public static void Caller()
+    {
+        Span<int> values = new int[] { 1, 2, 3 };
+        Increment({|#0:ref values[1]|});
+    }
+}";
+
+            var expected = VerifyCS.Diagnostic(RefKeywordUsageAnalyzer.DiagnosticId)
+                .WithLocation(0)
+                .WithArguments("Span element argument; bind it to a ref local before forwarding");
+            await VerifyCS.VerifyAnalyzerAsync(test, expected);
+        }
+
+        [TestMethod]
+        public async Task ArrayElementOutArgument_ShouldReportDiagnostic()
+        {
+            var test = @"
+class TestClass
+{
+    private static void Reset(out int value) => value = 0;
+
+    public static void Caller(int[] values)
+    {
+        Reset({|#0:out values[0]|});
+    }
+}";
+
+            var expected = VerifyCS.Diagnostic(RefKeywordUsageAnalyzer.DiagnosticId)
+                .WithLocation(0)
+                .WithArguments("array element argument; bind it to a ref local before forwarding");
+            await VerifyCS.VerifyAnalyzerAsync(test, expected);
+        }
+
+        [TestMethod]
+        public async Task RefLocalArgument_ShouldNotReportDiagnostic()
+        {
+            var test = @"
+class TestClass
+{
+    private static void Increment(ref int value) => value++;
+
+    public static void Caller(int[] values)
+    {
+        ref int value = ref values[0];
+        Increment(ref value);
+    }
+}";
+
+            await VerifyPreviewAnalyzerAsync(test);
+        }
+
         [TestMethod]
         public async Task OutParameter_ShouldNotReportDiagnostic()
         {