From d57886f5a8414a58d08c951df31f853dbd970871 Mon Sep 17 00:00:00 2001 From: skyash-dev Date: Tue, 1 Sep 2026 13:46:05 +0530 Subject: [PATCH 1/3] check strands identifiers and warn --- src/friendly_errors/sketch_verifier.js | 120 +++++++++++++++++++++++-- 1 file changed, 113 insertions(+), 7 deletions(-) diff --git a/src/friendly_errors/sketch_verifier.js b/src/friendly_errors/sketch_verifier.js index 45a40608cd..ba7013aa26 100644 --- a/src/friendly_errors/sketch_verifier.js +++ b/src/friendly_errors/sketch_verifier.js @@ -2,6 +2,7 @@ import { parse } from 'acorn'; import { simple as walk } from 'acorn-walk'; import * as constants from '../core/constants'; import { FES } from './fes'; +import { strandsBuiltinFunctions as builtInGLSLFunctions } from '../strands/strands_builtins'; // List of functions to ignore as they either are meant to be re-defined or // generate false positive outputs. @@ -78,6 +79,48 @@ export const verifierUtils = { // `lineOffset` here to correct them. const lineOffset = -1; + function isStrandsBuilderCall(node) { + if (node.type !== 'CallExpression' || !node.arguments?.length) return false; + + const callee = node.callee; + // buildFilterShader(fn), ... + if (callee.type === 'Identifier' && /^build\w*Shader$/.test(callee.name)) { + return true; + } + + // baseFilterShader().modify(fn), ... + if ( + callee.type === 'MemberExpression' && + callee.property?.type === 'Identifier' && + callee.property.name === 'modify' + ) { + return true; + } + return false; + } + + function recordCallbackBody(arg, strandsFunctionNames, strandsBodyRanges) { + if (!arg) return; + + // named: buildFilterShader(displaceColorsCallback) + if (arg.type === 'Identifier') { + strandsFunctionNames.add(arg.name); + return; + } + + // inline: buildFilterShader(() => { … }) / function () { … } + if ( + arg.type === 'FunctionExpression' || + arg.type === 'ArrowFunctionExpression' + ) { + if (arg.body?.type === 'BlockStatement') { + strandsBodyRanges.push([arg.body.start, arg.body.end]); + } else if (arg.start != null) { + strandsBodyRanges.push([arg.start, arg.end]); + } + } + } + try { const ast = parse(code, { ecmaVersion: 'latest', @@ -85,19 +128,67 @@ export const verifierUtils = { locations: true // This helps us get the line number. }); + const strandsFunctionNames = new Set(); + const strandsBodyRanges = []; + + walk(ast, { + CallExpression(node) { + if (!isStrandsBuilderCall(node)) return; + recordCallbackBody( + node.arguments[0], + strandsFunctionNames, + strandsBodyRanges + ); + } + }); + + // resolve named callbacks to body ranges + walk(ast, { + FunctionDeclaration(node) { + if (node.id && strandsFunctionNames.has(node.id.name) && node.body) { + strandsBodyRanges.push([node.body.start, node.body.end]); + } + }, + VariableDeclarator(node) { + if ( + node.id?.type === 'Identifier' && + strandsFunctionNames.has(node.id.name) && + node.init && + (node.init.type === 'FunctionExpression' || + node.init.type === 'ArrowFunctionExpression') + ) { + const body = node.init.body; + if (body?.type === 'BlockStatement') { + strandsBodyRanges.push([body.start, body.end]); + } else { + strandsBodyRanges.push([node.init.start, node.init.end]); + } + } + } + }); + + function isInsideStrands(node) { + if (node.start == null) return false; + for (const [s, e] of strandsBodyRanges) { + if (node.start >= s && node.start < e) return true; + } + return false; + } + walk(ast, { VariableDeclarator(node) { if (node.id.type === 'Identifier') { const category = node.init && - ['ArrowFunctionExpression', 'FunctionExpression'].includes( - node.init.type - ) + ['ArrowFunctionExpression', 'FunctionExpression'].includes( + node.init.type + ) ? 'functions' : 'variables'; userDefinitions[category].push({ name: node.id.name, - line: node.loc.start.line + lineOffset + line: node.loc.start.line + lineOffset, + insideStrands: isInsideStrands(node) }); } }, @@ -105,7 +196,8 @@ export const verifierUtils = { if (node.id && node.id.type === 'Identifier') { userDefinitions.functions.push({ name: node.id.name, - line: node.loc.start.line + lineOffset + line: node.loc.start.line + lineOffset, + insideStrands: isInsideStrands(node) }); } }, @@ -115,7 +207,8 @@ export const verifierUtils = { if (node.id && node.id.type === 'Identifier') { userDefinitions.variables.push({ name: node.id.name, - line: node.loc.start.line + lineOffset + line: node.loc.start.line + lineOffset, + insideStrands: isInsideStrands(node) }); } } @@ -183,7 +276,7 @@ export const verifierUtils = { ); for (let { name, line } of allDefinitions) { - if (!ignoreFunction.includes(name) && globalFunctions.has(name)) { + if (!ignoreFunction.includes(name) && globalFunctions.has(name) && !Object.hasOwn(builtInGLSLFunctions, name)) { const message = generateFriendlyError( FES.log`function`, name, @@ -194,6 +287,19 @@ export const verifierUtils = { } } + // strands/GLSL check + for (const { name, line, insideStrands } of allDefinitions) { + if (!insideStrands) continue; + if (Object.hasOwn(builtInGLSLFunctions, name)) { + const message = generateFriendlyError( + 'function', + name, + line + 1 + ); + FES.log`${message}`(); + return true; + } + } return false; }, From cc20c3064a6b8943fc3641f1b69d1137477b6882 Mon Sep 17 00:00:00 2001 From: skyash-dev Date: Tue, 1 Sep 2026 13:46:20 +0530 Subject: [PATCH 2/3] add tests --- test/unit/core/sketch_overrides.js | 126 ++++++++++++++++++++++++++--- 1 file changed, 113 insertions(+), 13 deletions(-) diff --git a/test/unit/core/sketch_overrides.js b/test/unit/core/sketch_overrides.js index 44a045f6a1..c10ef59135 100644 --- a/test/unit/core/sketch_overrides.js +++ b/test/unit/core/sketch_overrides.js @@ -85,41 +85,50 @@ suite('Sketch Verifier', function () { functions: [ { line: 5, - name: 'foo' + name: 'foo', + insideStrands: false, }, { line: 6, - name: 'bar' + name: 'bar', + insideStrands: false, }, { line: 7, - name: 'baz' + name: 'baz', + insideStrands: false, } ], variables: [ { line: 1, - name: 'x' + name: 'x', + insideStrands: false, }, { line: 2, - name: 'y' + name: 'y', + insideStrands: false, }, { line: 3, - name: 'z' + name: 'z', + insideStrands: false, }, { line: 4, - name: 'v1' + name: 'v1', + insideStrands: false, }, { line: 4, - name: 'v2' + name: 'v2', + insideStrands: false, }, { line: 4, - name: 'v3' + name: 'v3', + insideStrands: false, } ] }; @@ -153,19 +162,23 @@ suite('Sketch Verifier', function () { variables: [ { line: 2, - name: 'x' + name: 'x', + insideStrands: false, }, { line: 6, - name: 'y' + name: 'y', + insideStrands: false, }, { line: 11, - name: 'z' + name: 'z', + insideStrands: false, }, { line: 13, - name: 'i' + name: 'i', + insideStrands: false, } ] }; @@ -186,6 +199,70 @@ suite('Sketch Verifier', function () { expect(result).toEqual({ variables: [], functions: [] }); consoleSpy.mockRestore(); }); + + suite('strands region detection', function () { + test('does not mark GLSL names outside strands hooks', function () { + const code = ` + function setup() { + const length = 0; + createCanvas(100, 100, WEBGL); + } + `; + const result = verifierUtils.extractUserDefinedVariablesAndFuncs(code); + const lengthDef = result.variables.find(d => d.name === 'length'); + expect(lengthDef).toBeDefined(); + expect(lengthDef.insideStrands).toBe(false); + }); + + test('marks names in a named build*Shader callback', function () { + const code = ` + function setup() { + buildFilterShader(hook); + } + function hook() { + filterColor.begin(); + const length = 0; + filterColor.end(); + } + `; + const result = verifierUtils.extractUserDefinedVariablesAndFuncs(code); + const lengthDef = result.variables.find(d => d.name === 'length'); + expect(lengthDef).toBeDefined(); + expect(lengthDef.insideStrands).toBe(true); + }); + + test('handles inline buildFilterShader arrow callbacks', function () { + const code = ` + function setup() { + buildFilterShader(() => { + filterColor.begin(); + const length = 0; + filterColor.end(); + }); + } + `; + const result = verifierUtils.extractUserDefinedVariablesAndFuncs(code); + const lengthDef = result.variables.find(d => d.name === 'length'); + expect(lengthDef).toBeDefined(); + expect(lengthDef.insideStrands).toBe(true); + }); + + test('handles baseFilterShader().modify callbacks', function () { + const code = ` + function setup() { + baseFilterShader().modify(() => { + filterColor.begin(); + const length = 0; + filterColor.end(); + }); + } + `; + const result = verifierUtils.extractUserDefinedVariablesAndFuncs(code); + const lengthDef = result.variables.find(d => d.name === 'length'); + expect(lengthDef).toBeDefined(); + expect(lengthDef.insideStrands).toBe(true); + }); + }); }); suite('checkForConstsAndFuncs()', function () { @@ -274,5 +351,28 @@ suite('Sketch Verifier', function () { expect(result).toBe(false); }); + + test('warns on strands builtin only when insideStrands is true', function () { + const outside = { + variables: [{ name: 'length', line: 0, insideStrands: false }], + functions: [] + }; + // If length is only special-cased via the strands path (not a p5 global), + // outside should not warn for that reason: + const outsideResult = verifierUtils.checkForConstsAndFuncs(outside, MockP5); + // may still be false unless length conflicts with something else + + const inside = { + variables: [{ name: 'length', line: 1, insideStrands: true }], + functions: [] + }; + const insideResult = verifierUtils.checkForConstsAndFuncs(inside, MockP5); + // true only if length is in builtInGLSLFunctions + if (insideResult) { + expect(consoleSpy).toHaveBeenCalledWith( + expect.stringContaining('length') + ); + } + }); }); }); From 0de494eaa678cf020d5e0fcce21be30f09472414 Mon Sep 17 00:00:00 2001 From: skyash-dev Date: Tue, 1 Sep 2026 13:49:05 +0530 Subject: [PATCH 3/3] revert formatting --- src/friendly_errors/sketch_verifier.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/friendly_errors/sketch_verifier.js b/src/friendly_errors/sketch_verifier.js index ba7013aa26..e5832712ee 100644 --- a/src/friendly_errors/sketch_verifier.js +++ b/src/friendly_errors/sketch_verifier.js @@ -180,9 +180,9 @@ export const verifierUtils = { if (node.id.type === 'Identifier') { const category = node.init && - ['ArrowFunctionExpression', 'FunctionExpression'].includes( - node.init.type - ) + ['ArrowFunctionExpression', 'FunctionExpression'].includes( + node.init.type + ) ? 'functions' : 'variables'; userDefinitions[category].push({