Fix handling of internals which had several regressions

CHANGES.rst

@@ -3,6 +3,16 @@ Changes
 
 In next release ...
 
+- The list of names previously disallowed for use as variables in
+  templates such as "int" and "float" has been trimmed significantly,
+  not because it's a good idea to use such names but because the list
+  of disallowed names was not exhaustive and complicated the compiler
+  code; and perhaps more importantly, the technical reason for
+  disallowing the names in the first place no longer applies.
+
+- Fix a regression where generated template code would suboptimal due
+  to incorrect handling of internal variables.
+
 - Always cook templates in debug mode, even when using
   `CHAMELEON_CACHE` option to persist generated code on disk.
 

src/chameleon/astutil.py

@@ -194,8 +194,6 @@ def visit(self, node) -> None:
         annotation = node_annotations.get(node)
         if annotation is not None:
             assert hasattr(annotation, '_fields')
-            node = annotation
-
         super().visit(node)
 
     def apply_transform(self, node):

src/chameleon/codegen.py

@@ -13,7 +13,6 @@
 from ast import ImportFrom
 from ast import Module
 from ast import NodeTransformer
-from ast import NodeVisitor
 from ast import alias
 from ast import unparse
 
@@ -48,39 +47,42 @@ def wrapper(*vargs, **kwargs):
         symbols = dict(zip(args, vargs + defaults))
         symbols.update(kwargs)
 
-        class Visitor(NodeVisitor):
-            def visit_FunctionDef(self, node) -> None:
-                self.generic_visit(node)
-
+        class Transformer(NodeTransformer):
+            def visit_FunctionDef(self, node) -> AST:
                 name = symbols.get(node.name, self)
-                if name is not self:
-                    node_annotations[node] = FunctionDef(
-                        name=name,
-                        args=node.args,
-                        body=node.body,
-                        decorator_list=getattr(node, "decorator_list", []),
-                        lineno=None,
-                    )
-
-            def visit_Name(self, node) -> None:
+                if name is self:
+                    return self.generic_visit(node)
+
+                return FunctionDef(
+                    name=name,
+                    args=node.args,
+                    body=list(map(self.visit, node.body)),
+                    decorator_list=getattr(node, "decorator_list", []),
+                    lineno=None,
+                )
+
+            def visit_Name(self, node) -> AST:
                 value = symbols.get(node.id, self)
-                if value is not self:
-                    if isinstance(value, str):
-                        value = load(value)
-                    if isinstance(value, type) or value in reverse_builtin_map:
-                        name = reverse_builtin_map.get(value)
-                        if name is not None:
-                            value = Builtin(name)
-                        else:
-                            value = Symbol(value)
-
-                    assert node not in node_annotations
-                    assert hasattr(value, '_fields')
-                    node_annotations[node] = value
+                if value is self:
+                    if node.id == 'None' or \
+                       getattr(builtins, node.id, None) is not None:
+                        return Builtin(node.id)
+                    return node
+
+                if isinstance(value, type) or value in reverse_builtin_map:
+                    name = reverse_builtin_map.get(value)
+                    if name is not None:
+                        return Builtin(name)
+                    return Symbol(value)
+
+                if isinstance(value, str):
+                    value = load(value)
+
+                return value
 
         expr = parse(textwrap.dedent(source), mode=mode)
 
-        Visitor().visit(expr)
+        Transformer().visit(expr)
         return expr.body
 
     assert isinstance(source, str)

src/chameleon/compiler.py

@@ -54,22 +54,12 @@
 
 log = logging.getLogger('chameleon.compiler')
 
+# Disallowing the use of the following symbols to avoid misunderstandings.
 COMPILER_INTERNALS_OR_DISALLOWED = {
     "econtext",
     "rcontext",
-    "target_language",
-    "str",
-    "int",
-    "float",
-    "len",
-    "bool",
-    "None",
-    "True",
-    "False",
-    "RuntimeError",
 }
 
-
 RE_MANGLE = re.compile(r'[^\w_]')
 RE_NAME = re.compile('^%s$' % NAME)
 
@@ -153,8 +143,8 @@ def indent(s):
             if __tt is int or __tt is float:
                 target = str(target)
             else:
