Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #4558: Stack trace line numbers for scripts that compile CoffeeScript #4645

Merged
merged 4 commits into from
Aug 23, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions Cakefile
Original file line number Diff line number Diff line change
Expand Up @@ -341,11 +341,15 @@ task 'doc:source:watch', 'watch and continually rebuild the annotated source doc


task 'release', 'build and test the CoffeeScript source, and build the documentation', ->
invoke 'build:full'
invoke 'build:browser:full'
invoke 'doc:site'
invoke 'doc:test'
invoke 'doc:source'
execSync '''
cake build:full
cake build:browser
cake test:browser
cake test:integrations
cake doc:site
cake doc:test
cake doc:source''', stdio: 'inherit'


task 'bench', 'quick benchmark of compilation time', ->
{Rewriter} = require './lib/coffeescript/rewriter'
Expand Down
2 changes: 1 addition & 1 deletion docs/v2/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -4682,7 +4682,7 @@ <h2 class="header">
else
initializeScrollspyFromHash window.location.hash
# Initializing the code editors might’ve thrown off our vertical scroll position
document.getElementById(window.location.hash.slice(1)).scrollIntoView()
document.getElementById(window.location.hash.slice(1).replace(/try:.*/, '')).scrollIntoView()

</script>

Expand Down
2 changes: 1 addition & 1 deletion documentation/v2/docs.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -125,4 +125,4 @@ $(document).ready ->
else
initializeScrollspyFromHash window.location.hash
# Initializing the code editors might’ve thrown off our vertical scroll position
document.getElementById(window.location.hash.slice(1)).scrollIntoView()
document.getElementById(window.location.hash.slice(1).replace(/try:.*/, '')).scrollIntoView()
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.

54 changes: 38 additions & 16 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 @@ -49,9 +49,9 @@ withPrettyErrors = (fn) ->
# a stack trace. Assuming that most of the time, code isn’t throwing
# exceptions, it’s probably more efficient to compile twice only when we
# need a stack trace, rather than always generating a source map even when
# it’s not likely to be used. Save in form of `filename`: `(source)`
# it’s not likely to be used. Save in form of `filename`: [`(source)`]
sources = {}
# Also save source maps if generated, in form of `filename`: `(source map)`.
# Also save source maps if generated, in form of `(source)`: [`(source map)`].
sourceMaps = {}

# Compile CoffeeScript code to JavaScript, using the Coffee/Jison compiler.
Expand All @@ -75,7 +75,8 @@ exports.compile = compile = withPrettyErrors (code, options) ->

checkShebangLine filename, code

sources[filename] = code
sources[filename] ?= []
sources[filename].push code
map = new SourceMap if generateSourceMap

tokens = lexer.tokenize code, options
Expand Down Expand Up @@ -124,8 +125,9 @@ exports.compile = compile = withPrettyErrors (code, options) ->
js = "// #{header}\n#{js}"

if generateSourceMap
v3SourceMap = map.generate(options, code)
sourceMaps[filename] = map
v3SourceMap = map.generate options, code
sourceMaps[filename] ?= []
sourceMaps[filename].push map

if options.inlineMap
encoded = base64encode JSON.stringify v3SourceMap
Expand Down Expand Up @@ -264,16 +266,36 @@ formatSourcePosition = (frame, getSourceMapping) ->
else
fileLocation

getSourceMap = (filename) ->
if sourceMaps[filename]?
sourceMaps[filename]
# 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.
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>']?
sourceMaps['<anonymous>']
else if sources[filename]?
answer = compile 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
literate: helpers.isLiterate filename
Expand All @@ -287,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