Functions as params (#853)

* Added proper validaton for functions as params * Fixed lint errors and added hover test * fixed some places with wrong function type
rokucommunity · Jul 24, 2023 · 53c7160 · 53c7160
1 parent b4ff08e
commit 53c7160
Show file tree

Hide file tree

Showing 22 changed files with 369 additions and 203 deletions.
diff --git a/src/Program.ts b/src/Program.ts
@@ -36,10 +36,10 @@ import { BooleanType } from './types/BooleanType';
 import { DoubleType } from './types/DoubleType';
 import { DynamicType } from './types/DynamicType';
 import { FloatType } from './types/FloatType';
-import { FunctionType } from './types/FunctionType';
 import { LongIntegerType } from './types/LongIntegerType';
 import { ObjectType } from './types/ObjectType';
 import { VoidType } from './types/VoidType';
+import { FunctionType } from './types/FunctionType';
 
 const startOfSourcePkgPath = `source${path.sep}`;
 const bslibNonAliasedRokuModulesPkgPath = s`source/roku_modules/rokucommunity_bslib/bslib.brs`;
@@ -114,7 +114,7 @@ export class Program {
         this.globalScope.symbolTable.addSymbol('double', undefined, DoubleType.instance, SymbolTypeFlag.typetime);
         this.globalScope.symbolTable.addSymbol('dynamic', undefined, DynamicType.instance, SymbolTypeFlag.typetime);
         this.globalScope.symbolTable.addSymbol('float', undefined, FloatType.instance, SymbolTypeFlag.typetime);
-        this.globalScope.symbolTable.addSymbol('function', undefined, new FunctionType(DynamicType.instance), SymbolTypeFlag.typetime);
+        this.globalScope.symbolTable.addSymbol('function', undefined, new FunctionType(), SymbolTypeFlag.typetime);
         this.globalScope.symbolTable.addSymbol('integer', undefined, IntegerType.instance, SymbolTypeFlag.typetime);
         this.globalScope.symbolTable.addSymbol('longinteger', undefined, LongIntegerType.instance, SymbolTypeFlag.typetime);
         this.globalScope.symbolTable.addSymbol('object', undefined, new ObjectType(), SymbolTypeFlag.typetime);

diff --git a/src/Scope.ts b/src/Scope.ts
@@ -18,7 +18,7 @@ import { URI } from 'vscode-uri';
 import { LogLevel } from './Logger';
 import type { BrsFile } from './files/BrsFile';
 import type { DependencyGraph, DependencyChangedEvent } from './DependencyGraph';
-import { isBrsFile, isMethodStatement, isClassStatement, isConstStatement, isEnumStatement, isFunctionStatement, isFunctionType, isXmlFile, isEnumMemberStatement, isNamespaceStatement, isNamespaceType, isReferenceType } from './astUtils/reflection';
+import { isBrsFile, isMethodStatement, isClassStatement, isConstStatement, isEnumStatement, isFunctionStatement, isXmlFile, isEnumMemberStatement, isNamespaceStatement, isNamespaceType, isReferenceType, isCallableType } from './astUtils/reflection';
 import { SymbolTable, SymbolTypeFlag } from './SymbolTable';
 import type { Statement } from './parser/AstNode';
 import type { BscType } from './types/BscType';
@@ -973,7 +973,7 @@ export class Scope {
                 const lowerVarName = varName.toLowerCase();
 
                 //if the var is a function
-                if (isFunctionType(varDeclaration.getType())) {
+                if (isCallableType(varDeclaration.getType())) {
                     //local var function with same name as stdlib function
                     if (
                         //has same name as stdlib

diff --git a/src/astUtils/reflection.ts b/src/astUtils/reflection.ts
@@ -6,6 +6,7 @@ import type { BscFile, File, TypedefProvider } from '../interfaces';
 import type { InvalidType } from '../types/InvalidType';
 import type { VoidType } from '../types/VoidType';
 import { InternalWalkMode } from './visitors';
+import type { TypedFunctionType } from '../types/TypedFunctionType';
 import type { FunctionType } from '../types/FunctionType';
 import type { StringType } from '../types/StringType';
 import type { BooleanType } from '../types/BooleanType';
@@ -29,6 +30,7 @@ import type { ArrayType } from '../types/ArrayType';
 import type { InheritableType } from '../types/InheritableType';
 import { BscTypeKind } from '../types/BscTypeKind';
 import type { NamespaceType } from '../types/NamespaceType';
+import type { BaseFunctionType } from '../types/BaseFunctionType';
 
 // File reflection
 export function isBrsFile(file: (BscFile | File)): file is BrsFile {
@@ -262,6 +264,9 @@ export function isTypeCastExpression(element: any): element is TypeCastExpressio
 export function isStringType(value: any): value is StringType {
     return value?.kind === BscTypeKind.StringType;
 }
+export function isTypedFunctionType(value: any): value is TypedFunctionType {
+    return value?.kind === BscTypeKind.TypedFunctionType;
+}
 export function isFunctionType(value: any): value is FunctionType {
     return value?.kind === BscTypeKind.FunctionType;
 }
@@ -327,6 +332,10 @@ export function isInheritableType(target): target is InheritableType {
     return isClassType(target) || isInterfaceType(target);
 }
 
+export function isCallableType(target): target is BaseFunctionType {
+    return isFunctionType(target) || isTypedFunctionType(target);
+}
+
 const numberConstructorNames = [
     IntegerType.name,
     LongIntegerType.name,

diff --git a/src/bscPlugin/hover/HoverProcessor.spec.ts b/src/bscPlugin/hover/HoverProcessor.spec.ts
@@ -355,5 +355,26 @@ describe('HoverProcessor', () => {
             // TODO: Add comments for class methods/properties
             expect(hover?.contents).to.eql([`${fence('function MyKlass.getName() as string')}`]);
         });
+
+        it('finds functions as params', () => {
+            program.setFile('source/main.brs', `
+                function getStrLength(name as string) as integer
+                    return len(name)
+                end function
+
+                sub tryManyParams(someFunc as function)
+                    print someFunc(1, 2, "hello", "world")
+                end sub
+
+                sub test()
+                    tryManyParams(getStrLength)
+                end sub
+            `);
+            program.validate();
+            // print some|Func(1, 2, "hello", "world")
+            let hover = program.getHover('source/main.brs', util.createPosition(6, 31))[0];
+            expect(hover?.range).to.eql(util.createRange(6, 26, 6, 34));
+            expect(hover?.contents).to.eql([fence('someFunc as function')]);
+        });
     });
 });
diff --git a/src/bscPlugin/hover/HoverProcessor.ts b/src/bscPlugin/hover/HoverProcessor.ts
@@ -1,5 +1,5 @@
 import { SourceNode } from 'source-map';
-import { isBrsFile, isClassType, isFunctionType, isInterfaceType, isNewExpression, isTypeExpression, isXmlFile } from '../../astUtils/reflection';
+import { isBrsFile, isClassType, isInterfaceType, isNewExpression, isTypeExpression, isTypedFunctionType, isXmlFile } from '../../astUtils/reflection';
 import type { BrsFile } from '../../files/BrsFile';
 import type { XmlFile } from '../../files/XmlFile';
 import type { Hover, ProvideHoverEvent, TypeChainEntry } from '../../interfaces';
@@ -12,7 +12,7 @@ import { SymbolTypeFlag } from '../../SymbolTable';
 import type { Expression } from '../../parser/AstNode';
 import type { Scope } from '../../Scope';
 import type { FunctionScope } from '../../FunctionScope';
-import type { FunctionType } from '../../types/FunctionType';
+import type { TypedFunctionType } from '../../types/TypedFunctionType';
 import type { ClassType } from '../../types/ClassType';
 import type { InterfaceType } from '../../types/InterfaceType';
 
@@ -84,7 +84,7 @@ export class HoverProcessor {
         }
     }
 
-    private getFunctionTypeHover(token: Token, expression: Expression, expressionType: FunctionType, scope: Scope) {
+    private getFunctionTypeHover(token: Token, expression: Expression, expressionType: TypedFunctionType, scope: Scope) {
         const lowerTokenText = token.text.toLowerCase();
         let result = fence(expressionType.toString());
 
@@ -158,7 +158,7 @@ export class HoverProcessor {
                 const fullName = processedTypeChain.fullNameOfItem || token.text;
                 const useCustomTypeHover = isInTypeExpression || expression?.findAncestor(isNewExpression);
                 let hoverContent = fence(`${fullName} as ${exprType.toString()}`);
-                if (isFunctionType(exprType)) {
+                if (isTypedFunctionType(exprType)) {
                     exprType.setName(fullName);
                     hoverContent = this.getFunctionTypeHover(token, expression, exprType, scope);
                 } else if (useCustomTypeHover && (isClassType(exprType) || isInterfaceType(exprType))) {

diff --git a/src/bscPlugin/validation/ScopeValidator.spec.ts b/src/bscPlugin/validation/ScopeValidator.spec.ts
@@ -125,6 +125,18 @@ describe('ScopeValidator', () => {
                 DiagnosticMessages.mismatchArgumentCount(1, 2)
             ]);
         });
+
+
+        it('allows any number of parameters in a function used as an argument', () => {
+            program.setFile('source/file.brs', `
+                    sub tryManyParams(someFunc as function)
+                        someFunc(1, 2, "hello", "world")
+                    end sub
+                `);
+            program.validate();
+            //should have no errors
+            expectZeroDiagnostics(program);
+        });
     });
 
     describe('argumentTypeMismatch', () => {
@@ -202,6 +214,7 @@ describe('ScopeValidator', () => {
 
                 sub printFunction(value as function)
                     print value
+                    print value(1)
                 end sub
 
                 interface Video
@@ -660,5 +673,59 @@ describe('ScopeValidator', () => {
                 DiagnosticMessages.argumentTypeMismatch('integer', 'string').message
             ]);
         });
+
+        it('allows any parameter types in a function passed as an argument', () => {
+            program.setFile('source/file.brs', `
+                    function getStrLength(name as string) as integer
+                        return len(name)
+                    end function
+
+                    sub tryManyParams(someFunc as function)
+                        print someFunc(1, 2, "hello", "world")
+                    end sub
+
+                    sub test()
+                        tryManyParams(getStrLength)
+                    end sub
+                `);
+            program.validate();
+            //should have no errors
+            expectZeroDiagnostics(program);
+        });
+
+        it('allows a inline function as an argument of type function', () => {
+            program.setFile('source/file.brs', `
+                    sub tryManyParams(someFunc as function)
+                        print someFunc(1, 2, "hello", "world")
+                    end sub
+
+                    sub test()
+                        tryManyParams(sub (i as integer)
+                            print i
+                        end sub)
+                    end sub
+                `);
+            program.validate();
+            //should have no errors
+            expectZeroDiagnostics(program);
+        });
+
+        it('validates when a non-function is used as an argument expecting a function', () => {
+            program.setFile('source/file.brs', `
+                    sub tryManyParams(someFunc as function)
+                        print someFunc(1, 2, "hello", "world")
+                    end sub
+
+                    sub test()
+                        notAFunction = 3.14
+                        tryManyParams(notAFunction)
+                    end sub
+                `);
+            program.validate();
+            //should have an error that the argument is not a function
+            expectDiagnostics(program, [
+                DiagnosticMessages.argumentTypeMismatch('float', 'function').message
+            ]);
+        });
     });
 });
diff --git a/src/bscPlugin/validation/ScopeValidator.ts b/src/bscPlugin/validation/ScopeValidator.ts
@@ -1,5 +1,5 @@
 import { URI } from 'vscode-uri';
-import { isBinaryExpression, isBrsFile, isFunctionType, isLiteralExpression, isNamespaceStatement, isTypeExpression, isXmlScope } from '../../astUtils/reflection';
+import { isBinaryExpression, isBrsFile, isLiteralExpression, isNamespaceStatement, isTypeExpression, isTypedFunctionType, isXmlScope } from '../../astUtils/reflection';
 import { Cache } from '../../Cache';
 import { DiagnosticMessages } from '../../DiagnosticMessages';
 import type { BrsFile } from '../../files/BrsFile';
@@ -384,7 +384,7 @@ export class ScopeValidator {
         //validate all function calls
         for (let expCall of file.functionCalls) {
             const funcType = expCall.expression?.callee?.getType({ flags: SymbolTypeFlag.runtime });
-            if (funcType?.isResolvable() && isFunctionType(funcType)) {
+            if (funcType?.isResolvable() && isTypedFunctionType(funcType)) {
                 funcType.setName(expCall.name);
 
                 //get min/max parameter count for callable

diff --git a/src/files/BrsFile.spec.ts b/src/files/BrsFile.spec.ts
@@ -5,7 +5,7 @@ import type { Callable, CommentFlag, VariableDeclaration } from '../interfaces';
 import { Program } from '../Program';
 import { BooleanType } from '../types/BooleanType';
 import { DynamicType } from '../types/DynamicType';
-import { FunctionType } from '../types/FunctionType';
+import { TypedFunctionType } from '../types/TypedFunctionType';
 import { IntegerType } from '../types/IntegerType';
 import { StringType } from '../types/StringType';
 import { BrsFile } from './BrsFile';
@@ -1107,6 +1107,7 @@ describe('BrsFile', () => {
             file.parse(`
                 sub Main()
                     doWork = function(callback as function)
+                        callback()
                     end function
                 end sub
             `);
@@ -1698,7 +1699,7 @@ describe('BrsFile', () => {
                 lineIndex: 2,
                 name: 'sayHi'
             });
-            expect(file.functionScopes[0].variableDeclarations[0].getType()).instanceof(FunctionType);
+            expect(file.functionScopes[0].variableDeclarations[0].getType()).instanceof(TypedFunctionType);
 
             expect(file.functionScopes[1].variableDeclarations).to.be.length(1);
             expect(file.functionScopes[1].variableDeclarations[0]).to.deep.include(<VariableDeclaration>{

diff --git a/src/files/BrsFile.ts b/src/files/BrsFile.ts
@@ -22,7 +22,7 @@ import { BrsTranspileState } from '../parser/BrsTranspileState';
 import { Preprocessor } from '../preprocessor/Preprocessor';
 import { LogLevel } from '../Logger';
 import { serializeError } from 'serialize-error';
-import { isMethodStatement, isClassStatement, isDottedGetExpression, isFunctionStatement, isFunctionType, isLiteralExpression, isNamespaceStatement, isStringType, isVariableExpression, isXmlFile, isImportStatement, isFieldStatement, isEnumStatement, isConstStatement, isFunctionExpression } from '../astUtils/reflection';
+import { isMethodStatement, isClassStatement, isDottedGetExpression, isFunctionStatement, isLiteralExpression, isNamespaceStatement, isStringType, isVariableExpression, isXmlFile, isImportStatement, isFieldStatement, isEnumStatement, isConstStatement, isFunctionExpression, isCallableType } from '../astUtils/reflection';
 import { createVisitor, WalkMode } from '../astUtils/visitors';
 import type { DependencyGraph } from '../DependencyGraph';
 import { CommentFlagProcessor } from '../CommentFlagProcessor';
@@ -812,7 +812,7 @@ export class BrsFile {
                 names[variable.name.toLowerCase()] = true;
                 result.push({
                     label: variable.name,
-                    kind: isFunctionType(variable.getType()) ? CompletionItemKind.Function : CompletionItemKind.Variable
+                    kind: isCallableType(variable.getType()) ? CompletionItemKind.Function : CompletionItemKind.Variable
                 });
             }