-                render = getattr(target, "__html__", None)
-                if render is None:
+                __markup = getattr(target, "__html__", None)
+                if __markup is None:
                     __converted = translate(
                         target,
                         domain=__i18n_domain,
@@ -165,7 +155,7 @@ def indent(s):
                         if target is __converted \
                         else __converted
                 else:
-                    target = render()""")
+                    target = __markup()""")
 
 
 emit_func_convert = template(
@@ -184,8 +174,8 @@ def func(target):
             if __tt is int or __tt is float:
                 target = str(target)
             else:
-                render = getattr(target, "__html__", None)
-                if render is None:
+                __markup = getattr(target, "__html__", None)
+                if __markup is None:
                     __converted = translate(
                         target,
                         domain=__i18n_domain,
@@ -196,7 +186,7 @@ def func(target):
                         if target is __converted \
                         else __converted
                 else:
-                    target = render()
+                    target = __markup()
 
         return target"""
 )
@@ -235,8 +225,8 @@ def func(target, quote, quote_entity, default, default_marker):
         elif __tt is not str:
             if __tt is int or __tt is float:
                 return str(target)
-            render = getattr(target, "__html__", None)
-            if render is None:
+            __markup = getattr(target, "__html__", None)
+            if __markup is None:
                 __converted = translate(
                     target,
                     domain=__i18n_domain,
@@ -246,7 +236,7 @@ def func(target, quote, quote_entity, default, default_marker):
                 target = str(target) if target is __converted \
                          else __converted
             else:
-                return render()
+                return __markup()
 
         if target is not None:
             try:
@@ -274,8 +264,8 @@ class EmitText(Node):
     _fields = "s",
 
 
-class Scope(Node):
-    """Set a local output scope.
+class TranslationContext(Node):
+    """Set a local output context.
 
     This is used for the translation machinery.
     """
@@ -771,6 +761,8 @@ def __call__(self, node):
         if name.startswith('__') or name in self.internals:
             return node
 
+        # Some expressions allow setting variables which we transform to
+        # storing them as template context.
         if isinstance(node.ctx, ast.Store):
             return store_econtext(name)
 
@@ -785,7 +777,7 @@ def __call__(self, node):
                 "get(key, name)",
                 mode="eval",
                 key=ast.Str(s=name),
-                name=load(name),
+                name=Builtin(name),
             )
 
         # Otherwise, simply acquire it from the dynamic context.
@@ -1032,7 +1024,7 @@ def __init__(
         ast.fix_missing_locations(module)
 
         class Generator(TemplateCodeGenerator):
-            scopes = [Scope()]
+            scopes = [TranslationContext()]
             tokens = []
 
             def visit_EmitText(self, node) -> ast.AST:
@@ -1056,7 +1048,7 @@ def visit_Name(self, node) -> ast.AST:
                                 return load(identifier)
                 return node
 
-            def visit_Scope(self, node) -> list[ast.AST]:
+            def visit_TranslationContext(self, node) -> list[ast.AST]:
                 self.scopes.append(node)
                 stmts = list(map(self.visit, node.body))
                 self.scopes.pop()
@@ -1270,8 +1262,10 @@ def visit_Domain(self, node):
 
     def visit_Target(self, node):
         backup = "__previous_i18n_target_%s" % mangle(id(node))
+        tmp = "__tmp_%s" % mangle(id(node))
         return template("BACKUP = target_language", BACKUP=backup) + \
-            self._engine(node.expression, store("target_language")) + \
+            self._engine(node.expression, store(tmp)) + \
+            [ast.Assign([store("target_language")], load(tmp))] + \
             self.visit(node.node) + \
             template("target_language = BACKUP", BACKUP=backup)
 
@@ -1466,7 +1460,7 @@ def visit_Translate(self, node):
 
         # Visit body to generate the message body
         code = self.visit(node.node)
-        body.append(Scope(code, append, stream))
+        body.append(TranslationContext(code, append, stream))
 
         # Reduce white space and assign as message id
         msgid = identifier("msgid", id(node))
@@ -1714,7 +1708,7 @@ def visit_Name(self, node):
 
         # generate code
         code = self.visit(node.node)
-        body.append(Scope(code, append))
+        body.append(TranslationContext(code, append))
 
         # output msgid
         text = Text('${%s}' % node.name)

src/chameleon/tales.py

@@ -502,7 +502,7 @@ def __init__(self, expression) -> None:
         self.expression = expression
 
     def __call__(self, target, engine):
-        ignore = store("_ignore")
+        ignore = store("__ignore")
         compiler = engine.parse(self.expression, False)
         body = compiler.assign_value(ignore)