-
-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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.
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
Add new sketch verifier to FES #7293
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
0a7cec8
add new sketch verifier to FES; it currently can read all user-define…
sproutleaf 135187f
add test file for sketch verifier
sproutleaf 9067152
remove all the silly definitions in index.html
sproutleaf 607e61c
Merge remote-tracking branch 'upstream/dev-2.0' into dev-2.0
sproutleaf 07f4b1b
update sketch verifier to use acorn/acorn walk and include line numbe…
sproutleaf File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,41 +1,44 @@ | ||
<!DOCTYPE html> | ||
<html> | ||
|
||
<head> | ||
<title>P5 test</title> | ||
<meta http-equiv="pragma" content="no-cache" /> | ||
<meta http-equiv="cache-control" content="no-cache" /> | ||
<meta charset="utf-8"> | ||
|
||
<style> | ||
body{ | ||
margin:0; | ||
overflow: hidden; | ||
} | ||
body { | ||
margin: 0; | ||
overflow: hidden; | ||
} | ||
</style> | ||
</head> | ||
|
||
<body> | ||
<script type="module"> | ||
import p5 from '../src/app.js'; | ||
// import calculation from './src/math/calculation.js'; | ||
<script type="module"> | ||
import p5 from '../src/app.js'; | ||
// import calculation from './src/math/calculation.js'; | ||
|
||
// p5.registerAddon(calculation); | ||
// p5.registerAddon(calculation); | ||
|
||
const sketch = function(p){ | ||
p.setup = function(){ | ||
p.createCanvas(200, 200); | ||
}; | ||
const sketch = function (p) { | ||
p.setup = function () { | ||
p.createCanvas(200, 200); | ||
}; | ||
|
||
p.draw = function(){ | ||
p.background(0, 50, 50); | ||
p.circle(100, 100, 50); | ||
p.draw = function () { | ||
p.background(0, 50, 50); | ||
p.circle(100, 100, 50); | ||
|
||
p.fill('white'); | ||
p.textSize(30); | ||
p.text('hello', 10, 30); | ||
}; | ||
}; | ||
p.fill('white'); | ||
p.textSize(30); | ||
p.text('hello', 10, 30); | ||
}; | ||
}; | ||
|
||
new p5(sketch); | ||
</script> | ||
new p5(sketch); | ||
</script> | ||
</body> | ||
|
||
</html> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,7 @@ | ||
import validateParams from './param_validator.js'; | ||
import sketchVerifier from './sketch_verifier.js'; | ||
|
||
export default function (p5) { | ||
p5.registerAddon(validateParams); | ||
p5.registerAddon(sketchVerifier); | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,126 @@ | ||
import * as acorn from 'acorn'; | ||
import * as walk from 'acorn-walk'; | ||
|
||
/** | ||
* @for p5 | ||
* @requires core | ||
*/ | ||
function sketchVerifier(p5, fn) { | ||
/** | ||
* Fetches the contents of a script element in the user's sketch. | ||
* | ||
* @method fetchScript | ||
* @param {HTMLScriptElement} script | ||
* @returns {Promise<string>} | ||
*/ | ||
fn.fetchScript = async function (script) { | ||
if (script.src) { | ||
try { | ||
const contents = await fetch(script.src).then((res) => res.text()); | ||
return contents; | ||
} catch (error) { | ||
// TODO: Handle CORS error here. | ||
console.error('Error fetching script:', error); | ||
return ''; | ||
} | ||
} else { | ||
return script.textContent; | ||
} | ||
} | ||
|
||
/** | ||
* Extracts the user's code from the script fetched. Note that this method | ||
* assumes that the user's code is always the last script element in the | ||
* sketch. | ||
* | ||
* @method getUserCode | ||
* @returns {Promise<string>} The user's code as a string. | ||
*/ | ||
fn.getUserCode = async function () { | ||
// TODO: think of a more robust way to get the user's code. Refer to | ||
// https://github.com/processing/p5.js/pull/7293. | ||
const scripts = document.querySelectorAll('script'); | ||
const userCodeScript = scripts[scripts.length - 1]; | ||
const userCode = await fn.fetchScript(userCodeScript); | ||
|
||
return userCode; | ||
} | ||
|
||
/** | ||
* Extracts the user-defined variables and functions from the user code with | ||
* the help of Espree parser. | ||
* | ||
* @method extractUserDefinedVariablesAndFuncs | ||
* @param {string} code - The code to extract variables and functions from. | ||
* @returns {Object} An object containing the user's defined variables and functions. | ||
* @returns {Array<{name: string, line: number}>} [userDefinitions.variables] Array of user-defined variable names and their line numbers. | ||
* @returns {Array<{name: string, line: number}>} [userDefinitions.functions] Array of user-defined function names and their line numbers. | ||
*/ | ||
fn.extractUserDefinedVariablesAndFuncs = function (code) { | ||
const userDefinitions = { | ||
variables: [], | ||
functions: [] | ||
}; | ||
// The line numbers from the parser are consistently off by one, add | ||
// `lineOffset` here to correct them. | ||
const lineOffset = -1; | ||
|
||
try { | ||
const ast = acorn.parse(code, { | ||
ecmaVersion: 2021, | ||
sourceType: 'module', | ||
locations: true // This helps us get the line number. | ||
}); | ||
|
||
walk.simple(ast, { | ||
VariableDeclarator(node) { | ||
if (node.id.type === 'Identifier') { | ||
const category = node.init && ['ArrowFunctionExpression', 'FunctionExpression'].includes(node.init.type) | ||
? 'functions' | ||
: 'variables'; | ||
userDefinitions[category].push({ | ||
name: node.id.name, | ||
line: node.loc.start.line + lineOffset | ||
}); | ||
} | ||
}, | ||
FunctionDeclaration(node) { | ||
if (node.id && node.id.type === 'Identifier') { | ||
userDefinitions.functions.push({ | ||
name: node.id.name, | ||
line: node.loc.start.line + lineOffset | ||
}); | ||
} | ||
}, | ||
// We consider class declarations to be a special form of variable | ||
// declaration. | ||
ClassDeclaration(node) { | ||
if (node.id && node.id.type === 'Identifier') { | ||
userDefinitions.variables.push({ | ||
name: node.id.name, | ||
line: node.loc.start.line + lineOffset | ||
}); | ||
} | ||
} | ||
}); | ||
} catch (error) { | ||
// TODO: Replace this with a friendly error message. | ||
console.error('Error parsing code:', error); | ||
} | ||
|
||
return userDefinitions; | ||
} | ||
|
||
fn.run = async function () { | ||
const userCode = await fn.getUserCode(); | ||
const userDefinedVariablesAndFuncs = fn.extractUserDefinedVariablesAndFuncs(userCode); | ||
|
||
return userDefinedVariablesAndFuncs; | ||
} | ||
} | ||
|
||
export default sketchVerifier; | ||
|
||
if (typeof p5 !== 'undefined') { | ||
sketchVerifier(p5, p5.prototype); | ||
} |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if there's a more robust way to get the user's script, since e.g. some sketches are spread across multiple files, or might import other addon libraries. We definitely don't need to be perfect here, but since multi-file sketches are pretty common on OpenProcessing, maybe we could do something like, filter out all the scripts that look like library code, e.g. because their
src
attribute matches/p5(\.\w+)\.js/
, and then run the parser on the remaining scripts?We could also exclude srcs that come from a known CDN domain like
cdnjs.cloudflare.com
orcdn.jsdelivr.net
orunpkg.com
. This is what OpenProcessing uses for its additional library toggles, so we could exclude more library code that way, although with some false negatives still.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One way I was thinking about is to only grab from
<script>
tag that are in<body>
which assumes that library import<script>
tags are usually included in the<head>
element while user code are included in the<body>
tag, but that is mostly convention and not a guarantee so I'm not sure if it would be a good solution.The other thing is that, if I remember correctly, if any
<script>
tag is usingtype=module
they shouldn't be grabbed as well because these are scoped automatically.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a TODO!