Generate more concise syntax and compiler errors

[brandonwillard]

This PR refactors the exception/error classes and their handling, keeps Hy source strings and their corresponding file information (if any) closer to the origin of an exception (so that calling code isn't responsible for annotating exceptions), and provides minimally intrusive traceback print-out filtering via a context manager that temporarily alters sys.excepthook (this is automatically enabled for Hy command-line sessions).

New environment and package variables have been introduced, as well:

• HY_COLORED_ERRORS, and package variable, hy.errors._hy_colored_errors, that enables/disables manual error coloring, and
• HY_COLORED_AST_OBJECTS, and package variable, hy.models._hy_colored_ast_objects, that enables/disables AST object coloring.

Also, source caching has been added to the Hy interpreter. It's modeled after the approach used by IPython via linecache. Now, for example, traces will show the relevant source lines for functions, objects, etc., defined exclusively in the REPL (see the example below).

• The lexer needs to be more informative #657
• Turn off syntax highlighting of hy expressions by default #1510
• Colored reprs don't play nicely with HyMacroExpansionError #1429
• Support the debugger from the repl #1397
• Ugly error message for bad require #1486 (the error message part)

Notes

As mentioned above, the sys.excepthook provided here only hides internal frames from the traceback print-out, and not the actual traceback object, so, for instance, it does not affect the stack in pdb.

There are other, more powerful and direct approaches that involve altering and/or creating low-level traceback objects (e.g. Jinja's). These allow for a better debugging experience (e.g. without any compiler or parser frames), but they're also more complicated and distribution dependent. In the long run, I think it's better to have custom tracebacks, so I'll keep an eye on good/better ways to do this; for example, this approach is interesting.

Example

Before

hy 0.15.0+39.gd2319dc using CPython(default) 3.6.6 on Linux
=> (with [file (open "a.hy" "w")] (file.write "(print 'hi\""))
11
=> (import a)
Traceback (most recent call last):
  File "/home/bwillard/projects/code/python/hy-master/hy/importer.py", line 138, in hy_eval
    eval(ast_compile(_ast, "<eval_body>", "exec"), namespace)
  File "<eval_body>", line 1, in <module>
  File "<frozen importlib._bootstrap>", line 971, in _find_and_load
  File "<frozen importlib._bootstrap>", line 955, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 665, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 674, in exec_module
  File "<frozen importlib._bootstrap_external>", line 781, in get_code
  File "/home/bwillard/projects/code/python/hy-master/hy/importer.py", line 228, in _hy_source_to_code
    hy_tree = hy_parse(source)
  File "/home/bwillard/projects/code/python/hy-master/hy/importer.py", line 65, in hy_parse
    return HyExpression([HySymbol("do")] + tokenize(source + "\n"))
  File "/home/bwillard/projects/code/python/hy-master/hy/lex/__init__.py", line 19, in tokenize
    return parser.parse(lexer.lex(buf))
  File "/home/bwillard/apps/anaconda3/envs/hy-master-cpython36/lib/python3.6/site-packages/rply/parser.py", line 23, in parse
    t, symstack, statestack, state
  File "/home/bwillard/apps/anaconda3/envs/hy-master-cpython36/lib/python3.6/site-packages/rply/parser.py", line 80, in _reduce_production
    value = p.func(targ)
  File "/home/bwillard/projects/code/python/hy-master/hy/lex/parser.py", line 223, in t_partial_string
    raise PrematureEndOfInput("Premature end of input")
hy.lex.exceptions.PrematureEndOfInput: LexException: Premature end of input

=> (require blah)
Traceback (most recent call last):
  File "/home/bwillard/projects/code/python/hy-master/hy/importer.py", line 120, in hy_eval
    _ast, expr = hy_compile(hytree, module_name, get_expr=True)
  File "/home/bwillard/projects/code/python/hy-master/hy/compiler.py", line 1716, in hy_compile
    result = compiler.compile(tree)
  File "/home/bwillard/projects/code/python/hy-master/hy/compiler.py", line 350, in compile
    ret = self.compile_atom(tree)
  File "/home/bwillard/projects/code/python/hy-master/hy/compiler.py", line 344, in compile_atom
    return Result() + _model_compilers[type(atom)](self, atom)
  File "/home/bwillard/projects/code/python/hy-master/hy/compiler.py", line 1584, in compile_expression
    self, expr, unmangle(sroot), *parse_tree)
  File "/home/bwillard/projects/code/python/hy-master/hy/compiler.py", line 573, in compile_do
    return self._compile_branch(body)
  File "/home/bwillard/projects/code/python/hy-master/hy/compiler.py", line 444, in _compile_branch
    ret += self.compile(exprs[-1])
  File "/home/bwillard/projects/code/python/hy-master/hy/compiler.py", line 361, in compile
    raise_empty(HyCompileError, e, sys.exc_info()[2])
  File "<string>", line 1, in raise_empty
hy.errors.HyCompileError: Internal Compiler Bug 😱
⤷ ModuleNotFoundError: No module named 'blah'
Compilation traceback:
File "/home/bwillard/projects/code/python/hy-master/hy/compiler.py", line 350, in compile
    ret = self.compile_atom(tree)
  File "/home/bwillard/projects/code/python/hy-master/hy/compiler.py", line 344, in compile_atom
    return Result() + _model_compilers[type(atom)](self, atom)
  File "/home/bwillard/projects/code/python/hy-master/hy/compiler.py", line 1584, in compile_expression
    self, expr, unmangle(sroot), *parse_tree)
  File "/home/bwillard/projects/code/python/hy-master/hy/compiler.py", line 1154, in compile_import_or_require
    importlib.import_module(module)
  File "/home/bwillard/apps/anaconda3/envs/hy-master-cpython36/lib/python3.6/importlib/__init__.py", line 126, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 994, in _gcd_import
  File "<frozen importlib._bootstrap>", line 971, in _find_and_load
  File "<frozen importlib._bootstrap>", line 953, in _find_and_load_unlocked
