Repository: hylang/hy

Issue:
Improvement of error messages for pattern macros
Advancements to `funcpaserlib` such as https://github.com/vlasovskikh/funcparserlib/issues/52 should allow improving error messages produced while parsing calls to Hy's core pattern macros. In an error like the below example, `expected: some(...)` could probably be changed to `expected symbol`.

```
Traceback (most recent call last):
  File "<string>", line 1
    (defmacro 1)
              ^
hy.errors.HySyntaxError: parse error for pattern macro 'defmacro': got unexpected token: hy.models.Integer(1), expected: some(...)
```

It's conceivable that most of the patterns for Hy's core macros are too complex to admit useful automatic descriptions of the kind of form that's expected. If so, it's probably better to forget about this than to hand-write descriptions for lots of patterns. Manually writing lots of error messages like that is the sort of thing I intended to avoid by using `funcparserlib` in the first place.


A simpler sort of improvement would be rewording "unexpected end of input", as provoked by e.g. `(defmacro)`, to something that makes it clearer that the form, as opposed to the entire file, ended prematurely.

Discussion:
Kodiologist: I've made it so that case now says "expected: `annotate` form or Symbol". But if you want other improvements, I think you'll have to enumerate them. There's sort of no end to how much refinement one could theoretically add with `.named`. I think.
scauligi: Awesome! I'll take a pass to see if there's anything else I can find to include in this PR, but otherwise we can always add further improvements as we stumble across them.
Kodiologist: Sounds good.
scauligi: woof felt kind of like doing a mini-audit of the language
also ended up shaking out a couple other bugs which I'll file as issues for now since I'm not sure when I'll have time to dig into them

---

`except` seems to fail the whole expression:
```hy
=> (try (except))
... got unexpected token: hy.models.Expression([
  hy.models.Symbol('except')]), expected: end of macro call
```
it also gives a different (but also wrong) error when the `try` has a body, eg:
```hy
=> (try (. re S) (except 3))
                 ^
hy.errors.HySyntaxError: parse error for pattern macro 'try': got unexpected token: hy.models.Expression([
  hy.models.Symbol('.'),
  hy.models.Symbol('re'),
  hy.models.Symbol('S')]), expected: some(...)
```

---

`defclass` fails when the superclass list is missing:
```hy
=> (defclass a (def woops []))
               ^
hy.errors.HySyntaxError: parse error for pattern macro 'defclass': got unexpected token: hy.models.Expression([
  hy.models.Symbol('def'),
  hy.models.Symbol('woops'),
  hy.models.List()]), expected: end of macro call
```

---

forms taking zero-or-more symbols: (`global`/`nonlocal`) only consider ending:
```hy
=> (global 3)
... got unexpected token: hy.models.Integer(3), expected: end of macro call
=> (global a b 3)
... got unexpected token: hy.models.Integer(3), expected: end of macro call
```
would be better as `expected: Symbol or end of macro call`

similar for slightly more complex forms (`.`, `import`/`require`):
```hy
=> (. thing 3)
... got unexpected token: hy.models.Integer(3), expected: end of macro call
```
`(. thing)` is technically valid but probably not where we want to guide the user

---

forms with binding lists: (`for`,`with`,`with/a`, `let`):
```hy
=> (for)
... got unexpected end of macro call, expected: List (i.e., `[ … ]`)
```
we can name this something like `bindings_list` but I don't feel strongly about this

similar for (`fn`,`defn`,`defn/a`,`defmacro`) and something like `params_list` but I feel even less strongly about it

---

