From e190ac68a85d639269f349eddf899f6bf9d40839 Mon Sep 17 00:00:00 2001 From: alexgrover <41912104+alexgrover@users.noreply.github.com> Date: Mon, 14 Sep 2026 18:40:45 +0200 Subject: [PATCH] fix(transpiler): user-defined functions named after a constant namespace Pine allows a script to declare a function whose name matches a constant namespace (`position(x) => close + x`) and to keep using that namespace's members in the same script. Two transpiler passes broke this: - The pineToJS codegen collision pass renamed the declaration to `position_$N` but left bare call sites alone (it assumed a bare callee is always the built-in, which is only true for VARIABLE collisions such as `fill = 3` alongside `fill(p1, p2)`), so `plot(position(14))` resolved to the constants object -> `TypeError: position is not a function`. The pass now records which collision names were declared as functions and renames their bare callees too. - The parser's `name -> name_var` rewrite (for a variable sharing a user function's name) fired on the namespace base of `position.top_right` once a `position()` function existed -> `ReferenceError: position_var is not defined`. It now skips a collision-name identifier followed by `.`. Affects every entry of NAMESPACE_COLLISION_NAMES and makes adding names to that list (e.g. `scale`, #305) safe for scripts that use them as functions. Tests: tests/transpiler/namespace-identifier-collision.test.ts (7 new cases: bare call, member-access coexistence, argument position, nested/indirect calls, variable-collision guard, `scale` forward guard). Co-authored-by: Cursor --- CHANGELOG.md | 8 ++ src/transpiler/pineToJS/codegen.ts | 41 ++++++-- src/transpiler/pineToJS/parser.ts | 8 +- .../namespace-identifier-collision.test.ts | 96 +++++++++++++++++++ 4 files changed, 142 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 343e9a89..73a1b4b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ # Change Log +## [Unreleased] + +### Fixed + +- **User-defined functions named after a constant namespace crashed at the call site**: Pine allows a script to declare a function whose name matches a constant namespace (`position(x) => close + x`, `font(x) => …`) and to keep using the namespace's members in the same script. The pineToJS codegen collision pass renamed the *declaration* to `position_$N` but deliberately left bare call sites untouched (it assumed a bare callee is always the built-in, which holds for variable collisions like `fill = 3` + `fill(p1, p2)`), so `plot(position(14))` resolved to the constants object → `TypeError: position is not a function`. The pass now records which collision names were declared as functions and renames their bare callees too. Separately, the parser's `name → name_var` rewrite (a variable sharing a user function's name) fired on the namespace base of `position.top_right` once a `position()` function existed → `ReferenceError: position_var is not defined`; it now skips a collision-name identifier followed by `.`. Affects every entry of `NAMESPACE_COLLISION_NAMES`, and makes adding new names to that list (e.g. `scale`, PR #305) safe for scripts that use them as function names. Tests: `tests/transpiler/namespace-identifier-collision.test.ts` (function-call, member-access coexistence, nested / indirect calls, variable-collision guard). + +--- + ## [v0.9.33] ### Fixed diff --git a/src/transpiler/pineToJS/codegen.ts b/src/transpiler/pineToJS/codegen.ts index a7b4ca97..f2c75ed7 100644 --- a/src/transpiler/pineToJS/codegen.ts +++ b/src/transpiler/pineToJS/codegen.ts @@ -23,6 +23,12 @@ export class CodeGenerator { // Maps user-defined function names to their ordered parameter names. // Used to resolve named arguments to correct positional slots. private functionParams: Map; + // Collision names (NAMESPACE_COLLISION_NAMES) that the user declared as a + // FUNCTION. A bare call `name(...)` to one of these can only be the user + // function (a constants namespace is not callable), so its callees must + // follow the `_$N` rename — unlike variable collisions, where `fill(...)` + // still means the built-in. + private userFunctionCollisions: Set; constructor(options: { indentStr?: string; sourceCode?: string; includeSourceComments?: boolean } = {}) { this.indent = 0; this.indentStr = options.indentStr || ' '; @@ -33,6 +39,7 @@ export class CodeGenerator { this.includeSourceComments = options.includeSourceComments || false; // default false this.paramRenameCounter = 0; this.functionParams = new Map(); + this.userFunctionCollisions = new Set(); } generate(ast) { @@ -40,6 +47,7 @@ export class CodeGenerator { this.indent = 0; this.lastCommentedLine = -1; this.functionParams = new Map(); + this.userFunctionCollisions = new Set(); if (ast.type === 'Program') { // Pre-scan: collect user-defined function parameter lists and @@ -70,7 +78,11 @@ export class CodeGenerator { * 1. Pine namespace collisions (NAMESPACE_COLLISION_NAMES — e.g. `fill`, * `size`, `color`, `line`): user variable would shadow the namespace * destructured from `$.pine`. The CALL SITE `fill(...)` is the - * namespace, NOT the renamed variable, so callees are NOT renamed. + * namespace, NOT the renamed variable, so callees are NOT renamed — + * UNLESS the user declared a FUNCTION with that name + * (`position(x) => close + x`, valid Pine): then a bare call + * `position(14)` can only be the user function and its callees ARE + * renamed (tracked in `userFunctionCollisions`). * * 2. JS reserved keyword collisions (JS_RESERVED_WORDS — e.g. `delete`, * `super`, `static`): the generated JS would fail to parse @@ -148,11 +160,16 @@ export class CodeGenerator { // name visible at the call site (`obj.delete()` looks up `delete`, // not `delete_$0`), breaking UFCS retargeting in ExpressionTransformer. if (node.type === 'FunctionDeclaration') { - if (node.id?.type === 'Identifier' && - !node.id.isMethod && - this.isReservedName(node.id.name) && - !renameMap.has(node.id.name)) { - renameMap.set(node.id.name, `${node.id.name}_$${this.paramRenameCounter++}`); + if (node.id?.type === 'Identifier' && !node.id.isMethod && this.isReservedName(node.id.name)) { + if (!renameMap.has(node.id.name)) { + renameMap.set(node.id.name, `${node.id.name}_$${this.paramRenameCounter++}`); + } + // Remember that this collision name is a user FUNCTION so that + // bare call sites `name(...)` follow the rename (see + // renameVariableRefsInAST). Overloads share one entry. + if (NAMESPACE_COLLISION_NAMES.has(node.id.name)) { + this.userFunctionCollisions.add(node.id.name); + } } } @@ -188,10 +205,14 @@ export class CodeGenerator { // Two cases: // - JS_RESERVED_WORDS rename (e.g. user `method delete` → `delete_$N`): // the callee IS the user function — must be renamed. - // - NAMESPACE_COLLISION_NAMES rename (e.g. user `var fill = ...` while - // also calling the built-in `fill(...)`): the callee here refers to - // the namespace, not the renamed user variable — leave it alone. - if (JS_RESERVED_WORDS.has(node.callee.name)) { + // - NAMESPACE_COLLISION_NAMES rename of a user VARIABLE (e.g. + // `var fill = ...` while also calling the built-in `fill(...)`): + // the callee here refers to the namespace, not the renamed + // variable — leave it alone. + // - NAMESPACE_COLLISION_NAMES rename of a user FUNCTION + // (`position(x) => ...` then `position(14)`): the callee IS the + // user function — must be renamed. + if (JS_RESERVED_WORDS.has(node.callee.name) || this.userFunctionCollisions.has(node.callee.name)) { node.callee.name = renameMap.get(node.callee.name)!; } // else: skip callee diff --git a/src/transpiler/pineToJS/parser.ts b/src/transpiler/pineToJS/parser.ts index 1ce17a9f..3d90f7eb 100644 --- a/src/transpiler/pineToJS/parser.ts +++ b/src/transpiler/pineToJS/parser.ts @@ -36,6 +36,7 @@ import { SwitchCase, VariableDeclarationKind, } from './ast'; +import { NAMESPACE_COLLISION_NAMES } from '../settings'; export class Parser { private tokens: Token[]; @@ -1748,7 +1749,12 @@ export class Parser { if ( this.functionNames.has(name) && this.peek().type !== TokenType.LPAREN && - !this.isCurrentFunctionParam(name) + !this.isCurrentFunctionParam(name) && + // `position.top_right` after a user function `position(x) => ...` + // is the constants namespace, not a variable sharing the + // function's name — leave the base identifier untouched so the + // codegen collision pass can treat it as a namespace access. + !(this.peek().type === TokenType.DOT && NAMESPACE_COLLISION_NAMES.has(name)) ) { name = name + '_var'; } diff --git a/tests/transpiler/namespace-identifier-collision.test.ts b/tests/transpiler/namespace-identifier-collision.test.ts index 93275704..e70fff44 100644 --- a/tests/transpiler/namespace-identifier-collision.test.ts +++ b/tests/transpiler/namespace-identifier-collision.test.ts @@ -127,3 +127,99 @@ plot(dayofweek, "p") expect(v).toBeLessThanOrEqual(7); }); }); + +// Pine also allows a user-defined FUNCTION to share its name with a constant +// namespace (`position(x) => close + x` is a valid, working script). +// The codegen rename pass renamed the declaration to `position_$N` but left +// the bare call site `position(14)` alone (it assumed a bare callee is always +// the namespace), so the call resolved to the constants object → +// `TypeError: position is not a function`. Separately, the parser's +// `name → name_var` rewrite (a variable sharing a UDF's name) fired on the +// namespace base of `position.top_right` → `ReferenceError: position_var`. +describe('user-defined FUNCTION named after a constant namespace (valid Pine)', () => { + it('position(x) => ...: bare call resolves to the user function', async () => { + const { plots } = await newPineTS().run(` +//@version=6 +indicator("udf position") +position(x)=>close+x +plot(position(14), "p") +plot(close, "c") +`); + expect(lastValue(plots, 'p')).toBeCloseTo(lastValue(plots, 'c') + 14, 8); + }); + + it('position(x) => ... coexists with position.top_right member access', async () => { + const { plots } = await newPineTS().run(` +//@version=6 +indicator("udf position + namespace", overlay = true) +position(x)=>close+x +var table t = table.new(position.top_right, 1, 1) +plot(position(2), "p") +plot(close, "c") +`); + expect(lastValue(plots, 'p')).toBeCloseTo(lastValue(plots, 'c') + 2, 8); + }); + + it('font(x) => ... coexists with font.family_monospace passed as an argument', async () => { + const { plots } = await newPineTS().run(` +//@version=6 +indicator("udf font", overlay = true) +font(x)=>x*2 +if barstate.islast + label.new(bar_index, close, "x", text_font_family = font.family_monospace) +plot(font(21), "p") +`); + expect(lastValue(plots, 'p')).toBe(42); + }); + + it('UDF call nested inside another expression and used as a named argument', async () => { + const { plots } = await newPineTS().run(` +//@version=6 +indicator("udf nested") +order(x)=>x+1 +plot(math.max(order(1), order(2)) + order(0), "p") +`); + // max(2, 3) + 1 + expect(lastValue(plots, 'p')).toBe(4); + }); + + it('UDF calling itself by name from inside another UDF', async () => { + const { plots } = await newPineTS().run(` +//@version=6 +indicator("udf indirect") +currency(x)=>x*10 +wrap(y)=>currency(y)+1 +plot(wrap(3), "p") +`); + expect(lastValue(plots, 'p')).toBe(31); + }); + + it('a variable named after a namespace is still renamed without touching namespace calls', async () => { + // Guard: the callee-rename must only apply when the user declared a + // FUNCTION. Here `fill` is a user variable; the built-in `fill(...)` + // must still reach the namespace. + const { plots } = await newPineTS().run(` +//@version=6 +indicator("var fill + builtin fill") +fill = 3 +p1 = plot(close, "a") +p2 = plot(close + fill, "b") +fill(p1, p2, color.new(color.blue, 90)) +plot(fill, "p") +`); + expect(lastValue(plots, 'p')).toBe(3); + }); + + // `scale` is not yet a collision name (see PR #305). This must keep working + // both before and after it becomes one. + it('scale(x) => ... works as a user function', async () => { + const { plots } = await newPineTS().run(` +//@version=6 +indicator("udf scale") +scale(x)=>close+x +plot(scale(14), "p") +plot(close, "c") +`); + expect(lastValue(plots, 'p')).toBeCloseTo(lastValue(plots, 'c') + 14, 8); + }); +});