Repository: rokucommunity/brighterscript
Pull request #955 (2023-11-22 14:26:34)

Add `optional` modifier for interface and class members
Adds the ability to mark interface fields, interface methods, and class fields as `optional`. We don't really do anything with them right now, but this is in preparation for the upcoming type tracking features. 

rokucommunity-bot[bot]: Hey there! I just built a new temporary npm package based on f1ecf269452ff723f683621228f97fa195c72385. You can download it [here](https://github.com/rokucommunity/brighterscript/releases/download/v0.0.0-packages/brighterscript-0.65.10-optional-interface-and-class.20231122143724.tgz) or install it by running the following command: 
```bash
npm install https://github.com/rokucommunity/brighterscript/releases/download/v0.0.0-packages/brighterscript-0.65.10-optional-interface-and-class.20231122143724.tgz
```
rokucommunity-bot[bot]: Hey there! I just built a new temporary npm package based on c70f42f3db31605e246c04666977554feda03788. You can download it [here](https://github.com/rokucommunity/brighterscript/releases/download/v0.0.0-packages/brighterscript-0.65.10-optional-interface-and-class.20231122144826.tgz) or install it by running the following command: 
```bash
npm install https://github.com/rokucommunity/brighterscript/releases/download/v0.0.0-packages/brighterscript-0.65.10-optional-interface-and-class.20231122144826.tgz
```
rokucommunity-bot[bot]: Hey there! I just built a new temporary npm package based on eeb7723bd420c080dec3ad5eb64a0bd2756d2abe. You can download it [here](https://github.com/rokucommunity/brighterscript/releases/download/v0.0.0-packages/brighterscript-0.65.11-optional-interface-and-class.20231207150522.tgz) or install it by running the following command: 
```bash
npm install https://github.com/rokucommunity/brighterscript/releases/download/v0.0.0-packages/brighterscript-0.65.11-optional-interface-and-class.20231207150522.tgz
```

Patch:
diff --git a/docs/classes.md b/docs/classes.md
index 4e2ff8ef..3961c2cb 100644
--- a/docs/classes.md
+++ b/docs/classes.md
@@ -608,3 +608,41 @@ sub main()
 end sub
 ```
 </details>
+
+
+## Optional fields
+
+Classes can include optional fields, denoted with the keyword `optional` before the name. If a class has an optional field, it signifies that the value of the field may be `invalid`. Optional fields in a class do not change how the class or its members are validated.
+
+```brighterscript
+class Video
+    url as string
+    length as float
+    optional subtitleUrl as string
+    optional rating as string
+    optional genre as string
+end class
+```
+
+<details>
+  <summary>View the transpiled BrightScript code</summary>
+
+```BrightScript
+function __Video_builder()
+    instance = {}
+    instance.new = sub()
+        m.url = invalid
+        m.length = invalid
+        m.subtitleUrl = invalid
+        m.rating = invalid
+        m.genre = invalid
+    end sub
+    return instance
+end function
+function Video()
+    instance = __Video_builder()
+    instance.new()
+    return instance
+end function
+```
+</details>
diff --git a/docs/interfaces.md b/docs/interfaces.md
index d8905969..c480ba09 100644
--- a/docs/interfaces.md
+++ b/docs/interfaces.md
@@ -107,5 +107,29 @@ end interface
   <summary>View the transpiled BrightScript code</summary>
 
 ```BrightScript
+
+```
+</details>
+
+## Optional members
+
+Interfaces can include optional members. Optional members are denoted with the keyword `optional` before the name of fields, or before the `function` or `sub` keyword for methods. Interfaces with optional members will not incur a validation diagnostic if an object that is missing that member is passed to it, yet optional members will be included in completion results, and if referenced, they are inferred to be their declared type.
+
+```brighterscript
+interface Video
+    url as string
+    length as float
+    optional subtitleUrl as string
+    optional rating as string
+    optional genre as string
+end interface
 ```
+
+<details>
+  <summary>View the transpiled BrightScript code</summary>
+
+```BrightScript
+
+```
+
 </details>
diff --git a/src/files/BrsFile.Class.spec.ts b/src/files/BrsFile.Class.spec.ts
index e139cb53..83659bb2 100644
--- a/src/files/BrsFile.Class.spec.ts
+++ b/src/files/BrsFile.Class.spec.ts
@@ -73,6 +73,29 @@ describe('BrsFile BrighterScript classes', () => {
         ]);
     });
 
+    it('allows class named `optional`', () => {
+        program.setFile('source/main.bs', `
+            class optional
+                thing = 1
+            end class
+        `);
+        program.validate();
+        expectZeroDiagnostics(program);
+    });
+
+    it('supports optional fields', () => {
+        program.setFile('source/main.bs', `
+            class Movie
+                name as string
+                optional subtitles as string
+                public optional isRepeatEnabled as boolean
+                private optional wasPlayed
+            end class
+        `);
+        program.validate();
+        expectZeroDiagnostics(program);
+    });
+
     it('access modifier is option for override', () => {
         let file = program.setFile<BrsFile>({ src: `${rootDir}/source/main.bs`, dest: 'source/main.bs' }, `
             class Animal
diff --git a/src/files/BrsFile.spec.ts b/src/files/BrsFile.spec.ts
index 9533114a..9a71d26a 100644
--- a/src/files/BrsFile.spec.ts
+++ b/src/files/BrsFile.spec.ts
@@ -100,6 +100,23 @@ describe('BrsFile', () => {
         }]);
     });
 
