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

* Don't throw an error in the console when loading a try: URL

* Handle the possibility of compiling multiple scripts with the same filename, or multiple anonymous scripts

* Fix #4558: Much more robust caching of sources and source maps, more careful lookup of source maps especially for CoffeeScript code compiled within a Coffee script (. . . within a Coffee script, etc.)

* Reimplement `cake release` to just use the shell to avoid the issues plaguing that command (something to do with module caching perhaps)

Cakefile

@@ -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'
+  execSync '''
-  invoke 'build:browser:full'
+    cake build:full
-  invoke 'doc:site'
+    cake build:browser
-  invoke 'doc:test'
+    cake test:browser
-  invoke 'doc:source'
+    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'

docs/v2/index.html

@@ -4682,7 +4682,7 @@ $(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()
 
 </script>
 

documentation/v2/docs.coffee

@@ -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()

lib/coffeescript/coffeescript.js

@@ -4,7 +4,8 @@
   // on Node.js/V8, or to run CoffeeScript directly in the browser. This module
   // contains the main entry functions for tokenizing, parsing, and compiling
   // source CoffeeScript into JavaScript.
-  var Lexer, SourceMap, base64encode, checkShebangLine, compile, formatSourcePosition, getSourceMap, helpers, lexer, packageJson, parser, sourceMaps, sources, withPrettyErrors;
+  var FILE_EXTENSIONS, Lexer, SourceMap, base64encode, checkShebangLine, compile, formatSourcePosition, getSourceMap, helpers, lexer, packageJson, parser, sourceMaps, sources, withPrettyErrors,
+    indexOf = [].indexOf;
 
   ({Lexer} = require('./lexer'));
 
@@ -21,7 +22,7 @@
   // 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;
@@ -67,10 +68,10 @@
   // 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.
@@ -93,7 +94,10 @@
     generateSourceMap = options.sourceMap || options.inlineMap || (options.filename == null);
     filename = options.filename || '<anonymous>';
     checkShebangLine(filename, code);
-    sources[filename] = code;
+    if (sources[filename] == null) {
+      sources[filename] = [];
+    }
+    sources[filename].push(code);
     if (generateSourceMap) {
       map = new SourceMap;
     }
@@ -158,7 +162,10 @@
     }
     if (generateSourceMap) {
       v3SourceMap = map.generate(options, code);
-      sourceMaps[filename] = map;
+      if (sourceMaps[filename] == null) {
+        sourceMaps[filename] = [];
+      }
+      sourceMaps[filename].push(map);
     }
     if (options.inlineMap) {
       encoded = base64encode(JSON.stringify(v3SourceMap));
@@ -311,17 +318,43 @@
     }
   };
 
-  getSourceMap = function(filename) {
+  getSourceMap = function(filename, line, column) {
-    var answer;
+    var answer, i, map, ref, ref1, sourceLocation;
-    if (sourceMaps[filename] != null) {
+    if (!(filename === '<anonymous>' || (ref = filename.slice(filename.lastIndexOf('.')), indexOf.call(FILE_EXTENSIONS, ref) >= 0))) {
-      return sourceMaps[filename];
+      // Skip files that we didn’t compile, like Node system files that appear in
-    // CoffeeScript compiled in a browser may get compiled with `options.filename`
+      // the stack trace, as they never have source maps.
-    // of `<anonymous>`, but the browser may request the stack trace with the
+      return null;
-    // filename of the script file.
+    }
+    if (filename !== '<anonymous>' && (sourceMaps[filename] != null)) {
+      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>'] != null) {
-      return sourceMaps['<anonymous>'];
+      ref1 = sourceMaps['<anonymous>'];
-    } else if (sources[filename] != null) {
+      // Work backwards from the most recent anonymous source maps, until we find
-      answer = compile(sources[filename], {
+      // 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 (i = ref1.length - 1; i >= 0; i += -1) {
+        map = ref1[i];
+        sourceLocation = map.sourceLocation([line - 1, column - 1]);
+        if (((sourceLocation != null ? sourceLocation[0] : void 0) != null) && (sourceLocation[1] != null)) {
+          return map;
+        }
+      }
+    }
+    // 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] != null) {
+      answer = compile(sources[filename][sources[filename].length - 1], {
         filename: filename,
         sourceMap: true,
         literate: helpers.isLiterate(filename)
@@ -340,7 +373,7 @@
     var frame, frames, getSourceMapping;
     getSourceMapping = function(filename, line, column) {
       var answer, sourceMap;
-      sourceMap = getSourceMap(filename);
+      sourceMap = getSourceMap(filename, line, column);
       if (sourceMap != null) {
         answer = sourceMap.sourceLocation([line - 1, column - 1]);
       }

src/coffeescript.coffee

@@ -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
@@ -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.
@@ -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
@@ -124,8 +125,9 @@ exports.compile = compile = withPrettyErrors (code, options) ->
     js = "// #{header}\n#{js}"
 
   if generateSourceMap
-    v3SourceMap = map.generate(options, code)
+    v3SourceMap = map.generate options, code
-    sourceMaps[filename] = map
+    sourceMaps[filename] ?= []
+    sourceMaps[filename].push map
 
   if options.inlineMap
     encoded = base64encode JSON.stringify v3SourceMap
@@ -264,16 +266,36 @@ formatSourcePosition = (frame, getSourceMapping) ->
   else
     fileLocation
 
-getSourceMap = (filename) ->
+getSourceMap = (filename, line, column) ->
-  if sourceMaps[filename]?
+  # Skip files that we didn’t compile, like Node system files that appear in
-    sourceMaps[filename]
+  # the stack trace, as they never have source maps.
-  # CoffeeScript compiled in a browser may get compiled with `options.filename`
+  return null unless filename is '<anonymous>' or filename.slice(filename.lastIndexOf('.')) in FILE_EXTENSIONS
-  # of `<anonymous>`, but the browser may request the stack trace with the
+
-  # filename of the script file.
+  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>']
+    # Work backwards from the most recent anonymous source maps, until we find
-  else if sources[filename]?
+    # one that works. This isn’t foolproof; there is a chance that multiple
-    answer = compile sources[filename],
+    # 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
@@ -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
 

test/error_messages.coffee

@@ -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')"
@@ -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 = ->
@@ -111,15 +111,15 @@ 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)
@@ -127,6 +127,23 @@ test "#4418 stack traces for compiled strings reference the correct line number"
   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', '''