Repository: rokucommunity/brighterscript
Pull request #1096 (2024-03-08 18:59:32)

Fix classname as variable for completions and validations

https://github.com/rokucommunity/brighterscript/assets/810290/96313697-7232-4e9e-bdc3-60876fc83783

When a Class constructor function is used outside of `new` it is treated as a function type, allowing code completion and validation based on that.

This PR also adds member (and method) validation on primitive types in general


Patch:
diff --git a/src/Scope.spec.ts b/src/Scope.spec.ts
index 5be4a386..e2b3e40d 100644
--- a/src/Scope.spec.ts
+++ b/src/Scope.spec.ts
@@ -8,7 +8,7 @@ import PluginInterface from './PluginInterface';
 import { expectDiagnostics, expectDiagnosticsIncludes, expectTypeToBe, expectZeroDiagnostics, trim } from './testHelpers.spec';
 import { Logger } from './Logger';
 import type { BrsFile } from './files/BrsFile';
-import type { NamespaceStatement } from './parser/Statement';
+import type { ForEachStatement, NamespaceStatement } from './parser/Statement';
 import type { CompilerPlugin, OnScopeValidateEvent } from './interfaces';
 import { DiagnosticOrigin } from './interfaces';
 import { SymbolTypeFlag } from './SymbolTypeFlag';
@@ -23,12 +23,13 @@ import { FloatType } from './types/FloatType';
 import { NamespaceType } from './types/NamespaceType';
 import { DoubleType } from './types/DoubleType';
 import { UnionType } from './types/UnionType';
-import { isFunctionStatement, isNamespaceStatement } from './astUtils/reflection';
+import { isForEachStatement, isFunctionStatement, isNamespaceStatement } from './astUtils/reflection';
 import { ArrayType } from './types/ArrayType';
 import { AssociativeArrayType } from './types/AssociativeArrayType';
 import { InterfaceType } from './types/InterfaceType';
 import { ComponentType } from './types/ComponentType';
 import * as path from 'path';
