From f6160c755a59a8d8203fb58ad66b03b4f13e158e Mon Sep 17 00:00:00 2001 From: Bob Tolbert Date: Sun, 22 Dec 2013 12:56:03 -0700 Subject: [PATCH] Much better version of new error messages. This version is much simpler. At the point that the exception is raised, we don't have access to the actual source, just the current expression. but as the exception percolates up, we can intercept it, add the source and the re-raise it. Then at the final point, in the cmdline handler, we can choose to let the entire traceback print, or just the simpler, direct error message. And even with the full traceback, the last bit is nicely formatted just like the shorter, simpler message. The error message is colored if clint is installed, but to avoid yet another dependency, you get monochrome without clint. I'm sure there is a better way to do the markup, the current method is kludgy but works. I wish there was more shared code between HyTypeError and LexException but they are kind of different in some fundamental ways. This doesn't work (yet) with runtime errors generated from Python, like NameError, but I have a method that can catch NameError and turn it into a more pleasing output. Finally, there is no obvious way to raise HyTypeError from pure Hy code, so methods in core/language.hy throw ugly TypeError/ValueError. --- hy/cmdline.py | 49 ++++++++++++++++++++++++++------- hy/compiler.py | 55 ++++++++++++++++++++++++++++++++++--- hy/errors.py | 39 ++++++++++++++++++++++++++ hy/importer.py | 26 +++++++++++++----- hy/lex/__init__.py | 5 ++-- hy/lex/exceptions.py | 39 ++++++++++++++++++++++++-- hy/lex/parser.py | 7 ++--- tests/compilers/test_ast.py | 14 ++++++---- tests/test_bin.py | 2 +- 9 files changed, 200 insertions(+), 36 deletions(-) diff --git a/hy/cmdline.py b/hy/cmdline.py index eb621b0..e40862d 100644 --- a/hy/cmdline.py +++ b/hy/cmdline.py @@ -5,6 +5,7 @@ # Copyright (c) 2013 Konrad Hinsen # Copyright (c) 2013 Thom Neale # Copyright (c) 2013 Will Kahn-Greene +# Copyright (c) 2013 Bob Tolbert # # Permission is hereby granted, free of charge, to any person obtaining a # copy of this software and associated documentation files (the "Software"), @@ -32,7 +33,7 @@ import sys import hy from hy.lex import LexException, PrematureEndOfInput, tokenize -from hy.compiler import hy_compile +from hy.compiler import hy_compile, HyTypeError from hy.importer import ast_compile, import_buffer_to_module from hy.completer import completion @@ -83,8 +84,11 @@ class HyREPL(code.InteractiveConsole): tokens = tokenize(source) except PrematureEndOfInput: return True - except LexException: - self.showsyntaxerror(filename) + except LexException as e: + if e.source is None: + e.source = source + e.filename = filename + print(e) return False try: @@ -92,6 +96,12 @@ class HyREPL(code.InteractiveConsole): if self.spy: print_python_code(_ast) code = ast_compile(_ast, filename, symbol) + except HyTypeError as e: + if e.source is None: + e.source = source + e.filename = filename + print(e) + return False except Exception: self.showtraceback() return False @@ -154,22 +164,34 @@ def ideas_macro(): require("hy.cmdline", "__console__") require("hy.cmdline", "__main__") +SIMPLE_TRACEBACKS = True + def run_command(source): try: import_buffer_to_module("__main__", source) - except LexException as exc: - # TODO: This would be better if we had line, col info. - print(source) - print(repr(exc)) - return 1 + except (HyTypeError, LexException) as e: + if SIMPLE_TRACEBACKS: + print(e) + return 1 + raise + except Exception: + raise return 0 def run_file(filename): from hy.importer import import_file_to_module - import_file_to_module("__main__", filename) - return 0 # right? + try: + import_file_to_module("__main__", filename) + except (HyTypeError, LexException) as e: + if SIMPLE_TRACEBACKS: + print(e) + return 1 + raise + except Exception: + raise + return 0 def run_repl(hr=None, spy=False): @@ -218,6 +240,9 @@ def cmdline_handler(scriptname, argv): parser.add_argument("-v", action="version", version=VERSION) + parser.add_argument("--show_tracebacks", action="store_true", + help="show complete tracebacks for Hy exceptions") + # this will contain the script/program name and any arguments for it. parser.add_argument('args', nargs=argparse.REMAINDER, help=argparse.SUPPRESS) @@ -228,6 +253,10 @@ def cmdline_handler(scriptname, argv): options = parser.parse_args(argv[1:]) + if options.show_tracebacks: + global SIMPLE_TRACEBACKS + SIMPLE_TRACEBACKS = False + # reset sys.argv like Python sys.argv = options.args or [""] diff --git a/hy/compiler.py b/hy/compiler.py index 1c7c156..9fa4c69 100644 --- a/hy/compiler.py +++ b/hy/compiler.py @@ -4,6 +4,7 @@ # Copyright (c) 2013 Julien Danjou # Copyright (c) 2013 Nicolas Dandrimont # Copyright (c) 2013 James King +# Copyright (c) 2013 Bob Tolbert # # Permission is hereby granted, free of charge, to any person obtaining a # copy of this software and associated documentation files (the "Software"), @@ -93,11 +94,52 @@ class HyTypeError(TypeError): def __init__(self, expression, message): super(HyTypeError, self).__init__(message) self.expression = expression + self.message = message + self.source = None + self.filename = None def __str__(self): - return (super(HyTypeError, self).__str__() + " (line %s, column %d)" - % (self.expression.start_line, - self.expression.start_column)) + from hy.errors import colored + + line = self.expression.start_line + start = self.expression.start_column + end = self.expression.end_column + + source = [] + if self.source is not None: + source = self.source.split("\n")[line-1:self.expression.end_line] + + if line == self.expression.end_line: + length = end - start + else: + length = len(source[0]) - start + + result = "" + + result += ' File "%s", line %d, column %d\n\n' % (self.filename, + line, + start) + + if len(source) == 1: + result += ' %s\n' % colored.red(source[0]) + result += ' %s%s\n' % (' '*(start-1), + colored.green('^' + '-'*(length-1) + '^')) + if len(source) > 1: + result += ' %s\n' % colored.red(source[0]) + result += ' %s%s\n' % (' '*(start-1), + colored.green('^' + '-'*length)) + if len(source) > 2: # write the middle lines + for line in source[1:-1]: + result += ' %s\n' % colored.red("".join(line)) + result += ' %s\n' % colored.green("-"*len(line)) + + # write the last line + result += ' %s\n' % colored.red("".join(source[-1])) + result += ' %s\n' % colored.green('-'*(end-1) + '^') + + result += colored.yellow("HyTypeError: %s\n\n" % self.message) + + return result _compile_table = {} @@ -341,7 +383,7 @@ def checkargs(exact=None, min=None, max=None, even=None): if min is not None and (len(expression) - 1) < min: _raise_wrong_args_number( expression, - "`%%s' needs at least %d arguments, got %%d" % (min)) + "`%%s' needs at least %d arguments, got %%d." % (min)) if max is not None and (len(expression) - 1) > max: _raise_wrong_args_number( @@ -430,6 +472,8 @@ class HyASTCompiler(object): # nested; so let's re-raise this exception, let's not wrap it in # another HyCompileError! raise + except HyTypeError as e: + raise except Exception as e: raise HyCompileError(e, sys.exc_info()[2]) @@ -1584,6 +1628,9 @@ class HyASTCompiler(object): fn.replace(ofn) # Get the object we want to take an attribute from + if len(expression) < 2: + raise HyTypeError(expression, + "attribute access requires object") func = self.compile(expression.pop(1)) # And get the attribute diff --git a/hy/errors.py b/hy/errors.py index 323d67e..be932eb 100644 --- a/hy/errors.py +++ b/hy/errors.py @@ -1,4 +1,5 @@ # Copyright (c) 2013 Paul Tagliamonte +# Copyright (c) 2013 Bob Tolbert # # Permission is hereby granted, free of charge, to any person obtaining a # copy of this software and associated documentation files (the "Software"), @@ -25,3 +26,41 @@ class HyError(Exception): Exception. """ pass + + +try: + from clint.textui import colored +except: + class colored: + + @staticmethod + def black(foo): + return foo + + @staticmethod + def red(foo): + return foo + + @staticmethod + def green(foo): + return foo + + @staticmethod + def yellow(foo): + return foo + + @staticmethod + def blue(foo): + return foo + + @staticmethod + def magenta(foo): + return foo + + @staticmethod + def cyan(foo): + return foo + + @staticmethod + def white(foo): + return foo diff --git a/hy/importer.py b/hy/importer.py index b32b71a..35cd685 100644 --- a/hy/importer.py +++ b/hy/importer.py @@ -1,4 +1,5 @@ # Copyright (c) 2013 Paul Tagliamonte +# Copyright (c) 2013 Bob Tolbert # # Permission is hereby granted, free of charge, to any person obtaining a # copy of this software and associated documentation files (the "Software"), @@ -19,10 +20,9 @@ # DEALINGS IN THE SOFTWARE. from py_compile import wr_long, MAGIC -from hy.compiler import hy_compile +from hy.compiler import hy_compile, HyTypeError from hy.models import HyObject -from hy.lex import tokenize - +from hy.lex import tokenize, LexException from io import open import marshal @@ -73,17 +73,29 @@ def import_file_to_module(module_name, fpath): mod = imp.new_module(module_name) mod.__file__ = fpath eval(ast_compile(_ast, fpath, "exec"), mod.__dict__) + except (HyTypeError, LexException) as e: + if e.source is None: + fp = open(fpath, 'rt') + e.source = fp.read() + fp.close() + e.filename = fpath + raise except Exception: sys.modules.pop(module_name, None) raise - return mod def import_buffer_to_module(module_name, buf): - _ast = import_buffer_to_ast(buf, module_name) - mod = imp.new_module(module_name) - eval(ast_compile(_ast, "", "exec"), mod.__dict__) + try: + _ast = import_buffer_to_ast(buf, module_name) + mod = imp.new_module(module_name) + eval(ast_compile(_ast, "", "exec"), mod.__dict__) + except (HyTypeError, LexException) as e: + if e.source is None: + e.source = buf + e.filename = '' + raise return mod diff --git a/hy/lex/__init__.py b/hy/lex/__init__.py index 651f89a..8153aa8 100644 --- a/hy/lex/__init__.py +++ b/hy/lex/__init__.py @@ -33,6 +33,5 @@ def tokenize(buf): return parser.parse(lexer.lex(buf)) except LexingError as e: pos = e.getsourcepos() - raise LexException( - "Could not identify the next token at line %s, column %s" % ( - pos.lineno, pos.colno)) + raise LexException("Could not identify the next token.", + pos.lineno, pos.colno) diff --git a/hy/lex/exceptions.py b/hy/lex/exceptions.py index 21b2700..4c4b760 100644 --- a/hy/lex/exceptions.py +++ b/hy/lex/exceptions.py @@ -1,4 +1,5 @@ # Copyright (c) 2013 Nicolas Dandrimont +# Copyright (c) 2013 Bob Tolbert # # Permission is hereby granted, free of charge, to any person obtaining a # copy of this software and associated documentation files (the "Software"), @@ -23,9 +24,43 @@ from hy.errors import HyError class LexException(HyError): """Error during the Lexing of a Hython expression.""" - pass + def __init__(self, message, lineno, colno): + super(LexException, self).__init__(message) + self.message = message + self.lineno = lineno + self.colno = colno + self.source = None + self.filename = '' + + def __str__(self): + from hy.errors import colored + + line = self.lineno + start = self.colno + + result = "" + + source = self.source.split("\n") + + if line > 0 and start > 0: + result += ' File "%s", line %d, column %d\n\n' % (self.filename, + line, + start) + + if len(self.source) > 0: + source_line = source[line-1] + else: + source_line = "" + + result += ' %s\n' % colored.red(source_line) + result += ' %s%s\n' % (' '*(start-1), colored.green('^')) + + result += colored.yellow("LexException: %s\n\n" % self.message) + + return result class PrematureEndOfInput(LexException): """We got a premature end of input""" - pass + def __init__(self, message): + super(PrematureEndOfInput, self).__init__(message, -1, -1) diff --git a/hy/lex/parser.py b/hy/lex/parser.py index 9e11069..72f0d8f 100644 --- a/hy/lex/parser.py +++ b/hy/lex/parser.py @@ -258,12 +258,11 @@ def t_identifier(p): def error_handler(token): tokentype = token.gettokentype() if tokentype == '$end': - raise PrematureEndOfInput + raise PrematureEndOfInput("Premature end of input") else: raise LexException( - "Ran into a %s where it wasn't expected at line %s, column %s" % - (tokentype, token.source_pos.lineno, token.source_pos.colno) - ) + "Ran into a %s where it wasn't expected." % tokentype, + token.source_pos.lineno, token.source_pos.colno) parser = pg.build() diff --git a/tests/compilers/test_ast.py b/tests/compilers/test_ast.py index c6590fa..43b33ea 100644 --- a/tests/compilers/test_ast.py +++ b/tests/compilers/test_ast.py @@ -22,6 +22,7 @@ from __future__ import unicode_literals from hy import HyString +from hy.models import HyObject from hy.compiler import hy_compile, HyCompileError, HyTypeError from hy.lex import tokenize @@ -42,10 +43,14 @@ def can_compile(expr): def cant_compile(expr): - expr = tokenize(expr) try: - hy_compile(expr, "__main__") + hy_compile(tokenize(expr), "__main__") assert False + except HyTypeError as e: + # Anything that can't be compiled should raise a user friendly + # error, otherwise it's a compiler bug. + assert isinstance(e.expression, HyObject) + assert e.message except HyCompileError as e: # Anything that can't be compiled should raise a user friendly # error, otherwise it's a compiler bug. @@ -422,8 +427,7 @@ def test_compile_error(): """Ensure we get compile error in tricky cases""" try: can_compile("(fn [] (= 1))") - except HyCompileError as e: - assert(str(e) - == "`=' needs at least 2 arguments, got 1 (line 1, column 8)") + except HyTypeError as e: + assert(e.message == "`=' needs at least 2 arguments, got 1.") else: assert(False) diff --git a/tests/test_bin.py b/tests/test_bin.py index 06cd455..8f73f56 100644 --- a/tests/test_bin.py +++ b/tests/test_bin.py @@ -64,7 +64,7 @@ def test_bin_hy_cmd(): ret = run_cmd("hy -c \"(koan\"") assert ret[0] == 1 - assert "PrematureEndOfInput" in ret[1] + assert "Premature end of input" in ret[1] def test_bin_hy_icmd():