Skip to content

Commit

Permalink
Fixes Class Constructor used as function transpilation and validati…
Browse files Browse the repository at this point in the history
…on (#972)

* Created failing test

* Class constructors can be passed as arguments when parameter expects a function
  • Loading branch information
markwpearce authored Nov 30, 2023
1 parent be2236d commit d850411
Show file tree
Hide file tree
Showing 5 changed files with 205 additions and 4 deletions.
36 changes: 36 additions & 0 deletions src/bscPlugin/validation/ScopeValidator.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -786,6 +786,42 @@ describe('ScopeValidator', () => {
]);
});

it('allows a class constructor to be passed as arg to param typed `as function`', () => {
program.setFile('source/file.bs', `
sub callSomeFunc(someFunc as function)
someFunc()
end sub
class MyKlass
end class
sub doStuff()
callSomeFunc(MyKlass)
end sub
`);
program.validate();
expectZeroDiagnostics(program);
});

it('allows a namespaced class constructor to be passed as arg to param typed `as function`', () => {
program.setFile('source/file.bs', `
sub callSomeFunc(someFunc as function)
someFunc()
end sub
namespace Alpha
class MyKlass
end class
sub doStuff()
callSomeFunc(MyKlass)
end sub
end namespace
`);
program.validate();
expectZeroDiagnostics(program);
});

it('allows any variable to passed as argument to an untyped param with default type invalid', () => {
program.setFile('source/util.brs', `
sub doSomething(x = invalid)
Expand Down
17 changes: 14 additions & 3 deletions src/bscPlugin/validation/ScopeValidator.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import { URI } from 'vscode-uri';
import { isAssignmentStatement, isBrsFile, isClassType, isDottedGetExpression, isDynamicType, isEnumMemberType, isEnumType, isFunctionExpression, isLiteralExpression, isNamespaceStatement, isNamespaceType, isObjectType, isPrimitiveType, isTypedFunctionType, isUnionType, isVariableExpression, isXmlScope } from '../../astUtils/reflection';
import { isAssignmentStatement, isBrsFile, isCallableType, isClassStatement, isClassType, isDottedGetExpression, isDynamicType, isEnumMemberType, isEnumType, isFunctionExpression, isLiteralExpression, isNamespaceStatement, isNamespaceType, isObjectType, isPrimitiveType, isTypedFunctionType, isUnionType, isVariableExpression, isXmlScope } from '../../astUtils/reflection';
import { Cache } from '../../Cache';
import { DiagnosticMessages } from '../../DiagnosticMessages';
import type { BrsFile } from '../../files/BrsFile';
import { DiagnosticOrigin } from '../../interfaces';
import type { BsDiagnostic, BsDiagnosticWithOrigin, ExtraSymbolData, OnScopeValidateEvent, TypeChainEntry, TypeCompatibilityData } from '../../interfaces';
import { SymbolTypeFlag } from '../../SymbolTable';
import type { AssignmentStatement, DottedSetStatement, EnumStatement, ReturnStatement } from '../../parser/Statement';
import type { AssignmentStatement, DottedSetStatement, EnumStatement, NamespaceStatement, ReturnStatement } from '../../parser/Statement';
import util from '../../util';
import { nodes, components } from '../../roku-types';
import type { BRSComponentData } from '../../roku-types';
Expand All @@ -20,6 +20,7 @@ import type { BscType } from '../../types';
import type { File } from '../../files/File';
import { InsideSegmentWalkMode } from '../../AstValidationSegmenter';
import { TokenKind } from '../../lexer/TokenKind';
import { ParseMode } from '../../parser/Parser';

/**
* The lower-case names of all platform-included scenegraph nodes
Expand Down Expand Up @@ -311,13 +312,23 @@ export class ScopeValidator {
}
let paramIndex = 0;
for (let arg of expression.args) {
const argType = arg.getType({ flags: SymbolTypeFlag.runtime });
const data = {} as ExtraSymbolData;
let argType = arg.getType({ flags: SymbolTypeFlag.runtime, data: data });

const paramType = funcType.params[paramIndex]?.type;
if (!paramType) {
// unable to find a paramType -- maybe there are more args than params
break;
}

if (isCallableType(paramType) && isClassType(argType) && isClassStatement(data.definingNode)) {
// the param is expecting a function, but we're passing a Class... are we actually passing the constructor? then we're ok!
const namespace = expression.findAncestor<NamespaceStatement>(isNamespaceStatement);
if (file.calleeIsKnownFunction(arg, namespace?.getName(ParseMode.BrighterScript))) {
argType = data.definingNode.getConstructorType();
}
}

const compatibilityData: TypeCompatibilityData = {};
if (!paramType?.isTypeCompatible(argType, compatibilityData)) {
this.addMultiScopeDiagnostic({
Expand Down
124 changes: 123 additions & 1 deletion src/files/BrsFile.Class.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1064,6 +1064,128 @@ describe('BrsFile BrighterScript classes', () => {
end function
`, 'trim', 'source/main.bs');
});

