fix(transpiler): user-defined functions named after a constant namespace

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 ### 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).
 - **`display.a - display.b` produced `NaN`, and `display.a + display.b` a raw concatenation**: Pine's `display.*` constants are a set type — `+` unions two displays and `-` removes one's surfaces from the other (`display.all - display.price_scale` is the reference manual's own example). The runtime kept them as member-name strings, so native `-` yielded `NaN` (hosts then fell back to their default, typically showing the plot everywhere) and `+` glued names in source order (`display.all + display.none` → `'allnone'`). A new transpiler post-process (`transformDisplayArithmetic`) routes `+` / `-` with a `display.*` operand — literal members, chained expressions, or a variable combined with a member — to `display.__union` / `display.__minus`, which compute the set and report it as the canonical concatenation of member names in the order pane, data_window, status_line, price_scale (`'all'` / `'none'` for the full / empty set — the shape hosts already parse). So `display.all - display.none` → `'all'`, `display.none - display.all` → `'none'`, `display.all - display.price_scale` → `'panedata_windowstatus_line'`, `display.pane + display.pane` → `'pane'`. Plain member values and the `display.*` enum are unchanged. Test: `tests/namespaces/plot/display-arithmetic.test.ts`.
 
 ---

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<string, string[]>;
+    // 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<string>;
     constructor(options: { indentStr?: string; sourceCode?: string; includeSourceComments?: boolean } = {}) {
         this.indent = 0;
         this.indentStr = options.indentStr || '  ';
@@ -33,13 +39,15 @@ export class CodeGenerator {
         this.includeSourceComments = options.includeSourceComments || false; // default false
         this.paramRenameCounter = 0;
         this.functionParams = new Map();
+        this.userFunctionCollisions = new Set();
     }
 
     generate(ast) {
         this.output = [];
         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

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';
             }

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);
+    });
+});