Skip to content

Commit 268e8db

Browse files
chrisdpclaude
chrisdp
and
claude
authored
Diagnose reserved BrightScript builtins used as values (#1697)
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 1feac17 commit 268e8db

5 files changed

Lines changed: 356 additions & 2 deletions

File tree

‎src/DiagnosticMessages.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -770,6 +770,11 @@ export let DiagnosticMessages = {
770770
message: `'${featureName}' requires Roku firmware version ${minimumVersion} or higher (current target is ${configuredVersion})`,
771771
code: 1146,
772772
severity: DiagnosticSeverity.Error
773+
}),
774+
reservedBuiltinUsedAsValue: (name: string) => ({
775+
message: `'${name}' is a reserved builtin and can only be used as a function call (e.g. '${name}(...)')`,
776+
code: 1147,
777+
severity: DiagnosticSeverity.Error
773778
})
774779
};
775780

‎src/bscPlugin/validation/BrsFileValidator.spec.ts‎

Lines changed: 254 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -608,4 +608,258 @@ describe('BrsFileValidator', () => {
608608
});
609609
});
610610
});
611+
612+
describe('unreferencable builtins', () => {
613+
const reservedBuiltinCode = DiagnosticMessages.reservedBuiltinUsedAsValue('').code;
614+
615+
function reservedBuiltinDiagnostics() {
616+
return program.getDiagnostics().filter(diagnostic => diagnostic.code === reservedBuiltinCode);
617+
}
618+
619+
function expectFlagged(names: string[]) {
620+
expect(
621+
reservedBuiltinDiagnostics().map(diagnostic => diagnostic.message)
622+
).to.eql(
623+
names.map(name => DiagnosticMessages.reservedBuiltinUsedAsValue(name).message)
624+
);
625+
}
626+
627+
function expectNotFlagged() {
628+
expect(reservedBuiltinDiagnostics()).to.eql([]);
629+
}
630+
631+
it('flags `x = ObjFun` (RHS value read)', () => {
632+
program.setFile('source/main.brs', `
633+
sub a()
634+
x = ObjFun
635+
print x
636+
end sub
637+
`);
638+
program.validate();
639+
expectFlagged(['ObjFun']);
640+
});
641+
642+
it('flags `print type(ObjFun)` (passed as argument)', () => {
643+
program.setFile('source/main.brs', `
644+
sub a()
645+
print type(ObjFun)
646+
end sub
647+
`);
648+
program.validate();
649+
expectFlagged(['ObjFun']);
650+
});
651+
652+
it('flags `f(ObjFun, 2)` (passed by value)', () => {
653+
program.setFile('source/main.brs', `
654+
sub a()
655+
f(ObjFun, 2)
656+
end sub
657+
sub f(arg1, arg2)
658+
end sub
659+
`);
660+
program.validate();
661+
expectFlagged(['ObjFun']);
662+
});
663+
664+
it('flags `x = type` (RHS value read)', () => {
665+
program.setFile('source/main.brs', `
666+
sub a()
667+
x = type
668+
print x
669+
end sub
670+
`);
671+
program.validate();
672+
expectFlagged(['type']);
673+
});
674+
675+
it('does not flag `ObjFun(m)` (canonical call)', () => {
676+
program.setFile('source/main.brs', `
677+
sub a()
678+
ObjFun(m, "")
679+
end sub
680+
`);
681+
program.validate();
682+
expectNotFlagged();
683+
});
684+
685+
it('does not flag `type(123)` (canonical call)', () => {
686+
program.setFile('source/main.brs', `
687+
sub a()
688+
print type(123)
689+
end sub
690+
`);
691+
program.validate();
692+
expectNotFlagged();
693+
});
694+
695+
it('does not flag `m.ObjFun = 1` (property assignment)', () => {
696+
program.setFile('source/main.brs', `
697+
sub a()
698+
m.ObjFun = 1
699+
end sub
700+
`);
701+
program.validate();
702+
expectNotFlagged();
703+
});
704+
705+
it('does not flag `m.type = 1` (property assignment)', () => {
706+
program.setFile('source/main.brs', `
707+
sub a()
708+
m.type = 1
709+
end sub
710+
`);
711+
program.validate();
712+
expectNotFlagged();
713+
});
714+
715+
it('does not flag `{ ObjFun: 1 }` (AA literal key)', () => {
716+
program.setFile('source/main.brs', `
717+
sub a()
718+
aa = { ObjFun: 1 }
719+
end sub
720+
`);
721+
program.validate();
722+
expectNotFlagged();
723+
});
724+
725+
it('does not flag `{ type: 1 }` (AA literal key)', () => {
726+
program.setFile('source/main.brs', `
727+
sub a()
728+
aa = { type: 1 }
729+
end sub
730+
`);
731+
program.validate();
732+
expectNotFlagged();
733+
});
734+
735+
it('does not flag a BrighterScript `type Name = ...` statement', () => {
736+
program.setFile('source/main.bs', `
737+
type MyAlias = string or integer
738+
`);
739+
program.validate();
740+
expectNotFlagged();
741+
});
742+
743+
it('case-insensitive match for OBJFUN, ObjFun, objfun', () => {
744+
program.setFile('source/main.brs', `
745+
sub a()
746+
x = OBJFUN
747+
y = objfun
748+
end sub
749+
`);
750+
program.validate();
751+
expectFlagged(['OBJFUN', 'objfun']);
752+
});
753+
754+
//per-builtin coverage for each device-verified entry in UnreferencableBuiltins.
755+
//each pair: (1) bare value read flags, (2) canonical call form does not flag.
756+
757+
it('flags `x = Box` (RHS value read)', () => {
758+
program.setFile('source/main.brs', `sub a()\nx = Box\nend sub`);
759+
program.validate();
760+
expectFlagged(['Box']);
761+
});
762+
763+
it('does not flag `Box(1)` (canonical call)', () => {
764+
program.setFile('source/main.brs', `sub a()\nx = Box(1)\nend sub`);
765+
program.validate();
766+
expectNotFlagged();
767+
});
768+
769+
it('flags `x = CreateObject` (RHS value read)', () => {
770+
program.setFile('source/main.brs', `sub a()\nx = CreateObject\nend sub`);
771+
program.validate();
772+
expectFlagged(['CreateObject']);
773+
});
774+
775+
it('does not flag `CreateObject("roSGNode", "Node")` (canonical call)', () => {
776+
program.setFile('source/main.brs', `sub a()\nx = CreateObject("roSGNode", "Node")\nend sub`);
777+
program.validate();
778+
expectNotFlagged();
779+
});
780+
781+
it('flags `x = GetGlobalAA` (RHS value read)', () => {
782+
program.setFile('source/main.brs', `sub a()\nx = GetGlobalAA\nend sub`);
783+
program.validate();
784+
expectFlagged(['GetGlobalAA']);
785+
});
786+
787+
it('does not flag `GetGlobalAA()` (canonical call)', () => {
788+
program.setFile('source/main.brs', `sub a()\nx = GetGlobalAA()\nend sub`);
789+
program.validate();
790+
expectNotFlagged();
791+
});
792+
793+
it('flags `x = GetLastRunCompileError` (RHS value read)', () => {
794+
program.setFile('source/main.brs', `sub a()\nx = GetLastRunCompileError\nend sub`);
795+
program.validate();
796+
expectFlagged(['GetLastRunCompileError']);
797+
});
798+
799+
it('does not flag `GetLastRunCompileError()` (canonical call)', () => {
800+
program.setFile('source/main.brs', `sub a()\nx = GetLastRunCompileError()\nend sub`);
801+
program.validate();
802+
expectNotFlagged();
803+
});
804+
805+
it('flags `x = GetLastRunRunTimeError` (RHS value read)', () => {
806+
program.setFile('source/main.brs', `sub a()\nx = GetLastRunRunTimeError\nend sub`);
807+
program.validate();
808+
expectFlagged(['GetLastRunRunTimeError']);
809+
});
810+
811+
it('does not flag `GetLastRunRunTimeError()` (canonical call)', () => {
812+
program.setFile('source/main.brs', `sub a()\nx = GetLastRunRunTimeError()\nend sub`);
813+
program.validate();
814+
expectNotFlagged();
815+
});
816+
817+
it('flags `x = Pos` (RHS value read)', () => {
818+
program.setFile('source/main.brs', `sub a()\nx = Pos\nend sub`);
819+
program.validate();
820+
expectFlagged(['Pos']);
821+
});
822+
823+
it('does not flag `Pos(0)` (canonical call)', () => {
824+
program.setFile('source/main.brs', `sub a()\nx = Pos(0)\nend sub`);
825+
program.validate();
826+
expectNotFlagged();
827+
});
828+
829+
it('flags `x = Run` (RHS value read)', () => {
830+
program.setFile('source/main.brs', `sub a()\nx = Run\nend sub`);
831+
program.validate();
832+
expectFlagged(['Run']);
833+
});
834+
835+
it('does not flag `Run("pkg:/source/foo.brs")` (canonical call)', () => {
836+
program.setFile('source/main.brs', `sub a()\nx = Run("pkg:/source/foo.brs")\nend sub`);
837+
program.validate();
838+
expectNotFlagged();
839+
});
840+
841+
it('flags `x = Tab` (RHS value read)', () => {
842+
program.setFile('source/main.brs', `sub a()\nx = Tab\nend sub`);
843+
program.validate();
844+
expectFlagged(['Tab']);
845+
});
846+
847+
it('does not flag `Tab(5)` (canonical call)', () => {
848+
program.setFile('source/main.brs', `sub a()\nx = Tab(5)\nend sub`);
849+
program.validate();
850+
expectNotFlagged();
851+
});
852+
853+
it('flags `x = eval` (RHS value read)', () => {
854+
program.setFile('source/main.brs', `sub a()\nx = eval\nend sub`);
855+
program.validate();
856+
expectFlagged(['eval']);
857+
});
858+
859+
it('does not flag `eval("print 1")` (canonical call)', () => {
860+
program.setFile('source/main.brs', `sub a()\neval("print 1")\nend sub`);
861+
program.validate();
862+
expectNotFlagged();
863+
});
864+
});
611865
});

