-
Notifications
You must be signed in to change notification settings - Fork 4
Transpiler Phase 1: Core Language Features #15
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
Transpiler Phase 1: Core Language Features #15
Conversation
Co-authored-by: deepentropy <8287111+deepentropy@users.noreply.github.com>
Co-authored-by: deepentropy <8287111+deepentropy@users.noreply.github.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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.
Pull request overview
This PR implements Phase 1 of the PineScript v6 transpiler, adding support for essential language constructs including control flow (if/else, for, while, switch), the reassignment operator (:=), history access ([n]), tuple destructuring, and ternary expressions. The implementation parses these PineScript constructs into an AST and generates idiomatic TypeScript code.
Key Changes
- Added parsing and code generation for 9 new language constructs (if/else, for loops, while loops, switch expressions, ternary operator, history access, tuple destructuring, break/continue)
- Implemented indentation-based block parsing for PineScript's Python-like syntax
- Extended AST node interface with
name,operator, andstepproperties
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 16 comments.
| File | Description |
|---|---|
| packages/oakscript-engine/tests/transpiler-phase1.test.ts | Comprehensive test suite with 33 tests covering all new language features |
| packages/oakscript-engine/src/transpiler/PineParser.ts | Parser implementation for new constructs with indentation handling and backtracking support |
| packages/oakscript-engine/src/transpiler/PineToTS.ts | Code generator for TypeScript output including IIFE-wrapped switch expressions |
Comments suppressed due to low confidence (3)
packages/oakscript-engine/src/transpiler/PineParser.ts:1
- [nitpick] Ternary expressions are unnecessarily wrapped in parentheses. While not incorrect, this adds visual noise to the generated code. Consider removing the outer parentheses unless the ternary is part of a larger expression that requires them for precedence.
/**
packages/oakscript-engine/src/transpiler/PineParser.ts:1
- Break and continue statements don't consume the rest of the line or advance past newlines. This could cause the parser to incorrectly interpret subsequent tokens on the same line. Add
this.skipToEndOfLine();after creating these statements to ensure clean line handling.
/**
packages/oakscript-engine/src/transpiler/PineParser.ts:798
- Unused variable startPos.
const startPos = this.position;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
| if (this.peek() !== '=') { | ||
| this.position = savedPosition; | ||
| return null; | ||
| } | ||
| this.advance(); // skip '=' |
Copilot
AI
Nov 30, 2025
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.
Tuple destructuring only accepts = but should also support := for reassignment. PineScript allows [a, b] := func() for reassigning existing variables. Check for both = and := operators. After the check, if it's :=, advance twice (for : and =).
| this.skipWhitespace(); | ||
|
|
||
| // Must have '=' after tuple | ||
| if (this.peek() !== '=') { |
Copilot
AI
Nov 30, 2025
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.
Need to check if next character is = to avoid false positives. The current check could succeed when encountering == (equality operator). Use this.peek() !== '=' || this.peekNext() === '=' to ensure it's a single = and not ==.
| if (this.peek() !== '=') { | |
| if (this.peek() !== '=' || this.peekNext() === '=') { |
| const vars = varName.split(','); | ||
| this.emit(`for (const [${vars.join(', ')}] of ${iterableExpr}.entries()) {`); |
Copilot
AI
Nov 30, 2025
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.
The destructured variables are joined with ', ' (comma-space) which is inconsistent with tuple destructuring at line 506 that uses no spaces. Additionally, the parser stores comma-separated names without spaces, but split(',') on such a string followed by join(', ') adds spaces. This creates inconsistent spacing in generated code. Use varName directly: for (const [${varName}] of ....
| const vars = varName.split(','); | |
| this.emit(`for (const [${vars.join(', ')}] of ${iterableExpr}.entries()) {`); | |
| this.emit(`for (const [${varName}] of ${iterableExpr}.entries()) {`); |
| // else if - inline the else if | ||
| this.emit('} else ' + this.generateIfStatementInline(alternate)); |
Copilot
AI
Nov 30, 2025
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.
[nitpick] The else if generation uses string concatenation which produces inline code that may not respect current indentation. This creates inconsistent formatting compared to other control structures. Consider emitting the } else part separately and then generating the if statement normally to maintain proper indentation.
| // else if - inline the else if | |
| this.emit('} else ' + this.generateIfStatementInline(alternate)); | |
| // else if - emit closing brace, else, then generate the next if statement | |
| this.emit('}'); | |
| this.emit('else '); | |
| this.generateIfStatement(alternate); |
| lines.push('}'); | ||
| } | ||
|
|
||
| return lines.join('\n' + indent.repeat(this.indent)); |
Copilot
AI
Nov 30, 2025
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.
The generateIfStatementInline method joins lines with indentation repeated this.indent times, but the lines themselves already include indent (which is a single indent string). This will produce incorrect indentation where each line gets progressively more indented. The join should use \n followed by the current indent level calculated once.
| return lines.join('\n' + indent.repeat(this.indent)); | |
| return lines.join('\n'); |
|
|
||
| const condExpr = this.generateExpression(condition); | ||
| const lines: string[] = []; | ||
| const indent = this.getIndentString(); |
Copilot
AI
Nov 30, 2025
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.
[nitpick] The variable indent represents a single indentation unit (e.g., ' '), not the current indentation level. This is confusing when later used in indent.repeat(this.indent). Rename to indentUnit or singleIndent for clarity.
| // Save position for potential keyword lookahead | ||
| const startPos = this.position; | ||
| const name = this.parseIdentifier(); | ||
|
|
||
| // Check if this is the 'switch' keyword | ||
| if (name === 'switch') { | ||
| return this.parseSwitchExpression(); | ||
| } |
Copilot
AI
Nov 30, 2025
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.
The startPos variable is saved but never used. Either remove this unused variable or add a comment explaining why it's preserved (perhaps for future error reporting).
| describe('Ternary Operator `?:`', () => { | ||
| it('should parse ternary expression', () => { | ||
| const parser = new PineParser(); | ||
| const { ast, errors } = parser.parse('x = condition ? 1 : 2'); |
Copilot
AI
Nov 30, 2025
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.
Unused variable ast.
| const { ast, errors } = parser.parse('x = condition ? 1 : 2'); | |
| const { errors } = parser.parse('x = condition ? 1 : 2'); |
| describe('Break and Continue Statements', () => { | ||
| it('should parse break statement', () => { | ||
| const parser = new PineParser(); | ||
| const { ast, errors } = parser.parse(`for i = 0 to 10 |
Copilot
AI
Nov 30, 2025
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.
Unused variable ast.
| const { ast, errors } = parser.parse(`for i = 0 to 10 | |
| const { errors } = parser.parse(`for i = 0 to 10 |
|
|
||
| it('should parse continue statement', () => { | ||
| const parser = new PineParser(); | ||
| const { ast, errors } = parser.parse(`for i = 0 to 10 |
Copilot
AI
Nov 30, 2025
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.
Unused variable ast.
| const { ast, errors } = parser.parse(`for i = 0 to 10 | |
| const { errors } = parser.parse(`for i = 0 to 10 |
Implements essential PineScript v6 language constructs required by standard indicators. The existing transpiler only supported basic expressions, variable declarations, and simple function calls.
New Parser Constructs
:=reassignment operator - Separate from=assignmentif/else if/elseblocks - Indentation-based block parsingforloops - Standardfor i = 0 to N by stepandfor item in arraywith destructuringwhileloops - Condition-based loopsswitchexpressions - PineScript v6 value-returning switch with=>cases[n]- Series historical access (e.g.,close[1])[a, b, c] = func())break/continuestatements?:- Properly separated from binary operator parsingCode Generation
Transpiles to idiomatic TypeScript:
Testing
33 new test cases covering parsing and transpilation for all constructs.
Original prompt
Transpiler Phase 1: Core Language Features
Objective
Enhance the oakscript-engine transpiler (
packages/oakscript-engine/src/transpiler/) to support essential PineScript v6 language constructs that are required by most standard indicators.Current State
The transpiler currently supports:
var)ta.sma())indicator()declarationplot()functionFeatures to Implement
1.
:=Reassignment Operator (Critical)PineScript uses
:=for reassigning existing variables.TypeScript output:
2.
if/elseBlocks (Critical)Support both single-line and multi-line if statements.
Parser requirements:
ifkeywordelse ifchainselseclause3.
forLoops (Critical)TypeScript output:
4.
whileLoops (Critical)TypeScript output:
5.
switchExpressions (Critical)PineScript v6 switch returns a value.
TypeScript output:
6. Ternary Operator
?:(Critical)Already partially supported, but ensure it works with Series operations.
7. History Operator
[n](Critical)Access historical values of series.
TypeScript output:
8. Multi-Value Return / Tuple Destructuring (High Priority)
TypeScript output:
9.
breakandcontinueStatementsImplementation Details
PineParser.ts Changes
Add these methods to the parser: