From 3f9cdcf1faaa34ce7322cea79be724003c57d51e Mon Sep 17 00:00:00 2001 From: Demian Ferreiro Date: Wed, 31 Jul 2013 08:27:49 -0300 Subject: [PATCH] Change how error messages are shown Instead of throwing the syntax errors with their source file location and needing to then catch them and call a `prettyErrorMessage` function in order to get the formatted error message, now syntax errors know how to pretty-print themselves (their `toString` method gets overridden). An intermediate `catch` & re-`throw` is needed at the level of `CoffeeScript.compile` and friends. But the benefit of this approach is that now libraries that use the `CoffeeScript` object directly don't need to bother catching the possible compilation errors and calling a special function in order to get the nice error messages; they can just print the error itself (or let it bubble up) and the error will know how to pretty-print itself. --- lib/coffee-script/coffee-script.js | 38 +++++++++++++++++-------- lib/coffee-script/command.js | 5 ++-- lib/coffee-script/helpers.js | 39 ++++++++++++++------------ lib/coffee-script/repl.js | 17 +++++------ src/coffee-script.coffee | 21 ++++++++++---- src/command.coffee | 3 +- src/helpers.coffee | 45 +++++++++++++++--------------- src/repl.coffee | 6 ++-- test/error_messages.coffee | 12 ++++---- 9 files changed, 107 insertions(+), 79 deletions(-) diff --git a/lib/coffee-script/coffee-script.js b/lib/coffee-script/coffee-script.js index 05717f41..b7f33c90 100644 --- a/lib/coffee-script/coffee-script.js +++ b/lib/coffee-script/coffee-script.js @@ -1,6 +1,6 @@ // Generated by CoffeeScript 1.6.3 (function() { - var Lexer, Module, SourceMap, child_process, compile, compileFile, ext, fileExtensions, findExtension, fork, formatSourcePosition, fs, getSourceMap, helpers, lexer, loadFile, parser, path, sourceMaps, vm, _i, _len, + var Lexer, Module, SourceMap, child_process, compile, compileFile, ext, fileExtensions, findExtension, fork, formatSourcePosition, fs, getSourceMap, helpers, lexer, loadFile, parser, path, sourceMaps, vm, withPrettyErrors, _i, _len, __hasProp = {}.hasOwnProperty, __indexOf = [].indexOf || function(item) { for (var i = 0, l = this.length; i < l; i++) { if (i in this && this[i] === item) return i; } return -1; }; @@ -26,11 +26,25 @@ exports.helpers = helpers; - exports.compile = compile = function(code, options) { + withPrettyErrors = function(fn) { + return function(code, options) { + var err; + if (options == null) { + options = {}; + } + try { + return fn.call(this, code, options); + } catch (_error) { + err = _error; + err.code || (err.code = code); + err.filename || (err.filename = options.filename); + throw err; + } + }; + }; + + exports.compile = compile = withPrettyErrors(function(code, options) { var answer, currentColumn, currentLine, fragment, fragments, header, js, map, merge, newLines, _i, _len; - if (options == null) { - options = {}; - } merge = helpers.merge; if (options.sourceMap) { map = new SourceMap; @@ -73,19 +87,19 @@ } else { return js; } - }; + }); - exports.tokens = function(code, options) { + exports.tokens = withPrettyErrors(function(code, options) { return lexer.tokenize(code, options); - }; + }); - exports.nodes = function(source, options) { + exports.nodes = withPrettyErrors(function(source, options) { if (typeof source === 'string') { return parser.parse(lexer.tokenize(source, options)); } else { return parser.parse(source); } - }; + }); exports.run = function(code, options) { var answer, mainModule, _ref; @@ -178,8 +192,8 @@ }); } catch (_error) { err = _error; - err.filename = filename; - err.code = stripped; + err.filename || (err.filename = filename); + err.code || (err.code = stripped); throw err; } return answer; diff --git a/lib/coffee-script/command.js b/lib/coffee-script/command.js index 1ee917b5..3fba3b95 100644 --- a/lib/coffee-script/command.js +++ b/lib/coffee-script/command.js @@ -150,7 +150,7 @@ }; compileScript = function(file, input, base) { - var compiled, err, message, o, options, t, task, useColors; + var compiled, err, message, o, options, t, task; if (base == null) { base = null; } @@ -195,8 +195,7 @@ if (CoffeeScript.listeners('failure').length) { return; } - useColors = process.stdout.isTTY && !process.env.NODE_DISABLE_COLORS; - message = helpers.prettyErrorMessage(err, file || '[stdin]', input, useColors); + message = err.stack || ("" + err); if (o.watch) { return printLine(message + '\x07'); } else { diff --git a/lib/coffee-script/helpers.js b/lib/coffee-script/helpers.js index a7da6c89..95b03111 100644 --- a/lib/coffee-script/helpers.js +++ b/lib/coffee-script/helpers.js @@ -1,6 +1,6 @@ // Generated by CoffeeScript 1.6.3 (function() { - var buildLocationData, extend, flatten, last, repeat, _ref; + var buildLocationData, extend, flatten, last, repeat, syntaxErrorToString, _ref; exports.starts = function(string, literal, start) { return literal === string.substr(start, literal.length); @@ -188,38 +188,41 @@ exports.throwSyntaxError = function(message, location) { var error; - if (location.last_line == null) { - location.last_line = location.first_line; - } - if (location.last_column == null) { - location.last_column = location.first_column; - } error = new SyntaxError(message); error.location = location; + error.toString = syntaxErrorToString; + delete error.stack; throw error; }; - exports.prettyErrorMessage = function(error, filename, code, useColors) { - var codeLine, colorize, end, first_column, first_line, last_column, last_line, marker, message, start, _ref1; - if (!error.location) { - return error.stack || ("" + error); + syntaxErrorToString = function() { + var codeLine, colorize, colorsEnabled, end, filename, first_column, first_line, last_column, last_line, marker, start, _ref1, _ref2; + if (!(this.code && this.location)) { + return Error.prototype.toString.call(this); } - filename = error.filename || filename; - code = error.code || code; - _ref1 = error.location, first_line = _ref1.first_line, first_column = _ref1.first_column, last_line = _ref1.last_line, last_column = _ref1.last_column; - codeLine = code.split('\n')[first_line]; + _ref1 = this.location, first_line = _ref1.first_line, first_column = _ref1.first_column, last_line = _ref1.last_line, last_column = _ref1.last_column; + if (last_line == null) { + last_line = first_line; + } + if (last_column == null) { + last_column = first_column; + } + filename = this.filename || '[stdin]'; + codeLine = this.code.split('\n')[first_line]; start = first_column; end = first_line === last_line ? last_column + 1 : codeLine.length; marker = repeat(' ', start) + repeat('^', end - start); - if (useColors) { + if (typeof process !== "undefined" && process !== null) { + colorsEnabled = process.stdout.isTTY && !process.env.NODE_DISABLE_COLORS; + } + if ((_ref2 = this.colorful) != null ? _ref2 : colorsEnabled) { colorize = function(str) { return "\x1B[1;31m" + str + "\x1B[0m"; }; codeLine = codeLine.slice(0, start) + colorize(codeLine.slice(start, end)) + codeLine.slice(end); marker = colorize(marker); } - message = "" + filename + ":" + (first_line + 1) + ":" + (first_column + 1) + ": error: " + error.message + "\n" + codeLine + "\n" + marker; - return message; + return "" + filename + ":" + (first_line + 1) + ":" + (first_column + 1) + ": error: " + this.message + "\n" + codeLine + "\n" + marker; }; }).call(this); diff --git a/lib/coffee-script/repl.js b/lib/coffee-script/repl.js index 6c792913..76b48aec 100644 --- a/lib/coffee-script/repl.js +++ b/lib/coffee-script/repl.js @@ -1,6 +1,6 @@ // Generated by CoffeeScript 1.6.3 (function() { - var CoffeeScript, addHistory, addMultilineHandler, fs, merge, nodeREPL, path, prettyErrorMessage, replDefaults, vm, _ref; + var CoffeeScript, addHistory, addMultilineHandler, fs, merge, nodeREPL, path, replDefaults, vm; fs = require('fs'); @@ -12,17 +12,17 @@ CoffeeScript = require('./coffee-script'); - _ref = require('./helpers'), merge = _ref.merge, prettyErrorMessage = _ref.prettyErrorMessage; + merge = require('./helpers').merge; replDefaults = { prompt: 'coffee> ', historyFile: process.env.HOME ? path.join(process.env.HOME, '.coffee_history') : void 0, historyMaxInputSize: 10240, "eval": function(input, context, filename, cb) { - var Assign, Block, Literal, Value, ast, err, js, _ref1; + var Assign, Block, Literal, Value, ast, err, js, _ref; input = input.replace(/\uFF00/g, '\n'); input = input.replace(/^\(([\s\S]*)\n\)$/m, '$1'); - _ref1 = require('./nodes'), Block = _ref1.Block, Assign = _ref1.Assign, Value = _ref1.Value, Literal = _ref1.Literal; + _ref = require('./nodes'), Block = _ref.Block, Assign = _ref.Assign, Value = _ref.Value, Literal = _ref.Literal; try { ast = CoffeeScript.nodes(input); ast = new Block([new Assign(new Value(new Literal('_')), ast, '=')]); @@ -33,7 +33,8 @@ return cb(null, vm.runInContext(js, context, filename)); } catch (_error) { err = _error; - return cb(prettyErrorMessage(err, filename, input, true)); + err.code || (err.code = input); + return cb(err); } } }; @@ -132,13 +133,13 @@ module.exports = { start: function(opts) { - var build, major, minor, repl, _ref1; + var build, major, minor, repl, _ref; if (opts == null) { opts = {}; } - _ref1 = process.versions.node.split('.').map(function(n) { + _ref = process.versions.node.split('.').map(function(n) { return parseInt(n); - }), major = _ref1[0], minor = _ref1[1], build = _ref1[2]; + }), major = _ref[0], minor = _ref[1], build = _ref[2]; if (major === 0 && minor < 8) { console.warn("Node 0.8.0+ required for CoffeeScript REPL"); process.exit(1); diff --git a/src/coffee-script.coffee b/src/coffee-script.coffee index 6426214d..ffa2cb0c 100644 --- a/src/coffee-script.coffee +++ b/src/coffee-script.coffee @@ -20,6 +20,17 @@ fileExtensions = ['.coffee', '.litcoffee', '.coffee.md'] # Expose helpers for testing. exports.helpers = helpers +# Function wrapper to add source file information to SyntaxErrors thrown by the +# lexer/parser/compiler. +withPrettyErrors = (fn) -> + (code, options = {}) -> + try + fn.call @, code, options + catch err + err.code or= code + err.filename or= options.filename + throw err + # Compile CoffeeScript code to JavaScript, using the Coffee/Jison compiler. # # If `options.sourceMap` is specified, then `options.filename` must also be specified. All @@ -29,7 +40,7 @@ exports.helpers = helpers # in which case this returns a `{js, v3SourceMap, sourceMap}` # object, where sourceMap is a sourcemap.coffee#SourceMap object, handy for doing programatic # lookups. -exports.compile = compile = (code, options = {}) -> +exports.compile = compile = withPrettyErrors (code, options) -> {merge} = helpers if options.sourceMap @@ -70,13 +81,13 @@ exports.compile = compile = (code, options = {}) -> js # Tokenize a string of CoffeeScript code, and return the array of tokens. -exports.tokens = (code, options) -> +exports.tokens = withPrettyErrors (code, options) -> lexer.tokenize code, options # Parse a string of CoffeeScript code or an array of lexed tokens, and # return the AST. You can then compile it by calling `.compile()` on the root, # or traverse it by using `.traverseChildren()` with a callback. -exports.nodes = (source, options) -> +exports.nodes = withPrettyErrors (source, options) -> if typeof source is 'string' parser.parse lexer.tokenize source, options else @@ -150,8 +161,8 @@ compileFile = (filename, sourceMap) -> # As the filename and code of a dynamically loaded file will be different # from the original file compiled with CoffeeScript.run, add that # information to error so it can be pretty-printed later. - err.filename = filename - err.code = stripped + err.filename or= filename + err.code or= stripped throw err answer diff --git a/src/command.coffee b/src/command.coffee index 7350e1aa..885f7340 100644 --- a/src/command.coffee +++ b/src/command.coffee @@ -141,8 +141,7 @@ compileScript = (file, input, base=null) -> catch err CoffeeScript.emit 'failure', err, task return if CoffeeScript.listeners('failure').length - useColors = process.stdout.isTTY and not process.env.NODE_DISABLE_COLORS - message = helpers.prettyErrorMessage err, file or '[stdin]', input, useColors + message = err.stack or "#{err}" if o.watch printLine message + '\x07' else diff --git a/src/helpers.coffee b/src/helpers.coffee index 5344503e..04cba722 100644 --- a/src/helpers.coffee +++ b/src/helpers.coffee @@ -134,44 +134,45 @@ exports.isCoffee = (file) -> /\.((lit)?coffee|coffee\.md)$/.test file # Determine if a filename represents a Literate CoffeeScript file. exports.isLiterate = (file) -> /\.(litcoffee|coffee\.md)$/.test file -# Throws a SyntaxError with a source file location data attached to it in a -# property called `location`. +# Throws a SyntaxError from a given location. +# The error's `toString` will return an error message following the "standard" +# format ::: plus the line with the error and a +# marker showing where the error is. exports.throwSyntaxError = (message, location) -> - location.last_line ?= location.first_line - location.last_column ?= location.first_column error = new SyntaxError message error.location = location + error.toString = syntaxErrorToString + + # Prevent compiler error from showing the compiler's stacktrace. + delete error.stack + throw error -# Creates a nice error message like, following the "standard" format -# ::: plus the line with the error and a marker -# showing where the error is. -exports.prettyErrorMessage = (error, filename, code, useColors) -> - return error.stack or "#{error}" unless error.location +syntaxErrorToString = -> + return Error::toString.call @ unless @code and @location - # Prefer original source file information stored in the error if present. - filename = error.filename or filename - code = error.code or code + {first_line, first_column, last_line, last_column} = @location + last_line ?= first_line + last_column ?= first_column - {first_line, first_column, last_line, last_column} = error.location - codeLine = code.split('\n')[first_line] + filename = @filename or '[stdin]' + codeLine = @code.split('\n')[first_line] start = first_column # Show only the first line on multi-line errors. end = if first_line is last_line then last_column + 1 else codeLine.length marker = repeat(' ', start) + repeat('^', end - start) - if useColors + # Check to see if we're running on a color-enabled TTY. + if process? + colorsEnabled = process.stdout.isTTY and not process.env.NODE_DISABLE_COLORS + + if @colorful ? colorsEnabled colorize = (str) -> "\x1B[1;31m#{str}\x1B[0m" codeLine = codeLine[...start] + colorize(codeLine[start...end]) + codeLine[end..] marker = colorize marker - message = """ - #{filename}:#{first_line + 1}:#{first_column + 1}: error: #{error.message} + """ + #{filename}:#{first_line + 1}:#{first_column + 1}: error: #{@message} #{codeLine} #{marker} """ - - # Uncomment to add stacktrace. - #message += "\n#{error.stack}" - - message diff --git a/src/repl.coffee b/src/repl.coffee index 2f0ec585..3430f43b 100644 --- a/src/repl.coffee +++ b/src/repl.coffee @@ -3,7 +3,7 @@ path = require 'path' vm = require 'vm' nodeREPL = require 'repl' CoffeeScript = require './coffee-script' -{merge, prettyErrorMessage} = require './helpers' +{merge} = require './helpers' replDefaults = prompt: 'coffee> ', @@ -29,7 +29,9 @@ replDefaults = js = ast.compile bare: yes, locals: Object.keys(context) cb null, vm.runInContext(js, context, filename) catch err - cb prettyErrorMessage(err, filename, input, yes) + # AST's `compile` does not add source code information to syntax errors. + err.code or= input + cb err addMultilineHandler = (repl) -> {rli, inputStream, outputStream} = repl diff --git a/test/error_messages.coffee b/test/error_messages.coffee index 01320f4c..199d1f48 100644 --- a/test/error_messages.coffee +++ b/test/error_messages.coffee @@ -4,12 +4,10 @@ # Ensure that errors of different kinds (lexer, parser and compiler) are shown # in a consistent way. -{prettyErrorMessage} = CoffeeScript.helpers - assertErrorFormat = (code, expectedErrorFormat) -> throws (-> CoffeeScript.run code), (err) -> - message = prettyErrorMessage err, 'test.coffee', code - eq expectedErrorFormat, message + err.colorful = no + eq expectedErrorFormat, "#{err}" yes test "lexer errors formating", -> @@ -18,7 +16,7 @@ test "lexer errors formating", -> insideOutObject = }{ ''', ''' - test.coffee:2:19: error: unmatched } + [stdin]:2:19: error: unmatched } insideOutObject = }{ ^ ''' @@ -28,7 +26,7 @@ test "parser error formating", -> foo in bar or in baz ''', ''' - test.coffee:1:15: error: unexpected RELATION + [stdin]:1:15: error: unexpected RELATION foo in bar or in baz ^^ ''' @@ -38,7 +36,7 @@ test "compiler error formatting", -> evil = (foo, eval, bar) -> ''', ''' - test.coffee:1:14: error: parameter name "eval" is not allowed + [stdin]:1:14: error: parameter name "eval" is not allowed evil = (foo, eval, bar) -> ^^^^ '''