Debugger API improvements (#1382)

Jint.Tests/Runtime/Debugger/BreakPointTests.cs

@@ -192,6 +192,7 @@ public void DebuggerStatementTriggersBreak()
             bool didBreak = false;
             engine.DebugHandler.Break += (sender, info) =>
             {
+                Assert.Equal(PauseType.DebuggerStatement, info.PauseType);
                 didBreak = true;
                 return StepMode.None;
             };
@@ -232,6 +233,33 @@ public void DebuggerStatementDoesNotTriggerBreakWhenStepping()
             Assert.False(didBreak);
         }
 
+        [Fact]
+        public void DebuggerStatementDoesNotTriggerBreakWhenAtBreakPoint()
+        {
+            string script = @"'dummy';
+debugger;
+'dummy';";
+
+            var engine = new Engine(options => options
+                .DebugMode()
+                .DebuggerStatementHandling(DebuggerStatementHandling.Script)
+                .InitialStepMode(StepMode.None));
+
+            int breakCount = 0;
+
+            engine.DebugHandler.BreakPoints.Set(new BreakPoint(2, 0));
+
+            engine.DebugHandler.Break += (sender, info) =>
+            {
+                Assert.Equal(PauseType.Break, info.PauseType);
+                breakCount++;
+                return StepMode.None;
+            };
+
+            engine.Execute(script);
+            Assert.Equal(1, breakCount);
+        }
+
         [Fact]
         public void BreakPointDoesNotTriggerBreakWhenStepping()
         {

Jint.Tests/Runtime/Debugger/DebugHandlerTests.cs

@@ -31,7 +31,7 @@ public void AvoidsPauseRecursion()
 
                 if (info.ReachedLiteral("target"))
                 {
-                    var obj = info.CurrentScopeChain.Global.GetBindingValue("obj") as ObjectInstance;
+                    var obj = info.CurrentScopeChain[0].GetBindingValue("obj") as ObjectInstance;
                     var prop = obj.GetOwnProperty("name");
                     // This is where reentrance would occur:
                     var value = prop.Get.Invoke(engine);

Jint.Tests/Runtime/Debugger/ScopeTests.cs

@@ -299,7 +299,8 @@ function power(a)
             {
                 Assert.Collection(info.CurrentScopeChain,
                     scope => AssertScope(scope, DebugScopeType.Local, "arguments", "a"),
-                    scope => AssertScope(scope, DebugScopeType.Closure, "b", "power"), // a, this, arguments shadowed by local
+                    // a, arguments shadowed by local - but still exist in this scope
+                    scope => AssertScope(scope, DebugScopeType.Closure, "a", "arguments", "b", "power"),
                     scope => AssertScope(scope, DebugScopeType.Script, "x", "y", "z"),
                     scope => AssertScope(scope, DebugScopeType.Global, "add"));
             });
@@ -327,7 +328,7 @@ function add(a, b)
                 Assert.Collection(info.CurrentScopeChain,
                     scope => AssertScope(scope, DebugScopeType.Block, "y"),
                     scope => AssertScope(scope, DebugScopeType.Local, "arguments", "a", "b"),
-                    scope => AssertScope(scope, DebugScopeType.Script, "x", "z"), // y shadowed
+                    scope => AssertScope(scope, DebugScopeType.Script, "x", "y", "z"), // y is shadowed, but still in the scope
                     scope => AssertScope(scope, DebugScopeType.Global, "add"));
             });
         }
@@ -391,7 +392,7 @@ function add(a, b)
                     scope => AssertScope(scope, DebugScopeType.Block, "x"),
                     scope => AssertScope(scope, DebugScopeType.Block, "y"),
                     scope => AssertScope(scope, DebugScopeType.Local, "arguments", "a", "b"),
-                    scope => AssertScope(scope, DebugScopeType.Script, "z"), // x, y shadowed
+                    scope => AssertScope(scope, DebugScopeType.Script, "x", "y", "z"), // x, y are shadowed, but still in the scope
                     scope => AssertScope(scope, DebugScopeType.Global, "add"));
             });
         }

Jint.Tests/Runtime/Debugger/StepModeTests.cs

@@ -413,7 +413,7 @@ public void StepNotTriggeredWhenRunning()
                 function test()
                 {
                     'dummy';
-                    'øummy';
+                    'dummy';
                 }";
 
             var engine = new Engine(options => options
@@ -432,5 +432,52 @@ function test()
 
             Assert.Equal(1, stepCount);
         }
