Skip to content
16 changes: 15 additions & 1 deletion src/strands/p5.strands.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/
import { glslBackend } from './strands_glslBackend';

import { transpileStrandsToJS } from './strands_transpiler';
import { transpileStrandsToJS, detectOutsideVariableReferences } from './strands_transpiler';
import { BlockType } from './ir_types';

import { createDirectedAcyclicGraph } from './ir_dag'
Expand Down Expand Up @@ -70,6 +70,20 @@ function strands(p5, fn) {
// TODO: expose this, is internal for debugging for now.
const options = { parser: true, srcLocations: false };

// 0. Detect outside variable references in uniforms (before transpilation)
if (options.parser) {
const sourceString = `(${shaderModifier.toString()})`;
const errors = detectOutsideVariableReferences(sourceString);
if (errors.length > 0) {
// Show errors to the user
for (const error of errors) {
p5._friendlyError(
`p5.strands: ${error.message}`
);
}
}
}

// 1. Transpile from strands DSL to JS
let strandsCallback;
if (options.parser) {
Expand Down
115 changes: 114 additions & 1 deletion src/strands/strands_transpiler.js
Original file line number Diff line number Diff line change
Expand Up @@ -863,7 +863,120 @@ const ASTCallbacks = {
return replaceInNode(node);
}
}
export function transpileStrandsToJS(p5, sourceString, srcLocations, scope) {
/**
* Analyzes strand code to detect outside variable references in uniform initializers
* This runs before transpilation to provide helpful errors to users
*
* @param {string} sourceString - The strand code to analyze
* @returns {Array<{variable: string, uniform: string, message: string}>} - Array of errors if any
*/
export function detectOutsideVariableReferences(sourceString) {
try {
const ast = parse(sourceString, { ecmaVersion: 2021 });

// Step 1: Collect all variable declarations in the strand code
const declaredVars = new Set();
const scopeChain = [new Set()]; // Track nested scopes

function collectDeclarations(node, ancestors) {
// Add current block's local vars to scope chain
if (node.type === 'BlockStatement') {
scopeChain.push(new Set());
}

// Collect variable declarations
if (node.type === 'VariableDeclaration') {
for (const decl of node.declarations) {
if (decl.id.type === 'Identifier') {
declaredVars.add(decl.id.name);
if (scopeChain.length > 0) {
scopeChain[scopeChain.length - 1].add(decl.id.name);
}
}
}
}

// Close block scope when exiting
if (node.type === 'BlockStatement' && scopeChain.length > 1) {
scopeChain.pop();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now it looks like scopeChain doesn't get used really -- it gets added to and then popped off the array before anything reads from it. Maybe consider doing the check all in one step, so that when you encounter a variable identifier, you can check to see if it should be visible in the current scope?

}
}

// Walk the AST to collect declared variables
const collectCallbacks = {
VariableDeclaration: collectDeclarations,
BlockStatement: collectDeclarations
};

ancestor(ast, collectCallbacks);

// Step 2: Find uniform initializers and extract their variable references
const errors = [];
const uniformCallbacks = {
VariableDeclarator(node, _state, ancestors) {
if (nodeIsUniform(node.init)) {
// Found a uniform initializer
const uniformName = node.id.name;

// Extract variables referenced in the uniform initializer function
const referencedVars = new Set();

// Walk the uniform function body to find all variable references
// Uniform functions have signature: uniformXXX(name, defaultValue?)
// The defaultValue (second arg) is optional - it's what we need to check
if (node.init.arguments && node.init.arguments.length >= 2) {
const funcArg = node.init.arguments[1];
if (funcArg && (funcArg.type === 'FunctionExpression' || funcArg.type === 'ArrowFunctionExpression')) {
const uniformBody = funcArg.body;

// Walk the body to collect all identifier references
const walkReferences = (n, ancestors) => {
if (n.type === 'Identifier') {
// Skip function parameters and built-in properties
const ignoreNames = ['__p5', 'p5', 'window', 'global', 'undefined', 'null'];
if (!ignoreNames.includes(n.name) &&
(n.name[0] !== '_' || n.name.startsWith('__p5'))) {
// Check if this identifier is used as a property
const isProperty = ancestors && ancestors.some(anc =>
anc.type === 'MemberExpression' && anc.property === n
);

if (!isProperty) {
referencedVars.add(n.name);
}
}
}
};

ancestor(uniformBody, { Identifier: walkReferences });
}
}

// Step 3: Check if any referenced variables aren't declared
for (const varName of referencedVars) {
if (!declaredVars.has(varName)) {
errors.push({
variable: varName,
uniform: uniformName,
message: `Variable "${varName}" referenced in uniform "${uniformName}" is not declared in the strand context.`
});
}
}
}
}
};

// Walk again to find uniforms and analyze their references
ancestor(ast, uniformCallbacks);

return errors;
} catch (error) {
// If parsing fails, return empty array - transpilation will catch it
return [];
}
}

export function transpileStrandsToJS(p5, sourceString, srcLocations, scope) {
const ast = parse(sourceString, {
ecmaVersion: 2021,
locations: srcLocations
Expand Down
46 changes: 46 additions & 0 deletions test/unit/strands/strands_transpiler.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import { detectOutsideVariableReferences } from '../../../src/strands/strands_transpiler.js';
import { suite, test } from '../../../test/js/spec.js';

suite('Strands Transpiler - Outside Variable Detection', function() {
test('should detect undeclared variable in uniform', function() {
// Simulate code that references mouseX (not declared in strand context)
const code = `
const myUniform = uniform('color', () => {
return mouseX; // mouseX is not declared
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you might be checking for a different scenario than we need. In the callback of a uniform creation function, that's the right spot to have p5 globals. So this is ok:

baseMaterialShader.modify(() => {
  const myUniform = uniformFloat(() => mouseX)
  getWorldPosition((inputs) => {
    inputs.position.x += myUniform
    return inputs
  })
})

...but this should show a warning:

baseMaterialShader.modify(() => {
  getWorldPosition((inputs) => {
    inputs.position.x += mouseX
    return inputs
  })
})

Additionally, just uniform() on its own isn't a p5 function, it will always have a type after it, like uniformFloat or uniformVec2.

});
`;

const errors = detectOutsideVariableReferences(code);
assert.ok(errors.length > 0, 'Should detect at least one error');
assert.ok(errors.some(e => e.variable === 'mouseX'), 'Should detect mouseX');
});

test('should not error when variable is declared', function() {
// Variable is declared before use
const code = `
let myVar = 5;
const myUniform = uniform('color', () => {
return myVar; // myVar is declared
});
`;

const errors = detectOutsideVariableReferences(code);
assert.equal(errors.length, 0, 'Should not detect errors');
});

test('should detect multiple undeclared variables', function() {
const code = `
const myUniform = uniform('color', () => {
return mouseX + windowWidth; // Both not declared
});
`;

const errors = detectOutsideVariableReferences(code);
assert.equal(errors.length, 2, 'Should detect both mouseX and windowWidth');
});

test('should handle empty code', function() {
const errors = detectOutsideVariableReferences('');
assert.equal(errors.length, 0, 'Empty code should have no errors');
});
});