Fix location information for programs

kpdecker · kpdecker · commit 93faffa54916 · 2015-06-26T16:58:34.000-05:00
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
diff --git a/lib/handlebars/compiler/helpers.js b/lib/handlebars/compiler/helpers.js
@@ -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);
+}
+
+
diff --git a/spec/ast.js b/spec/ast.js
@@ -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() {
@@ -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);
      });
   });
 
diff --git a/spec/parser.js b/spec/parser.js
@@ -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 }
+    }));
+  });
 });
diff --git a/src/handlebars.yy b/src/handlebars.yy
@@ -9,7 +9,7 @@ root
   ;
 
 program
-  : statement* -> new yy.Program($1, null, {}, yy.locInfo(@$))
+  : statement* -> yy.prepareProgram($1)
   ;
 
 statement
@@ -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 };

Original file line number	Diff line number	Diff line change
`@@ -9,7 +9,7 @@ root`
`9`	`9`	`;`
`10`	`10`
`11`	`11`	`program`
`12`		`- : statement* -> new yy.Program($1, null, {}, yy.locInfo(@$))`
	`12`	`+ : statement* -> yy.prepareProgram($1)`
`13`	`13`	`;`
`14`	`14`
`15`	`15`	`statement`
`@@ -57,7 +57,7 @@ inverseAndProgram`
`57`	`57`	`inverseChain`
`58`	`58`	`: openInverseChain program inverseChain? {`
`59`	`59`	`var inverse = yy.prepareBlock($1, $2, $3, $3, false, @$),`
`60`		`- program = new yy.Program([inverse], null, {}, yy.locInfo(@$));`
	`60`	`+ program = yy.prepareProgram([inverse], $2.loc);`
`61`	`61`	`program.chained = true;`
`62`	`62`
`63`	`63`	`$$ = { strip: $1.strip, program: program, chain: true };`