As an aside, it's a little annoying that funcparserlib bakes model `repr` strings into the errors; it would be nicer if we could have eg `unexpected form '3` instead of `unexpected token hy.models.Integer(3)`, but at least my initial attempts at get around that felt like more trouble than it's worth

Patch:
diff --git a/NEWS.rst b/NEWS.rst
index 345094bb..f9acfdcc 100644
--- a/NEWS.rst
+++ b/NEWS.rst
@@ -16,6 +16,11 @@ New Features
 ------------------------------
 * `defn`, `defn/a`, and `defclass` now support type parameters.
 
+Misc. Improvements
+------------------------------
+* Some syntax errors raised by core macros now have more informative
+  messages.
+
 0.27.0 (released 2023-07-06)
 =============================
 
diff --git a/hy/core/result_macros.py b/hy/core/result_macros.py
index 1fad8e85..2db198d5 100644
--- a/hy/core/result_macros.py
+++ b/hy/core/result_macros.py
@@ -72,7 +72,9 @@ def pvalue(root, wanted):
 
 
 def maybe_annotated(target):
-    return pexpr(sym("annotate") + target + FORM) | target >> (lambda x: (x, None))
+    return (
+       pexpr(sym("annotate") + target + FORM).named('`annotate` form') |
+       (target >> (lambda x: (x, None))))
 
 
 def dotted(name):
@@ -727,9 +729,9 @@ loopers = many(
 )
 
 
-@pattern_macro(
-    ["for"], [brackets(loopers), many(notpexpr("else")) + maybe(dolike("else"))]
-)
+@pattern_macro(["for"], [
+    brackets(loopers, name = 'square-bracketed loop clauses'),
+    many(notpexpr("else")) + maybe(dolike("else"))])
 @pattern_macro(["lfor", "sfor", "gfor"], [loopers, FORM])
 @pattern_macro(["dfor"], [loopers, finished])
 # Here `finished` is a hack replacement for FORM + FORM:
diff --git a/hy/macros.py b/hy/macros.py
index d684310f..c1ee5604 100644
--- a/hy/macros.py
+++ b/hy/macros.py
@@ -68,7 +68,7 @@ def pattern_macro(names, pattern, shadow=None):
                     raise hy_compiler._syntax_error(
                         expr[min(e.state.pos + 1, len(expr) - 1)],
                         "parse error for pattern macro '{}': {}".format(
-                            root, e.msg.replace("<EOF>", "end of form")
+                            root, e.msg.replace("end of input", "end of macro call")
                         ),
                     )
                 return fn(hy_compiler, expr, root, *parse_tree)
diff --git a/hy/model_patterns.py b/hy/model_patterns.py
index 14d06a33..1d703395 100644
--- a/hy/model_patterns.py
+++ b/hy/model_patterns.py
@@ -31,11 +31,11 @@ from hy.models import (
     Tuple,
 )
 
-FORM = some(lambda _: True)
-SYM = some(lambda x: isinstance(x, Symbol))
-KEYWORD = some(lambda x: isinstance(x, Keyword))
-STR = some(lambda x: isinstance(x, String))  # matches literal strings only!
-LITERAL = some(lambda x: isinstance(x, (String, Integer, Float, Complex, Bytes)))
+FORM = some(lambda _: True).named('form')
+SYM = some(lambda x: isinstance(x, Symbol)).named('Symbol')
+KEYWORD = some(lambda x: isinstance(x, Keyword)).named('Keyword')
+STR = some(lambda x: isinstance(x, String)).named('String')  # matches literal strings only!
+LITERAL = some(lambda x: isinstance(x, (String, Integer, Float, Complex, Bytes))).named('literal')
 
 
 def sym(wanted):
@@ -49,9 +49,10 @@ def keepsym(wanted):
 
 
 def _sym(wanted, f=lambda x: x):
+    name = '`' + wanted + '`'
     if wanted.startswith(":"):
-        return f(a(Keyword(wanted[1:])))
-    return f(some(lambda x: x == Symbol(wanted)))
+        return f(a(Keyword(wanted[1:]))).named(name)
+    return f(some(lambda x: x == Symbol(wanted))).named(name)
 
 
 def whole(parsers):
@@ -64,29 +65,23 @@ def whole(parsers):
     return reduce(add, parsers) + skip(finished)
 
 
-def _grouped(group_type, parsers):
-    return some(lambda x: isinstance(x, group_type)) >> (
-        lambda x: group_type(whole(parsers).parse(x)).replace(x, recursive=False)
+def _grouped(group_type, syntax_example, name, parsers):
+    return (
+        some(lambda x: isinstance(x, group_type)).named(name or
+            f'{group_type.__name__} (i.e., `{syntax_example}`)') >>
+        (lambda x: group_type(whole(parsers).parse(x)).replace(x, recursive=False))
     )
-
-
-def brackets(*parsers):
+def brackets(*parsers, name = None):
     "Parse the given parsers inside square brackets."
-    return _grouped(List, parsers)
-
-
-def in_tuple(*parsers):
-    return _grouped(Tuple, parsers)
-
-
-def braces(*parsers):
+    return _grouped(List, '[ … ]', name, parsers)
+def in_tuple(*parsers, name = None):
+    return _grouped(Tuple, '#( … )', name, parsers)
+def braces(*parsers, name = None):
     "Parse the given parsers inside curly braces"
-    return _grouped(Dict, parsers)
-
-
-def pexpr(*parsers):
+    return _grouped(Dict, '{ … }', name, parsers)
+def pexpr(*parsers, name = None):
     "Parse the given parsers inside a parenthesized expression."
-    return _grouped(Expression, parsers)
+    return _grouped(Expression, '( … )', name, parsers)
 
 
 def dolike(head):


Test patch:
diff --git a/tests/compilers/test_ast.py b/tests/compilers/test_ast.py
index d3d91a5d..56e90389 100644
--- a/tests/compilers/test_ast.py
+++ b/tests/compilers/test_ast.py
@@ -628,3 +628,15 @@ def test_module_prelude():
 def test_pragma():
     cant_compile("(pragma)")
     cant_compile("(pragma :native-code :namespaced-symbols :give-user-a-pony)")
+
+
+def test_error_with_expectation():
+    def check(code, expected):
+        assert cant_compile(code).msg.endswith("expected: " + expected)
+
+    check("(defmacro)",    "Symbol")
+    check("(quote)",       "form")
+    check("(py)",          "String")
+    check("(py a)",        "String")
+    check('(py "foo" a)',  "end of macro call")
+    check('(for a)',       "square-bracketed loop clauses")