Skip to content

Commit

Permalink
Bulletproof AST.helpers.helperExpression
Browse files Browse the repository at this point in the history 
Avoid undefined values and potential false positives from other type values such as partials.

Fixes handlebars-lang#1055
  • Loading branch information
@kpdecker @flenter
kpdecker authored and flenter committed Aug 27, 2015
1 parent 3659ba7 commit 606628f
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 15 deletions.
4 changes: 3 additions & 1 deletion lib/handlebars/compiler/ast.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,9 @@ let AST = {
// * it is an eligible helper, and
// * it has at least one parameter or hash segment
helperExpression: function(node) {
return !!(node.type === 'SubExpression' || node.params.length || node.hash);
return (node.type === 'SubExpression')
|| ((node.type === 'MustacheStatement' || node.type === 'BlockStatement')
&& !!((node.params && node.params.length) || node.hash));
},

scopedId: function(path) {
Expand Down
66 changes: 52 additions & 14 deletions spec/ast.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ describe('ast', function() {
return;
}

var AST = Handlebars.AST;

var LOCATION_INFO = {
start: {
line: 1,
Expand All @@ -23,7 +25,7 @@ describe('ast', function() {

describe('MustacheStatement', function() {
it('should store args', function() {
var mustache = new handlebarsEnv.AST.MustacheStatement({}, null, null, true, {}, LOCATION_INFO);
var mustache = new AST.MustacheStatement({}, null, null, true, {}, LOCATION_INFO);
equals(mustache.type, 'MustacheStatement');
equals(mustache.escaped, true);
testLocationInfoStorage(mustache);
Expand All @@ -37,8 +39,8 @@ describe('ast', function() {
});

it('stores location info', function() {
var mustacheNode = new handlebarsEnv.AST.MustacheStatement([{ original: 'foo'}], null, null, false, {});
var block = new handlebarsEnv.AST.BlockStatement(
var mustacheNode = new AST.MustacheStatement([{ original: 'foo'}], null, null, false, {});
var block = new AST.BlockStatement(
mustacheNode,
null, null,
{body: []},
Expand All @@ -52,78 +54,114 @@ describe('ast', function() {
});
describe('PathExpression', function() {
it('stores location info', function() {
var idNode = new handlebarsEnv.AST.PathExpression(false, 0, [], 'foo', LOCATION_INFO);
var idNode = new AST.PathExpression(false, 0, [], 'foo', LOCATION_INFO);
testLocationInfoStorage(idNode);
});
});

describe('Hash', function() {
it('stores location info', function() {
var hash = new handlebarsEnv.AST.Hash([], LOCATION_INFO);
var hash = new AST.Hash([], LOCATION_INFO);
testLocationInfoStorage(hash);
});
});

describe('ContentStatement', function() {
it('stores location info', function() {
var content = new handlebarsEnv.AST.ContentStatement('HI', LOCATION_INFO);
var content = new AST.ContentStatement('HI', LOCATION_INFO);
testLocationInfoStorage(content);
});
});

describe('CommentStatement', function() {
it('stores location info', function() {
var comment = new handlebarsEnv.AST.CommentStatement('HI', {}, LOCATION_INFO);
var comment = new AST.CommentStatement('HI', {}, LOCATION_INFO);
testLocationInfoStorage(comment);
});
});

describe('NumberLiteral', function() {
it('stores location info', function() {
var integer = new handlebarsEnv.AST.NumberLiteral('6', LOCATION_INFO);
var integer = new AST.NumberLiteral('6', LOCATION_INFO);
testLocationInfoStorage(integer);
});
});

describe('StringLiteral', function() {
it('stores location info', function() {
var string = new handlebarsEnv.AST.StringLiteral('6', LOCATION_INFO);
var string = new AST.StringLiteral('6', LOCATION_INFO);
testLocationInfoStorage(string);
});
});

describe('BooleanLiteral', function() {
it('stores location info', function() {
var bool = new handlebarsEnv.AST.BooleanLiteral('true', LOCATION_INFO);
var bool = new AST.BooleanLiteral('true', LOCATION_INFO);
testLocationInfoStorage(bool);
});
});

describe('PartialStatement', function() {
it('provides default params', function() {
var pn = new handlebarsEnv.AST.PartialStatement('so_partial', undefined, {}, {}, LOCATION_INFO);
var pn = new AST.PartialStatement('so_partial', undefined, {}, {}, LOCATION_INFO);
equals(pn.params.length, 0);
});
it('stores location info', function() {
var pn = new handlebarsEnv.AST.PartialStatement('so_partial', [], {}, {}, LOCATION_INFO);
var pn = new AST.PartialStatement('so_partial', [], {}, {}, LOCATION_INFO);
testLocationInfoStorage(pn);
});
});

describe('Program', function() {
it('storing location info', function() {
var pn = new handlebarsEnv.AST.Program([], null, {}, LOCATION_INFO);
var pn = new AST.Program([], null, {}, LOCATION_INFO);
testLocationInfoStorage(pn);
});
});

describe('SubExpression', function() {
it('provides default params', function() {
var pn = new handlebarsEnv.AST.SubExpression('path', undefined, {}, LOCATION_INFO);
var pn = new AST.SubExpression('path', undefined, {}, LOCATION_INFO);
equals(pn.params.length, 0);
});
});

describe('helpers', function() {
describe('#helperExpression', function() {
it('should handle mustache statements', function() {
equals(AST.helpers.helperExpression(new AST.MustacheStatement('foo', [], undefined, false, {}, LOCATION_INFO)), false);
equals(AST.helpers.helperExpression(new AST.MustacheStatement('foo', [1], undefined, false, {}, LOCATION_INFO)), true);
equals(AST.helpers.helperExpression(new AST.MustacheStatement('foo', [], {}, false, {}, LOCATION_INFO)), true);
});
it('should handle block statements', function() {
equals(AST.helpers.helperExpression(new AST.BlockStatement('foo', [], undefined, false, {}, LOCATION_INFO)), false);
equals(AST.helpers.helperExpression(new AST.BlockStatement('foo', [1], undefined, false, {}, LOCATION_INFO)), true);
equals(AST.helpers.helperExpression(new AST.BlockStatement('foo', [], {}, false, {}, LOCATION_INFO)), true);
});
it('should handle subexpressions', function() {
equals(AST.helpers.helperExpression(new AST.SubExpression()), true);
});
it('should work with non-helper nodes', function() {
equals(AST.helpers.helperExpression(new AST.Program([], [], {}, LOCATION_INFO)), false);

equals(AST.helpers.helperExpression(new AST.PartialStatement()), false);
equals(AST.helpers.helperExpression(new AST.ContentStatement('a', LOCATION_INFO)), false);
equals(AST.helpers.helperExpression(new AST.CommentStatement('a', {}, LOCATION_INFO)), false);

equals(AST.helpers.helperExpression(new AST.PathExpression(false, 0, ['a'], 'a', LOCATION_INFO)), false);

equals(AST.helpers.helperExpression(new AST.StringLiteral('a', LOCATION_INFO)), false);
equals(AST.helpers.helperExpression(new AST.NumberLiteral(1, LOCATION_INFO)), false);
equals(AST.helpers.helperExpression(new AST.BooleanLiteral(true, LOCATION_INFO)), false);
equals(AST.helpers.helperExpression(new AST.UndefinedLiteral(LOCATION_INFO)), false);
equals(AST.helpers.helperExpression(new AST.NullLiteral(LOCATION_INFO)), false);

equals(AST.helpers.helperExpression(new AST.Hash([], LOCATION_INFO)), false);
equals(AST.helpers.helperExpression(new AST.HashPair('foo', 'bar', LOCATION_INFO)), false);
});
});
});

describe('Line Numbers', function() {
var ast, body;

Expand Down

0 comments on commit 606628f

Please sign in to comment.