Fix if statement sequence points and SequencePointInserter

src/Neo.Compiler.CSharp/CompilationContext.cs

@@ -350,7 +350,7 @@ public JObject CreateDebugInformation(string folder = "")
             foreach (var m in methodsConverted.Where(p => p.SyntaxNode is not null))
             {
                 List<JString> sequencePoints = new();
-                foreach (var ins in m.Instructions.Where(i => i.SourceLocation is not null))
+                foreach (var ins in m.Instructions.Where(i => i.SourceLocation?.SourceTree is not null))
                 {
                     var doc = ins.SourceLocation!.SourceTree!.FilePath;
                     if (!string.IsNullOrEmpty(folder))

src/Neo.Compiler.CSharp/MethodConvert/Expression/Expression.cs

@@ -22,12 +22,15 @@ partial class MethodConvert
 {
     private void ConvertExpression(SemanticModel model, ExpressionSyntax syntax)
     {
+        using var sequence = InsertSequencePoint(syntax);
+
         Optional<object?> constant = model.GetConstantValue(syntax);
         if (constant.HasValue)
         {
             Push(constant.Value);
             return;
         }
+
         switch (syntax)
         {
             case AnonymousObjectCreationExpressionSyntax expression:

src/Neo.Compiler.CSharp/MethodConvert/SourceConvert.cs

@@ -33,19 +33,16 @@ private void ConvertSource(SemanticModel model)
                 if (syntax.Body is not null)
                     ConvertStatement(model, syntax.Body);
                 else if (syntax.ExpressionBody is not null)
-                    using (InsertSequencePoint(syntax.ExpressionBody.Expression))
-                        ConvertExpression(model, syntax.ExpressionBody.Expression);
+                    ConvertExpression(model, syntax.ExpressionBody.Expression);
                 else
                     ConvertNoBody(syntax);
                 break;
             case ArrowExpressionClauseSyntax syntax:
-                using (InsertSequencePoint(syntax))
-                    ConvertExpression(model, syntax.Expression);
+                ConvertExpression(model, syntax.Expression);
                 break;
             case BaseMethodDeclarationSyntax syntax:
                 if (syntax.Body is null)
-                    using (InsertSequencePoint(syntax.ExpressionBody!.Expression))
-                        ConvertExpression(model, syntax.ExpressionBody.Expression);
+                    ConvertExpression(model, syntax.ExpressionBody!.Expression);
                 else
                     ConvertStatement(model, syntax.Body);
                 break;

src/Neo.Compiler.CSharp/MethodConvert/Statement/DoStatement.cs

@@ -51,8 +51,7 @@ private void ConvertDoStatement(SemanticModel model, DoStatementSyntax syntax)
             startTarget.Instruction = AddInstruction(OpCode.NOP);
             ConvertStatement(model, syntax.Statement);
             continueTarget.Instruction = AddInstruction(OpCode.NOP);
-            using (InsertSequencePoint(syntax.Condition))
-                ConvertExpression(model, syntax.Condition);
+            ConvertExpression(model, syntax.Condition);
             Jump(OpCode.JMPIF_L, startTarget);
             breakTarget.Instruction = AddInstruction(OpCode.NOP);
             PopContinueTarget();

src/Neo.Compiler.CSharp/MethodConvert/Statement/ExpressionStatement.cs

@@ -41,7 +41,7 @@ partial class MethodConvert
         private void ConvertExpressionStatement(SemanticModel model, ExpressionStatementSyntax syntax)
         {
             ITypeSymbol type = model.GetTypeInfo(syntax.Expression).Type!;
-            using (InsertSequencePoint(syntax))
+            using (InsertSequencePoint(syntax.Expression))
             {
                 ConvertExpression(model, syntax.Expression);
                 if (type.SpecialType != SpecialType.System_Void)

src/Neo.Compiler.CSharp/MethodConvert/Statement/ForStatement.cs

@@ -91,8 +91,7 @@ private void ConvertForStatement(SemanticModel model, ForStatementSyntax syntax)
             }
             else
             {
-                using (InsertSequencePoint(syntax.Condition))
-                    ConvertExpression(model, syntax.Condition);
+                ConvertExpression(model, syntax.Condition);
                 Jump(OpCode.JMPIF_L, startTarget);
             }
             breakTarget.Instruction = AddInstruction(OpCode.NOP);

src/Neo.Compiler.CSharp/MethodConvert/Statement/IfStatement.cs

@@ -47,21 +47,31 @@ partial class MethodConvert
         private void ConvertIfStatement(SemanticModel model, IfStatementSyntax syntax)
         {
             JumpTarget elseTarget = new();
-            using (InsertSequencePoint(syntax.Condition))
-                ConvertExpression(model, syntax.Condition);
-            Jump(OpCode.JMPIFNOT_L, elseTarget);
-            ConvertStatement(model, syntax.Statement);
-            if (syntax.Else is null)
-            {
-                elseTarget.Instruction = AddInstruction(OpCode.NOP);
-            }
-            else
+
+            ConvertExpression(model, syntax.Condition);
+
+            using (InsertSequencePoint(syntax.Statement))
             {
-                JumpTarget endTarget = new();
-                Jump(OpCode.JMP_L, endTarget);
-                elseTarget.Instruction = AddInstruction(OpCode.NOP);
-                ConvertStatement(model, syntax.Else.Statement);
-                endTarget.Instruction = AddInstruction(OpCode.NOP);
+                Jump(OpCode.JMPIFNOT_L, elseTarget);
+                ConvertStatement(model, syntax.Statement);
+
+                if (syntax.Else is null)
+                {
+                    elseTarget.Instruction = AddInstruction(OpCode.NOP);
+                }
+                else
+                {
+                    JumpTarget endTarget = new();
+                    Jump(OpCode.JMP_L, endTarget);
+
+                    using (InsertSequencePoint(syntax.Else.Statement))
+                    {
+                        elseTarget.Instruction = AddInstruction(OpCode.NOP);
+                        ConvertStatement(model, syntax.Else.Statement);
+                    }
+
+                    endTarget.Instruction = AddInstruction(OpCode.NOP);
+                }
             }
         }
     }

src/Neo.Compiler.CSharp/SequencePointInserter.cs

@@ -17,19 +17,30 @@ namespace Neo.Compiler
     class SequencePointInserter : IDisposable
     {
         private readonly IReadOnlyList<Instruction> instructions;
-        private readonly SyntaxNodeOrToken syntax;
+        private readonly Location? location;
         private readonly int position;
 
         public SequencePointInserter(IReadOnlyList<Instruction> instructions, SyntaxNodeOrToken syntax)
         {
             this.instructions = instructions;
-            this.syntax = syntax;
+            this.location = syntax.GetLocation();
             this.position = instructions.Count;
+
+            // No location must be removed
+
+            if (this.location?.SourceTree is null)
+                this.location = null;
         }
+
         void IDisposable.Dispose()
         {
-            if (position < instructions.Count)
-                instructions[position].SourceLocation = syntax.GetLocation();
+            for (int x = position; x < instructions.Count; x++)
+            {
+                if (instructions[x].SourceLocation is null)
+                {
+                    instructions[x].SourceLocation = location;
+                }
+            }
         }
     }
 }

src/Neo.SmartContract.Testing/Coverage/Formats/CoberturaFormat.ContractCoverageWriter.cs

@@ -24,9 +24,10 @@ public ContractCoverageWriter(CoveredContract contract, NeoDebugInfo debugInfo)
 
             public void WritePackage(XmlWriter writer)
             {
-                var (lineCount, hitCount) = GetLineRate(Contract, DebugInfo.Methods.SelectMany(m => m.SequencePoints));
+                var allMethods = DebugInfo.Methods.SelectMany(m => m.SequencePoints).ToArray();
+                var (lineCount, hitCount) = GetLineRate(Contract, allMethods);
                 var lineRate = CoverageBase.CalculateHitRate(lineCount, hitCount);
-                var (branchCount, branchHit) = GetBranchRate(Contract, DebugInfo.Methods);
+                var (branchCount, branchHit) = GetBranchRate(Contract, allMethods);
                 var branchRate = CoverageBase.CalculateHitRate(branchCount, branchHit);
 
                 writer.WriteStartElement("package");
@@ -65,9 +66,10 @@ public void WritePackage(XmlWriter writer)
 
             internal void WriteClass(XmlWriter writer, string name, string filename, IEnumerable<NeoDebugInfo.Method> methods)
             {
-                var (lineCount, hitCount) = GetLineRate(Contract, methods.SelectMany(m => m.SequencePoints));
+                var allMethods = methods.SelectMany(m => m.SequencePoints).ToArray();
+                var (lineCount, hitCount) = GetLineRate(Contract, allMethods);
                 var lineRate = CoverageBase.CalculateHitRate(lineCount, hitCount);
-                var (branchCount, branchHit) = GetBranchRate(Contract, methods);
+                var (branchCount, branchHit) = GetBranchRate(Contract, allMethods);
                 var branchRate = CoverageBase.CalculateHitRate(branchCount, branchHit);
 
                 writer.WriteStartElement("class");
@@ -103,7 +105,7 @@ internal void WriteMethod(XmlWriter writer, NeoDebugInfo.Method method)
                 var signature = string.Join(", ", method.Parameters.Select(p => p.Type));
                 var (lineCount, hitCount) = GetLineRate(Contract, method.SequencePoints);
                 var lineRate = CoverageBase.CalculateHitRate(lineCount, hitCount);
-                var (branchCount, branchHit) = GetBranchRate(Contract, method);
+                var (branchCount, branchHit) = GetBranchRate(Contract, method.SequencePoints);
                 var branchRate = CoverageBase.CalculateHitRate(branchCount, branchHit);
 
                 writer.WriteStartElement("method");

src/Neo.SmartContract.Testing/Coverage/Formats/CoberturaFormat.cs

@@ -42,11 +42,13 @@ internal void WriteReport(XmlWriter writer, IReadOnlyList<(CoveredContract Contr
 
             foreach (var entry in coverage)
             {
-                var (lineCount, hitCount) = GetLineRate(entry.Contract, entry.DebugInfo.Methods.SelectMany(m => m.SequencePoints));
+                var allPoints = entry.DebugInfo.Methods.SelectMany(m => m.SequencePoints).ToArray();
+
+                var (lineCount, hitCount) = GetLineRate(entry.Contract, allPoints);
                 linesValid += lineCount;
                 linesCovered += hitCount;
 
-                var (branchCount, branchHit) = GetBranchRate(entry.Contract, entry.DebugInfo.Methods);
+                var (branchCount, branchHit) = GetBranchRate(entry.Contract, allPoints);
                 branchesValid += branchCount;
                 branchesCovered += branchHit;
             }
@@ -84,24 +86,11 @@ internal void WriteReport(XmlWriter writer, IReadOnlyList<(CoveredContract Contr
             writer.WriteEndElement();
         }
 
-        private static (int branchCount, int branchHit) GetBranchRate(CoveredContract contract, IEnumerable<NeoDebugInfo.Method> methods)
-        {
-            int branchCount = 0, branchHit = 0;
-            foreach (var method in methods)
-            {
-                var rate = GetBranchRate(contract, method);
-
-                branchCount += rate.branchCount;
-                branchHit += rate.branchHit;
-            }
-            return (branchCount, branchHit);
-        }
-
-        private static (int branchCount, int branchHit) GetBranchRate(CoveredContract contract, NeoDebugInfo.Method method)
+        private static (int branchCount, int branchHit) GetBranchRate(CoveredContract contract, IEnumerable<NeoDebugInfo.SequencePoint> sequencePoints)
         {
             int branchCount = 0, branchHit = 0;
 
-            foreach (var sp in method.SequencePoints)
+            foreach (var sp in sequencePoints)
             {
                 if (contract.TryGetBranch(sp.Address, out var branch))
                 {
@@ -113,14 +102,14 @@ private static (int branchCount, int branchHit) GetBranchRate(CoveredContract co
             return (branchCount, branchHit);
         }
 
-        private static (int lineCount, int hitCount) GetLineRate(CoveredContract contract, IEnumerable<NeoDebugInfo.SequencePoint> lines)
+        private static (int lineCount, int hitCount) GetLineRate(CoveredContract contract, IEnumerable<NeoDebugInfo.SequencePoint> sequencePoints)
         {
             int lineCount = 0, hitCount = 0;
 
-            foreach (var line in lines)
+            foreach (var sp in sequencePoints)
             {
                 lineCount++;
-                if (contract.TryGetLine(line.Address, out var hit) && hit.Hits > 0)
+                if (contract.TryGetLine(sp.Address, out var hit) && hit.Hits > 0)
                 {
                     hitCount++;
                 }

tests/Neo.Compiler.CSharp.UnitTests/UnitTest_DebugInfo.cs

@@ -25,7 +25,8 @@ public void Test_DebugInfo()
             Assert.IsInstanceOfType(debugInfo["methods"], typeof(JArray));
             Assert.AreEqual(1, (debugInfo["methods"] as JArray).Count);
             Assert.AreEqual("Neo.Compiler.CSharp.UnitTests.TestClasses.Contract_Event,Main", (debugInfo["methods"] as JArray)[0]["name"].AsString());
-            Assert.AreEqual("3[0]17:13-17:30;7[0]18:13-18:33;13[0]19:9-19:10", string.Join(';', ((debugInfo["methods"] as JArray)[0]["sequence-points"] as JArray).Select(u => u.AsString())));
+            Assert.AreEqual("3[0]17:28-17:29;4[0]17:13-17:29;5[0]17:13-17:29;6[0]17:13-17:29;7[0]18:28-18:32;8[0]18:28-18:32;10[0]18:13-18:32;11[0]18:13-18:32;12[0]18:13-18:32;13[0]19:9-19:10",
+                string.Join(';', ((debugInfo["methods"] as JArray)[0]["sequence-points"] as JArray).Select(u => u.AsString())));
             Assert.IsTrue(debugInfo.ContainsProperty("events"));
             Assert.IsInstanceOfType(debugInfo["events"], typeof(JArray));
             Assert.AreEqual(1, (debugInfo["events"] as JArray).Count);

tests/Neo.SmartContract.Framework.TestContracts/Contract_SequencePointInserter.cs

@@ -0,0 +1,15 @@
+using Neo.SmartContract.Framework.Attributes;
+using Neo.SmartContract.Framework.Services;
+using Neo.SmartContract.Framework.Native;
+
+namespace Neo.SmartContract.Framework.UnitTests.TestClasses
+{
+    public class Contract_SequencePointInserter : SmartContract
+    {
+        public static int test(int a)
+        {
+            if (a == 1) return 23;
+            return 45;
+        }
+    }
+}

tests/Neo.SmartContract.Framework.UnitTests/Services/SequencePointInserterTest.cs

@@ -0,0 +1,60 @@
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+using Neo.Json;
+using Neo.VM;
+using System.Linq;
+
+namespace Neo.SmartContract.Framework.UnitTests.Services
+{
+    [TestClass]
+    public class SequencePointInserterTest
+    {
+        private TestEngine.TestEngine testengine;
+
+        [TestInitialize]
+        public void Init()
+        {
+            var system = TestBlockchain.TheNeoSystem;
+            var snapshot = system.GetSnapshot().CreateSnapshot();
+
+            testengine = new TestEngine.TestEngine(snapshot: snapshot);
+            Assert.IsTrue(testengine.AddEntryScript(Utils.Extensions.TestContractRoot + "Contract_SequencePointInserter.cs").Success);
+        }
+
+        [TestMethod]
+        public void Test_SequencePointInserter()
+        {
+            var points = (testengine.DebugInfo["methods"][0]["sequence-points"] as JArray).Select(u => u.GetString()).ToArray();
+
+            // Ensure that all the instructions have sequence point
+
+            var ip = 0;
+            Script script = testengine.Nef.Script;
+
+            while (ip < script.Length)
+            {
+                var instruction = script.GetInstruction(ip);
+
+                if (ip != 0) // Avoid INITSLOT
+                {
+                    Assert.IsTrue(points.Any(u => u.StartsWith($"{ip}[")), $"Offset {ip} with '{instruction.OpCode}' is not in sequence points.");
+                }
+
+                ip += instruction.Size;
+            }
+        }
+
+        [TestMethod]
+        public void Test_If()
+        {
+            testengine.Reset();
+            var ret = testengine.ExecuteTestCaseStandard("test", 1);
+            Assert.AreEqual(1, ret.Count);
+            Assert.AreEqual(23, ret.Pop().GetInteger());
+
+            testengine.Reset();
+            ret = testengine.ExecuteTestCaseStandard("test", 0);
+            Assert.AreEqual(1, ret.Count);
+            Assert.AreEqual(45, ret.Pop().GetInteger());
+        }
+    }
+}