Update sketch verifier to check for redefinitions and print friendly messages

src/core/friendly_errors/sketch_verifier.js

@@ -1,11 +1,64 @@
 import * as acorn from 'acorn';
 import * as walk from 'acorn-walk';
+import * as constants from '../constants';
 
 /**
  * @for p5
  * @requires core
  */
 function sketchVerifier(p5, fn) {
+  // List of functions to ignore as they either are meant to be re-defined or
+  // generate false positive outputs.
+  const ignoreFunction = [
+    'setup',
+    'draw',
+    'preload',
+    'deviceMoved',
+    'deviceTurned',
+    'deviceShaken',
+    'doubleClicked',
+    'mousePressed',
+    'mouseReleased',
+    'mouseMoved',
+    'mouseDragged',
+    'mouseClicked',
+    'mouseWheel',
+    'touchStarted',
+    'touchMoved',
+    'touchEnded',
+    'keyPressed',
+    'keyReleased',
+    'keyTyped',
+    'windowResized',
+    'name',
+    'parent',
+    'toString',
+    'print',
+    'stop',
+    'onended'
+  ];
+
+  // Mapping names of p5 types to their constructor functions.
+  // p5Constructors:
+  //   - Color: f()
+  //   - Graphics: f()
+  //   - Vector: f()
+  // and so on.
+  const p5Constructors = {};
+
+  fn.loadP5Constructors = function () {
+    // Make a list of all p5 classes to be used for argument validation
+    // This must be done only when everything has loaded otherwise we get
+    // an empty array
+    for (let key of Object.keys(p5)) {
+      // Get a list of all constructors in p5. They are functions whose names
+      // start with a capital letter
+      if (typeof p5[key] === 'function' && key[0] !== key[0].toLowerCase()) {
+        p5Constructors[key] = p5[key];
+      }
+    }
+  }
+
   /**
    * Fetches the contents of a script element in the user's sketch.
    * 
@@ -111,16 +164,116 @@ function sketchVerifier(p5, fn) {
     return userDefinitions;
   }
 
+  /**
+   * Checks user-defined variables and functions for conflicts with p5.js
+   * constants and global functions.
+   * 
+   * This function performs two main checks:
+   * 1. Verifies if any user definition conflicts with p5.js constants.
+   * 2. Checks if any user definition conflicts with global functions from
+   * p5.js renderer classes.
+   * 
+   * If a conflict is found, it reports a friendly error message and halts
+   * further checking.
+   * 
+   * @param {Object} userDefinitions - An object containing user-defined variables and functions.
+   * @param {Array<{name: string, line: number}>} userDefinitions.variables - Array of user-defined variable names and their line numbers.
+   * @param {Array<{name: string, line: number}>} userDefinitions.functions - Array of user-defined function names and their line numbers.
+   * @returns {boolean} - Returns true if a conflict is found, false otherwise.
+   */
+  fn.checkForConstsAndFuncs = function (userDefinitions) {
+    const allDefinitions = [
+      ...userDefinitions.variables,
+      ...userDefinitions.functions
+    ];
+
+    // Helper function that generates a friendly error message that contains
+    // the type of redefinition (constant or function), the name of the
+    // redefinition, the line number in user's code, and a link to its
+    // reference on the p5.js website.
+    function generateFriendlyError(errorType, name, line) {
+      const url = `https://p5js.org/reference/#/p5/${name}`;
+      const message = `${errorType} "${name}" on line ${line} is being redeclared and conflicts with a p5.js ${errorType.toLowerCase()}. JavaScript does not declaring a ${errorType.toLowerCase()} more than once. p5.js reference: ${url}.`;
+      return message;
+    }
+
+    // Helper function that checks if a user definition has already been defined
+    // in the p5.js library, either as a constant or as a function.
+    function checkForRedefinition(name, libValue, line, type) {
+      try {
+        const userValue = eval("name");
+        if (libValue !== userValue) {
+          let message = generateFriendlyError(type, name, line);
+          console.log(message);
+          return true;
+        }
+      } catch (e) {
+        // If eval fails, the function hasn't been redefined
+        return false;
+      }
+      return false;
+    }
+
+    // Checks for constant redefinitions.
+    for (let { name, line } of allDefinitions) {
+      const libDefinition = constants[name];
+      if (libDefinition !== undefined) {
+        if (checkForRedefinition(name, libDefinition, line, "Constant")) {
+          return true;
+        }
+      }
+    }
+
+    // Get the names of all p5.js functions which are available globally
+    const classesWithGlobalFns = ['Renderer', 'Renderer2D', 'RendererGL'];
+    const globalFunctions = new Set(
+      classesWithGlobalFns.flatMap(className =>
+        Object.keys(p5Constructors[className]?.prototype || {})
+      )
+    );
+
+    for (let { name, line } of allDefinitions) {
+      if (!ignoreFunction.includes(name) && globalFunctions.has(name)) {
+        for (let className of classesWithGlobalFns) {
+          const prototypeFunc = p5Constructors[className]?.prototype[name];
+          if (prototypeFunc && checkForRedefinition(name, prototypeFunc, line, "Function")) {
+            return true;
+          }
+        }
+      }
+    }
+
+    // Additional check for other p5 constructors
+    const otherP5ConstructorKeys = Object.keys(p5Constructors).filter(
+      key => !classesWithGlobalFns.includes(key)
+    );
+    for (let { name, line } of allDefinitions) {
+      for (let key of otherP5ConstructorKeys) {
+        if (p5Constructors[key].prototype[name] !== undefined) {
+          const prototypeFunc = p5Constructors[key].prototype[name];
+          if (prototypeFunc && checkForRedefinition(name, prototypeFunc, line, "Function")) {
+            return true;
+          }
+        }
+      }
+    }
+
+    return false;
+  }
+
   fn.run = async function () {
     const userCode = await fn.getUserCode();
     const userDefinedVariablesAndFuncs = fn.extractUserDefinedVariablesAndFuncs(userCode);
 
-    return userDefinedVariablesAndFuncs;
+    if (fn.checkForConstsAndFuncs(userDefinedVariablesAndFuncs)) {
+      return;
+    }
   }
 }
 
 export default sketchVerifier;
 
 if (typeof p5 !== 'undefined') {
   sketchVerifier(p5, p5.prototype);
+  p5.prototype.loadP5Constructors();
 }

