Skip to content

Commit 07f4b1b

Browse files
committed
update sketch verifier to use acorn/acorn walk and include line numbers in results
1 parent 607e61c commit 07f4b1b

File tree

4 files changed

+155
-56
lines changed

4 files changed

+155
-56
lines changed

package-lock.json

Lines changed: 14 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,9 @@
2222
},
2323
"version": "1.9.4",
2424
"dependencies": {
25+
"acorn": "^8.12.1",
26+
"acorn-walk": "^8.3.4",
2527
"colorjs.io": "^0.5.2",
26-
"espree": "^10.2.0",
2728
"file-saver": "^1.3.8",
2829
"gifenc": "^1.0.3",
2930
"libtess": "^1.2.2",
@@ -83,4 +84,4 @@
8384
"pre-commit": "lint-staged"
8485
}
8586
}
86-
}
87+
}

src/core/friendly_errors/sketch_verifier.js

Lines changed: 49 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import * as espree from 'espree';
1+
import * as acorn from 'acorn';
2+
import * as walk from 'acorn-walk';
23

34
/**
45
* @for p5
@@ -14,8 +15,14 @@ function sketchVerifier(p5, fn) {
1415
*/
1516
fn.fetchScript = async function (script) {
1617
if (script.src) {
17-
const contents = await fetch(script.src).then((res) => res.text());
18-
return contents;
18+
try {
19+
const contents = await fetch(script.src).then((res) => res.text());
20+
return contents;
21+
} catch (error) {
22+
// TODO: Handle CORS error here.
23+
console.error('Error fetching script:', error);
24+
return '';
25+
}
1926
} else {
2027
return script.textContent;
2128
}
@@ -30,6 +37,8 @@ function sketchVerifier(p5, fn) {
3037
* @returns {Promise<string>} The user's code as a string.
3138
*/
3239
fn.getUserCode = async function () {
40+
// TODO: think of a more robust way to get the user's code. Refer to
41+
// https://github.com/processing/p5.js/pull/7293.
3342
const scripts = document.querySelectorAll('script');
3443
const userCodeScript = scripts[scripts.length - 1];
3544
const userCode = await fn.fetchScript(userCodeScript);
@@ -42,59 +51,58 @@ function sketchVerifier(p5, fn) {
4251
* the help of Espree parser.
4352
*
4453
* @method extractUserDefinedVariablesAndFuncs
45-
* @param {string} codeStr - The code to extract variables and functions from.
54+
* @param {string} code - The code to extract variables and functions from.
4655
* @returns {Object} An object containing the user's defined variables and functions.
47-
* @returns {string[]} [userDefinitions.variables] Array of user-defined variable names.
48-
* @returns {strings[]} [userDefinitions.functions] Array of user-defined function names.
56+
* @returns {Array<{name: string, line: number}>} [userDefinitions.variables] Array of user-defined variable names and their line numbers.
57+
* @returns {Array<{name: string, line: number}>} [userDefinitions.functions] Array of user-defined function names and their line numbers.
4958
*/
50-
fn.extractUserDefinedVariablesAndFuncs = function (codeStr) {
59+
fn.extractUserDefinedVariablesAndFuncs = function (code) {
5160
const userDefinitions = {
5261
variables: [],
5362
functions: []
5463
};
64+
// The line numbers from the parser are consistently off by one, add
65+
// `lineOffset` here to correct them.
66+
const lineOffset = -1;
5567

5668
try {
57-
const ast = espree.parse(codeStr, {
69+
const ast = acorn.parse(code, {
5870
ecmaVersion: 2021,
5971
sourceType: 'module',
60-
ecmaFeatures: {
61-
jsx: true
62-
}
72+
locations: true // This helps us get the line number.
6373
});
6474

65-
function traverse(node) {
66-
const { type, declarations, id, init } = node;
67-
68-
switch (type) {
69-
case 'VariableDeclaration':
70-
declarations.forEach(({ id, init }) => {
71-
if (id.type === 'Identifier') {
72-
const category = init && ['ArrowFunctionExpression', 'FunctionExpression'].includes(init.type)
73-
? 'functions'
74-
: 'variables';
75-
userDefinitions[category].push(id.name);
76-
}
75+
walk.simple(ast, {
76+
VariableDeclarator(node) {
77+
if (node.id.type === 'Identifier') {
78+
const category = node.init && ['ArrowFunctionExpression', 'FunctionExpression'].includes(node.init.type)
79+
? 'functions'
80+
: 'variables';
81+
userDefinitions[category].push({
82+
name: node.id.name,
83+
line: node.loc.start.line + lineOffset
84+
});
85+
}
86+
},
87+
FunctionDeclaration(node) {
88+
if (node.id && node.id.type === 'Identifier') {
89+
userDefinitions.functions.push({
90+
name: node.id.name,
91+
line: node.loc.start.line + lineOffset
92+
});
93+
}
94+
},
95+
// We consider class declarations to be a special form of variable
96+
// declaration.
97+
ClassDeclaration(node) {
98+
if (node.id && node.id.type === 'Identifier') {
99+
userDefinitions.variables.push({
100+
name: node.id.name,
101+
line: node.loc.start.line + lineOffset
77102
});
78-
break;
79-
case 'FunctionDeclaration':
80-
if (id?.type === 'Identifier') {
81-
userDefinitions.functions.push(id.name);
82-
}
83-
break;
84-
}
85-
86-
for (const key in node) {
87-
if (node[key] && typeof node[key] === 'object') {
88-
if (Array.isArray(node[key])) {
89-
node[key].forEach(child => traverse(child));
90-
} else {
91-
traverse(node[key]);
92-
}
93103
}
94104
}
95-
}
96-
97-
traverse(ast);
105+
});
98106
} catch (error) {
99107
// TODO: Replace this with a friendly error message.
100108
console.error('Error parsing code:', error);

test/unit/core/sketch_overrides.js

Lines changed: 89 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -74,9 +74,49 @@ suite('Validate Params', function () {
7474
`;
7575

7676
const result = mockP5Prototype.extractUserDefinedVariablesAndFuncs(code);
77-
78-
expect(result.variables).toEqual(['x', 'y', 'z', 'v1', 'v2', 'v3']);
79-
expect(result.functions).toEqual(['foo', 'bar', 'baz']);
77+
const expectedResult = {
78+
"functions": [
79+
{
80+
"line": 5,
81+
"name": "foo",
82+
},
83+
{
84+
"line": 6,
85+
"name": "bar",
86+
},
87+
{
88+
"line": 7,
89+
"name": "baz",
90+
},
91+
],
92+
"variables": [
93+
{
94+
"line": 1,
95+
"name": "x",
96+
},
97+
{
98+
"line": 2,
99+
"name": "y",
100+
},
101+
{
102+
"line": 3,
103+
"name": "z",
104+
},
105+
{
106+
"line": 4,
107+
"name": "v1",
108+
},
109+
{
110+
"line": 4,
111+
"name": "v2",
112+
},
113+
{
114+
"line": 4,
115+
"name": "v3",
116+
},
117+
],
118+
};
119+
expect(result).toEqual(expectedResult);
80120
});
81121

82122
// Sketch verifier should ignore the following types of lines:
@@ -101,9 +141,29 @@ suite('Validate Params', function () {
101141
`;
102142

103143
const result = mockP5Prototype.extractUserDefinedVariablesAndFuncs(code);
104-
105-
expect(result.variables).toEqual(['x', 'y', 'z', 'i']);
106-
expect(result.functions).toEqual([]);
144+
const expectedResult = {
145+
"functions": [],
146+
"variables": [
147+
{
148+
"line": 2,
149+
"name": "x",
150+
},
151+
{
152+
"line": 6,
153+
"name": "y",
154+
},
155+
{
156+
"line": 11,
157+
"name": "z",
158+
},
159+
{
160+
"line": 13,
161+
"name": "i",
162+
},
163+
],
164+
};
165+
166+
expect(result).toEqual(expectedResult);
107167
});
108168

109169
test('Handles parsing errors', function () {
@@ -130,13 +190,31 @@ suite('Validate Params', function () {
130190
mockP5Prototype.getUserCode = vi.fn(() => Promise.resolve(mockScript));
131191

132192
const result = await mockP5Prototype.run();
193+
const expectedResult = {
194+
"functions": [
195+
{
196+
"line": 3,
197+
"name": "foo",
198+
},
199+
{
200+
"line": 4,
201+
"name": "bar",
202+
},
203+
],
204+
"variables": [
205+
{
206+
"line": 1,
207+
"name": "x",
208+
},
209+
{
210+
"line": 2,
211+
"name": "y",
212+
},
213+
],
214+
};
133215

134216
expect(mockP5Prototype.getUserCode).toHaveBeenCalledTimes(1);
135-
136-
expect(result).toEqual({
137-
variables: ['x', 'y'],
138-
functions: ['foo', 'bar']
139-
});
217+
expect(result).toEqual(expectedResult);
140218
});
141219
});
142220
});

0 commit comments

Comments
 (0)