Skip to content

Commit

Permalink
Prevent unused variable warnings on ternary and null coalescence expr…
Browse files Browse the repository at this point in the history
…essions (#1101)

* Prevent unused variable warnings on expressions

* Update src/parser/Expression.ts

---------

Co-authored-by: Bronley Plumb <bronley@gmail.com>
  • Loading branch information
agohof and TwitchBronBron authored Mar 13, 2024
1 parent 54cdd42 commit 1c2133d
Show file tree
Hide file tree
Showing 4 changed files with 112 additions and 8 deletions.
13 changes: 7 additions & 6 deletions src/parser/Expression.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1393,8 +1393,9 @@ export class TernaryExpression extends Expression {

transpile(state: BrsTranspileState) {
let result = [] as TranspileResult;
let consequentInfo = util.getExpressionInfo(this.consequent!);
let alternateInfo = util.getExpressionInfo(this.alternate!);
const file = state.file;
let consequentInfo = util.getExpressionInfo(this.consequent!, file);
let alternateInfo = util.getExpressionInfo(this.alternate!, file);

//get all unique variable names used in the consequent and alternate, and sort them alphabetically so the output is consistent
let allUniqueVarNames = [...new Set([...consequentInfo.uniqueVarNames, ...alternateInfo.uniqueVarNames])].sort();
Expand All @@ -1409,7 +1410,7 @@ export class TernaryExpression extends Expression {
this.questionMarkToken,
//write all the scope variables as parameters.
//TODO handle when there are more than 31 parameters
`(function(__bsCondition, ${allUniqueVarNames.join(', ')})`
`(function(${['__bsCondition', ...allUniqueVarNames].join(', ')})`
),
state.newline,
//double indent so our `end function` line is still indented one at the end
Expand All @@ -1433,7 +1434,7 @@ export class TernaryExpression extends Expression {
state.indent(-1),
state.sourceNode(this.questionMarkToken, 'end function)('),
...this.test.transpile(state),
state.sourceNode(this.questionMarkToken, `, ${allUniqueVarNames.join(', ')})`)
state.sourceNode(this.questionMarkToken, `${['', ...allUniqueVarNames].join(', ')})`)
);
state.blockDepth--;
} else {
Expand Down Expand Up @@ -1476,8 +1477,8 @@ export class NullCoalescingExpression extends Expression {

transpile(state: BrsTranspileState) {
let result = [] as TranspileResult;
let consequentInfo = util.getExpressionInfo(this.consequent);
let alternateInfo = util.getExpressionInfo(this.alternate);
let consequentInfo = util.getExpressionInfo(this.consequent, state.file);
let alternateInfo = util.getExpressionInfo(this.alternate, state.file);

//get all unique variable names used in the consequent and alternate, and sort them alphabetically so the output is consistent
let allUniqueVarNames = [...new Set([...consequentInfo.uniqueVarNames, ...alternateInfo.uniqueVarNames])].sort();
Expand Down
46 changes: 46 additions & 0 deletions src/parser/tests/expression/NullCoalescenceExpression.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -280,5 +280,51 @@ describe('NullCoalescingExpression', () => {
end sub
`);
});

it('ignores enum variable names', () => {
testTranspile(`
enum Direction
up = "up"
end enum
sub main()
d = invalid
a = d ?? Direction.up
end sub
`, `
sub main()
d = invalid
a = (function(d)
__bsConsequent = d
if __bsConsequent <> invalid then
return __bsConsequent
else
return "up"
end if
end function)(d)
end sub
`);
});

it('ignores const variable names', () => {
testTranspile(`
const USER = "user"
sub main()
settings = {}
a = m.defaults.getAccount(settings.name) ?? USER
end sub
`, `
sub main()
settings = {}
a = (function(m, settings)
__bsConsequent = m.defaults.getAccount(settings.name)
if __bsConsequent <> invalid then
return __bsConsequent
else
return "user"
end if
end function)(m, settings)
end sub
`);
});
});
});
49 changes: 49 additions & 0 deletions src/parser/tests/expression/TernaryExpression.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,55 @@ describe('ternary expressions', () => {
`);
});

it('ignores enum variable names', () => {
testTranspile(`
enum Direction
up = "up"
down = "down"
end enum
sub main()
d = Direction.up
theDir = d = Direction.up ? Direction.up : false
end sub
`, `
sub main()
d = "up"
theDir = (function(__bsCondition)
if __bsCondition then
return "up"
else
return false
end if
end function)(d = "up")
end sub
`);
});

it('ignores const variable names', () => {
testTranspile(`
enum Direction
up = "up"
down = "down"
end enum
const UP = "up"
sub main()
d = Direction.up
theDir = d = Direction.up ? UP : Direction.down
end sub
`, `
sub main()
d = "up"
theDir = (function(__bsCondition)
if __bsCondition then
return "up"
else
return "down"
end if
end function)(d = "up")
end sub
`);
});

it('supports scope-captured outer, and simple inner', () => {
testTranspile(
`
Expand Down
12 changes: 10 additions & 2 deletions src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1189,7 +1189,7 @@ export class Util {
* Gathers expressions, variables, and unique names from an expression.
* This is mostly used for the ternary expression
*/
public getExpressionInfo(expression: Expression): ExpressionInfo {
public getExpressionInfo(expression: Expression, file: BrsFile): ExpressionInfo {
const expressions = [expression];
const variableExpressions = [] as VariableExpression[];
const uniqueVarNames = new Set<string>();
Expand All @@ -1213,7 +1213,15 @@ export class Util {
//handle the expression itself (for situations when expression is a VariableExpression)
expressionWalker(expression);

return { expressions: expressions, varExpressions: variableExpressions, uniqueVarNames: [...uniqueVarNames] };
const scope = file.program.getFirstScopeForFile(file);
const filteredVarNames = [...uniqueVarNames].filter((varName: string) => {
const varNameLower = varName.toLowerCase();
// TODO: include namespaces in this filter
return !scope.getEnumMap().has(varNameLower) &&
!scope.getConstMap().has(varNameLower);
});

return { expressions: expressions, varExpressions: variableExpressions, uniqueVarNames: filteredVarNames };
}


Expand Down

0 comments on commit 1c2133d

Please sign in to comment.