‎src/bscPlugin/validation/BrsFileValidator.ts‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
1-
import { isAliasStatement, isBody, isClassStatement, isCommentStatement, isConstStatement, isDottedGetExpression, isDottedSetStatement, isEnumStatement, isForEachStatement, isForStatement, isFunctionExpression, isFunctionStatement, isImportStatement, isIndexedGetExpression, isIndexedSetStatement, isInterfaceStatement, isLibraryStatement, isLiteralExpression, isNamespaceStatement, isTypecastStatement, isTypeStatement, isUnaryExpression, isWhileStatement } from '../../astUtils/reflection';
1+
import { isAliasStatement, isBody, isCallExpression, isClassStatement, isCommentStatement, isConstStatement, isDottedGetExpression, isDottedSetStatement, isEnumStatement, isForEachStatement, isForStatement, isFunctionExpression, isFunctionStatement, isImportStatement, isIndexedGetExpression, isIndexedSetStatement, isInterfaceStatement, isLibraryStatement, isLiteralExpression, isNamespaceStatement, isTypecastStatement, isTypeStatement, isUnaryExpression, isWhileStatement } from '../../astUtils/reflection';
22
import { createVisitor, WalkMode } from '../../astUtils/visitors';
33
import { DiagnosticMessages } from '../../DiagnosticMessages';
44
import type { BrsFile } from '../../files/BrsFile';
55
import type { OnFileValidateEvent } from '../../interfaces';
6-
import { TokenKind } from '../../lexer/TokenKind';
6+
import { TokenKind, UnreferencableBuiltins } from '../../lexer/TokenKind';
77
import type { AstNode, Expression, Statement } from '../../parser/AstNode';
88
import { CallExpression, type FunctionExpression, type LiteralExpression } from '../../parser/Expression';
99
import { ParseMode } from '../../parser/Parser';
@@ -197,6 +197,23 @@ export class BrsFileValidator {
197197
},
198198
ContinueStatement: (node) => {
199199
this.validateContinueStatement(node);
200+
},
201+
VariableExpression: (node) => {
202+
//flag reserved unreferencable builtins (e.g. `ObjFun`, `type`) used in non-call position.
203+
//these compile cleanly as values today but are device compile errors
204+
//(`Syntax Error. Builtin function call expected`).
205+
const name = node.name?.text;
206+
if (
207+
name &&
208+
UnreferencableBuiltins.has(name.toLowerCase()) &&
209+
//only valid use is as the callee of a CallExpression
210+
!(isCallExpression(node.parent) && node.parent.callee === node)
211+
) {
212+
this.event.file.addDiagnostic({
213+
...DiagnosticMessages.reservedBuiltinUsedAsValue(name),
214+
range: node.name.range
215+
});
216+
}
200217
}
201218
});
202219

