Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes Class Constructor used as function transpilation and validation #972

Merged
merged 2 commits into from
Nov 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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