=> (defn test [x]
... (print(+ 1 x))
... )
=> (test 'a)
Traceback (most recent call last):
  File "/home/bwillard/projects/code/python/hy-master/hy/importer.py", line 141, in hy_eval
    return eval(ast_compile(expr, "<eval>", "eval"), namespace)
  File "<eval>", line 1, in <module>
  File "<eval_body>", line 1, in test
TypeError: unsupported operand type(s) for +: 'int' and 'HySymbol'
=> (defmacro blah [x] `(print ~@z))
<function <lambda> at 0x7f1351095730>
=> (defmacro bleh [q] `(print (blah ~q)))
<function <lambda> at 0x7f1351095840>
=> (bleh y)
  File "<input>", unknown location
HyMacroExpansionError: expanding `blah': NameError("name 'z' is not defined",)

=> (defn foo []
... (setv bar 42)
... (import pdb) (pdb.set-trace))
=> (foo)
--Return--
> <eval_body>(1)foo()->None
(Pdb) l
[EOF]

After

hy 0.15.0+45.ga8d6a48 using CPython(default) 3.6.6 on Linux
=> (with [file (open "a.hy" "w")] (file.write "(print 'hi\""))
11
=> (import a)
Traceback (most recent call last):
  File "stdin-99c0d4eb813cb56c622500ab4696f5bbb3cb3660", line 1, in <module>
    (import a)
  File "<frozen importlib._bootstrap>", line 971, in _find_and_load
  File "<frozen importlib._bootstrap>", line 955, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 665, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 674, in exec_module
  File "<frozen importlib._bootstrap_external>", line 781, in get_code
  File "/tmp/a.hy", line 1
    (print 'hi"
              ^
hy.lex.exceptions.PrematureEndOfInput: Partial string literal
=> (require blah)
Traceback (most recent call last):
  File "stdin-97db2eacebe610e14f46e56e428d68ec1c466f2f", line 1, in <module>
    (require blah)
hy.errors.HyRequireError: No module named 'blah'
=> (defn test [x]
... (print (+ 1 x))
... )
None
=> (test 'a)
Traceback (most recent call last):
  File "stdin-e199a43c50b388bff5e7901a7a1f02f5525d862d", line 1, in <module>
    (test 'a)
  File "stdin-71d0b2f8a864d8760f5a5eab4efa209694ee2714", line 2, in test
    (print (+ 1 x))
TypeError: unsupported operand type(s) for +: 'int' and 'HySymbol'
=> (defmacro blah [x] `(print ~@z))
<function <lambda> at 0x7fd6741319d8>
=> (defmacro bleh [q] `(print (blah ~q)))
<function <lambda> at 0x7fd674131268>
=> (bleh y)
Traceback (most recent call last):
  File "stdin-ff397ee5ec5c34f03703a2acccc9f62ed9b3a9ae", line 1, in <module>
    (bleh y)
  File "stdin-a0f7e3bfef673f0bae54abd3721c76c25db16c65", line 1, in <lambda>
    (defmacro blah [x] `(print ~@z))