‎src/lexer/TokenKind.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -577,6 +577,27 @@ export const DisallowedLocalIdentifiersText = new Set([
577577
...DisallowedLocalIdentifiers.map(x => x.toLowerCase())
578578
]);
579579

580+
/**
581+
* Reserved BrightScript builtins that cannot be referenced — they compile only when invoked as a function call.
582+
* Any non-call reference (e.g. `x = type`, `print foo(ObjFun)`) is a device compile error
583+
* (most produce `Syntax Error. Builtin function call expected`, hex code &h9d;
584+
* `createobject` and `tab` produce a generic `Syntax Error` &h02 with the same outcome).
585+
* Names are lowercased for case-insensitive matching.
586+
*/
587+
export const UnreferencableBuiltins = new Set([
588+
'box',
589+
'createobject',
590+
'eval',
591+
'getglobalaa',
592+
'getlastruncompileerror',
593+
'getlastrunruntimeerror',
594+
'objfun',
595+
'pos',
596+
'run',
597+
'tab',
598+
'type'
599+
]);
600+
580601
/**
581602
* List of string versions of TokenKind and various globals that are NOT allowed as scope function names.
582603
* Used to throw more helpful "you can't use a reserved word as a function name" errors.

‎src/parser/Parser.spec.ts‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -928,6 +928,63 @@ describe('parser', () => {
928928
expect(diagnostics, `assigning to reserved word "${reservedWord}" should have been an error`).to.be.length.greaterThan(0);
929929
}
930930
});
931+
932+
describe('unreferencable builtins as parameter names', () => {
933+
it('flags `function f(Box)` (parameter name)', () => {
934+
let { diagnostics } = parse(`function f(Box)\nend function`);
935+
expectDiagnostics(diagnostics, [DiagnosticMessages.cannotUseReservedWordAsIdentifier('Box')]);
936+
});
937+
938+
it('flags `function f(CreateObject)` (parameter name)', () => {
939+
let { diagnostics } = parse(`function f(CreateObject)\nend function`);
940+
expectDiagnostics(diagnostics, [DiagnosticMessages.cannotUseReservedWordAsIdentifier('CreateObject')]);
941+
});
942+
943+
it('flags `function f(GetGlobalAA)` (parameter name)', () => {
944+
let { diagnostics } = parse(`function f(GetGlobalAA)\nend function`);
945+
expectDiagnostics(diagnostics, [DiagnosticMessages.cannotUseReservedWordAsIdentifier('GetGlobalAA')]);
946+
});
947+
948+
it('flags `function f(GetLastRunCompileError)` (parameter name)', () => {
949+
let { diagnostics } = parse(`function f(GetLastRunCompileError)\nend function`);
950+
expectDiagnostics(diagnostics, [DiagnosticMessages.cannotUseReservedWordAsIdentifier('GetLastRunCompileError')]);
951+
});
952+
953+
it('flags `function f(GetLastRunRunTimeError)` (parameter name)', () => {
954+
let { diagnostics } = parse(`function f(GetLastRunRunTimeError)\nend function`);
955+
expectDiagnostics(diagnostics, [DiagnosticMessages.cannotUseReservedWordAsIdentifier('GetLastRunRunTimeError')]);
956+
});
957+
958+
it('flags `function f(ObjFun)` (parameter name)', () => {
959+
let { diagnostics } = parse(`function f(ObjFun)\nend function`);
960+
expectDiagnostics(diagnostics, [DiagnosticMessages.cannotUseReservedWordAsIdentifier('ObjFun')]);
961+
});
962+
963+
it('flags `function f(Pos)` (parameter name)', () => {
964+
let { diagnostics } = parse(`function f(Pos)\nend function`);
965+
expectDiagnostics(diagnostics, [DiagnosticMessages.cannotUseReservedWordAsIdentifier('Pos')]);
966+
});
967+
968+
it('flags `function f(Run)` (parameter name)', () => {
969+
let { diagnostics } = parse(`function f(Run)\nend function`);
970+
expectDiagnostics(diagnostics, [DiagnosticMessages.cannotUseReservedWordAsIdentifier('Run')]);
971+
});
972+
973+
it('flags `function f(Tab)` (parameter name)', () => {
974+
let { diagnostics } = parse(`function f(Tab)\nend function`);
975+
expectDiagnostics(diagnostics, [DiagnosticMessages.cannotUseReservedWordAsIdentifier('Tab')]);
976+
});
977+
978+
it('flags `function f(type)` (parameter name)', () => {
979+
let { diagnostics } = parse(`function f(type)\nend function`);
980+
expectDiagnostics(diagnostics, [DiagnosticMessages.cannotUseReservedWordAsIdentifier('type')]);
981+
});
982+
983+
it('flags `function f(eval)` (parameter name)', () => {
984+
let { diagnostics } = parse(`function f(eval)\nend function`);
985+
expectDiagnostics(diagnostics, [DiagnosticMessages.cannotUseReservedWordAsIdentifier('eval')]);
986+
});
987+
});
931988
});
932989

933990
describe('import keyword', () => {

0 commit comments

Comments
 (0)