Skip to content

Commit

Permalink
Added test for throw in conditional compile block (#114)
Browse files Browse the repository at this point in the history
  • Loading branch information
@markwpearce
markwpearce authored Jul 2, 2024
1 parent 11e948d commit eeb2af4
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 11 deletions.
1 change: 1 addition & 0 deletions src/plugins/trackCodeFlow/index.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,7 @@ describe('trackCodeFlow', () => {
const expected = [
`04:LINT2002:Sub as void should not return a value`,
`11:LINT2002:Function as void should not return a value`,
`151:LINT2004:Not all code paths return a value`,
`15:LINT2006:Sub should consistently return a value`,
`18:LINT2004:Not all code paths return a value`,
`22:LINT2006:Function should consistently return a value`,
Expand Down
35 changes: 30 additions & 5 deletions src/plugins/trackCodeFlow/index.ts
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { BsDiagnostic, BrsFile, OnGetCodeActionsEvent, Statement, EmptyStatement, FunctionExpression, isForEachStatement, isForStatement, isIfStatement, isWhileStatement, createStackedVisitor, isBrsFile, isStatement, isExpression, WalkMode, isTryCatchStatement, isCatchStatement, CompilerPlugin, AfterScopeValidateEvent, AfterFileValidateEvent, util, isFunctionExpression } from 'brighterscript';
import { BsDiagnostic, BrsFile, OnGetCodeActionsEvent, Statement, EmptyStatement, FunctionExpression, isForEachStatement, isForStatement, isIfStatement, isWhileStatement, createStackedVisitor, isBrsFile, isStatement, isExpression, WalkMode, isTryCatchStatement, isCatchStatement, CompilerPlugin, AfterScopeValidateEvent, AfterFileValidateEvent, util, isFunctionExpression, InternalWalkMode, isConditionalCompileStatement } from 'brighterscript';
import { PluginContext } from '../../util';
import { createReturnLinter } from './returnTracking';
import { createVarLinter, resetVarContext, runDeferredValidation } from './varTracking';
Expand Down Expand Up @@ -50,6 +50,7 @@ export interface LintState {
ifs?: StatementInfo;
trys?: StatementInfo;
branch?: StatementInfo;
conditionalCompiles?: StatementInfo;
}

export default class TrackCodeFlow implements CompilerPlugin {
Expand Down Expand Up @@ -92,7 +93,8 @@ export default class TrackCodeFlow implements CompilerPlugin {
blocks: new WeakMap(),
ifs: undefined,
trys: undefined,
branch: undefined
branch: undefined,
conditionalCompiles: undefined
};
let curr: StatementInfo = {
stat: new EmptyStatement()
Expand All @@ -119,9 +121,11 @@ export default class TrackCodeFlow implements CompilerPlugin {

if (isIfStatement(opened)) {
state.ifs = curr;
} else if (isConditionalCompileStatement(opened)) {
state.conditionalCompiles = curr;
} else if (isTryCatchStatement(opened)) {
state.trys = curr;
} else if (!curr.parent || isIfStatement(curr.parent) || isTryCatchStatement(curr.parent) || isCatchStatement(curr.parent)) {
} else if (!curr.parent || isIfStatement(curr.parent) || isConditionalCompileStatement(curr.parent) || isTryCatchStatement(curr.parent) || isCatchStatement(curr.parent)) {
state.branch = curr;
}
state.parent = curr;
Expand All @@ -132,6 +136,10 @@ export default class TrackCodeFlow implements CompilerPlugin {
const { ifs, branch } = findIfBranch(state);
state.ifs = ifs;
state.branch = branch;
} else if (isConditionalCompileStatement(closed)) {
const { conditionalCompiles, branch } = findConditionalCompileBranch(state);
state.conditionalCompiles = conditionalCompiles;
state.branch = branch;
} else if (isTryCatchStatement(closed)) {
const { trys, branch } = findTryBranch(state);
state.trys = trys;
Expand All @@ -154,7 +162,7 @@ export default class TrackCodeFlow implements CompilerPlugin {
} else if (parent) {
varLinter.visitExpression(elem, parent, curr);
}
}, { walkMode: WalkMode.visitStatements | WalkMode.visitExpressions });
}, { walkMode: WalkMode.visitStatements | WalkMode.visitExpressions | InternalWalkMode.visitFalseConditionalCompilationBlocks });
} else {
// ensure empty functions are finalized
state.blocks.set(fun.body, curr);
Expand Down Expand Up @@ -201,6 +209,23 @@ function findIfBranch(state: LintState): { ifs?: StatementInfo; branch?: Stateme
};
}

// Find parent if and block where code flow is branched
function findConditionalCompileBranch(state: LintState): { conditionalCompiles?: StatementInfo; branch?: StatementInfo } {
const { blocks, parent, stack } = state;
for (let i = stack.length - 2; i >= 0; i--) {
if (isConditionalCompileStatement(stack[i])) {
return {
conditionalCompiles: blocks.get(stack[i]),
branch: blocks.get(stack[i + 1])
};
}
}
return {
conditionalCompiles: undefined,
branch: parent
};
}

// Find parent try and block where code flow is branched
function findTryBranch(state: LintState): { trys?: StatementInfo; branch?: StatementInfo } {
const { blocks, parent, stack } = state;
Expand All @@ -220,5 +245,5 @@ function findTryBranch(state: LintState): { trys?: StatementInfo; branch?: State

// `if` and `for/while` are considered as multi-branch
export function isBranchedStatement(stat: Statement) {
return isIfStatement(stat) || isForStatement(stat) || isForEachStatement(stat) || isWhileStatement(stat) || isTryCatchStatement(stat);
return isIfStatement(stat) || isForStatement(stat) || isForEachStatement(stat) || isWhileStatement(stat) || isTryCatchStatement(stat) || isConditionalCompileStatement(stat);
}
14 changes: 8 additions & 6 deletions src/plugins/trackCodeFlow/returnTracking.ts
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { BscFile, FunctionExpression, BsDiagnostic, DiagnosticTag, isReturnStatement, isIfStatement, isThrowStatement, TokenKind, util, ReturnStatement, ThrowStatement, isTryCatchStatement, isCatchStatement, isVoidType, SymbolTypeFlag } from 'brighterscript';
import { BscFile, FunctionExpression, BsDiagnostic, DiagnosticTag, isReturnStatement, isIfStatement, isThrowStatement, TokenKind, util, ReturnStatement, ThrowStatement, isTryCatchStatement, isCatchStatement, isVoidType, SymbolTypeFlag, isConditionalCompileStatement } from 'brighterscript';
import { LintState, StatementInfo } from '.';
import { PluginContext } from '../../util';

Expand Down Expand Up @@ -44,21 +44,21 @@ export function createReturnLinter(
tags: [DiagnosticTag.Unnecessary]
});
} else if (isReturnStatement(curr.stat)) {
const { ifs, trys, branch, parent } = state;
const { ifs, conditionalCompiles, trys, branch, parent } = state;
returns.push({
stat: curr.stat,
hasValue: !!curr.stat.value
});
// flag parent branch to return
const returnBlock = (ifs || trys) ? branch : parent;
const returnBlock = (ifs || conditionalCompiles || trys) ? branch : parent;
if (returnBlock && parent?.branches === 1) {
returnBlock.returns = true;
}
} else if (isThrowStatement(curr.stat)) {
const { ifs, trys, branch, parent } = state;
const { ifs, conditionalCompiles, trys, branch, parent } = state;
throws.push({ stat: curr.stat });
// flag parent branch to 'return'
const returnBlock = (ifs || trys) ? branch : parent;
const returnBlock = (ifs || conditionalCompiles || trys) ? branch : parent;
if (returnBlock && parent?.branches === 1) {
returnBlock.returns = true;
}
Expand All @@ -69,14 +69,16 @@ export function createReturnLinter(
const { parent } = state;
if (!parent) {
finalize(closed);
} else if (isIfStatement(closed.stat) || isTryCatchStatement(closed.stat) || isCatchStatement(closed.stat)) {
} else if (isIfStatement(closed.stat) || isConditionalCompileStatement(closed.stat) || isTryCatchStatement(closed.stat) || isCatchStatement(closed.stat)) {
if (closed.branches === 0) {
parent.returns = true;
parent.branches--;
}
} else if (closed.returns) {
if (isIfStatement(parent.stat)) {
parent.branches--;
} else if (isConditionalCompileStatement(parent.stat)) {
parent.branches--;
} else if (isTryCatchStatement(parent.stat)) {
parent.branches--;
} else if (isCatchStatement(parent.stat)) {
Expand Down
26 changes: 26 additions & 0 deletions test/project1/source/consistent-return.brs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,3 +129,29 @@ end function
sub some(o)
print o
end sub

#const debug = true

function conditionalCompileOk() as dynamic
#if DEBUG ' no error
return 1
#else
return 0
#end if
end function

function conditionalCompileOk2() as dynamic
#if DEBUG ' no error
throw "error"
#else
return 0
#end if
end function

function conditionalCompileError() as dynamic
#if DEBUG
' error - no return
#else
return 0
#end if
end function

0 comments on commit eeb2af4

Please sign in to comment.