Prevent more ways to assign to constants

NEWS.rst

@@ -42,6 +42,7 @@ Bug Fixes
 * Fixed error handling for non-symbol macro names
 * `doc` and `#doc` now work with names that require mangling.
 * Fixed compiler crash on `.` form with empty attributes.
+* Attempts to assign to constants are now more reliably detected.
 
 0.20.0 (released 2021-01-25)
 ==============================

hy/compiler.py

@@ -109,9 +109,6 @@ def ast_str(x, piecewise=False):
     "unquote", "unquote-splice", "unpack-mapping", "except"))
 
 
-def named_constant(expr, v):
-    return asty.Constant(expr, value=v)
-
 def special(names, pattern):
     """Declare special operators. The decorated method and the given pattern
     is assigned to _special_form_compilers for each of the listed names."""
@@ -541,13 +538,20 @@ def _storeize(self, expr, name, func=None):
             new_name = ast.Starred(
                 value=self._storeize(expr, name.value, func))
         else:
-            raise self._syntax_error(expr,
-                "Can't assign or delete a %s" % type(expr).__name__)
+            raise self._syntax_error(expr, "Can't assign or delete a " + (
+                "constant"
+                if isinstance(name, ast.Constant)
+                else type(expr).__name__))
 
         new_name.ctx = func()
         ast.copy_location(new_name, name)
         return new_name
 
