visionmedia/jade — pull request #1387 by doublerebel (2014-01-17 02:24:49+00:00)
Title: Bugfix, tests for filename of template errors during compile. Fixes #923

### Summary
As described in #923, error reports are incorrect when reporting an undefined variable.

Extending/including a .jade template that does not declare a `block` will cause runtime errors to be incorrectly attributed to the parent file, but with the correct line number from the file with the error.

i.e. the incorrect report:
```
test/fixtures/compile.with.layout.locals.error.jade:2
1| extends layout.locals.error.jade
> 2|

undefined is not a function
```

instead of the correct report:
```
test/fixtures/layout.locals.error.jade:2
1|
> 2| = foo()

undefined is not a function
```

### Cause
The problem is centered around [lib/parser.js:132](https://github.com/visionmedia/jade/blob/master/lib/parser.js#L132):
```js
expr.filename = this.options.filename;
```

When a `block` is not declared, `expr` is parsed as `Code` instead of `Block`, and `Code` does not have a filename attribute. On further inspection, the filename is incorrectly set for all `Nodes` that do not set their own .filename, i.e. `Tag`.

The effect of the error can be seen [in this parsing dump from Jade](https://gist.github.com/doublerebel/8466992#file-bad-jade-parsing-js-L15), where both the header and footer have filename set to blog/index.jade. This also has the side effect of multiple lines attributed to the same line number. Both these issues make debugging Jade templates quite difficult.

The code in parser.js causing the issue is from visionmedia/jade@763f96cf . In the same commit, the filename for blocks is defined similarly:
```js
expr.filename = this.filename;
```

### Solution
`options.filename` does not change from the original parent template file. However, `this.filename` is always correctly set to the (sub)file being parsed. Therefore, we can change [lib/parser.js:132](https://github.com/visionmedia/jade/blob/master/lib/parser.js#L132):
```js
expr.filename = expr.filename || this.filename;
```

All tests pass, and the [parser dump now looks correct](https://gist.github.com/doublerebel/8466992#file-goodjadeparsing-js-L15). `options.filename` is used in very few places, I cannot find a use case that requires `expr.filename` to be set to `options.filename` in the old manner.

The current tests only test for Syntax and Type Errors, which are thrown elsewhere and not subject to this bug. This behavior of extends and blocks is not documented and is unexpected. The test suite does test for extends/includes without block declarations, but only with one type of error (SyntaxError).

I have included additional tests, to cover these example cases. Additionally, my blog now renders correctly in production. Comparing many variables deep inside the parser can be very time-consuming, I will gladly add the expected behavior for block/extends into the docs if this PR is correct/accepted.

Thanks!!