+    it('allows namespaces with the name `optional`', () => {
+        program.setFile('source/main.bs', `
+            namespace optional
+                namespace optional
+                end namespace
+            end namespace
+            namespace alpha
+                namespace optional
+                end namespace
+            end namespace
+            namespace alpha.beta.optional
+            end namespace
+        `);
+        program.validate();
+        expectZeroDiagnostics(program);
+    });
+
     it('supports the third parameter in CreateObject', () => {
         program.setFile('source/main.brs', `
             sub main()
diff --git a/src/lexer/TokenKind.ts b/src/lexer/TokenKind.ts
index c6ea5151..a526af66 100644
--- a/src/lexer/TokenKind.ts
+++ b/src/lexer/TokenKind.ts
@@ -155,6 +155,7 @@ export enum TokenKind {
     Public = 'Public',
     Protected = 'Protected',
     Private = 'Private',
+    Optional = 'Optional',
     As = 'As',
     New = 'New',
     Override = 'Override',
@@ -301,6 +302,7 @@ export const Keywords: Record<string, TokenKind> = {
     private: TokenKind.Private,
     new: TokenKind.New,
     override: TokenKind.Override,
+    optional: TokenKind.Optional,
     namespace: TokenKind.Namespace,
     endnamespace: TokenKind.EndNamespace,
     'end namespace': TokenKind.EndNamespace,
@@ -427,6 +429,7 @@ export const AllowedProperties = [
     TokenKind.Class,
     TokenKind.New,
     TokenKind.Override,
+    TokenKind.Optional,
     TokenKind.Namespace,
     TokenKind.EndNamespace,
     TokenKind.Import,
@@ -471,6 +474,7 @@ export const AllowedLocalIdentifiers = [
     TokenKind.Class,
     TokenKind.New,
     TokenKind.Override,
+    TokenKind.Optional,
     TokenKind.Namespace,
     TokenKind.EndNamespace,
     TokenKind.Import,
diff --git a/src/parser/Parser.Class.spec.ts b/src/parser/Parser.Class.spec.ts
index 94e51928..f6919b89 100644
--- a/src/parser/Parser.Class.spec.ts
+++ b/src/parser/Parser.Class.spec.ts
@@ -6,6 +6,8 @@ import { Parser, ParseMode } from './Parser';
 import type { FunctionStatement, AssignmentStatement, FieldStatement } from './Statement';
 import { ClassStatement } from './Statement';
 import { NewExpression } from './Expression';
+import { expectDiagnosticsIncludes, expectZeroDiagnostics } from '../testHelpers.spec';
+import { isClassStatement } from '../astUtils/reflection';
 
 describe('parser class', () => {
     it('throws exception when used in brightscript scope', () => {
@@ -410,4 +412,88 @@ describe('parser class', () => {
             expect(diagnostics[0]?.message).to.not.exist;
         });
     });
+
+    describe('optional members', () => {
+        it('allows optional fields', () => {
+            let { statements, diagnostics } = Parser.parse(`
+                class HasOptional
+                    optional name as string
+                    optional height
+                end class
+            `, { mode: ParseMode.BrighterScript });
+            expectZeroDiagnostics(diagnostics);
+            expect(statements.length).to.eq(1);
+            expect(isClassStatement(statements[0])).to.be.true;
+            const klass = statements[0] as ClassStatement;
+            klass.fields.forEach(f => expect(f.isOptional).to.be.true);
+        });
+
+        it('allows fields named optional', () => {
+            let { statements, diagnostics } = Parser.parse(`
+                class IsJustOptional
+                    optional
+                    someThingElse
+                end class
+            `, { mode: ParseMode.BrighterScript });
+            expectZeroDiagnostics(diagnostics);
+            expect(statements.length).to.eq(1);
+            expect(isClassStatement(statements[0])).to.be.true;
+            const klass = statements[0] as ClassStatement;
+            klass.fields.forEach(f => expect(f.isOptional).to.be.false);
+        });
+
+        it('allows typed fields named optional', () => {
+            let { statements, diagnostics } = Parser.parse(`
+                class IsJustOptional
+                    optional as string
+                end class
+            `, { mode: ParseMode.BrighterScript });
+            expectZeroDiagnostics(diagnostics);
+            expect(statements.length).to.eq(1);
+            expect(isClassStatement(statements[0])).to.be.true;
+            const klass = statements[0] as ClassStatement;
+            klass.fields.forEach(f => expect(f.isOptional).to.be.false);
+        });
+
+        it('allows fields named optional that are also optional', () => {
+            let { statements, diagnostics } = Parser.parse(`
+                class IsJustOptional
+                    optional optional
+                end class
+            `, { mode: ParseMode.BrighterScript });
+            expectZeroDiagnostics(diagnostics);
+            expect(statements.length).to.eq(1);
+            expect(isClassStatement(statements[0])).to.be.true;
+            const klass = statements[0] as ClassStatement;
+            klass.fields.forEach(f => expect(f.isOptional).to.be.true);
+        });
+
+        it('disallows optional methods', () => {
+            let { statements, diagnostics } = Parser.parse(`
+                class HasOptional
+                    optional function getValue() as boolean
+                        return false
+                    end function
+                end class
+            `, { mode: ParseMode.BrighterScript });
+
+            expectDiagnosticsIncludes(diagnostics, [
+                DiagnosticMessages.expectedNewlineOrColon().message
+            ]);
+            expect(statements.length).to.eq(1);
+        });
+
+        it('allows fields named `as` that are also optional', () => {
+            let { statements, diagnostics } = Parser.parse(`
+                class IsJustOptional
+                    optional as
+                end class
+            `, { mode: ParseMode.BrighterScript });
+            expectZeroDiagnostics(diagnostics);
+            expect(statements.length).to.eq(1);
+            expect(isClassStatement(statements[0])).to.be.true;
+            const klass = statements[0] as ClassStatement;
+            klass.fields.forEach(f => expect(f.isOptional).to.be.true);
+        });
+    });
 });
diff --git a/src/parser/Parser.ts b/src/parser/Parser.ts
index 0f87b90e..fb043e0f 100644
--- a/src/parser/Parser.ts
+++ b/src/parser/Parser.ts
@@ -95,6 +95,7 @@ import { createStringLiteral, createToken } from '../astUtils/creators';
 import { Cache } from '../Cache';
 import type { Expression, Statement } from './AstNode';
 import { SymbolTable } from '../SymbolTable';
+import type { BscType } from '../types/BscType';
 
 export class Parser {
     /**
@@ -392,11 +393,16 @@ export class Parser {
     /**
      * Create a new InterfaceMethodStatement. This should only be called from within `interfaceDeclaration`
      */
-    private interfaceFieldStatement() {
+    private interfaceFieldStatement(optionalKeyword?: Token) {
         const name = this.identifier(...AllowedProperties);
-        let asToken = this.consumeToken(TokenKind.As);
-        let typeToken = this.typeToken();
-        const type = util.tokenToBscType(typeToken);
+        let asToken: Token;
+        let typeToken: Token;
+        let type: BscType;
+        if (this.check(TokenKind.As)) {
+            asToken = this.consumeToken(TokenKind.As);
+            typeToken = this.typeToken();
+            type = util.tokenToBscType(typeToken);
+        }
 
         if (!type) {
             this.diagnostics.push({
@@ -406,16 +412,16 @@ export class Parser {
             throw this.lastDiagnosticAsError();
         }
 
-        return new InterfaceFieldStatement(name, asToken, typeToken, type);
+        return new InterfaceFieldStatement(name, asToken, typeToken, type, optionalKeyword);
     }
 
     /**
      * Create a new InterfaceMethodStatement. This should only be called from within `interfaceDeclaration()`
      */
-    private interfaceMethodStatement() {
+    private interfaceMethodStatement(optionalKeyword?: Token) {
         const functionType = this.advance();
         const name = this.identifier(...AllowedProperties);
-        const leftParen = this.consumeToken(TokenKind.LeftParen);
+        const leftParen = this.consume(DiagnosticMessages.expectedToken(TokenKind.LeftParen), TokenKind.LeftParen);
 
         let params = [] as FunctionParameterExpression[];
         if (!this.check(TokenKind.RightParen)) {
@@ -454,7 +460,8 @@ export class Parser {
             rightParen,
             asToken,
             returnTypeToken,
-            util.tokenToBscType(returnTypeToken)
+            util.tokenToBscType(returnTypeToken),
+            optionalKeyword
         );
     }
 
@@ -467,7 +474,7 @@ export class Parser {
             DiagnosticMessages.expectedKeyword(TokenKind.Interface),
             TokenKind.Interface
         );
-        const nameToken = this.identifier();
+        const nameToken = this.identifier(...this.allowedLocalIdentifiers);
 
         let extendsToken: Token;
         let parentInterfaceName: NamespacedVariableNameExpression;
@@ -488,19 +495,24 @@ export class Parser {
                     this.annotationExpression();
                 }
 
+                const optionalKeyword = this.consumeTokenIf(TokenKind.Optional);
                 //fields
-                if (this.checkAny(TokenKind.Identifier, ...AllowedProperties) && this.checkNext(TokenKind.As)) {
+                if (this.checkAny(TokenKind.Identifier, ...AllowedProperties) && this.checkAnyNext(TokenKind.As, TokenKind.Newline, TokenKind.Comment)) {
+                    decl = this.interfaceFieldStatement(optionalKeyword);
+                    //field with name = 'optional'
+                } else if (optionalKeyword && this.checkAny(TokenKind.As, TokenKind.Newline, TokenKind.Comment)) {
+                    //rewind one place, so that 'optional' is the field name
+                    this.current--;
                     decl = this.interfaceFieldStatement();
 
                     //methods (function/sub keyword followed by opening paren)
-                } else if (this.checkAny(TokenKind.Function, TokenKind.Sub) && this.checkAny(TokenKind.Identifier, ...AllowedProperties)) {
-                    decl = this.interfaceMethodStatement();
+                } else if (this.checkAny(TokenKind.Function, TokenKind.Sub) && this.checkAnyNext(TokenKind.Identifier, ...AllowedProperties)) {
+                    decl = this.interfaceMethodStatement(optionalKeyword);
 
                     //comments
                 } else if (this.check(TokenKind.Comment)) {
                     decl = this.commentStatement();
                 }
-
                 if (decl) {
                     this.consumePendingAnnotations(decl);
                     body.push(decl);
@@ -548,7 +560,7 @@ export class Parser {
             TokenKind.Enum
         );
 
-        result.tokens.name = this.tryIdentifier();
+        result.tokens.name = this.tryIdentifier(...this.allowedLocalIdentifiers);
 
         this.consumeStatementSeparators();
         //gather up all members
@@ -725,6 +737,32 @@ export class Parser {
     }
 
     private fieldDeclaration(accessModifier: Token | null) {
+
+        let optionalKeyword = this.consumeTokenIf(TokenKind.Optional);
+
+        if (this.checkAny(TokenKind.Identifier, ...AllowedProperties)) {
+            if (this.check(TokenKind.As)) {
+                if (this.checkAnyNext(TokenKind.Comment, TokenKind.Newline)) {
+                    // as <EOL>
+                    // `as` is the field name
+                } else if (this.checkNext(TokenKind.As)) {
+                    //  as as ____
+                    // first `as` is the field name
+                } else if (optionalKeyword) {
+                    // optional as ____
+                    // optional is the field name, `as` starts type
+                    // rewind current token
+                    optionalKeyword = null;
+                    this.current--;
+                }
+            }
+        } else {
+            // no name after `optional` ... optional is the name
+            // rewind current token
+            optionalKeyword = null;
+            this.current--;
+        }
+
         let name = this.consume(
             DiagnosticMessages.expectedClassFieldIdentifier(),
             TokenKind.Identifier,
@@ -760,7 +798,8 @@ export class Parser {
             asToken,
             fieldType,
             equal,
-            initialValue
+            initialValue,
+            optionalKeyword
         );
     }
 
@@ -2840,6 +2879,15 @@ export class Parser {
         }
     }
 
+    /**
+     * Consume next token IF it matches the specified kind. Otherwise, do nothing and return undefined
+     */
+    private consumeTokenIf(tokenKind: TokenKind) {
+        if (this.match(tokenKind)) {
+            return this.previous();
+        }
+    }
+
     private consumeToken(tokenKind: TokenKind) {
         return this.consume(
             DiagnosticMessages.expectedToken(tokenKind),
diff --git a/src/parser/Statement.ts b/src/parser/Statement.ts
index e16e3f46..5f2fd7fc 100644
--- a/src/parser/Statement.ts
+++ b/src/parser/Statement.ts
@@ -1427,13 +1427,16 @@ export class InterfaceFieldStatement extends Statement implements TypedefProvide
         nameToken: Identifier,
         asToken: Token,
         typeToken: Token,
-        public type: BscType
+        public type: BscType,
+        optionalToken?: Token
     ) {
         super();
+        this.tokens.optional = optionalToken;
         this.tokens.name = nameToken;
         this.tokens.as = asToken;
         this.tokens.type = typeToken;
         this.range = util.createBoundingRange(
+            optionalToken,
             nameToken,
             asToken,
             typeToken
@@ -1443,6 +1446,7 @@ export class InterfaceFieldStatement extends Statement implements TypedefProvide
     public range: Range;
 
     public tokens = {} as {
+        optional?: Token;
         name: Identifier;
         as: Token;
         type: Token;
@@ -1452,6 +1456,10 @@ export class InterfaceFieldStatement extends Statement implements TypedefProvide
         return this.tokens.name.text;
     }
 
+    public get isOptional() {
+        return !!this.tokens.optional;
+    }
+
     walk(visitor: WalkVisitor, options: WalkOptions) {
         //nothing to walk
     }
@@ -1465,7 +1473,12 @@ export class InterfaceFieldStatement extends Statement implements TypedefProvide
                 state.indent()
             );
         }
-
+        if (this.isOptional) {
+            result.push(
+                this.tokens.optional.text,
+                ' '
+            );
+        }
         result.push(
             this.tokens.name.text
         );
@@ -1492,9 +1505,11 @@ export class InterfaceMethodStatement extends Statement implements TypedefProvid
         rightParen: Token,
         asToken?: Token,
         returnTypeToken?: Token,
-        public returnType?: BscType
+        public returnType?: BscType,
+        optionalToken?: Token
     ) {
         super();
+        this.tokens.optional = optionalToken;
         this.tokens.functionType = functionTypeToken;
         this.tokens.name = nameToken;
         this.tokens.leftParen = leftParen;
@@ -1505,6 +1520,7 @@ export class InterfaceMethodStatement extends Statement implements TypedefProvid
 
     public get range() {
         return util.createBoundingRange(
+            this.tokens.optional,
             this.tokens.functionType,
             this.tokens.name,
             this.tokens.leftParen,
@@ -1516,6 +1532,7 @@ export class InterfaceMethodStatement extends Statement implements TypedefProvid
     }
 
     public tokens = {} as {
+        optional?: Token;
         functionType: Token;
         name: Identifier;
         leftParen: Token;
@@ -1524,6 +1541,10 @@ export class InterfaceMethodStatement extends Statement implements TypedefProvid
         returnType: Token;
     };
 
+    public get isOptional() {
+        return !!this.tokens.optional;
+    }
+
     walk(visitor: WalkVisitor, options: WalkOptions) {
         //nothing to walk
     }
@@ -1538,6 +1559,13 @@ export class InterfaceMethodStatement extends Statement implements TypedefProvid
             );
         }
 
+        if (this.isOptional) {
+            result.push(
+                this.tokens.optional.text,
+                ' '
+            );
+        }
+
         result.push(
             this.tokens.functionType.text,
             ' ',
@@ -2071,12 +2099,12 @@ export class MethodStatement extends FunctionStatement {
 
         //check whether any calls to super exist
         let containsSuperCall =
-        this.func.body.statements.findIndex((x) => {
-            //is a call statement
-            return isExpressionStatement(x) && isCallExpression(x.expression) &&
-            //is a call to super
-            util.findBeginningVariableExpression(x.expression.callee as any).name.text.toLowerCase() === 'super';
-        }) !== -1;
+            this.func.body.statements.findIndex((x) => {
+                //is a call statement
+                return isExpressionStatement(x) && isCallExpression(x.expression) &&
+                    //is a call to super
+                    util.findBeginningVariableExpression(x.expression.callee as any).name.text.toLowerCase() === 'super';
+            }) !== -1;
 
         //if a call to super exists, quit here
         if (containsSuperCall) {
@@ -2163,7 +2191,8 @@ export class FieldStatement extends Statement implements TypedefProvider {
         readonly as?: Token,
         readonly type?: Token,
         readonly equal?: Token,
-        readonly initialValue?: Expression
+        readonly initialValue?: Expression,
+        readonly optional?: Token
     ) {
         super();
         this.range = util.createBoundingRange(
@@ -2192,6 +2221,10 @@ export class FieldStatement extends Statement implements TypedefProvider {
 
     public readonly range: Range;
 
+    public get isOptional() {
+        return !!this.optional;
+    }
+
     transpile(state: BrsTranspileState): TranspileResult {
         throw new Error('transpile not implemented for ' + Object.getPrototypeOf(this).constructor.name);
     }
@@ -2214,8 +2247,12 @@ export class FieldStatement extends Statement implements TypedefProvider {
 
             result.push(
                 this.accessModifier?.text ?? 'public',
-                ' ',
-                this.name?.text,
+                ' '
+            );
+            if (this.isOptional) {
+                result.push(this.optional.text, ' ');
+            }
+            result.push(this.name?.text,
                 ' as ',
                 type.toTypeString()
             );


Test patch:
diff --git a/src/parser/tests/statement/ConstStatement.spec.ts b/src/parser/tests/statement/ConstStatement.spec.ts
index 42a4817c..6bbc61f6 100644
--- a/src/parser/tests/statement/ConstStatement.spec.ts
+++ b/src/parser/tests/statement/ConstStatement.spec.ts
@@ -64,6 +64,21 @@ describe('ConstStatement', () => {
         `);
     });
 
+    it('allows const with the name `optional`', () => {
+        program.setFile('source/main.bs', `
+            const optional = true
+            namespace alpha
+                const optional = true
+            end namespace
+            sub main()
+                print optional
+                print alpha.optional
+            end sub
+        `);
+        program.validate();
+        expectZeroDiagnostics(program);
+    });
+
     describe('transpile', () => {
 
         it('transpiles simple consts', () => {
diff --git a/src/parser/tests/statement/Enum.spec.ts b/src/parser/tests/statement/Enum.spec.ts
index a12bcec8..806bb226 100644
--- a/src/parser/tests/statement/Enum.spec.ts
+++ b/src/parser/tests/statement/Enum.spec.ts
@@ -202,6 +202,15 @@ describe('EnumStatement', () => {
     });
 
     describe('validation', () => {
+        it('allows enums named `optional`', () => {
+            program.setFile('source/main.bs', `
+                enum optional
+                    thing = 1
+                end enum
+            `);
+            program.validate();
+            expectZeroDiagnostics(program);
+        });
 
         it('catches duplicate enums from same file', () => {
             program.setFile('source/main.bs', `