hy.errors.HyMacroExpansionError: expanding macro blah
  NameError: name 'z' is not defined
=> (defn foo []
... (setv bar 42)
... (import pdb) (pdb.set-trace))
None
=> (foo)
--Return--
> /home/bwillard/stdin-2741f77da2a8503d126c87cd48d527a65903cc14(3)foo()->None
(Pdb) l
  1  	(defn foo []
  2  	(setv bar 42)
  3  ->	(import pdb) (pdb.set-trace))
[EOF]

[Kodiologist]

It looks like a PrematureEndOfInput exception will display the whole file, or at least, a lot of it. Also, what's the arrow pointing at?

$ perl -E 'say foreach "a".."z"; say "("' >/tmp/test.hy && hy /tmp/test.hy
Traceback (most recent call last):
  File "/home/hippo/Desktop/hyenv/bin/hy", line 12, in <module>
    sys.exit(hy_main())
  File "/home/hippo/Desktop/hyenv/hy/hy/cmdline.py", line 368, in hy_main
    sys.exit(cmdline_handler("hy", sys.argv))
  File "/home/hippo/Desktop/hyenv/hy/hy/cmdline.py", line 354, in cmdline_handler
    runhy.run_path(filename, run_name='__main__')
  File "/usr/local/lib/python3.7/runpy.py", line 261, in run_path
    code, fname = _get_code_from_file(run_name, path_name)
  File "/home/hippo/Desktop/hyenv/hy/hy/importer.py", line 221, in _get_code_from_file
    code = _hy_code_from_file(fname, loader_type=HyLoader)
  File "/home/hippo/Desktop/hyenv/hy/hy/importer.py", line 200, in _hy_code_from_file
    code = loader.get_code(modname)
  File "<frozen importlib._bootstrap_external>", line 860, in get_code
  File "/home/hippo/Desktop/hyenv/hy/hy/importer.py", line 246, in _hy_source_to_code
    hy_tree = hy_parse(source, filename=path)
  File "/home/hippo/Desktop/hyenv/hy/hy/importer.py", line 76, in hy_parse
    reraise(type(e), e.args, None)
  File "/home/hippo/Desktop/hyenv/hy/hy/_compat.py", line 5, in reraise
    try:
  File "/tmp/test.hy", line 1
    a