+
+        [Fact]
+        public void SkipIsTriggeredWhenRunning()
+        {
+            string script = @"
+                'step';
+                'skip';
+                'skip';
+                debugger;
+                'step';
+                'step';
+                ";
+
+            var engine = new Engine(options => options
+                .DebugMode()
+                .DebuggerStatementHandling(DebuggerStatementHandling.Script)
+                .InitialStepMode(StepMode.Into));
+
+            int stepCount = 0;
+            int skipCount = 0;
+
+            engine.DebugHandler.Step += (sender, info) =>
+            {
+                Assert.True(TestHelpers.IsLiteral(info.CurrentNode, "step"));
+                stepCount++;
+                // Start running after first step
+                return stepCount == 1 ? StepMode.None : StepMode.Into;
+            };
+
+            engine.DebugHandler.Skip += (sender, info) =>
+            {
+                Assert.True(TestHelpers.IsLiteral(info.CurrentNode, "skip"));
+                skipCount++;
+                return StepMode.None;
+            };
+
+            engine.DebugHandler.Break += (sender, info) =>
+            {
+                // Back to stepping after debugger statement
+                return StepMode.Into;
+            };
+
+            engine.Execute(script);
+
+            Assert.Equal(2, skipCount);
+            Assert.Equal(3, stepCount);
+        }
     }
 }

Jint.Tests/Runtime/EngineTests.cs

@@ -1,11 +1,13 @@
 using System.Globalization;
 using System.Reflection;
 using Esprima;
+using Esprima.Ast;
 using Jint.Native;
 using Jint.Native.Array;
 using Jint.Native.Object;
 using Jint.Runtime;
 using Jint.Runtime.Debugger;
+using Jint.Tests.Runtime.Debugger;
 using Xunit.Abstractions;
 
 #pragma warning disable 618
@@ -1584,13 +1586,11 @@ private StepMode EngineStepVerifyDebugInfo(object sender, DebugInformation debug
             Assert.NotNull(debugInfo.CallStack);
             Assert.NotNull(debugInfo.CurrentNode);
             Assert.NotNull(debugInfo.CurrentScopeChain);
-            Assert.NotNull(debugInfo.CurrentScopeChain.Global);
-            Assert.NotNull(debugInfo.CurrentScopeChain.Local);
 
             Assert.Equal(2, debugInfo.CallStack.Count);
             Assert.Equal("func1", debugInfo.CurrentCallFrame.FunctionName);
-            var globalScope = debugInfo.CurrentScopeChain.Global;
-            var localScope = debugInfo.CurrentScopeChain.Local;
+            var globalScope = debugInfo.CurrentScopeChain.Single(s => s.ScopeType == DebugScopeType.Global);
+            var localScope = debugInfo.CurrentScopeChain.Single(s => s.ScopeType == DebugScopeType.Local);
             Assert.Contains("global", globalScope.BindingNames);
             Assert.Equal(true, globalScope.GetBindingValue("global").AsBoolean());
             Assert.Contains("local", localScope.BindingNames);
@@ -2882,6 +2882,136 @@ public void ShouldAllowOptionalChainingForMemberCall()
             Assert.True(array[1].IsUndefined());
         }
 
