Skip to content

Commit

Permalink
Fix location information for programs
Browse files Browse the repository at this point in the history
There appears to be a bug in our use of jison causing the parent location information to be reported to programs. I wasn’t able to work through what might be causing this so instead using the location information of the statements collection to generate the proper location information.

This is a bit of a hack but we are very far behind on the Jison release train and upgrading will likely be a less than pleasant task that doesn’t provide us much benefit.

Fixes #1024
  • Loading branch information
kpdecker committed Jun 26, 2015
1 parent d2fb3a4 commit 93faffa
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 6 deletions.
25 changes: 25 additions & 0 deletions lib/handlebars/compiler/helpers.js
Expand Up @@ -121,3 +121,28 @@ export function prepareBlock(openBlock, program, inverseAndProgram, close, inver
openBlock.strip, inverseStrip, close && close.strip,
this.locInfo(locInfo));
}

export function prepareProgram(statements, loc) {
if (!loc && statements.length) {
const first = statements[0].loc,
last = statements[statements.length - 1].loc;

if (first && last) {
loc = {
source: first.source,
start: {
line: first.start.line,
column: first.start.column
},
end: {
line: last.end.line,
column: last.end.column
}
};
}
}

return new this.Program(statements, null, {}, loc);
}


27 changes: 23 additions & 4 deletions spec/ast.js
Expand Up @@ -123,8 +123,18 @@ describe('ast', function() {
equals(node.loc.end.column, lastColumn);
}

ast = Handlebars.parse('line 1 {{line1Token}}\n line 2 {{line2token}}\n line 3 {{#blockHelperOnLine3}}\nline 4{{line4token}}\n' +
'line5{{else}}\n{{line6Token}}\n{{/blockHelperOnLine3}}');
ast = Handlebars.parse(
'line 1 {{line1Token}}\n' // 1
+ ' line 2 {{line2token}}\n' // 2
+ ' line 3 {{#blockHelperOnLine3}}\n' // 3
+ 'line 4{{line4token}}\n' // 4
+ 'line5{{else}}\n' // 5
+ '{{line6Token}}\n' // 6
+ '{{/blockHelperOnLine3}}\n' // 7
+ '{{#open}}\n' // 8
+ '{{else inverse}}\n' // 9
+ '{{else}}\n' // 10
+ '{{/open}}'); // 11
body = ast.body;

it('gets ContentNode line numbers', function() {
Expand Down Expand Up @@ -155,14 +165,23 @@ describe('ast', function() {
var blockHelperNode = body[5],
program = blockHelperNode.program;

testColumns(program, 3, 5, 8, 5);
testColumns(program, 3, 5, 31, 5);
});

it('correctly records the line numbers of an inverse of a block helper', function() {
var blockHelperNode = body[5],
inverse = blockHelperNode.inverse;

testColumns(inverse, 5, 7, 5, 0);
testColumns(inverse, 5, 7, 13, 0);
});

it('correctly records the line number of chained inverses', function() {
var chainInverseNode = body[7];

testColumns(chainInverseNode.program, 8, 9, 9, 0);
testColumns(chainInverseNode.inverse, 9, 10, 16, 0);
testColumns(chainInverseNode.inverse.body[0].program, 9, 10, 16, 0);
testColumns(chainInverseNode.inverse.body[0].inverse, 10, 11, 8, 0);
});
});

Expand Down
24 changes: 24 additions & 0 deletions spec/parser.js
Expand Up @@ -231,4 +231,28 @@ describe('parser', function() {
equals(astFor(new Handlebars.AST.Program([new Handlebars.AST.ContentStatement('Hello')], null)), 'CONTENT[ \'Hello\' ]\n');
});
});

it('GH1024 - should track program location properly', function() {
var p = Handlebars.parse('\n'
+ ' {{#if foo}}\n'
+ ' {{bar}}\n'
+ ' {{else}} {{baz}}\n'
+ '\n'
+ ' {{/if}}\n'
+ ' ');

// We really need a deep equals but for now this should be stable...
equals(JSON.stringify(p.loc), JSON.stringify({
start: { line: 1, column: 0 },
end: { line: 7, column: 4 }
}));
equals(JSON.stringify(p.body[1].program.loc), JSON.stringify({
start: { line: 2, column: 13 },
end: { line: 4, column: 7 }
}));
equals(JSON.stringify(p.body[1].inverse.loc), JSON.stringify({
start: { line: 4, column: 15 },
end: { line: 6, column: 5 }
}));
});
});
4 changes: 2 additions & 2 deletions src/handlebars.yy
Expand Up @@ -9,7 +9,7 @@ root
;

program
: statement* -> new yy.Program($1, null, {}, yy.locInfo(@$))
: statement* -> yy.prepareProgram($1)
;

statement
Expand Down Expand Up @@ -57,7 +57,7 @@ inverseAndProgram
inverseChain
: openInverseChain program inverseChain? {
var inverse = yy.prepareBlock($1, $2, $3, $3, false, @$),
program = new yy.Program([inverse], null, {}, yy.locInfo(@$));
program = yy.prepareProgram([inverse], $2.loc);
program.chained = true;

$$ = { strip: $1.strip, program: program, chain: true };
Expand Down

0 comments on commit 93faffa

Please sign in to comment.