it('allows namespaced class function as function parameters', async () => {
await testTranspile(`
namespace Alpha
function foo()
return 1
end function
function callSomeFunc(f as function)
return f()
end function
sub callFoo()
callSomeFunc(foo)
end sub
end namespace
`, `
function Alpha_foo()
return 1
end function
function Alpha_callSomeFunc(f as function)
return f()
end function
sub Alpha_callFoo()
Alpha_callSomeFunc(Alpha_foo)
end sub
`, 'trim', 'source/main.bs');
});

it('allows namespaced class constructors as function parameters', async () => {
await testTranspile(`
namespace Alpha
class Button
end class
function callSomeFunc(f as function)
return f()
end function
sub makeButton()
callSomeFunc(Button)
end sub
end namespace
`, `
function __Alpha_Button_builder()
instance = {}
instance.new = sub()
end sub
return instance
end function
function Alpha_Button()
instance = __Alpha_Button_builder()
instance.new()
return instance
end function
function Alpha_callSomeFunc(f as function)
return f()
end function
sub Alpha_makeButton()
Alpha_callSomeFunc(Alpha_Button)
end sub
`, 'trim', 'source/main.bs');
});

it('allows class constructors as functions in array', async () => {
await testTranspile(`
namespace Alpha
class Button
end class
class ButtonContainer
private button = new Alpha.Button()
sub new()
m.init()
end sub
sub init()
button = new Alpha.Button()
items = [m.button, button, Alpha.Button]
end sub
end class
end namespace
`, `
function __Alpha_Button_builder()
instance = {}
instance.new = sub()
end sub
return instance
end function
function Alpha_Button()
instance = __Alpha_Button_builder()
instance.new()
return instance
end function
function __Alpha_ButtonContainer_builder()
instance = {}
instance.new = sub()
m.button = Alpha_Button()
m.init()
end sub
instance.init = sub()
button = Alpha_Button()
items = [
m.button
Alpha_button
Alpha_Button
]
end sub
return instance
end function
function Alpha_ButtonContainer()
instance = __Alpha_ButtonContainer_builder()
instance.new()
return instance
end function
`, 'trim', 'source/main.bs');
});
});

it('detects using `new` keyword on non-classes', () => {
Expand Down Expand Up @@ -1740,7 +1862,7 @@ describe('BrsFile BrighterScript classes', () => {
program.validate();
});

it.skip('detects calling class constructors with too many parameters', () => {
it('detects calling class constructors with too many parameters', () => {
program.setFile('source/main.bs', `
class Parameterless
sub new()
Expand Down
26 changes: 26 additions & 0 deletions src/files/BrsFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -897,6 +897,32 @@ export class BrsFile implements File {
if (namespace.functionStatements.has(lowerCalleeName)) {
return true;
}
if (namespace.classStatements.has(lowerCalleeName)) {
return true;
}
}
}
}
return false;
}

/**
* Determine if the callee (i.e. function name) is a known function
*/
public calleeIsKnownFunction(callee: Expression, namespaceName?: string) {
//if we have a variable and a namespace
if (isVariableExpression(callee)) {
if (namespaceName) {
return this.calleeIsKnownNamespaceFunction(callee, namespaceName);
}
let scopes = this.program.getScopesForFile(this);
let lowerCalleeName = callee?.name?.text?.toLowerCase();
for (let scope of scopes) {
if (scope.getCallableByName(lowerCalleeName)) {
return true;
}
if (scope.getClass(lowerCalleeName)) {
return true;
}
}
}
Expand Down
6 changes: 6 additions & 0 deletions src/parser/Statement.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1953,6 +1953,12 @@ export class ClassStatement extends Statement implements TypedefProvider {
return `__${name}_builder`;
}

public getConstructorType() {
const constructorType = this.getConstructorFunction()?.getType({ flags: SymbolTypeFlag.runtime }) ?? new TypedFunctionType(null);
constructorType.returnType = this.getType({ flags: SymbolTypeFlag.runtime });
return constructorType;
}

/**
* Get the constructor function for this class (if exists), or undefined if not exist
*/
Expand Down

0 comments on commit d850411

Please sign in to comment.