From 4598c792bb0e6678a9803cb28522fb3678a673e5 Mon Sep 17 00:00:00 2001
From: Christian Lawson-Perfect <christianperfect@gmail.com>
Date: Thu, 19 Dec 2024 13:00:49 +0000
Subject: [PATCH] findvars on recursive custom functions doesn't include that
 function's name

When finding unbound variables inside the definition of a custom JME
function, the scope didn't yet know about the function being defined, so
a recursive call of that function would produce the function's own name
as a free variable.

This changes makeJMEFunction to add the function to the scope that is
used when finding unbound variables inside its definition.
---
 runtime/scripts/jme-variables.js |  4 +++-
 tests/jme-runtime.js             |  4 +++-
 tests/jme/jme-tests.mjs          | 12 ++++++++++++
 tests/numbas-runtime.js          |  4 +++-
 4 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/runtime/scripts/jme-variables.js b/runtime/scripts/jme-variables.js
index 69b9bcac5..e2abd7dd3 100644
--- a/runtime/scripts/jme-variables.js
+++ b/runtime/scripts/jme-variables.js
@@ -49,7 +49,9 @@ jme.variables = /** @lends Numbas.jme.variables */ {
      */
     makeJMEFunction: function(fn,scope) {
         fn.tree = jme.compile(fn.definition,scope,true);
-        var external_vars = jme.findvars(fn.tree,fn.paramNames.map(function(v) { return jme.normaliseName(v,scope) }),scope);
+        const nscope = new jme.Scope([scope]);
+        nscope.addFunction(fn);
+        var external_vars = jme.findvars(fn.tree,fn.paramNames.map(function(v) { return jme.normaliseName(v,scope) }),nscope);
         jme.findvarsOps[fn.name] = function(tree,boundvars,scope) {
             var vars = external_vars.slice();
             for(var i=0;i<tree.args.length;i++) {
diff --git a/tests/jme-runtime.js b/tests/jme-runtime.js
index 63d97095c..b122adf23 100644
--- a/tests/jme-runtime.js
+++ b/tests/jme-runtime.js
@@ -19971,7 +19971,9 @@ jme.variables = /** @lends Numbas.jme.variables */ {
      */
     makeJMEFunction: function(fn,scope) {
         fn.tree = jme.compile(fn.definition,scope,true);
-        var external_vars = jme.findvars(fn.tree,fn.paramNames.map(function(v) { return jme.normaliseName(v,scope) }),scope);
+        const nscope = new jme.Scope([scope]);
+        nscope.addFunction(fn);
+        var external_vars = jme.findvars(fn.tree,fn.paramNames.map(function(v) { return jme.normaliseName(v,scope) }),nscope);
         jme.findvarsOps[fn.name] = function(tree,boundvars,scope) {
             var vars = external_vars.slice();
             for(var i=0;i<tree.args.length;i++) {
diff --git a/tests/jme/jme-tests.mjs b/tests/jme/jme-tests.mjs
index 70b16c390..4c827d1eb 100644
--- a/tests/jme/jme-tests.mjs
+++ b/tests/jme/jme-tests.mjs
@@ -95,6 +95,18 @@ Numbas.queueScript('jme_tests',['qunit','jme','jme-rules','jme-display','jme-cal
         deepCloseEqual(assert, Numbas.jme.findvars(Numbas.jme.compile('a -> a(x)')),['x'],'findvars on lambda where an argument is another lambda');
         deepCloseEqual(assert, Numbas.jme.findvars(Numbas.jme.compile('[x,[y,z]] -> x+y+z+w')),['w'],'findvars on lambda with destructuring');
         deepCloseEqual(assert, Numbas.jme.findvars(Numbas.jme.compile('undefined_function(x)')),['x', 'undefined_function'],'Undefined function names are presumed to be missing variables.');
+
+        const fn = Numbas.jme.variables.makeFunction({
+            parameters: [{type:'number', name:'x'}],
+            definition: 'if(x>0,0,f(x-1))',
+            name: 'f',
+            language: 'jme'
+        }, Numbas.jme.builtinScope);
+
+        const scope = new Numbas.jme.Scope([Numbas.jme.builtinScope]);
+        scope.addFunction(fn);
+        const vars = Numbas.jme.findvars(Numbas.jme.compile('f(2)'), scope);
+        assert.deepEqual(vars, [], "Recursive custom functions don't produce their own names in findvars");
     });
 
     QUnit.test('findvars in HTML',function(assert) {
diff --git a/tests/numbas-runtime.js b/tests/numbas-runtime.js
index 1ccde1af0..a815af157 100644
--- a/tests/numbas-runtime.js
+++ b/tests/numbas-runtime.js
@@ -19307,7 +19307,9 @@ jme.variables = /** @lends Numbas.jme.variables */ {
      */
     makeJMEFunction: function(fn,scope) {
         fn.tree = jme.compile(fn.definition,scope,true);
-        var external_vars = jme.findvars(fn.tree,fn.paramNames.map(function(v) { return jme.normaliseName(v,scope) }),scope);
+        const nscope = new jme.Scope([scope]);
+        nscope.addFunction(fn);
+        var external_vars = jme.findvars(fn.tree,fn.paramNames.map(function(v) { return jme.normaliseName(v,scope) }),nscope);
         jme.findvarsOps[fn.name] = function(tree,boundvars,scope) {
             var vars = external_vars.slice();
             for(var i=0;i<tree.args.length;i++) {