feat(editor): add syntax error detection for JS and Python code blocks#3407
feat(editor): add syntax error detection for JS and Python code blocks#3407waleedlatif1 wants to merge 2 commits intostagingfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
@greptile |
|
@cursor review |
|
Skipping Bugbot: Bugbot is disabled for this repository. Visit the Bugbot dashboard to update your settings. |
Greptile SummaryThis PR adds real-time syntax error detection for JavaScript (using acorn) and Python (bracket/string matching) to the code editor panel, generalising the previously JSON-only validation warning icon to all code languages. Block references and env-var placeholders are sanitised before parsing to prevent false positives. Key changes:
Issues found:
Confidence Score: 4/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User types code] --> B{effectiveLanguage}
B -->|json| C[JSON.parse]
C -->|valid| E[isValidJson = true]
C -->|invalid| F[isValidJson = false]
B -->|javascript or python| G[sanitizeForParsing\nReplace ENV_VAR and Block references\nwith placeholder identifiers]
G -->|javascript| J[validateJavaScript via acorn\nTry module parse then async IIFE wrap]
G -->|python| P[validatePython\nBracket stack and string scanner]
J -->|no error| L[syntaxError = null]
J -->|error| N[syntaxError = adjusted line/col message]
P -->|no error| L
P -->|error| N
E & L --> R{isValid check}
F & N --> R
R -->|valid| S[onValidationChange true, null - immediate]
R -->|invalid| T[onValidationChange false, errorMsg - 150ms debounce]
T --> U[SubBlockComponent\nupdates isValidCode and codeErrorMessage]
S --> U
U --> V{code block and not valid}
V -->|yes| W[AlertTriangle icon with tooltip]
V -->|no| X[No icon shown]
Last reviewed commit: 136d5b5 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const handleValidationChange = (isValid: boolean, errorMessage?: string | null): void => { | ||
| setIsValidCode(isValid) | ||
| setCodeErrorMessage(errorMessage ?? null) | ||
| } |
There was a problem hiding this comment.
handleValidationChange not memoized — triggers unnecessary re-renders
handleValidationChange is a plain function created on every render. Because it is passed as the onValidationChange prop to Code (which is a memo component with default shallow comparison), every state update inside SubBlockComponent — including those caused by handleValidationChange itself — creates a new function reference, which then causes Code to re-render and re-fire the useEffect that has onValidationChange in its dependency array. This sets a fresh 150 ms timer for invalid code, and the cycle repeats once more before React's bail-out for equal state values finally stops it.
Wrapping the callback in useCallback with an empty dependency array (the setState functions from useState are stable) fixes this:
| const handleValidationChange = (isValid: boolean, errorMessage?: string | null): void => { | |
| setIsValidCode(isValid) | |
| setCodeErrorMessage(errorMessage ?? null) | |
| } | |
| const handleValidationChange = useCallback((isValid: boolean, errorMessage?: string | null): void => { | |
| setIsValidCode(isValid) | |
| setCodeErrorMessage(errorMessage ?? null) | |
| }, []) |
| const match = msg.match(/\((\d+):(\d+)\)/) | ||
| if (match) { | ||
| const adjustedLine = Number(match[1]) - 1 | ||
| if (adjustedLine < 1) return null | ||
| return `Syntax error at line ${adjustedLine}, col ${match[2]}: ${msg.replace(/\s*\(\d+:\d+\)/, '')}` |
There was a problem hiding this comment.
Inaccurate line numbers for unclosed-delimiter errors
When the parser fails because a delimiter is never closed (e.g. function foo() { with no closing }), acorn reports the error at the point where it is surprised — typically the wrapper's })() closing line, which may be two or more lines after the user's actual last line. For example, the 1-line snippet function foo() { produces a wrapped 4-line program; acorn reports the error somewhere around line 4, so adjustedLine becomes 3, pointing the user to a non-existent line 3. The column is also taken from the wrapper's close, not from the unmatched opener.
This is a known limitation of the wrapping approach, but it may confuse users who see "line 3" for a 1-line snippet. A best-effort mitigation is to clamp the reported line to the actual number of user code lines before surfacing it:
const userLineCount = code.split('\n').length
const clampedLine = Math.min(adjustedLine, userLineCount)
return `Syntax error at line ${clampedLine}, col ${match[2]}: ${msg.replace(/\s*\(\d+:\d+\)/, '')}`| while (i < code.length && code[i] !== ch && code[i] !== '\n') { | ||
| if (code[i] === '\\') i++ | ||
| i++ | ||
| } |
There was a problem hiding this comment.
\ line-continuation in strings skips line counter increment
When the single-line string scanner encounters \ followed by \n (a backslash-continuation), it advances i twice and skips the newline without incrementing line. Any bracket-mismatch reported on a subsequent code line will have its line number off by one for each backslash-continuation encountered earlier. This is a minor inaccuracy in error reporting.
To fix, increment line when the skipped character is \n:
while (i < code.length && code[i] !== ch && code[i] !== '\n') {
if (code[i] === '\\') {
i++
if (i < code.length && code[i] === '\n') line++
}
i++
}1 participant
Summary
<Block.output>references and{{ENV_VAR}}placeholders before parsing to avoid false positivesType of Change
Testing
Checklist