Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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');
});
});