Skip to content

Fix/fes warnings glsl keywords 9038 #9122

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
skyash-dev wants to merge 3 commits into
base: main
Choose a base branch
from
+223 −17
Open

Fix/fes warnings glsl keywords 9038 #9122

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d57886f
check strands identifiers and warn
skyash-dev Sep 1, 2026
cc20c30
add tests
skyash-dev Sep 1, 2026
0de494e
revert formatting
skyash-dev Sep 1, 2026
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
114 changes: 110 additions & 4 deletions src/friendly_errors/sketch_verifier.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -78,13 +79,102 @@ 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',
sourceType: 'module',
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') {
Expand All @@ -97,15 +187,17 @@ export const verifierUtils = {
: 'variables';
userDefinitions[category].push({
name: node.id.name,
line: node.loc.start.line + lineOffset
line: node.loc.start.line + lineOffset,
insideStrands: isInsideStrands(node)
});
}
},
FunctionDeclaration(node) {
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)
});
}
},
Expand All @@ -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)
});
}
}
Expand Down Expand Up @@ -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,
Expand All @@ -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;
},

Expand Down
126 changes: 113 additions & 13 deletions test/unit/core/sketch_overrides.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
]
};
Expand Down Expand Up @@ -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,
}
]
};
Expand All @@ -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 () {
Expand Down Expand Up @@ -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')
);
}
});
});
});
Loading