Feature/detect outside variables strands
Resolves #8172
Changes
This PR adds detection for outside variable references in p5.strands uniform initializers, providing helpful error messages to users when they reference variables that aren't declared in the strand context.
What was implemented:
- Created
detectOutsideVariableReferences()function insrc/strands/strands_transpiler.jsthat:- Uses Acorn AST parsing to analyze strand code
- Collects all variable declarations within the strand context
- Finds uniform initializer functions (e.g.,
uniformFloat('color', () => ...)) - Extracts variables referenced inside uniform initializers
- Checks if referenced variables are declared in the strand scope
- Returns helpful error messages for any undeclared variables
- Integrated detection to run before transpilation in
src/strands/p5.strands.js - Added unit tests in
test/unit/strands/strands_transpiler.jswith test cases for:- Detecting undeclared variables (e.g.,
mouseX,windowWidth) - No errors when variables are properly declared
- Multiple undeclared variables
- Edge cases
- Detecting undeclared variables (e.g.,
Approach: Following @davepagurek's feedback, this implementation:
- Runs as a separate analysis phase before transpilation
- Reuses the transpiler's Acorn-based structure for AST parsing
- Analyzes variable declarations to build a set of in-scope variables
- Detects when uniform initializer functions reference variables not in that set
- Provides specific, actionable error messages to users
This is more robust than checking for hardcoded variable names, as it analyzes the actual code structure and works with any user-defined variables.
Screenshots
N/A (infrastructure change - no visual impact)
PR Checklist
- [x] Code follows p5.js coding conventions and practices
- [x] Code is tested and all tests pass
- [x] Documentation is updated (if applicable)
- [x] Pull request targets the correct branch (dev-2.0)
- [x] Changes are minimal and focused on the issue
- [x] No breaking changes
Hey @davepagurek!
I've re-implemented the outside variable detection following your feedback:
- Uses AST analysis instead of hardcoded variable names
- Runs as a separate analysis phase before transpilation
- Properly handles the uniform function signature
- Includes unit tests Could you take a look when you have time? Let me know if you'd like any adjustments! Thanks!
@davepagurek Fixed! Now checking strand code (not uniform callbacks) and using the simpler two-pass approach. Tests updated. Ready for another look when you have time.
@davepagurek Thanks for catching those issues! Updated the tests to pass just the function body, and the scope checking should now properly exclude sibling scopes using the ancestor chain. Going to run the integration tests as you suggested - expect that will surface any issues I missed. Thanks for the detailed guidance! 💯
Hi @davepagurek , just following up, it’s been over a week since this PR was opened. Could you please approve the pending workflows or review it when you get a chance? Thanks!