+import { WalkMode } from './astUtils/visitors';
 
 describe('Scope', () => {
     let sinon = sinonImport.createSandbox();
@@ -1783,7 +1784,6 @@ describe('Scope', () => {
                     class MyClass
                         member as integer
                     end class
-
                 `);
                 program.validate();
                 expectDiagnostics(program, [
@@ -2021,7 +2021,7 @@ describe('Scope', () => {
             it('finds unknown members of primitive types', () => {
                 program.setFile(`source/main.bs`, `
                     sub fn(input as SomeKlass)
-                        piValue = input.getPi().noMethod()
+                        piValue = input.getPi().someProp
                     end sub
 
                     class SomeKlass
@@ -2031,11 +2031,28 @@ describe('Scope', () => {
                     end class
                 `);
                 program.validate();
-                //TODO: ideally, if this is a primitive type, we should know all the possible members
-                // This *SHOULD* be an error, but currently, during Runtime, an unknown member (from DottedtGetExpression) is returned as Dynamic.instance
-                expectZeroDiagnostics(program);
+                expectDiagnostics(program, [
+                    DiagnosticMessages.cannotFindName('someProp').message
+                ]);
             });
 
+            it('finds unknown methods of primitive types', () => {
+                program.setFile(`source/main.bs`, `
+                    sub fn(input as SomeKlass)
+                        piValue = input.getPi().noMethod()
+                    end sub
+
+                    class SomeKlass
+                        function getPi() as float
+                            return 3.14
+                        end function
+                    end class
+                `);
+                program.validate();
+                expectDiagnostics(program, [
+                    DiagnosticMessages.cannotFindName('noMethod').message
+                ]);
+            });
 
             it('finds members of arrays', () => {
                 program.setFile(`source/main.bs`, `
@@ -3331,6 +3348,31 @@ describe('Scope', () => {
             program.validate();
             expectZeroDiagnostics(program);
         });
+
+        it('resets m in a function in an AA literal to be an AssociativeArray type', () => {
+            const file = program.setFile<BrsFile>('source/class.bs', `
+                class TestKlass
+                    function getData() as object
+                        data = {
+                            data: [] as float[],
+                            sum: function() as float
+                                value = 0
+                                for each item in m.data
+                                    value += item
+                                end for
+                                return value
+                            end function
+                        }
+                        return data
+                    end function
+                end class
+            `);
+            program.validate();
+            expectZeroDiagnostics(program);
+            const forEachStmt = file.parser.ast.findChildren(isForEachStatement, { walkMode: WalkMode.visitAllRecursive })[0] as ForEachStatement;
+            const mType = forEachStmt.getSymbolTable().getSymbolType('m', { flags: SymbolTypeFlag.runtime });
+            expectTypeToBe(mType, AssociativeArrayType);
+        });
     });
 
     describe('unlinkSymbolTable', () => {
diff --git a/src/bscPlugin/completions/CompletionsProcessor.spec.ts b/src/bscPlugin/completions/CompletionsProcessor.spec.ts
index 75d46f40..975629e6 100644
--- a/src/bscPlugin/completions/CompletionsProcessor.spec.ts
+++ b/src/bscPlugin/completions/CompletionsProcessor.spec.ts
@@ -1059,6 +1059,48 @@ describe('CompletionsProcessor', () => {
             }]);
         });
 
+        it('treats class name as a function', () => {
+            program.setFile('source/main.bs', `
+                class SomeKlass
+                    name as string
+                end class
+
+                sub test()
+                    print SomeKlass.
+                end sub
+            `);
+
+            program.validate();
+            // print SomeKlass.|
+            let completions = program.getCompletions(`${rootDir}/source/main.bs`, Position.create(6, 36));
+            expectCompletionsIncludes(completions, [{
+                label: 'ifFunction',
+                kind: CompletionItemKind.Interface
+            }]);
+        });
+
+        it('treats namespaced class name as a function', () => {
+            program.setFile('source/main.bs', `
+                namespace alpha
+                    class SomeKlass
+                        name as string
+                    end class
+                end namespace
+
+                sub test()
+                    print alpha.SomeKlass.toStr()
+                end sub
+            `);
+
+            program.validate();
+            expectZeroDiagnostics(program);
+            // print alpha.SomeKlass.|
+            let completions = program.getCompletions(`${rootDir}/source/main.bs`, Position.create(8, 42));
+            expectCompletionsIncludes(completions, [{
+                label: 'ifFunction',
+                kind: CompletionItemKind.Interface
+            }]);
+        });
 
     });
 
diff --git a/src/bscPlugin/validation/BrsFileValidator.ts b/src/bscPlugin/validation/BrsFileValidator.ts
index 061d632b..bb62273d 100644
--- a/src/bscPlugin/validation/BrsFileValidator.ts
+++ b/src/bscPlugin/validation/BrsFileValidator.ts
@@ -1,4 +1,4 @@
-import { isArrayType, isBody, isBrsFile, isClassStatement, isConstStatement, isDottedGetExpression, isDottedSetStatement, isEnumStatement, isForEachStatement, isForStatement, isFunctionExpression, isFunctionStatement, isImportStatement, isIndexedGetExpression, isIndexedSetStatement, isInterfaceStatement, isLibraryStatement, isLiteralExpression, isNamespaceStatement, isUnaryExpression, isWhileStatement } from '../../astUtils/reflection';
+import { isAALiteralExpression, isArrayType, isBody, isBrsFile, isClassStatement, isConstStatement, isDottedGetExpression, isDottedSetStatement, isEnumStatement, isForEachStatement, isForStatement, isFunctionExpression, isFunctionStatement, isImportStatement, isIndexedGetExpression, isIndexedSetStatement, isInterfaceStatement, isLibraryStatement, isLiteralExpression, isNamespaceStatement, isUnaryExpression, isWhileStatement } from '../../astUtils/reflection';
 import { createVisitor, WalkMode } from '../../astUtils/visitors';
 import { DiagnosticMessages } from '../../DiagnosticMessages';
 import type { BrsFile } from '../../files/BrsFile';
@@ -139,7 +139,7 @@ export class BrsFileValidator {
                 }
             },
             FunctionExpression: (node) => {
-                if (!node.symbolTable.hasSymbol('m', SymbolTypeFlag.runtime)) {
+                if (!node.symbolTable.hasSymbol('m', SymbolTypeFlag.runtime) || node.findAncestor(isAALiteralExpression)) {
                     node.symbolTable.addSymbol('m', undefined, new AssociativeArrayType(), SymbolTypeFlag.runtime);
                 }
             },
diff --git a/src/bscPlugin/validation/ScopeValidator.spec.ts b/src/bscPlugin/validation/ScopeValidator.spec.ts
index a9da2526..9b77a2c8 100644
--- a/src/bscPlugin/validation/ScopeValidator.spec.ts
+++ b/src/bscPlugin/validation/ScopeValidator.spec.ts
@@ -1733,12 +1733,12 @@ describe('ScopeValidator', () => {
                 end class
 
                 sub doStuff()
-                    print klass.name ' only valid use of "Klass" is as a constructor: "new Klass()"
+                    print klass.name ' only valid use of "Klass" is as a constructor: "new Klass()", or as a function
                 end sub
             `);
             program.validate();
             expectDiagnostics(program, [
-                DiagnosticMessages.itemCannotBeUsedAsVariable('Klass')
+                DiagnosticMessages.cannotFindName('name', 'function.name')
             ]);
         });
 
@@ -1756,7 +1756,7 @@ describe('ScopeValidator', () => {
             `);
             program.validate();
             expectDiagnostics(program, [
-                DiagnosticMessages.itemCannotBeUsedAsVariable('Alpha.Klass')
+                DiagnosticMessages.cannotFindName('name', 'function.name')
             ]);
         });
 
