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.

Sign up for GitHub

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

Fix class transpile issue with child class constructor not inherriting parent params #1390

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Fix class transpile issue with child class constructor not inherriting parent params #1390

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
11626af
Add tests
TwitchBronBron Jan 6, 2025
e94e807
Fix test
luis-soares-sky Jan 6, 2025
9f829f2
Make sure the builder works
luis-soares-sky Jan 6, 2025
6f1cb12
Make sure the class function works too
luis-soares-sky Jan 6, 2025
60ccc63
Fix bug with nested classes which don't contain "new"
luis-soares-sky Jan 6, 2025
cee7507
Fix some test cases which expected the bug to happen
luis-soares-sky Jan 6, 2025
780363a
Fix crash, update test case
luis-soares-sky Jan 6, 2025
9bd7f77
Adjusted the test to account for mid-level ctor change
luis-soares-sky Jan 6, 2025
ce8c570
Fix lint issues
luis-soares-sky Jan 7, 2025
c71189b
Fix issue with missing ancestors when crossing different namespaces
luis-soares-sky Jan 10, 2025
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
97 changes: 84 additions & 13 deletions src/files/BrsFile.Class.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,7 @@ describe('BrsFile BrighterScript classes', () => {
`, undefined, 'source/main.bs');
});

it('works for simple class', () => {
it('works for simple class', () => {
testTranspile(`
class Duck
end class
Expand All @@ -470,6 +470,77 @@ describe('BrsFile BrighterScript classes', () => {
`, undefined, 'source/main.bs');
});

it('inherits the parameters of the last known constructor', () => {
testTranspile(`
class Animal
sub new(p1)
end sub
end class
class Bird extends Animal
end class
class Duck extends Bird
sub new(p1, p2)
super(p1)
m.p2 = p2
end sub
end class
class BabyDuck extends Duck
end class
`, `
function __Animal_builder()
instance = {}
instance.new = sub(p1)
end sub
return instance
end function
function Animal(p1)
instance = __Animal_builder()
instance.new(p1)
return instance
end function
function __Bird_builder()
instance = __Animal_builder()
instance.super0_new = instance.new
instance.new = sub(p1)
m.super0_new(p1)
end sub
return instance
end function
function Bird(p1)
instance = __Bird_builder()
instance.new(p1)
return instance
end function
function __Duck_builder()
instance = __Bird_builder()
instance.super1_new = instance.new
instance.new = sub(p1, p2)
m.super1_new(p1)
m.p2 = p2
end sub
return instance
end function
function Duck(p1, p2)
instance = __Duck_builder()
instance.new(p1, p2)
return instance
end function
function __BabyDuck_builder()
instance = __Duck_builder()
instance.super2_new = instance.new
instance.new = sub(p1, p2)
m.super2_new(p1, p2)
end sub
return instance
end function
function BabyDuck(p1, p2)
instance = __BabyDuck_builder()
instance.new(p1, p2)
return instance
end function
`);
});

it('registers the constructor and properly handles its parameters', () => {
testTranspile(`
class Duck
Expand Down Expand Up @@ -569,8 +640,8 @@ describe('BrsFile BrighterScript classes', () => {
function __Duck_builder()
instance = __Creature_builder()
instance.super0_new = instance.new
instance.new = sub()
m.super0_new()
instance.new = sub(name as string)
m.super0_new(name)
end sub
instance.super0_sayHello = instance.sayHello
instance.sayHello = function(text)
Expand All @@ -581,9 +652,9 @@ describe('BrsFile BrighterScript classes', () => {
end function
return instance
end function
function Duck()
function Duck(name as string)
instance = __Duck_builder()
instance.new()
instance.new(name)
return instance
end function
`, 'trim', 'source/main.bs'
Expand Down Expand Up @@ -774,8 +845,8 @@ describe('BrsFile BrighterScript classes', () => {
function __Duck_builder()
instance = __Animal_builder()
instance.super0_new = instance.new
instance.new = sub()
m.super0_new()
instance.new = sub(name as string)
m.super0_new(name)
end sub
instance.super0_move = instance.move
instance.move = sub(distanceInMeters as integer)
Expand All @@ -784,16 +855,16 @@ describe('BrsFile BrighterScript classes', () => {
end sub
return instance
end function
function Duck()
function Duck(name as string)
instance = __Duck_builder()
instance.new()
instance.new(name)
return instance
end function
function __BabyDuck_builder()
instance = __Duck_builder()
instance.super1_new = instance.new
instance.new = sub()
m.super1_new()
instance.new = sub(name as string)
m.super1_new(name)
end sub
instance.super1_move = instance.move
instance.move = sub(distanceInMeters as integer)
Expand All @@ -802,9 +873,9 @@ describe('BrsFile BrighterScript classes', () => {
end sub
return instance
end function
function BabyDuck()
function BabyDuck(name as string)
instance = __BabyDuck_builder()
instance.new()
instance.new(name)
return instance
end function

Expand Down
65 changes: 57 additions & 8 deletions src/parser/Statement.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* eslint-disable no-bitwise */
import type { Token, Identifier } from '../lexer/Token';
import { CompoundAssignmentOperators, TokenKind } from '../lexer/TokenKind';
import type { BinaryExpression, NamespacedVariableNameExpression, FunctionExpression, FunctionParameterExpression, LiteralExpression } from './Expression';
import { type BinaryExpression, type NamespacedVariableNameExpression, FunctionExpression, type FunctionParameterExpression, type LiteralExpression } from './Expression';
import { CallExpression, VariableExpression } from './Expression';
import { util } from '../util';
import type { Range } from 'vscode-languageserver';
Expand All @@ -11,7 +11,7 @@ import type { WalkVisitor, WalkOptions } from '../astUtils/visitors';
import { InternalWalkMode, walk, createVisitor, WalkMode, walkArray } from '../astUtils/visitors';
import { isCallExpression, isCommentStatement, isEnumMemberStatement, isExpression, isExpressionStatement, isFieldStatement, isFunctionExpression, isFunctionStatement, isIfStatement, isInterfaceFieldStatement, isInterfaceMethodStatement, isInvalidType, isLiteralExpression, isMethodStatement, isNamespaceStatement, isTypedefProvider, isUnaryExpression, isVoidType } from '../astUtils/reflection';
import type { TranspileResult, TypedefProvider } from '../interfaces';
import { createInvalidLiteral, createMethodStatement, createToken } from '../astUtils/creators';
import { createIdentifier, createInvalidLiteral, createMethodStatement, createToken } from '../astUtils/creators';
import { DynamicType } from '../types/DynamicType';
import type { BscType } from '../types/BscType';
import type { TranspileState } from './TranspileState';
Expand Down Expand Up @@ -2144,7 +2144,7 @@ export class ClassStatement extends Statement implements TypedefProvider {
let stmt = this as ClassStatement;
while (stmt) {
if (stmt.parentClassName) {
const namespace = this.findAncestor<NamespaceStatement>(isNamespaceStatement);
const namespace = stmt.findAncestor<NamespaceStatement>(isNamespaceStatement);
stmt = state.file.getClassFileLink(
stmt.parentClassName.getName(ParseMode.BrighterScript),
namespace?.getName(ParseMode.BrighterScript)
Expand Down Expand Up @@ -2173,6 +2173,21 @@ export class ClassStatement extends Statement implements TypedefProvider {
}) as MethodStatement;
}

/**
* Return the parameters for the first constructor function for this class
* @param ancestors The list of ancestors for this class
* @returns The parameters for the first constructor function for this class
*/
private getConstructorParams(ancestors: ClassStatement[]) {
for (let ancestor of ancestors) {
const ctor = ancestor?.getConstructorFunction();
if (ctor) {
return ctor.func.parameters;
}
}
return [];
}

/**
* Determine if the specified field was declared in one of the ancestor classes
*/
Expand Down Expand Up @@ -2226,10 +2241,38 @@ export class ClassStatement extends Statement implements TypedefProvider {
let body = this.body;
//inject an empty "new" method if missing
if (!this.getConstructorFunction()) {
body = [
createMethodStatement('new', TokenKind.Sub),
...this.body
];
if (ancestors.length === 0) {
body = [
createMethodStatement('new', TokenKind.Sub),
...this.body
];
} else {
const params = this.getConstructorParams(ancestors);
const call = new ExpressionStatement(
new CallExpression(
new VariableExpression(createToken(TokenKind.Identifier, 'super')),
createToken(TokenKind.LeftParen),
createToken(TokenKind.RightParen),
params.map(x => new VariableExpression(x.name))
)
);
body = [
new MethodStatement(
[],
createIdentifier('new'),
new FunctionExpression(
params.map(x => x.clone()),
new Block([call]),
createToken(TokenKind.Sub),
createToken(TokenKind.EndSub),
createToken(TokenKind.LeftParen),
createToken(TokenKind.RightParen)
),
null
),
...this.body
];
}
}

for (let statement of body) {
Expand Down Expand Up @@ -2289,8 +2332,14 @@ export class ClassStatement extends Statement implements TypedefProvider {
*/
private getTranspiledClassFunction(state: BrsTranspileState) {
let result = [] as TranspileResult;

const constructorFunction = this.getConstructorFunction();
const constructorParams = constructorFunction ? constructorFunction.func.parameters : [];
let constructorParams = [];
if (constructorFunction) {
constructorParams = constructorFunction.func.parameters;
} else {
constructorParams = this.getConstructorParams(this.getAncestors(state));
}

result.push(
state.sourceNode(this.classKeyword, 'function'),
Expand Down
Loading