+    def _nonconst(self, name):
+        if str(name) in ("None", "True", "False"):
+            raise self._syntax_error(name, "Can't assign to constant")
+        return name
+
     def _render_quoted_form(self, form, level):
         """
         Render a quoted form as a new HyExpression.
@@ -726,7 +730,7 @@ def _compile_catch_expression(self, expr, var, exceptions, body):
 
         name = None
         if len(exceptions) == 2:
-            name = ast_str(exceptions[0])
+            name = ast_str(self._nonconst(exceptions[0]))
 
         exceptions_list = exceptions[-1] if exceptions else HyList()
         if isinstance(exceptions_list, HyList):
@@ -960,7 +964,7 @@ def compile_with_expression(self, expr, root, args, body):
         # Initialize the tempvar to None in case the `with` exits
         # early with an exception.
         initial_assign = asty.Assign(
-            expr, targets=[name], value=named_constant(expr, None))
+            expr, targets=[name], value=asty.Constant(expr, value=None))
 
         node = asty.With if root == "with*" else asty.AsyncWith
         the_with = node(expr,
@@ -1220,7 +1224,7 @@ def compile_logical_or_and_and_operator(self, expr, operator, args):
         opnode, default = ops[operator]
         osym = expr[0]
         if len(args) == 0:
-            return named_constant(osym, default)
+            return asty.Constant(osym, value=default)
         elif len(args) == 1:
             return self.compile(args[0])
         ret = Result()
@@ -1282,7 +1286,7 @@ def _get_c_op(self, sym):
     def compile_compare_op_expression(self, expr, root, args):
         if len(args) == 1:
             return (self.compile(args[0]) +
-                    named_constant(expr, True))
+                    asty.Constant(expr, value=True))
 
         ops = [self._get_c_op(root) for _ in args[1:]]
         exprs, ret, _ = self._compile_collect(args)
@@ -1373,7 +1377,7 @@ def compile_augassign_expression(self, expr, root, target, values):
     @special((PY38, "setx"), [times(1, 1, SYM + FORM)])
     def compile_def_expression(self, expr, root, decls):
         if not decls:
-            return named_constant(expr, None)
+            return asty.Constant(expr, value=None)
 
         result = Result()
         is_assignment_expr = root == HySymbol("setx")
@@ -1408,18 +1412,11 @@ def _compile_assign(self, ann, name, value, *, is_assignment_expr = False):
         if ann is not None:
             # An annotation / annotated assignment is more strict with the target expression.
             invalid_name = not isinstance(ld_name.expr, (ast.Name, ast.Attribute, ast.Subscript))
-        else:
-            invalid_name = (str(name) in ("None", "True", "False")
-                            or isinstance(ld_name.expr, ast.Call))
-
-        if invalid_name:
-            raise self._syntax_error(name, "illegal target for {}".format(
-                                        "annotation" if annotate_only else "assignment"))
 
         if (result.temp_variables
                 and isinstance(name, HySymbol)
                 and '.' not in name):
-            result.rename(name)
+            result.rename(self._nonconst(name))
             if not is_assignment_expr:
                 # Throw away .expr to ensure that (setv ...) returns None.
                 result.expr = None
@@ -1479,7 +1476,7 @@ def make_not(operand):
 
             cond_compiled = (Result()
                 + asty.Assign(cond, targets=[self._storeize(cond, cond_var)],
-                              value=named_constant(cond, True))
+                              value=asty.Constant(cond, value=True))
                 + cond_var)
 
         orel = Result()
@@ -1604,7 +1601,8 @@ def _compile_arguments_set(self, decls, implicit_default_none, ret):
                 # positional args.
                 args_defaults.append(None)
 
-            args_ast.append(asty.arg(sym, arg=ast_str(sym), annotation=ann_ast))
+            args_ast.append(asty.arg(
+                sym, arg=ast_str(self._nonconst(sym)), annotation=ann_ast))
 
         return args_ast, args_defaults, ret
 
@@ -1638,7 +1636,7 @@ def compile_class_expression(self, expr, root, name, rest):
         return bases + asty.ClassDef(
             expr,
             decorator_list=[],
-            name=ast_str(name),
+            name=ast_str(self._nonconst(name)),
             keywords=keywords,
             starargs=None,
             kwargs=None,
@@ -1854,7 +1852,8 @@ def compile_symbol(self, symbol):
             self.imports[self._stdlib[ast_str(symbol)]].add(ast_str(symbol))
 
         if ast_str(symbol) in ("None", "False", "True"):
-            return named_constant(symbol, ast.literal_eval(ast_str(symbol)))
+            return asty.Constant(symbol, value =
+                ast.literal_eval(ast_str(symbol)))
 
         return asty.Name(symbol, id=ast_str(symbol), ctx=ast.Load())
 

tests/native_tests/language.hy

@@ -9,7 +9,7 @@
         [operator [or_]]
         pickle
         [typing [get-type-hints List Dict]]
-        [hy.errors [HyLanguageError]]
+        [hy.errors [HyLanguageError HySyntaxError]]
         pytest)
 (import sys)
 
@@ -66,20 +66,6 @@
   (assert (= (get foo 0) 12)))
 
 
-(defn test-setv-builtin []
-  "NATIVE: test that setv doesn't work on names Python can't assign to
-  and that we can't mangle"
-  (for [form '[
-      (setv None 1)
-      (defn None [] (print "hello"))
-      (setv False 1)
-      (setv True 0)
-      (defn True [] (print "hello"))]]
-    (with [e (pytest.raises SyntaxError)]
-      (eval form))
-    (assert (in "illegal target for assignment" (str e)))))
-
-
 (defn test-setv-pairs []
   "NATIVE: test that setv works on pairs of arguments"
   (setv a 1 b 2)
@@ -138,16 +124,28 @@
   (assert (= l [["eggs" "ham"]])))
 
 
-(defn test-store-errors []
-  "NATIVE: test that setv raises the correct errors when given wrong argument types"
-  (for [[form s] '[
-      [(setv (do 1 2) 1) "a non-expression"]
-      [(setv 1 1) "a HyInteger"]
-      [(setv {1 2} 1) "a HyDict"]
-      [(del 1 1) "a HyInteger"]]]
+(defn test-illegal-assignments []
+  (for [form '[
+      (setv (do 1 2) 1)
+      (setv 1 1)
+      (setv {1 2} 1)
+      (del 1 1)
+      ; https://github.com/hylang/hy/issues/1780
+      (setv None 1)
+      (setv False 1)
+      (setv True 1)
+      (defn None [] (print "hello"))
+      (defn True [] (print "hello"))
+      (defn f [True] (print "hello"))
+      (for [True [1 2 3]] (print "hello"))
+      (lfor  True [1 2 3]  True)
+      (lfor  :setv True 1  True)
+      (with [True x] (print "hello"))
+      (try 1 (except [True AssertionError] 2))
+      (defclass True [])]]
     (with [e (pytest.raises HyLanguageError)]
       (eval form))
-    (assert (= e.value.msg (+ "Can't assign or delete " s)))))
+    (assert (in "Can't assign" e.value.msg))))
 
 
 (defn test-no-str-as-sym []