Skip to content

Commit

Permalink
Fix jashkenas#4558: Much more robust caching of sources and source ma…
Browse files Browse the repository at this point in the history
…ps, more careful lookup of source maps especially for CoffeeScript code compiled within a Coffee script (. . . within a Coffee script, etc.)
  • Loading branch information
GeoffreyBooth committed Aug 16, 2017
1 parent cf66494 commit 482faef
Show file tree
Hide file tree
Showing 3 changed files with 102 additions and 32 deletions.
67 changes: 50 additions & 17 deletions lib/coffeescript/coffeescript.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

40 changes: 30 additions & 10 deletions src/coffeescript.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ packageJson = require '../../package.json'
# The current CoffeeScript version number.
exports.VERSION = packageJson.version

exports.FILE_EXTENSIONS = ['.coffee', '.litcoffee', '.coffee.md']
exports.FILE_EXTENSIONS = FILE_EXTENSIONS = ['.coffee', '.litcoffee', '.coffee.md']

# Expose helpers for testing.
exports.helpers = helpers
Expand Down Expand Up @@ -266,15 +266,35 @@ formatSourcePosition = (frame, getSourceMapping) ->
else
fileLocation

getSourceMap = (filename) ->
if sourceMaps[filename]?
sourceMaps[filename][sourceMaps[filename].length - 1]
getSourceMap = (filename, line, column) ->
# Skip files that we didn’t compile, like Node system files that appear in
# the stack trace, as they never have source maps.
return null unless filename is '<anonymous>' or filename.slice(filename.lastIndexOf('.')) in FILE_EXTENSIONS

if filename isnt '<anonymous>' and sourceMaps[filename]?
return sourceMaps[filename][sourceMaps[filename].length - 1]
# CoffeeScript compiled in a browser or via `CoffeeScript.compile` or `.run`
# may get compiled with `options.filename` that’s missing, which becomes
# `<anonymous>`; but the runtime might request the stack trace with the
# filename of the script file. See if we have a source map cached under
# `<anonymous>` that matches the error.
else if sourceMaps['<anonymous>']?
# CoffeeScript compiled in a browser may get compiled with `options.filename`
# of `<anonymous>`, but the browser may request the stack trace with the
# filename of the script file.
sourceMaps['<anonymous>'][sourceMaps['<anonymous>'].length - 1]
else if sources[filename]?
# Work backwards from the most recent anonymous source maps, until we find
# one that works. This isn’t foolproof; there is a chance that multiple
# source maps will have line/column pairs that match. But we have no other
# way to match them. `frame.getFunction().toString()` doesn’t always work,
# and it’s not foolproof either.
for map in sourceMaps['<anonymous>'] by -1
sourceLocation = map.sourceLocation [line - 1, column - 1]
return map if sourceLocation?[0]? and sourceLocation[1]?

# If all else fails, recompile this source to get a source map. We need the
# previous section (for `<anonymous>`) despite this option, because after it
# gets compiled we will still need to look it up from
# `sourceMaps['<anonymous>']` in order to find and return it. That’s why we
# start searching from the end in the previous block, because most of the
# time the source map we want is the last one.
if sources[filename]?
answer = compile sources[filename][sources[filename].length - 1],
filename: filename
sourceMap: yes
Expand All @@ -289,7 +309,7 @@ getSourceMap = (filename) ->
# positions.
Error.prepareStackTrace = (err, stack) ->
getSourceMapping = (filename, line, column) ->
sourceMap = getSourceMap filename
sourceMap = getSourceMap filename, line, column
answer = sourceMap.sourceLocation [line - 1, column - 1] if sourceMap?
if answer? then [answer[0] + 1, answer[1] + 1] else null

Expand Down
27 changes: 22 additions & 5 deletions test/error_messages.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ if require?
finally
fs.unlinkSync tempFile

test "#3890 Error.prepareStackTrace doesn't throw an error if a compiled file is deleted", ->
test "#3890: Error.prepareStackTrace doesn't throw an error if a compiled file is deleted", ->
# Adapted from https://github.com/atom/coffee-cash/blob/master/spec/coffee-cash-spec.coffee
filePath = path.join os.tmpdir(), 'PrepareStackTraceTestFile.coffee'
fs.writeFileSync filePath, "module.exports = -> throw new Error('hello world')"
Expand All @@ -90,7 +90,7 @@ if require?
doesNotThrow(-> error.stack)
notEqual error.stack.toString().indexOf(filePath), -1

test "#4418 stack traces for compiled files reference the correct line number", ->
test "#4418: stack traces for compiled files reference the correct line number", ->
filePath = path.join os.tmpdir(), 'StackTraceLineNumberTestFile.coffee'
fileContents = """
testCompiledFileStackTraceLineNumber = ->
Expand All @@ -111,22 +111,39 @@ if require?
eq /StackTraceLineNumberTestFile.coffee:(\d)/.exec(error.stack.toString())[1], '3'


test "#4418 stack traces for compiled strings reference the correct line number", ->
test "#4418: stack traces for compiled strings reference the correct line number", ->
try
CoffeeScript.run """
CoffeeScript.run '''
testCompiledStringStackTraceLineNumber = ->
# `a` on the next line is undefined and should throw a ReferenceError
console.log a if true
do testCompiledStringStackTraceLineNumber
"""
'''
catch error

# Make sure the line number reported is line 3 (the original Coffee source)
# and not line 6 (the generated JavaScript).
eq /testCompiledStringStackTraceLineNumber.*:(\d):/.exec(error.stack.toString())[1], '3'


test "#4558: compiling a string inside a script doesn’t screw up stack trace line number", ->
try
CoffeeScript.run '''
testCompilingInsideAScriptDoesntScrewUpStackTraceLineNumber = ->
if require?
CoffeeScript = require './lib/coffeescript'
CoffeeScript.compile ''
throw new Error 'Some Error'
do testCompilingInsideAScriptDoesntScrewUpStackTraceLineNumber
'''
catch error

# Make sure the line number reported is line 5 (the original Coffee source)
# and not line 10 (the generated JavaScript).
eq /testCompilingInsideAScriptDoesntScrewUpStackTraceLineNumber.*:(\d):/.exec(error.stack.toString())[1], '5'

test "#1096: unexpected generated tokens", ->
# Implicit ends
assertErrorFormat 'a:, b', '''
Expand Down

0 comments on commit 482faef

Please sign in to comment.