diff --git a/src/bscPlugin/validation/ScopeValidator.ts b/src/bscPlugin/validation/ScopeValidator.ts
index 1f2db499..6bd99d5c 100644
--- a/src/bscPlugin/validation/ScopeValidator.ts
+++ b/src/bscPlugin/validation/ScopeValidator.ts
@@ -1,5 +1,5 @@
 import { URI } from 'vscode-uri';
-import { isAssignmentStatement, isAssociativeArrayType, isBrsFile, isCallExpression, isCallableType, isClassStatement, isClassType, isDottedGetExpression, isDynamicType, isEnumMemberType, isEnumType, isFunctionExpression, isLiteralExpression, isNamespaceStatement, isNamespaceType, isNewExpression, isObjectType, isPrimitiveType, isTypedFunctionType, isUnionType, isVariableExpression, isXmlScope } from '../../astUtils/reflection';
+import { isAssignmentStatement, isAssociativeArrayType, isBrsFile, isCallExpression, isCallableType, isClassStatement, isClassType, isDottedGetExpression, isDynamicType, isEnumMemberType, isEnumType, isFunctionExpression, isLiteralExpression, isNamespaceStatement, isNamespaceType, isNewExpression, isObjectType, isPrimitiveType, isReferenceType, isTypedFunctionType, isUnionType, isVariableExpression, isXmlScope } from '../../astUtils/reflection';
 import { Cache } from '../../Cache';
 import { DiagnosticMessages } from '../../DiagnosticMessages';
 import type { BrsFile } from '../../files/BrsFile';