+        [Fact]
+        public void ExecuteShouldTriggerBeforeEvaluateEvent()
+        {
+            TestBeforeEvaluateEvent(
+                (engine, code) => engine.Execute(code),
+                expectedSource: "<anonymous>"
+            );
+        }
+
+        [Fact]
+        public void ExecuteWithSourceShouldTriggerBeforeEvaluateEvent()
+        {
+            TestBeforeEvaluateEvent(
+                (engine, code) => engine.Execute(code, "mysource"),
+                expectedSource: "mysource"
+            );
+        }
+
+        [Fact]
+        public void ExecuteWithParserOptionsShouldTriggerBeforeEvaluateEvent()
+        {
+            TestBeforeEvaluateEvent(
+                (engine, code) => engine.Execute(code, ParserOptions.Default),
+                expectedSource: "<anonymous>"
+            );
+        }
+
+        [Fact]
+        public void ExecuteWithSourceAndParserOptionsShouldTriggerBeforeEvaluateEvent()
+        {
+            TestBeforeEvaluateEvent(
+                (engine, code) => engine.Execute(code, "mysource", ParserOptions.Default),
+                expectedSource: "mysource"
+            );
+        }
+
+        [Fact]
+        public void EvaluateShouldTriggerBeforeEvaluateEvent()
+        {
+            TestBeforeEvaluateEvent(
+                (engine, code) => engine.Evaluate(code),
+                expectedSource: "<anonymous>"
+            );
+        }
+
+        [Fact]
+        public void EvaluateWithSourceShouldTriggerBeforeEvaluateEvent()
+        {
+            TestBeforeEvaluateEvent(
+                (engine, code) => engine.Evaluate(code, "mysource"),
+                expectedSource: "mysource"
+            );
+        }
+
+        [Fact]
+        public void EvaluateWithParserOptionsShouldTriggerBeforeEvaluateEvent()
+        {
+            TestBeforeEvaluateEvent(
+                (engine, code) => engine.Evaluate(code, ParserOptions.Default),
+                expectedSource: "<anonymous>"
+            );
+        }
+
+        [Fact]
+        public void EvaluateWithSourceAndParserOptionsShouldTriggerBeforeEvaluateEvent()
+        {
+            TestBeforeEvaluateEvent(
+                (engine, code) => engine.Evaluate(code, "mysource", ParserOptions.Default),
+                expectedSource: "mysource"
+            );
+        }
+
+        [Fact]
+        public void ImportModuleShouldTriggerBeforeEvaluateEvents()
+        {
+            var engine = new Engine();
+
+            const string module1 = "import dummy from 'module2';";
+            const string module2 = "export default 'dummy';";
+
+            var beforeEvaluateTriggeredCount = 0;
+            engine.DebugHandler.BeforeEvaluate += (sender, ast) =>
+            {
+                beforeEvaluateTriggeredCount++;
+                Assert.Equal(engine, sender);
+
+                switch (beforeEvaluateTriggeredCount)
+                {
+                    case 1:
+                        Assert.Equal("module1", ast.Location.Source);
+                        Assert.Collection(ast.Body,
+                            node => Assert.IsType<ImportDeclaration>(node)
+                        );
+                        break;
+                    case 2:
+                        Assert.Equal("module2", ast.Location.Source);
+                        Assert.Collection(ast.Body,
+                            node => Assert.IsType<ExportDefaultDeclaration>(node)
+                        );
+                        break;
+                }
+            };
+
+            engine.AddModule("module1", module1);
+            engine.AddModule("module2", module2);
+            engine.ImportModule("module1");
+
+            Assert.Equal(2, beforeEvaluateTriggeredCount);
+        }
+
+        private static void TestBeforeEvaluateEvent(Action<Engine, string> call, string expectedSource)
+        {
+            var engine = new Engine();
+
+            const string script = "'dummy';";
+
+            var beforeEvaluateTriggered = false;
+            engine.DebugHandler.BeforeEvaluate += (sender, ast) =>
+            {
+                beforeEvaluateTriggered = true;
+                Assert.Equal(engine, sender);
+                Assert.Equal(expectedSource, ast.Location.Source);
+                Assert.Collection(ast.Body, node => Assert.True(TestHelpers.IsLiteral(node, "dummy")));
+            };
+
+            call(engine, script);
+
+            Assert.True(beforeEvaluateTriggered);
+        }
+
         private class Wrapper
         {
             public Testificate Test { get; set; }

Jint.Tests/Runtime/Modules/DefaultModuleLoaderTests.cs

@@ -35,23 +35,6 @@ public void ShouldRejectPathsOutsideOfBasePath(string specifier)
         Assert.StartsWith(exc.Specifier, specifier);
     }
 
-    [Fact]
-    public void ShouldTriggerLoadedEvent()
-    {
-        var loader = new DefaultModuleLoader(ModuleTests.GetBasePath());
-        bool triggered = false;
-        loader.Loaded += (sender, source, module) =>
-        {
-            Assert.Equal(loader, sender);
-            Assert.NotNull(source);
-            Assert.NotNull(module);
-            triggered = true;
-        };
-        var engine = new Engine(options => options.EnableModules(loader));
-        engine.ImportModule("./modules/format-name.js");
-        Assert.True(triggered);
-    }
-
     [Fact]
     public void ShouldResolveBareSpecifiers()
     {

Jint/Engine.Modules.cs

@@ -41,6 +41,11 @@ internal ModuleRecord LoadModule(string? referencingModuleLocation, string speci
                 module = LoaderFromModuleLoader(moduleResolution);
             }
 
+            if (module is SourceTextModuleRecord sourceTextModule)
+            {
+                DebugHandler.OnBeforeEvaluate(sourceTextModule._source);
+            }
+
             return module;
         }
 

Jint/Engine.cs

@@ -292,6 +292,8 @@ public Engine Execute(Script script)
         /// </summary>
         private Engine ScriptEvaluation(ScriptRecord scriptRecord)
         {
+            DebugHandler.OnBeforeEvaluate(scriptRecord.EcmaScriptCode);
+
             var globalEnv = Realm.GlobalEnv;
 
             var scriptContext = new ExecutionContext(
@@ -363,7 +365,7 @@ public ManualPromise RegisterPromise()
 
             Action<JsValue> SettleWith(FunctionInstance settle) => value =>
             {
-                settle.Call(JsValue.Undefined, new[] {value});
+                settle.Call(JsValue.Undefined, new[] { value });
                 RunAvailableContinuations();
             };
 

Jint/Runtime/Debugger/BreakLocation.cs

@@ -18,7 +18,7 @@ public BreakLocation(int line, int column) : this(null, line, column)
 
     }
 
-    public BreakLocation(string source, Esprima.Position position) : this(source, position.Line, position.Column)
+    public BreakLocation(string? source, Esprima.Position position) : this(source, position.Line, position.Column)
     {
     }