b
c
[ and so on ]
y
z
(

    ^
hy.lex.exceptions.PrematureEndOfInput: Premature end of input

[brandonwillard]

Looks like it's pointing at the end of the file, where the paren isn't closed, but the indenting in the print-out stops after a.

[brandonwillard]

Just pushed a fix that should take care of that print-out.

> hy test.hy
Traceback (most recent call last):
  File "/home/bwillard/apps/anaconda3/envs/hy-dev/bin/hy", line 11, in <module>
    load_entry_point('hy', 'console_scripts', 'hy')()
  File "/home/bwillard/apps/anaconda3/envs/hy-dev/lib/python3.6/runpy.py", line 261, in run_path
    code, fname = _get_code_from_file(run_name, path_name)
  File "<frozen importlib._bootstrap_external>", line 781, in get_code
  File "/tmp/test.hy", line 27
    (
    ^
hy.lex.exceptions.PrematureEndOfInput: Premature end of input

[Kodiologist]

This is a huge commit. Can you group your changes into logical units instead?

[Kodiologist]

Would you please:

• Try to split the big commits into smaller logical units.
• Separate out refactoring and other code changes unrelated to the purpose of a commit (e.g., adding object to the base classes of HyREPL) into other commits.
• Generally speaking, don't make a commit that undoes or alters changes that were introduced by a previous commit in the same PR. Instead, change the previous commit itself.
• Avoid intermediate variables that are used only once (e.g., instead of x = a + b; z = x + y, write z = a + b + y).
• Remove comments with XXX, TODO, FIXME, or questions, or any other comments that do something other than describe the current state and usage of the code. If you wish, make issues out of these removed comments or post them to this PR.

Let me know when the PR is ready for review.

[brandonwillard]

All right, I further separated those commits by their functionality. Unfortunately, the first one is necessarily large-ish, due to how much code it affects; however, most of the changes are simple (e.g. adding new arguments for source and filename).

[Kodiologist]

It's okay. There will at least be fewer added lines once you introduce a helper for HyTypeError's new parameters.

[brandonwillard]

All right, added some helpers for the relevant errors.

[Kodiologist]

You mean like this?

raise HySyntaxError.from_expression(
    "Pythons < 3.5 allow only one "
    "`unpack-mapping` per call",
    expr, self.filename, self.source)

Why not something like this?

raise self.error(
    "Pythons < 3.5 allow only one "
    "`unpack-mapping` per call",
    expr)

Then you don't have to pass in self.filename and self.source every time.

[brandonwillard]

No, the helper functions are the additional class methods in HySyntaxError and LexException.

Sure, a method in the compiler could be added, too.

[brandonwillard]

Added a HyASTCompiler._syntax_error method.

[Kodiologist]

Much nicer.

[Kodiologist]

The commit message for "Make colored output configurable" mentions HY_FILTER_INTERNAL_ERRORS where I think you meant HY_COLORED_ERRORS (right?).

It seems that HY_FILTER_INTERNAL_ERRORS only has an effect when set to the empty string. I think it's counterintuitive that a binary environment variable would distinguish between an empty string and absence. How about inverting the variable and calling it HY_SHOW_INTERNAL_ERRORS or something?

At least at the tip of the PR, there's a regression such that a return value of None is no longer silenced at the REPL.

[Kodiologist]

I'm sorry, Brandon, I've only just noticed that you updated this. It seems that GitHub doesn't send a notification for force-pushes. I'll continue review soon.

[brandonwillard]

No problem; I try to respond with a message after force-pushes, since I also don't recall ever getting such notifications.

I believe I've addressed the matters in your last comment, except for the REPL return value. Is it by design that it doesn't display None?

The Python REPL doesn't either, but, if I recall, Clojure and some other Lisps do. The only immediate advantage— that I can think of right now—involves code that reads from the REPL output (e.g. comint in Emacs); it's often easier to handle situations in which output is guaranteed/consistent.

Regardless, it's easy to change.

[Kodiologist]

Is it by design that it doesn't display None?

Yeah, the non-display of None has to be implemented separately, since None doesn't str or repr to the empty string (and then there's the matter of the newline). I think it helps keep the output neater.

[Kodiologist]

So do that, if you don't mind, and then I'll look again.

[brandonwillard]

All right, now it doesn't print None expression values in the console

[Kodiologist]

Thanks. I just reviewed more of these commits and made more changes and I'm getting a "Permission denied" error when I try to push. Do you have the option to accept changes from reviewers switched off?

[brandonwillard]

Yes

[Kodiologist]

Can you turn it back on? Otherwise, I don't think there's a way for me to cleanly rewrite history. You can make a copy first with git branch clean-up-exceptions--backup or so.

[brandonwillard]

I'm fine with one reasonably sized set of final reviewer changes—which is what I thought the last one was—but a review process consisting of rebases is too confusing and time consuming.

Please describe the changes you think are necessary to merge this PR.

[Kodiologist]

I could save up all my edits till I'm done reviewing the entire PR, but I thought you'd rather get them as I make them, rather than get a giant stack of them all at once after a long time.

I've pushed my current version of clean-up-exceptions to my own fork. If you'd prefer to wait till I've finished reviewing the whole PR (which hopefully won't be that much longer), I can let you know when I'm done.

[Kodiologist]

So far, at any rate, I've reviewed up to and including "Improve correspondence with Python errors and console behavior", and made the following notable changes:

• Replaced usages of future_builtins.filter with generator expressions (and removed the import of future_builtins.map since it seemed to have no effect).

• Moved the introduction of _initialize_env_var back to the first commit where it's useful, and moved it from compat.py (since it doesn't have anything to do with portability) to __init__.py.

• Shortened the implementation of test_bin_hy_macro_require.

• Removed the following comment:

  def can_eval(expr):
  # XXX: filename and source are needed to avoid ridiculous pytest
  # issues.
  return hy_eval(hy_parse(expr))

[Kodiologist]

Okay, all done. Sorry for the long delay. I had a busy week and I expected there'd be a lot more work to do here, but I made only minor changes to the last two commits.

[Kodiologist]

If all looks good to you, replace your version of the branch with mine and we should be good to merge (pending the final Travis check).

[brandonwillard]

Finally had a chance to look at your branch; give it a push.

[Kodiologist]

Okay, I'll do it this afternoon. There will probably be some merge conflicts, but if those can be smoothed out without major issues and Travis has no surprises in store, I'll merge.