test/unit/core/sketch_overrides.js

@@ -1,13 +1,16 @@
 import sketchVerifier from '../../../src/core/friendly_errors/sketch_verifier.js';
 
-suite('Validate Params', function () {
+suite('Sketch Verifier', function () {
   const mockP5 = {
-    _validateParameters: vi.fn()
+    _validateParameters: vi.fn(),
+    Renderer: function () { },
   };
+  mockP5.Renderer.prototype.rect = vi.fn();
   const mockP5Prototype = {};
 
   beforeAll(function () {
     sketchVerifier(mockP5, mockP5Prototype);
+    mockP5Prototype.loadP5Constructors();
   });
 
   afterAll(function () {
@@ -179,42 +182,129 @@ suite('Validate Params', function () {
     });
   });
 
+  suite('checkForConstsAndFuncs()', function () {
+    // Set up for this suite of tests
+    let consoleSpy;
+
+    beforeEach(function () {
+      consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => { });
+    });
+
+    afterEach(function () {
+      consoleSpy.mockRestore();
+    });
+
+    test('Detects conflict with p5.js constant', function () {
+      const userDefinitions = {
+        variables: [{ name: 'PI', line: 1 }],
+        functions: []
+      };
+      const result = mockP5Prototype.checkForConstsAndFuncs(userDefinitions);
+
+      expect(result).toBe(true);
+      expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining('Constant "PI" on line 1 is being redeclared and conflicts with a p5.js constant'));
+    });
+
+    test('Detects conflict with p5.js global function', function () {
+      const userDefinitions = {
+        variables: [],
+        functions: [{ name: 'rect', line: 2 }]
+      };
+      const result = mockP5Prototype.checkForConstsAndFuncs(userDefinitions);
+
+      expect(result).toBe(true);
+      expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining('Function "rect" on line 2 is being redeclared and conflicts with a p5.js function'));
+    });
+
+    test('allows redefinition of whitelisted functions', function () {
+      const userDefinitions = {
+        variables: [],
+        functions: [
+          { name: 'setup', line: 1 },
+          { name: 'draw', line: 2 },
+          { name: 'preload', line: 3 }
+        ]
+      };
+
+      const result = mockP5Prototype.checkForConstsAndFuncs(userDefinitions);
+
+      expect(result).toBe(false);
+      expect(consoleSpy).not.toHaveBeenCalled();
+    });
+
+    test('Returns false when no conflicts are found', function () {
+      const userDefinitions = {
+        variables: [{ name: 'img', line: 1 }],
+        functions: [{ name: 'cut', line: 2 }]
+      };
+
+      const result = mockP5Prototype.checkForConstsAndFuncs(userDefinitions);
+
+      expect(result).toBe(false);
+    });
+  });
+
   suite('run()', function () {
-    test('Returns extracted variables and functions', async function () {
+    test('Extracts user-defined variables and functions and checks for conflicts', async function () {
       const mockScript = `
         let x = 5;
         const y = 10;
         function foo() {}
         const bar = () => {};
+        class MyClass {}
       `;
       mockP5Prototype.getUserCode = vi.fn(() => Promise.resolve(mockScript));
-
-      const result = await mockP5Prototype.run();
-      const expectedResult = {
-        "functions": [
-          {
-            "line": 3,
-            "name": "foo",
-          },
-          {
-            "line": 4,
-            "name": "bar",
-          },
+      mockP5Prototype.extractUserDefinedVariablesAndFuncs = vi.fn(() => ({
+        variables: [
+          { name: 'x', line: 1 },
+          { name: 'y', line: 2 },
+          { name: 'MyClass', line: 5 }
         ],
-        "variables": [
-          {
-            "line": 1,
-            "name": "x",
-          },
-          {
-            "line": 2,
-            "name": "y",
-          },
+        functions: [
+          { name: 'foo', line: 3 },
+          { name: 'bar', line: 4 }
+        ]
+      }));
+      mockP5Prototype.checkForConstsAndFuncs = vi.fn(() => false);
+
+      await mockP5Prototype.run();
+
+      expect(mockP5Prototype.getUserCode).toHaveBeenCalledTimes(1);
+      expect(mockP5Prototype.extractUserDefinedVariablesAndFuncs).toHaveBeenCalledWith(mockScript);
+      expect(mockP5Prototype.checkForConstsAndFuncs).toHaveBeenCalledWith({
+        variables: [
+          { name: 'x', line: 1 },
+          { name: 'y', line: 2 },
+          { name: 'MyClass', line: 5 }
         ],
-      };
+        functions: [
+          { name: 'foo', line: 3 },
+          { name: 'bar', line: 4 }
+        ]
+      });
+    });
+
+    test('Stops execution when a conflict is found', async function () {
+      const mockScript = `
+        let PI = 3.14;
+        function setup() {}
+      `;
+      mockP5Prototype.getUserCode = vi.fn(() => Promise.resolve(mockScript));
+      mockP5Prototype.extractUserDefinedVariablesAndFuncs = vi.fn(() => ({
+        variables: [{ name: 'PI', line: 1 }],
+        functions: [{ name: 'setup', line: 2 }]
+      }));
+      mockP5Prototype.checkForConstsAndFuncs = vi.fn(() => true);
+
+      const result = await mockP5Prototype.run();
 
       expect(mockP5Prototype.getUserCode).toHaveBeenCalledTimes(1);
-      expect(result).toEqual(expectedResult);
+      expect(mockP5Prototype.extractUserDefinedVariablesAndFuncs).toHaveBeenCalledWith(mockScript);
+      expect(mockP5Prototype.checkForConstsAndFuncs).toHaveBeenCalledWith({
+        variables: [{ name: 'PI', line: 1 }],
+        functions: [{ name: 'setup', line: 2 }]
+      });
+      expect(result).toBeUndefined();
     });
   });
 });