Make resolver threadsaf(er) again, fix isolation

This patch removes state from the compiled tree.
For one, the recursion detection on patterns keeps its state in
the `ResolverEnvironment` now, instead of in the compiled tree.
Also, the question of bidi isolation is now kept in the resolver
context, instead of in the compiled tree.
For single placeables, which don't get bidi-isolated ever, there's
now a distinct NeverIsolatingPlaceable, which also takes care of
resolving fluent values to strings.

This should fix the test cases in #108 and #109

Author: Pike

fluent.runtime/fluent/runtime/__init__.py

@@ -32,10 +32,10 @@ def __init__(self, locales, functions=None, use_isolating=True):
         if functions:
             _functions.update(functions)
         self._functions = _functions
-        self._use_isolating = use_isolating
+        self.use_isolating = use_isolating
         self._messages_and_terms = {}
         self._compiled = {}
-        self._compiler = Compiler(use_isolating=use_isolating)
+        self._compiler = Compiler()
         self._babel_locale = self._get_babel_locale()
         self._plural_form = babel.plural.to_python(self._babel_locale.plural_form)
 

fluent.runtime/fluent/runtime/prepare.py

@@ -4,9 +4,6 @@
 
 
 class Compiler(object):
-    def __init__(self, use_isolating=False):
-        self.use_isolating = use_isolating
-
     def __call__(self, item):
         if isinstance(item, FTL.BaseNode):
             return self.compile(item)
@@ -28,19 +25,17 @@ def compile_generic(self, nodename, **kwargs):
         return getattr(resolver, nodename)(**kwargs)
 
     def compile_Placeable(self, _, expression, **kwargs):
-        if self.use_isolating:
-            return resolver.IsolatingPlaceable(expression=expression, **kwargs)
         if isinstance(expression, resolver.Literal):
             return expression
         return resolver.Placeable(expression=expression, **kwargs)
 
     def compile_Pattern(self, _, elements, **kwargs):
         if (
             len(elements) == 1 and
-            isinstance(elements[0], resolver.IsolatingPlaceable)
+            isinstance(elements[0], resolver.Placeable)
         ):
             # Don't isolate isolated placeables
-            return elements[0].expression
+            return resolver.NeverIsolatingPlaceable(elements[0].expression)
         if any(
             not isinstance(child, resolver.Literal)
             for child in elements

fluent.runtime/fluent/runtime/resolver.py

@@ -53,7 +53,8 @@ class CurrentEnvironment(object):
 class ResolverEnvironment(object):
     context = attr.ib()
     errors = attr.ib()
-    part_count = attr.ib(default=0)
+    part_count = attr.ib(default=0, init=False)
+    active_patterns = attr.ib(factory=set, init=False)
     current = attr.ib(factory=CurrentEnvironment)
 
     @contextlib.contextmanager
@@ -105,15 +106,14 @@ class Pattern(FTL.Pattern, BaseResolver):
 
     def __init__(self, *args, **kwargs):
         super(Pattern, self).__init__(*args, **kwargs)
-        self.dirty = False
 
     def __call__(self, env):
-        if self.dirty:
+        if self in env.active_patterns:
             env.errors.append(FluentCyclicReferenceError("Cyclic reference"))
             return FluentNone()
         if env.part_count > self.MAX_PARTS:
             return ""
-        self.dirty = True
+        env.active_patterns.add(self)
         elements = self.elements
         remaining_parts = self.MAX_PARTS - env.part_count
         if len(self.elements) > remaining_parts:
@@ -124,7 +124,7 @@ def __call__(self, env):
             resolve(element(env), env) for element in elements
         )
         env.part_count += len(elements)
-        self.dirty = False
+        env.active_patterns.remove(self)
         return retval
 
 
@@ -144,13 +144,16 @@ def __call__(self, env):
 
 class Placeable(FTL.Placeable, BaseResolver):
     def __call__(self, env):
-        return self.expression(env)
+        inner = resolve(self.expression(env), env)
+        if not env.context.use_isolating:
+            return inner
+        return "\u2068" + inner + "\u2069"
 
 
-class IsolatingPlaceable(FTL.Placeable, BaseResolver):
+class NeverIsolatingPlaceable(FTL.Placeable, BaseResolver):
     def __call__(self, env):
-        inner = self.expression(env)
-        return "\u2068" + resolve(inner, env) + "\u2069"
+        inner = resolve(self.expression(env), env)
+        return inner
 
 
 class StringLiteral(FTL.StringLiteral, Literal):

fluent.runtime/tests/format/test_placeables.py

@@ -109,3 +109,47 @@ def test_allowed_self_reference(self):
         val, errs = self.ctx.format('self-parent-ref-ok.attr', {})
         self.assertEqual(val, 'Attribute Parent')
         self.assertEqual(len(errs), 0)
+
+
+class TestSingleElementPattern(unittest.TestCase):
+    def test_single_literal_number_isolating(self):
+        self.ctx = FluentBundle(['en-US'], use_isolating=True)
+        self.ctx.add_messages('foo = { 1 }')
+        val, errs = self.ctx.format('foo')
+        self.assertEqual(val, '1')
+        self.assertEqual(errs, [])
+
+    def test_single_literal_number_non_isolating(self):
+        self.ctx = FluentBundle(['en-US'], use_isolating=False)
+        self.ctx.add_messages('foo = { 1 }')
+        val, errs = self.ctx.format('foo')
+        self.assertEqual(val, '1')
+        self.assertEqual(errs, [])
+
+    def test_single_arg_number_isolating(self):
+        self.ctx = FluentBundle(['en-US'], use_isolating=True)
+        self.ctx.add_messages('foo = { $arg }')
+        val, errs = self.ctx.format('foo', {'arg': 1})
+        self.assertEqual(val, '1')
+        self.assertEqual(errs, [])
+
+    def test_single_arg_number_non_isolating(self):
+        self.ctx = FluentBundle(['en-US'], use_isolating=False)
+        self.ctx.add_messages('foo = { $arg }')
+        val, errs = self.ctx.format('foo', {'arg': 1})
+        self.assertEqual(val, '1')
+        self.assertEqual(errs, [])
+
+    def test_single_arg_missing_isolating(self):
+        self.ctx = FluentBundle(['en-US'], use_isolating=True)
+        self.ctx.add_messages('foo = { $arg }')
+        val, errs = self.ctx.format('foo')
+        self.assertEqual(val, 'arg')
+        self.assertEqual(len(errs), 1)
+
+    def test_single_arg_missing_non_isolating(self):
+        self.ctx = FluentBundle(['en-US'], use_isolating=False)
+        self.ctx.add_messages('foo = { $arg }')
+        val, errs = self.ctx.format('foo')
+        self.assertEqual(val, 'arg')
+        self.assertEqual(len(errs), 1)