From 80933a2cd95cbb04c9db40a8a102cdedf14b2b6a Mon Sep 17 00:00:00 2001 From: Shargon Date: Thu, 22 Feb 2024 05:24:35 -0800 Subject: [PATCH 01/19] Fix SequencePointInserter --- src/Neo.Compiler.CSharp/SequencePointInserter.cs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/Neo.Compiler.CSharp/SequencePointInserter.cs b/src/Neo.Compiler.CSharp/SequencePointInserter.cs index e883a3e2a..024e6bc2a 100644 --- a/src/Neo.Compiler.CSharp/SequencePointInserter.cs +++ b/src/Neo.Compiler.CSharp/SequencePointInserter.cs @@ -26,10 +26,16 @@ public SequencePointInserter(IReadOnlyList instructions, SyntaxNode this.syntax = syntax; this.position = instructions.Count; } + void IDisposable.Dispose() { - if (position < instructions.Count) - instructions[position].SourceLocation = syntax.GetLocation(); + for (int x = position; x < instructions.Count; x++) + { + if (instructions[position].SourceLocation is null) + { + instructions[position].SourceLocation = syntax.GetLocation(); + } + } } } } From bc79bacb65128f324ac0ae264eddba9b88d2a33d Mon Sep 17 00:00:00 2001 From: Shargon Date: Thu, 22 Feb 2024 05:28:52 -0800 Subject: [PATCH 02/19] reuse variable --- src/Neo.Compiler.CSharp/SequencePointInserter.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Neo.Compiler.CSharp/SequencePointInserter.cs b/src/Neo.Compiler.CSharp/SequencePointInserter.cs index 024e6bc2a..683b4e01b 100644 --- a/src/Neo.Compiler.CSharp/SequencePointInserter.cs +++ b/src/Neo.Compiler.CSharp/SequencePointInserter.cs @@ -29,11 +29,13 @@ public SequencePointInserter(IReadOnlyList instructions, SyntaxNode void IDisposable.Dispose() { + var location = syntax.GetLocation(); + for (int x = position; x < instructions.Count; x++) { if (instructions[position].SourceLocation is null) { - instructions[position].SourceLocation = syntax.GetLocation(); + instructions[position].SourceLocation = location; } } } From a27d64fbf83036ee8a0e11dd83a26dc76dcc3ce7 Mon Sep 17 00:00:00 2001 From: Fernando Diaz Toledano Date: Thu, 22 Feb 2024 15:12:21 +0100 Subject: [PATCH 03/19] clean code --- src/Neo.Compiler.CSharp/SequencePointInserter.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Neo.Compiler.CSharp/SequencePointInserter.cs b/src/Neo.Compiler.CSharp/SequencePointInserter.cs index 683b4e01b..cc9fdc288 100644 --- a/src/Neo.Compiler.CSharp/SequencePointInserter.cs +++ b/src/Neo.Compiler.CSharp/SequencePointInserter.cs @@ -17,20 +17,18 @@ namespace Neo.Compiler class SequencePointInserter : IDisposable { private readonly IReadOnlyList instructions; - private readonly SyntaxNodeOrToken syntax; + private readonly Location? location; private readonly int position; public SequencePointInserter(IReadOnlyList instructions, SyntaxNodeOrToken syntax) { this.instructions = instructions; - this.syntax = syntax; + this.location = syntax.GetLocation(); this.position = instructions.Count; } void IDisposable.Dispose() { - var location = syntax.GetLocation(); - for (int x = position; x < instructions.Count; x++) { if (instructions[position].SourceLocation is null) From d1133a40fed6d3f64a6d40c6f5e39af4f17b6a64 Mon Sep 17 00:00:00 2001 From: Fernando Diaz Toledano Date: Thu, 22 Feb 2024 15:20:33 +0100 Subject: [PATCH 04/19] Stop if was found --- src/Neo.Compiler.CSharp/SequencePointInserter.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Neo.Compiler.CSharp/SequencePointInserter.cs b/src/Neo.Compiler.CSharp/SequencePointInserter.cs index cc9fdc288..17dd2b8c3 100644 --- a/src/Neo.Compiler.CSharp/SequencePointInserter.cs +++ b/src/Neo.Compiler.CSharp/SequencePointInserter.cs @@ -35,6 +35,10 @@ void IDisposable.Dispose() { instructions[position].SourceLocation = location; } + else + { + break; + } } } } From 83b08ca5f9fa30ab70f5f75f83da495a239b3271 Mon Sep 17 00:00:00 2001 From: Shargon Date: Thu, 22 Feb 2024 06:46:52 -0800 Subject: [PATCH 05/19] Update src/Neo.Compiler.CSharp/SequencePointInserter.cs --- src/Neo.Compiler.CSharp/SequencePointInserter.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Neo.Compiler.CSharp/SequencePointInserter.cs b/src/Neo.Compiler.CSharp/SequencePointInserter.cs index 17dd2b8c3..516632f49 100644 --- a/src/Neo.Compiler.CSharp/SequencePointInserter.cs +++ b/src/Neo.Compiler.CSharp/SequencePointInserter.cs @@ -31,7 +31,7 @@ void IDisposable.Dispose() { for (int x = position; x < instructions.Count; x++) { - if (instructions[position].SourceLocation is null) + if (instructions[x].SourceLocation is null) { instructions[position].SourceLocation = location; } From 3ce4521e7b8cd3d2067d7ffe93f7e1e7846edf03 Mon Sep 17 00:00:00 2001 From: Shargon Date: Thu, 22 Feb 2024 06:47:34 -0800 Subject: [PATCH 06/19] Update SequencePointInserter.cs --- src/Neo.Compiler.CSharp/SequencePointInserter.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Neo.Compiler.CSharp/SequencePointInserter.cs b/src/Neo.Compiler.CSharp/SequencePointInserter.cs index 516632f49..48ad5afb0 100644 --- a/src/Neo.Compiler.CSharp/SequencePointInserter.cs +++ b/src/Neo.Compiler.CSharp/SequencePointInserter.cs @@ -33,7 +33,7 @@ void IDisposable.Dispose() { if (instructions[x].SourceLocation is null) { - instructions[position].SourceLocation = location; + instructions[x].SourceLocation = location; } else { From 6428df25a178c00231931fc47fb7107b1397d10d Mon Sep 17 00:00:00 2001 From: Fernando Diaz Toledano Date: Thu, 22 Feb 2024 17:54:41 +0100 Subject: [PATCH 07/19] Fix if branch --- src/Neo.Compiler.CSharp/CompilationContext.cs | 2 +- .../MethodConvert/Statement/IfStatement.cs | 34 +++++++---- .../SequencePointInserter.cs | 5 ++ .../CoberturaFormat.ContractCoverageWriter.cs | 12 ++-- .../Coverage/Formats/CoberturaFormat.cs | 29 +++------ .../UnitTest_DebugInfo.cs | 3 +- .../Contract_SequencePointInserter.cs | 15 +++++ .../Services/SequencePointInserterTest.cs | 60 +++++++++++++++++++ 8 files changed, 121 insertions(+), 39 deletions(-) create mode 100644 tests/Neo.SmartContract.Framework.TestContracts/Contract_SequencePointInserter.cs create mode 100644 tests/Neo.SmartContract.Framework.UnitTests/Services/SequencePointInserterTest.cs diff --git a/src/Neo.Compiler.CSharp/CompilationContext.cs b/src/Neo.Compiler.CSharp/CompilationContext.cs index 9f1f6bb57..c116b39a4 100644 --- a/src/Neo.Compiler.CSharp/CompilationContext.cs +++ b/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 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)) diff --git a/src/Neo.Compiler.CSharp/MethodConvert/Statement/IfStatement.cs b/src/Neo.Compiler.CSharp/MethodConvert/Statement/IfStatement.cs index dc1137690..914014600 100644 --- a/src/Neo.Compiler.CSharp/MethodConvert/Statement/IfStatement.cs +++ b/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 + + 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); + } + } } } } diff --git a/src/Neo.Compiler.CSharp/SequencePointInserter.cs b/src/Neo.Compiler.CSharp/SequencePointInserter.cs index 48ad5afb0..cbc459053 100644 --- a/src/Neo.Compiler.CSharp/SequencePointInserter.cs +++ b/src/Neo.Compiler.CSharp/SequencePointInserter.cs @@ -25,6 +25,11 @@ public SequencePointInserter(IReadOnlyList instructions, SyntaxNode this.instructions = instructions; 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() diff --git a/src/Neo.SmartContract.Testing/Coverage/Formats/CoberturaFormat.ContractCoverageWriter.cs b/src/Neo.SmartContract.Testing/Coverage/Formats/CoberturaFormat.ContractCoverageWriter.cs index 9bd0c28fb..6fa201694 100644 --- a/src/Neo.SmartContract.Testing/Coverage/Formats/CoberturaFormat.ContractCoverageWriter.cs +++ b/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 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"); diff --git a/src/Neo.SmartContract.Testing/Coverage/Formats/CoberturaFormat.cs b/src/Neo.SmartContract.Testing/Coverage/Formats/CoberturaFormat.cs index b03f827c2..4513f570a 100644 --- a/src/Neo.SmartContract.Testing/Coverage/Formats/CoberturaFormat.cs +++ b/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 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 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 lines) + private static (int lineCount, int hitCount) GetLineRate(CoveredContract contract, IEnumerable 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++; } diff --git a/tests/Neo.Compiler.CSharp.UnitTests/UnitTest_DebugInfo.cs b/tests/Neo.Compiler.CSharp.UnitTests/UnitTest_DebugInfo.cs index dba60a1d4..da4506116 100644 --- a/tests/Neo.Compiler.CSharp.UnitTests/UnitTest_DebugInfo.cs +++ b/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:13-17:30;4[0]17:13-17:30;5[0]17:13-17:30;6[0]17:13-17:30;7[0]18:13-18:33;8[0]18:13-18:33;10[0]18:13-18:33;11[0]18:13-18:33;12[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.IsTrue(debugInfo.ContainsProperty("events")); Assert.IsInstanceOfType(debugInfo["events"], typeof(JArray)); Assert.AreEqual(1, (debugInfo["events"] as JArray).Count); diff --git a/tests/Neo.SmartContract.Framework.TestContracts/Contract_SequencePointInserter.cs b/tests/Neo.SmartContract.Framework.TestContracts/Contract_SequencePointInserter.cs new file mode 100644 index 000000000..37b545308 --- /dev/null +++ b/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; + } + } +} diff --git a/tests/Neo.SmartContract.Framework.UnitTests/Services/SequencePointInserterTest.cs b/tests/Neo.SmartContract.Framework.UnitTests/Services/SequencePointInserterTest.cs new file mode 100644 index 000000000..d7ee8fb06 --- /dev/null +++ b/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 (instruction.OpCode != OpCode.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()); + } + } +} From 9d354fdec9b6679dda99d02c4f36f7ee0c77aedc Mon Sep 17 00:00:00 2001 From: Fernando Diaz Toledano Date: Thu, 22 Feb 2024 17:59:23 +0100 Subject: [PATCH 08/19] clean else --- src/Neo.Compiler.CSharp/MethodConvert/Statement/IfStatement.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Neo.Compiler.CSharp/MethodConvert/Statement/IfStatement.cs b/src/Neo.Compiler.CSharp/MethodConvert/Statement/IfStatement.cs index 914014600..b6f3c3c80 100644 --- a/src/Neo.Compiler.CSharp/MethodConvert/Statement/IfStatement.cs +++ b/src/Neo.Compiler.CSharp/MethodConvert/Statement/IfStatement.cs @@ -69,8 +69,9 @@ private void ConvertIfStatement(SemanticModel model, IfStatementSyntax syntax) { elseTarget.Instruction = AddInstruction(OpCode.NOP); ConvertStatement(model, syntax.Else.Statement); - endTarget.Instruction = AddInstruction(OpCode.NOP); } + + endTarget.Instruction = AddInstruction(OpCode.NOP); } } } From 8be8d686756447772cdcc44f2cb092680450cab9 Mon Sep 17 00:00:00 2001 From: Fernando Diaz Toledano Date: Thu, 22 Feb 2024 18:00:16 +0100 Subject: [PATCH 09/19] Remove breaks --- src/Neo.Compiler.CSharp/SequencePointInserter.cs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/Neo.Compiler.CSharp/SequencePointInserter.cs b/src/Neo.Compiler.CSharp/SequencePointInserter.cs index cbc459053..1ac5c68eb 100644 --- a/src/Neo.Compiler.CSharp/SequencePointInserter.cs +++ b/src/Neo.Compiler.CSharp/SequencePointInserter.cs @@ -40,10 +40,6 @@ void IDisposable.Dispose() { instructions[x].SourceLocation = location; } - else - { - break; - } } } } From 42fe78ad8ed9b3144c5a45062d297bf41b35922d Mon Sep 17 00:00:00 2001 From: Fernando Diaz Toledano Date: Thu, 22 Feb 2024 18:05:16 +0100 Subject: [PATCH 10/19] Clean ut --- .../Services/SequencePointInserterTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Neo.SmartContract.Framework.UnitTests/Services/SequencePointInserterTest.cs b/tests/Neo.SmartContract.Framework.UnitTests/Services/SequencePointInserterTest.cs index d7ee8fb06..b0f3ba66e 100644 --- a/tests/Neo.SmartContract.Framework.UnitTests/Services/SequencePointInserterTest.cs +++ b/tests/Neo.SmartContract.Framework.UnitTests/Services/SequencePointInserterTest.cs @@ -34,7 +34,7 @@ public void Test_SequencePointInserter() { var instruction = script.GetInstruction(ip); - if (instruction.OpCode != OpCode.INITSLOT) + if (ip != 0) // Avoid INITSLOT { Assert.IsTrue(points.Any(u => u.StartsWith($"{ip}[")), $"Offset {ip} with '{instruction.OpCode}' is not in sequence points."); } From 2493411509f76e8fd09bf0fec6c25068af8a09bc Mon Sep 17 00:00:00 2001 From: Fernando Diaz Toledano Date: Thu, 22 Feb 2024 18:27:38 +0100 Subject: [PATCH 11/19] Ensure all expressions are covered by sequence point --- src/Neo.Compiler.CSharp/MethodConvert/Expression/Expression.cs | 3 +++ tests/Neo.Compiler.CSharp.UnitTests/UnitTest_DebugInfo.cs | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Neo.Compiler.CSharp/MethodConvert/Expression/Expression.cs b/src/Neo.Compiler.CSharp/MethodConvert/Expression/Expression.cs index bf724756a..2b185ca15 100644 --- a/src/Neo.Compiler.CSharp/MethodConvert/Expression/Expression.cs +++ b/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 constant = model.GetConstantValue(syntax); if (constant.HasValue) { Push(constant.Value); return; } + switch (syntax) { case AnonymousObjectCreationExpressionSyntax expression: diff --git a/tests/Neo.Compiler.CSharp.UnitTests/UnitTest_DebugInfo.cs b/tests/Neo.Compiler.CSharp.UnitTests/UnitTest_DebugInfo.cs index da4506116..9138b6143 100644 --- a/tests/Neo.Compiler.CSharp.UnitTests/UnitTest_DebugInfo.cs +++ b/tests/Neo.Compiler.CSharp.UnitTests/UnitTest_DebugInfo.cs @@ -25,7 +25,7 @@ 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;4[0]17:13-17:30;5[0]17:13-17:30;6[0]17:13-17:30;7[0]18:13-18:33;8[0]18:13-18:33;10[0]18:13-18:33;11[0]18:13-18:33;12[0]18:13-18:33;13[0]19:9-19:10", + 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:30;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:33;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)); From 7f2d07495bfeae5e763213a0db2e8ef2367f28b8 Mon Sep 17 00:00:00 2001 From: Fernando Diaz Toledano Date: Thu, 22 Feb 2024 18:48:49 +0100 Subject: [PATCH 12/19] Remove insertions for one `ConvertExpression`, it's done inside --- src/Neo.Compiler.CSharp/MethodConvert/SourceConvert.cs | 9 +++------ .../MethodConvert/Statement/DoStatement.cs | 3 +-- .../MethodConvert/Statement/ForStatement.cs | 3 +-- .../MethodConvert/Statement/IfStatement.cs | 3 +-- 4 files changed, 6 insertions(+), 12 deletions(-) diff --git a/src/Neo.Compiler.CSharp/MethodConvert/SourceConvert.cs b/src/Neo.Compiler.CSharp/MethodConvert/SourceConvert.cs index add233beb..0d3fcfd82 100644 --- a/src/Neo.Compiler.CSharp/MethodConvert/SourceConvert.cs +++ b/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; diff --git a/src/Neo.Compiler.CSharp/MethodConvert/Statement/DoStatement.cs b/src/Neo.Compiler.CSharp/MethodConvert/Statement/DoStatement.cs index 1d818c15a..d245d6df4 100644 --- a/src/Neo.Compiler.CSharp/MethodConvert/Statement/DoStatement.cs +++ b/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(); diff --git a/src/Neo.Compiler.CSharp/MethodConvert/Statement/ForStatement.cs b/src/Neo.Compiler.CSharp/MethodConvert/Statement/ForStatement.cs index 2fc850a4e..cc3c93195 100644 --- a/src/Neo.Compiler.CSharp/MethodConvert/Statement/ForStatement.cs +++ b/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); diff --git a/src/Neo.Compiler.CSharp/MethodConvert/Statement/IfStatement.cs b/src/Neo.Compiler.CSharp/MethodConvert/Statement/IfStatement.cs index b6f3c3c80..35646608a 100644 --- a/src/Neo.Compiler.CSharp/MethodConvert/Statement/IfStatement.cs +++ b/src/Neo.Compiler.CSharp/MethodConvert/Statement/IfStatement.cs @@ -48,8 +48,7 @@ private void ConvertIfStatement(SemanticModel model, IfStatementSyntax syntax) { JumpTarget elseTarget = new(); - using (InsertSequencePoint(syntax.Condition)) - ConvertExpression(model, syntax.Condition); + ConvertExpression(model, syntax.Condition); using (InsertSequencePoint(syntax.Statement)) { From 59ed399cd318f204643fc936218a3e46893aeb57 Mon Sep 17 00:00:00 2001 From: Fernando Diaz Toledano Date: Thu, 22 Feb 2024 19:00:36 +0100 Subject: [PATCH 13/19] missmatch --- .../MethodConvert/Statement/ExpressionStatement.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Neo.Compiler.CSharp/MethodConvert/Statement/ExpressionStatement.cs b/src/Neo.Compiler.CSharp/MethodConvert/Statement/ExpressionStatement.cs index 364066246..6245a8236 100644 --- a/src/Neo.Compiler.CSharp/MethodConvert/Statement/ExpressionStatement.cs +++ b/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) From e460f1d5d4a96f72c53f25a45c241986cf520cd5 Mon Sep 17 00:00:00 2001 From: Fernando Diaz Toledano Date: Thu, 22 Feb 2024 19:01:54 +0100 Subject: [PATCH 14/19] fix ut --- tests/Neo.Compiler.CSharp.UnitTests/UnitTest_DebugInfo.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Neo.Compiler.CSharp.UnitTests/UnitTest_DebugInfo.cs b/tests/Neo.Compiler.CSharp.UnitTests/UnitTest_DebugInfo.cs index 9138b6143..e539d6def 100644 --- a/tests/Neo.Compiler.CSharp.UnitTests/UnitTest_DebugInfo.cs +++ b/tests/Neo.Compiler.CSharp.UnitTests/UnitTest_DebugInfo.cs @@ -25,7 +25,7 @@ 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:28-17:29;4[0]17:13-17:29;5[0]17:13-17:29;6[0]17:13-17:30;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:33;13[0]19:9-19:10", + 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)); From 89f7af071109d24d117cbad06a23c233f6d1cbab Mon Sep 17 00:00:00 2001 From: Fernando Diaz Toledano Date: Thu, 22 Feb 2024 19:28:47 +0100 Subject: [PATCH 15/19] Fix branch if --- .../MethodConvert/Statement/IfStatement.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Neo.Compiler.CSharp/MethodConvert/Statement/IfStatement.cs b/src/Neo.Compiler.CSharp/MethodConvert/Statement/IfStatement.cs index 35646608a..0b1199d6d 100644 --- a/src/Neo.Compiler.CSharp/MethodConvert/Statement/IfStatement.cs +++ b/src/Neo.Compiler.CSharp/MethodConvert/Statement/IfStatement.cs @@ -48,10 +48,10 @@ private void ConvertIfStatement(SemanticModel model, IfStatementSyntax syntax) { JumpTarget elseTarget = new(); - ConvertExpression(model, syntax.Condition); - - using (InsertSequencePoint(syntax.Statement)) + using (InsertSequencePoint(syntax)) { + ConvertExpression(model, syntax.Condition); + Jump(OpCode.JMPIFNOT_L, elseTarget); ConvertStatement(model, syntax.Statement); From 7c65c624431ebc8c3db5f3d261860ca041d48df1 Mon Sep 17 00:00:00 2001 From: Fernando Diaz Toledano Date: Thu, 22 Feb 2024 19:32:47 +0100 Subject: [PATCH 16/19] 100% branch coverage in template nep17 --- src/Neo.SmartContract.Testing/TestingStandards/Nep17Tests.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Neo.SmartContract.Testing/TestingStandards/Nep17Tests.cs b/src/Neo.SmartContract.Testing/TestingStandards/Nep17Tests.cs index 710d72f69..ee5019c25 100644 --- a/src/Neo.SmartContract.Testing/TestingStandards/Nep17Tests.cs +++ b/src/Neo.SmartContract.Testing/TestingStandards/Nep17Tests.cs @@ -144,6 +144,11 @@ public virtual void TestTransfer() Assert.AreEqual(initialSupply, Contract.TotalSupply); AssertTransferEvent(Alice.Account, Bob.Account, 3); + // Check 0 tokens transfer + + Assert.IsTrue(Contract.Transfer(Alice.Account, Bob.Account, 0)); + AssertTransferEvent(Alice.Account, Bob.Account, 0); + // Invoke invalid transfers Assert.ThrowsException(() => Assert.IsTrue(Contract.Transfer(Alice.Account, Bob.Account, -1))); From 74f554ef51629e6200291f38e7025f6be47cf60d Mon Sep 17 00:00:00 2001 From: Fernando Diaz Toledano Date: Thu, 22 Feb 2024 19:47:42 +0100 Subject: [PATCH 17/19] fix --- .../Coverage/Formats/CoberturaFormat.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Neo.SmartContract.Testing/Coverage/Formats/CoberturaFormat.cs b/src/Neo.SmartContract.Testing/Coverage/Formats/CoberturaFormat.cs index 4513f570a..05a2a588c 100644 --- a/src/Neo.SmartContract.Testing/Coverage/Formats/CoberturaFormat.cs +++ b/src/Neo.SmartContract.Testing/Coverage/Formats/CoberturaFormat.cs @@ -128,6 +128,8 @@ private static (int lineCount, int hitCount) GetLineRate(CoveredContract contrac foreach (var line in lines) { + if (line.Offset > last) break; + if (contract.TryGetBranch(address, out var branch)) // IsBranchInstruction { //yield return (address, ins.OpCode); @@ -152,7 +154,7 @@ public static int GetLineLastAddress(CoverageHit[] lines, NeoDebugInfo.Method me foreach (var line in lines) { - if (line.Offset >= nextSPAddress) + if (line.Offset > nextSPAddress) { return address; } From 6e5a631359e810a1158c75de169fc9ebb40e9030 Mon Sep 17 00:00:00 2001 From: Fernando Diaz Toledano Date: Thu, 22 Feb 2024 19:58:12 +0100 Subject: [PATCH 18/19] Optimize GetBranchInstructions --- .../Formats/CoberturaFormat.ContractCoverageWriter.cs | 7 +++---- .../Coverage/Formats/CoberturaFormat.cs | 8 +++----- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/Neo.SmartContract.Testing/Coverage/Formats/CoberturaFormat.ContractCoverageWriter.cs b/src/Neo.SmartContract.Testing/Coverage/Formats/CoberturaFormat.ContractCoverageWriter.cs index 6fa201694..0922a69d3 100644 --- a/src/Neo.SmartContract.Testing/Coverage/Formats/CoberturaFormat.ContractCoverageWriter.cs +++ b/src/Neo.SmartContract.Testing/Coverage/Formats/CoberturaFormat.ContractCoverageWriter.cs @@ -145,16 +145,15 @@ internal void WriteLine(XmlWriter writer, NeoDebugInfo.Method method, NeoDebugIn writer.WriteAttributeString("condition-coverage", $"{branchRate * 100:N}% ({branchHit}/{branchCount})"); writer.WriteStartElement("conditions"); - foreach (var (address, opCode) in GetBranchInstructions(Contract, method, sp)) + foreach ((var address, var branchI) in GetBranchInstructions(Contract, method, sp)) { - var (condBranchCount, condContinueCount) = Contract.TryGetBranch(address, out var brach) ? - (brach.Count, brach.Hits) : (0, 0); + var (condBranchCount, condContinueCount) = (branchI.Count, branchI.Hits); var coverage = condBranchCount == 0 ? 0m : 1m; coverage += condContinueCount == 0 ? 0m : 1m; writer.WriteStartElement("condition"); writer.WriteAttributeString("number", $"{address}"); - writer.WriteAttributeString("type", $"{opCode}"); + writer.WriteAttributeString("type", $"{branchI.Offset}"); // TODO: Opcode? writer.WriteAttributeString("coverage", $"{coverage / 2m * 100m}%"); writer.WriteEndElement(); } diff --git a/src/Neo.SmartContract.Testing/Coverage/Formats/CoberturaFormat.cs b/src/Neo.SmartContract.Testing/Coverage/Formats/CoberturaFormat.cs index 05a2a588c..7fbc668c9 100644 --- a/src/Neo.SmartContract.Testing/Coverage/Formats/CoberturaFormat.cs +++ b/src/Neo.SmartContract.Testing/Coverage/Formats/CoberturaFormat.cs @@ -118,7 +118,7 @@ private static (int lineCount, int hitCount) GetLineRate(CoveredContract contrac return (lineCount, hitCount); } - public static IEnumerable<(int address, OpCode opCode)> GetBranchInstructions( + public static IEnumerable<(int address, CoverageBranch branch)> GetBranchInstructions( CoveredContract contract, NeoDebugInfo.Method method, NeoDebugInfo.SequencePoint sequencePoint ) { @@ -132,8 +132,7 @@ private static (int lineCount, int hitCount) GetLineRate(CoveredContract contrac if (contract.TryGetBranch(address, out var branch)) // IsBranchInstruction { - //yield return (address, ins.OpCode); - yield return (address, OpCode.NOP); + yield return (address, branch); } } } @@ -149,8 +148,7 @@ public static int GetLineLastAddress(CoverageHit[] lines, NeoDebugInfo.Method me else { var nextSPAddress = method.SequencePoints[index + 1].Address; - var point = method.SequencePoints[index]; - var address = point.Address; + var address = method.SequencePoints[index].Address; foreach (var line in lines) { From ef32060265147f94671f8734d1fbeef71d28cfe4 Mon Sep 17 00:00:00 2001 From: Fernando Diaz Toledano Date: Thu, 22 Feb 2024 20:25:11 +0100 Subject: [PATCH 19/19] Allow to use license in reporting --- .../Coverage/CoverageReporting.cs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/Neo.SmartContract.Testing/Coverage/CoverageReporting.cs b/src/Neo.SmartContract.Testing/Coverage/CoverageReporting.cs index 4e52d85e8..1468b40dc 100644 --- a/src/Neo.SmartContract.Testing/Coverage/CoverageReporting.cs +++ b/src/Neo.SmartContract.Testing/Coverage/CoverageReporting.cs @@ -10,14 +10,22 @@ public class CoverageReporting /// /// Coverage file /// Output dir + /// License /// True if was success - public static bool CreateReport(string file, string outputDir) + public static bool CreateReport(string file, string outputDir, string? license = null) { try { // Reporting - Program.Main(new string[] { $"-reports:{Path.GetFullPath(file)}", $"-targetdir:{Path.GetFullPath(outputDir)}" }); + if (string.IsNullOrEmpty(license)) + { + Program.Main(new string[] { $"-reports:{Path.GetFullPath(file)}", $"-targetdir:{Path.GetFullPath(outputDir)}" }); + } + else + { + Program.Main(new string[] { $"-reports:{Path.GetFullPath(file)}", $"-targetdir:{Path.GetFullPath(outputDir)}", $"-license:{license}" }); + } return true; } catch { }