@@ -582,9 +582,9 @@ export class ScopeValidator {
         const shouldIgnoreLHS = this.ignoreUnresolvedAssignmentLHS(expression, exprType, typeData?.definingNode);
 
         if (!this.isTypeKnown(exprType) && !shouldIgnoreLHS) {
-            if (expression.getType({ flags: oppositeSymbolType })?.isResolvable()) {
+            if (expression.getType({ flags: oppositeSymbolType, isExistenceTest: true })?.isResolvable()) {
                 const oppoSiteTypeChain = [];
-                const invalidlyUsedResolvedType = expression.getType({ flags: oppositeSymbolType, typeChain: oppoSiteTypeChain });
+                const invalidlyUsedResolvedType = expression.getType({ flags: oppositeSymbolType, typeChain: oppoSiteTypeChain, isExistenceTest: true });
                 const typeChainScan = util.processTypeChain(oppoSiteTypeChain);
                 if (isUsedAsType) {
                     this.addMultiScopeDiagnostic({
@@ -592,12 +592,19 @@ export class ScopeValidator {
                         range: expression.range,
                         file: file
                     });
-                } else {
+                } else if (invalidlyUsedResolvedType && !isReferenceType(invalidlyUsedResolvedType)) {
                     this.addMultiScopeDiagnostic({
                         ...DiagnosticMessages.itemCannotBeUsedAsVariable(invalidlyUsedResolvedType.toString()),
                         range: expression.range,
                         file: file
                     });
+                } else {
+                    const typeChainScan = util.processTypeChain(typeChain);
+                    this.addMultiScopeDiagnostic({
+                        file: file as BscFile,
+                        ...DiagnosticMessages.cannotFindName(typeChainScan.itemName, typeChainScan.fullNameOfItem),
+                        range: typeChainScan.range
+                    });
                 }
 
             } else {
@@ -676,7 +683,6 @@ export class ScopeValidator {
         let isFirst = true;
         for (let i = 0; i < typeChain.length - 1; i++) { // do not look at final entry - we CAN use the constructor as a variable
             const tce = typeChain[i];
-
             lowerNameSoFar += `${lowerNameSoFar ? '.' : ''}${tce.name.toLowerCase()}`;
             if (!isNamespaceType(tce.type)) {
                 if (isFirst && containingNamespaceName) {
diff --git a/src/files/BrsFile.spec.ts b/src/files/BrsFile.spec.ts
index fd9c6af9..db2d4ca4 100644
--- a/src/files/BrsFile.spec.ts
+++ b/src/files/BrsFile.spec.ts
@@ -2836,7 +2836,7 @@ describe('BrsFile', () => {
                 program.setFile('source/main.bs', `
                     sub test()
                         someNode = createObject("roSGNode", "Rectangle")
-                        someNode@.someFunction(test.value)
+                        someNode@.someFunction({test: "value"})
                     end sub
                 `);
                 program.validate();
diff --git a/src/interfaces.ts b/src/interfaces.ts
index 5e256f26..684bfba4 100644
--- a/src/interfaces.ts
+++ b/src/interfaces.ts
@@ -833,6 +833,7 @@ export interface GetTypeOptions {
     ignoreCall?: boolean; // get the type of this expression, NOT it's return type
     onlyCacheResolvedTypes?: boolean;
     ignoreCacheForRetrieval?: boolean;
+    isExistenceTest?: boolean;
 }
 
 export class TypeChainEntry {
diff --git a/src/parser/Expression.ts b/src/parser/Expression.ts
index b4e4a1c7..50f31306 100644
--- a/src/parser/Expression.ts
+++ b/src/parser/Expression.ts
@@ -10,7 +10,7 @@ import * as fileUrl from 'file-url';
 import type { WalkOptions, WalkVisitor } from '../astUtils/visitors';
 import { WalkMode } from '../astUtils/visitors';
 import { walk, InternalWalkMode, walkArray } from '../astUtils/visitors';
-import { isAALiteralExpression, isAAMemberExpression, isArrayLiteralExpression, isArrayType, isCallExpression, isCallableType, isCallfuncExpression, isComponentType, isDottedGetExpression, isEscapedCharCodeLiteralExpression, isFunctionExpression, isFunctionStatement, isIntegerType, isInterfaceMethodStatement, isLiteralBoolean, isLiteralExpression, isLiteralNumber, isLiteralString, isLongIntegerType, isMethodStatement, isNamespaceStatement, isNewExpression, isReferenceType, isStringType, isTypeCastExpression, isUnaryExpression, isVariableExpression } from '../astUtils/reflection';
+import { isAALiteralExpression, isAAMemberExpression, isArrayLiteralExpression, isArrayType, isCallExpression, isCallableType, isCallfuncExpression, isComponentType, isDottedGetExpression, isEscapedCharCodeLiteralExpression, isFunctionExpression, isFunctionStatement, isIntegerType, isInterfaceMethodStatement, isLiteralBoolean, isLiteralExpression, isLiteralNumber, isLiteralString, isLongIntegerType, isMethodStatement, isNamespaceStatement, isNewExpression, isPrimitiveType, isReferenceType, isStringType, isTypeCastExpression, isUnaryExpression, isVariableExpression } from '../astUtils/reflection';
 import type { GetTypeOptions, TranspileResult, TypedefProvider } from '../interfaces';
 import { TypeChainEntry } from '../interfaces';
 import { VoidType } from '../types/VoidType';
@@ -30,6 +30,7 @@ import type { ComponentType } from '../types/ComponentType';
 import { createToken } from '../astUtils/creators';
 import { TypedFunctionType } from '../types';
 import { SymbolTypeFlag } from '../SymbolTypeFlag';
+import { FunctionType } from '../types/FunctionType';
 
 export type ExpressionVisitor = (expression: Expression, parent: Expression) => void;
 
@@ -180,7 +181,7 @@ export class CallExpression extends Expression {
         if (isCallableType(calleeType) && (!isReferenceType(calleeType.returnType) || calleeType.returnType?.isResolvable())) {
             return calleeType.returnType;
         }
-        if (!isReferenceType(calleeType) && (calleeType as any).returnType?.isResolvable()) {
+        if (!isReferenceType(calleeType) && (calleeType as any)?.returnType?.isResolvable()) {
             return (calleeType as any).returnType;
         }
         return new TypePropertyReferenceType(calleeType, 'returnType');
@@ -552,7 +553,12 @@ export class DottedGetExpression extends Expression {
 
     getType(options: GetTypeOptions) {
         const objType = this.obj?.getType(options);
-        const result = objType?.getMemberType(this.tokens.name?.text, options);
+        let result = objType?.getMemberType(this.tokens.name?.text, options);
+
+        if (util.isClassUsedAsFunction(result, this, options)) {
+            // treat this class constructor as a function
+            result = FunctionType.instance;
+        }
         options.typeChain?.push(new TypeChainEntry({
             name: this.tokens.name?.text,
             type: result,
@@ -560,8 +566,10 @@ export class DottedGetExpression extends Expression {
             range: this.tokens.name?.range ?? this.range,
             kind: this.kind
         }));
-        if (result || options.flags & SymbolTypeFlag.typetime) {
-            // All types should be known at typetime
+        if (result ||
+            options.flags & SymbolTypeFlag.typetime ||
+            (isPrimitiveType(objType) || isCallableType(objType))) {
+            // All types should be known at typeTime, or the obj is well known
             return result;
         }
         // It is possible at runtime that a value has been added dynamically to an object, or something
@@ -1175,6 +1183,11 @@ export class VariableExpression extends Expression {
         if (!resultType) {
             const symbolTable = this.getSymbolTable();
             resultType = symbolTable?.getSymbolType(nameKey, { ...options, fullName: nameKey, tableProvider: () => this.getSymbolTable() });
+
+            if (util.isClassUsedAsFunction(resultType, this, options)) {
+                resultType = FunctionType.instance;
+            }
+
         }
         options.typeChain?.push(new TypeChainEntry({ name: nameKey, type: resultType, data: options.data, range: this.range, kind: this.kind }));
         return resultType;
diff --git a/src/types/BscType.ts b/src/types/BscType.ts
index 469e05bb..af70f887 100644
--- a/src/types/BscType.ts
+++ b/src/types/BscType.ts
@@ -38,6 +38,7 @@ export abstract class BscType {
     }
 
     getMemberTable() {
+        this.addBuiltInInterfaces();
         return this.memberTable;
     }
 
diff --git a/src/util.ts b/src/util.ts
index 5239ab39..4dab1872 100644
--- a/src/util.ts
+++ b/src/util.ts
@@ -11,7 +11,7 @@ import { URI } from 'vscode-uri';
 import * as xml2js from 'xml2js';
 import type { BsConfig, FinalizedBsConfig } from './BsConfig';
 import { DiagnosticMessages } from './DiagnosticMessages';
-import type { CallableContainer, BsDiagnostic, FileReference, CallableContainerMap, CompilerPluginFactory, CompilerPlugin, ExpressionInfo, TranspileResult, TypeChainEntry, TypeChainProcessResult } from './interfaces';
+import type { CallableContainer, BsDiagnostic, FileReference, CallableContainerMap, CompilerPluginFactory, CompilerPlugin, ExpressionInfo, TranspileResult, TypeChainEntry, TypeChainProcessResult, GetTypeOptions } from './interfaces';
 import { BooleanType } from './types/BooleanType';
 import { DoubleType } from './types/DoubleType';
 import { DynamicType } from './types/DynamicType';
@@ -26,7 +26,7 @@ import type { CallExpression, CallfuncExpression, DottedGetExpression, FunctionP
 import { Logger, LogLevel } from './Logger';
 import { isToken, type Identifier, type Locatable, type Token } from './lexer/Token';
 import { TokenKind } from './lexer/TokenKind';
-import { isAnyReferenceType, isBinaryExpression, isBooleanType, isBrsFile, isCallExpression, isCallfuncExpression, isDottedGetExpression, isDoubleType, isDynamicType, isEnumMemberType, isExpression, isFloatType, isIndexedGetExpression, isInvalidType, isLongIntegerType, isNumberType, isStringType, isTypeExpression, isTypedArrayExpression, isVariableExpression, isXmlAttributeGetExpression, isXmlFile } from './astUtils/reflection';
+import { isAnyReferenceType, isBinaryExpression, isBooleanType, isBrsFile, isCallExpression, isCallfuncExpression, isClassType, isDottedGetExpression, isDoubleType, isDynamicType, isEnumMemberType, isExpression, isFloatType, isIndexedGetExpression, isInvalidType, isLongIntegerType, isNewExpression, isNumberType, isStringType, isTypeExpression, isTypedArrayExpression, isVariableExpression, isXmlAttributeGetExpression, isXmlFile } from './astUtils/reflection';
 import { WalkMode } from './astUtils/visitors';
 import { SourceNode } from 'source-map';
 import * as requireRelative from 'require-relative';
@@ -2118,6 +2118,18 @@ export class Util {
         }
         return false;
     }
+
+    public isClassUsedAsFunction(potentialClassType: BscType, expression: Expression, options: GetTypeOptions) {
+        // eslint-disable-next-line no-bitwise
+        if (options.flags & SymbolTypeFlag.runtime &&
+            isClassType(potentialClassType) &&
+            !options.isExistenceTest &&
+            potentialClassType.name.toLowerCase() === this.getAllDottedGetPartsAsString(expression).toLowerCase() &&
+            !expression.findAncestor(isNewExpression)) {
+            return true;
+        }
+        return false;
+    }
 }
 
 /**


Test patch:
diff --git a/src/parser/tests/expression/TemplateStringExpression.spec.ts b/src/parser/tests/expression/TemplateStringExpression.spec.ts
index 9d50d6ef..717500ae 100644
--- a/src/parser/tests/expression/TemplateStringExpression.spec.ts
+++ b/src/parser/tests/expression/TemplateStringExpression.spec.ts
@@ -149,11 +149,11 @@ describe('TemplateStringExpression', () => {
         it('properly transpiles one line template string with expressions', async () => {
             await testTranspile(`
                 sub main()
-                    a = \`hello \${LINE_NUM.text} world \${"template" + "".getChars()} test\`
+                    a = \`hello \${LINE_NUM.text} world \${"template" + "".trim()} test\`
                 end sub
             `, `
                 sub main()
-                    a = ("hello " + bslib_toString(LINE_NUM.text) + " world " + bslib_toString("template" + "".getChars()) + " test")
+                    a = ("hello " + bslib_toString(LINE_NUM.text) + " world " + bslib_toString("template" + "".trim()) + " test")
                 end sub
             `);
         });
@@ -220,12 +220,12 @@ describe('TemplateStringExpression', () => {
 
         it('properly transpiles more complex multiline template string', async () => {
             await testTranspile(`
-                sub main()
-                    a = \`I am multiline\n\${a.isRunning()}\nmore\`
+                sub main(data)
+                    a = \`I am multiline\n\${data.isRunning()}\nmore\`
                 end sub
             `, `
-                sub main()
-                    a = ("I am multiline" + chr(10) + bslib_toString(a.isRunning()) + chr(10) + "more")
+                sub main(data)
+                    a = ("I am multiline" + chr(10) + bslib_toString(data.isRunning()) + chr(10) + "more")
                 end sub
             `);
         });
diff --git a/src/parser/tests/statement/ConstStatement.spec.ts b/src/parser/tests/statement/ConstStatement.spec.ts
index 10387e43..db57ae65 100644
--- a/src/parser/tests/statement/ConstStatement.spec.ts
+++ b/src/parser/tests/statement/ConstStatement.spec.ts
@@ -184,11 +184,11 @@ describe('ConstStatement', () => {
             await testTranspile(`
                 const API_KEY ="ABC"
                 sub main()
-                    print API_KEY.toString()
+                    print API_KEY.toStr()
                 end sub
             `, `
                 sub main()
-                    print "ABC".toString()
+                    print "ABC".toStr()
                 end sub
             `);
         });