From 82f985f62461eb722eab9f2b83bcacb0600b9cba Mon Sep 17 00:00:00 2001
From: Luke Plant <L.Plant.98@cantab.net>
Date: Mon, 28 Jan 2019 11:40:08 +0300
Subject: [PATCH 1/7] Updated fluent.runtime to fluent.syntax 0.10 and
 implemented new features.

---
 fluent.runtime/fluent/runtime/__init__.py     |   6 +-
 fluent.runtime/fluent/runtime/resolver.py     | 144 ++++++++++--------
 fluent.runtime/fluent/runtime/utils.py        |  31 ++++
 fluent.runtime/setup.py                       |   2 +-
 .../tests/format/test_parameterized_terms.py  | 144 ++++++++++++++++++
 .../tests/format/test_select_expression.py    |  42 +++++
 fluent.runtime/tests/test_bundle.py           |  12 ++
 fluent.runtime/tox.ini                        |   2 +-
 8 files changed, 318 insertions(+), 65 deletions(-)
 create mode 100644 fluent.runtime/tests/format/test_parameterized_terms.py

diff --git a/fluent.runtime/fluent/runtime/__init__.py b/fluent.runtime/fluent/runtime/__init__.py
index 55df3368..9631da9c 100644
--- a/fluent.runtime/fluent/runtime/__init__.py
+++ b/fluent.runtime/fluent/runtime/__init__.py
@@ -41,8 +41,10 @@ def add_messages(self, source):
         # TODO - warn/error about duplicates
         for item in resource.body:
             if isinstance(item, (Message, Term)):
-                if item.id.name not in self._messages_and_terms:
-                    self._messages_and_terms[item.id.name] = item
+                prefix = "-" if isinstance(item, Term) else ""
+                full_id = prefix + item.id.name
+                if full_id not in self._messages_and_terms:
+                    self._messages_and_terms[full_id] = item
 
     def has_message(self, message_id):
         if message_id.startswith('-'):
diff --git a/fluent.runtime/fluent/runtime/resolver.py b/fluent.runtime/fluent/runtime/resolver.py
index 19adc241..52bc5935 100644
--- a/fluent.runtime/fluent/runtime/resolver.py
+++ b/fluent.runtime/fluent/runtime/resolver.py
@@ -1,20 +1,19 @@
 from __future__ import absolute_import, unicode_literals
 
+import contextlib
 from datetime import date, datetime
 from decimal import Decimal
 
 import attr
 import six
 
-from fluent.syntax.ast import (AttributeExpression, CallExpression, Message,
-                               MessageReference, NumberLiteral, Pattern,
-                               Placeable, SelectExpression, StringLiteral, Term,
-                               TermReference, TextElement, VariableReference,
-                               VariantExpression, VariantList, Identifier)
+from fluent.syntax.ast import (AttributeExpression, CallExpression, Identifier, Message, MessageReference,
+                               NumberLiteral, Pattern, Placeable, SelectExpression, StringLiteral, Term, TermReference,
+                               TextElement, VariableReference, VariantExpression, VariantList)
 
-from .errors import FluentCyclicReferenceError, FluentReferenceError
+from .errors import FluentCyclicReferenceError, FluentFormatError, FluentReferenceError
 from .types import FluentDateType, FluentNone, FluentNumber, fluent_date, fluent_number
-from .utils import numeric_to_native
+from .utils import numeric_to_native, reference_to_id, unknown_reference_error_obj
 
 try:
     from functools import singledispatch
@@ -37,13 +36,38 @@
 PDI = "\u2069"
 
 
+@attr.s
+class CurrentEnvironment(object):
+    # The parts of ResolverEnvironment that we want to mutate (and restore)
+    # temporarily for some parts of a call chain.
+    args = attr.ib()
+    error_for_missing_arg = attr.ib(default=True)
+
+
 @attr.s
 class ResolverEnvironment(object):
     context = attr.ib()
-    args = attr.ib()
     errors = attr.ib()
     dirty = attr.ib(factory=set)
     part_count = attr.ib(default=0)
+    current = attr.ib(factory=CurrentEnvironment)
+
+    @contextlib.contextmanager
+    def modified(self, **replacements):
+        """
+        Context manager that modifies the 'current' attribute of the
+        environment, restoring the old data at the end.
+        """
+        # CurrentEnvironment only has immutable args at the moment, so the
+        # shallow copy returned by attr.evolve is fine.
+        old_current = self.current
+        self.current = attr.evolve(old_current, **replacements)
+        yield self
+        self.current = old_current
+
+    def modified_for_term_reference(self, args=None):
+        return self.modified(args=args if args is not None else {},
+                             error_for_missing_arg=False)
 
 
 def resolve(context, message, args):
@@ -55,7 +79,7 @@ def resolve(context, message, args):
     """
     errors = []
     env = ResolverEnvironment(context=context,
-                              args=args,
+                              current=CurrentEnvironment(args=args),
                               errors=errors)
     return fully_resolve(message, env), errors
 
@@ -156,33 +180,34 @@ def handle_number_expression(number_expression, env):
 
 @handle.register(MessageReference)
 def handle_message_reference(message_reference, env):
-    name = message_reference.id.name
-    return handle(lookup_reference(name, env), env)
+    return handle(lookup_reference(message_reference, env), env)
 
 
 @handle.register(TermReference)
 def handle_term_reference(term_reference, env):
-    name = term_reference.id.name
-    return handle(lookup_reference(name, env), env)
+    with env.modified_for_term_reference():
+        return handle(lookup_reference(term_reference, env), env)
 
 
-def lookup_reference(name, env):
-    message = None
-    try:
-        message = env.context._messages_and_terms[name]
-    except LookupError:
-        if name.startswith("-"):
-            env.errors.append(
-                FluentReferenceError("Unknown term: {0}"
-                                     .format(name)))
+def lookup_reference(ref, env):
+    ref_id = reference_to_id(ref)
+    if "." in ref_id:
+        parent_id, attr_name = ref_id.split('.')
+        if parent_id not in env.context._messages_and_terms:
+            env.errors.append(unknown_reference_error_obj(ref_id))
+            return FluentNone(ref_id)
         else:
-            env.errors.append(
-                FluentReferenceError("Unknown message: {0}"
-                                     .format(name)))
-    if message is None:
-        message = FluentNone(name)
-
-    return message
+            parent = env.context._messages_and_terms[parent_id]
+            for attribute in parent.attributes:
+                if attribute.id.name == attr_name:
+                    return attribute.value
+            env.errors.append(unknown_reference_error_obj(ref_id))
+            return parent
+    else:
+        if ref_id not in env.context._messages_and_terms:
+            env.errors.append(unknown_reference_error_obj(ref_id))
+            return FluentNone(ref_id)
+        return env.context._messages_and_terms[ref_id]
 
 
 @handle.register(FluentNone)
@@ -200,10 +225,11 @@ def handle_none(none, env):
 def handle_variable_reference(argument, env):
     name = argument.id.name
     try:
-        arg_val = env.args[name]
+        arg_val = env.current.args[name]
     except LookupError:
-        env.errors.append(
-            FluentReferenceError("Unknown external: {0}".format(name)))
+        if env.current.error_for_missing_arg:
+            env.errors.append(
+                FluentReferenceError("Unknown external: {0}".format(name)))
         return FluentNone(name)
 
     if isinstance(arg_val,
@@ -217,21 +243,8 @@ def handle_variable_reference(argument, env):
 
 
 @handle.register(AttributeExpression)
-def handle_attribute_expression(attribute, env):
-    parent_id = attribute.ref.id.name
-    attr_name = attribute.name.name
-    message = lookup_reference(parent_id, env)
-    if isinstance(message, FluentNone):
-        return message
-
-    for message_attr in message.attributes:
-        if message_attr.id.name == attr_name:
-            return handle(message_attr.value, env)
-
-    env.errors.append(
-        FluentReferenceError("Unknown attribute: {0}.{1}"
-                             .format(parent_id, attr_name)))
-    return handle(message, env)
+def handle_attribute_expression(attribute_ref, env):
+    return handle(lookup_reference(attribute_ref, env), env)
 
 
 @handle.register(VariantList)
@@ -318,7 +331,7 @@ def handle_indentifier(identifier, env):
 
 @handle.register(VariantExpression)
 def handle_variant_expression(expression, env):
-    message = lookup_reference(expression.ref.id.name, env)
+    message = lookup_reference(expression.ref, env)
     if isinstance(message, FluentNone):
         return message
 
@@ -334,21 +347,30 @@ def handle_variant_expression(expression, env):
 
 @handle.register(CallExpression)
 def handle_call_expression(expression, env):
-    function_name = expression.callee.name
-    try:
-        function = env.context._functions[function_name]
-    except LookupError:
-        env.errors.append(FluentReferenceError("Unknown function: {0}"
-                                               .format(function_name)))
-        return FluentNone(function_name + "()")
-
     args = [handle(arg, env) for arg in expression.positional]
     kwargs = {kwarg.name.name: handle(kwarg.value, env) for kwarg in expression.named}
-    try:
-        return function(*args, **kwargs)
-    except Exception as e:
-        env.errors.append(e)
-        return FluentNone(function_name + "()")
+
+    if isinstance(expression.callee, (TermReference, AttributeExpression)):
+        term = lookup_reference(expression.callee, env)
+        if args:
+            env.errors.append(FluentFormatError("Ignored positional arguments passed to term '{0}'"
+                                                .format(reference_to_id(expression.callee))))
+        with env.modified_for_term_reference(args=kwargs):
+            return handle(term, env)
+    else:
+        function_name = expression.callee.id.name
+        try:
+            function = env.context._functions[function_name]
+        except LookupError:
+            env.errors.append(FluentReferenceError("Unknown function: {0}"
+                                                   .format(function_name)))
+            return FluentNone(function_name + "()")
+
+        try:
+            return function(*args, **kwargs)
+        except Exception as e:
+            env.errors.append(e)
+            return FluentNone(function_name + "()")
 
 
 @handle.register(FluentNumber)
diff --git a/fluent.runtime/fluent/runtime/utils.py b/fluent.runtime/fluent/runtime/utils.py
index 1f67dd26..f76fa7c0 100644
--- a/fluent.runtime/fluent/runtime/utils.py
+++ b/fluent.runtime/fluent/runtime/utils.py
@@ -1,3 +1,8 @@
+from fluent.syntax.ast import AttributeExpression, TermReference
+
+from .errors import FluentReferenceError
+
+
 def numeric_to_native(val):
     """
     Given a numeric string (as defined by fluent spec),
@@ -9,3 +14,29 @@ def numeric_to_native(val):
         return float(val)
     else:
         return int(val)
+
+
+def reference_to_id(ref):
+    """
+    Returns a string reference for a MessageReference, TermReference or AttributeExpression
+    AST node.
+
+    e.g.
+       message
+       message.attr
+       -term
+       -term.attr
+    """
+    if isinstance(ref, AttributeExpression):
+        return "{0}.{1}".format(reference_to_id(ref.ref),
+                                ref.name.name)
+    return ('-' if isinstance(ref, TermReference) else '') + ref.id.name
+
+
+def unknown_reference_error_obj(ref_id):
+    if '.' in ref_id:
+        return FluentReferenceError("Unknown attribute: {0}".format(ref_id))
+    elif ref_id.startswith('-'):
+        return FluentReferenceError("Unknown term: {0}".format(ref_id))
+    else:
+        return FluentReferenceError("Unknown message: {0}".format(ref_id))
diff --git a/fluent.runtime/setup.py b/fluent.runtime/setup.py
index fe7e6b8e..326dcc01 100755
--- a/fluent.runtime/setup.py
+++ b/fluent.runtime/setup.py
@@ -26,7 +26,7 @@
       ],
       packages=['fluent', 'fluent.runtime'],
       install_requires=[
-          'fluent>=0.9,<0.10',
+          'fluent.syntax>=0.10',
           'attrs',
           'babel',
           'pytz',
diff --git a/fluent.runtime/tests/format/test_parameterized_terms.py b/fluent.runtime/tests/format/test_parameterized_terms.py
new file mode 100644
index 00000000..d127c1bb
--- /dev/null
+++ b/fluent.runtime/tests/format/test_parameterized_terms.py
@@ -0,0 +1,144 @@
+from __future__ import absolute_import, unicode_literals
+
+import unittest
+
+from fluent.runtime import FluentBundle
+from fluent.runtime.errors import FluentFormatError, FluentReferenceError
+
+from ..utils import dedent_ftl
+
+
+class TestParameterizedTerms(unittest.TestCase):
+
+    def setUp(self):
+        self.ctx = FluentBundle(['en-US'], use_isolating=False)
+        self.ctx.add_messages(dedent_ftl("""
+            -thing = { $article ->
+                  *[definite] the thing
+                   [indefinite] a thing
+            }
+            thing-no-arg = { -thing }
+            thing-no-arg-alt = { -thing() }
+            thing-with-arg = { -thing(article: "indefinite") }
+            thing-positional-arg = { -thing("foo") }
+            thing-fallback = { -thing(article: "somethingelse") }
+            bad-term = { -missing() }
+        """))
+
+    def test_argument_omitted(self):
+        val, errs = self.ctx.format('thing-no-arg', {})
+        self.assertEqual(val, 'the thing')
+        self.assertEqual(errs, [])
+
+    def test_argument_omitted_alt(self):
+        val, errs = self.ctx.format('thing-no-arg-alt', {})
+        self.assertEqual(val, 'the thing')
+        self.assertEqual(errs, [])
+
+    def test_with_argument(self):
+        val, errs = self.ctx.format('thing-with-arg', {})
+        self.assertEqual(val, 'a thing')
+        self.assertEqual(errs, [])
+
+    def test_positional_arg(self):
+        val, errs = self.ctx.format('thing-positional-arg', {})
+        self.assertEqual(val, 'the thing')
+        self.assertEqual(errs, [FluentFormatError("Ignored positional arguments passed to term '-thing'")])
+
+    def test_fallback(self):
+        val, errs = self.ctx.format('thing-fallback', {})
+        self.assertEqual(val, 'the thing')
+        self.assertEqual(errs, [])
+
+    def test_no_implicit_access_to_external_args(self):
+        # The '-thing' term should not get passed article="indefinite"
+        val, errs = self.ctx.format('thing-no-arg', {'article': 'indefinite'})
+        self.assertEqual(val, 'the thing')
+        self.assertEqual(errs, [])
+
+    def test_bad_term(self):
+        val, errs = self.ctx.format('bad-term', {})
+        self.assertEqual(val, '-missing')
+        self.assertEqual(errs, [FluentReferenceError('Unknown term: -missing')])
+
+
+class TestParameterizedTermAttributes(unittest.TestCase):
+
+    def setUp(self):
+        self.ctx = FluentBundle(['en-US'], use_isolating=False)
+        self.ctx.add_messages(dedent_ftl("""
+            -brand = Cool Thing
+                .status = { $version ->
+                    [v2]     available
+                   *[v1]     deprecated
+                }
+
+            attr-with-arg = { -brand } is { -brand.status(version: "v2") ->
+                 [available]   available, yay!
+                *[deprecated]  deprecated, sorry
+            }
+
+            -other = { $arg ->
+                        [a]  ABC
+                       *[d]  DEF
+                     }
+
+            missing-attr-ref = { -other.missing(arg: "a") ->
+                 [ABC]  ABC option
+                *[DEF]  DEF option
+            }
+        """))
+
+    def test_with_argument(self):
+        val, errs = self.ctx.format('attr-with-arg', {})
+        self.assertEqual(val, 'Cool Thing is available, yay!')
+        self.assertEqual(errs, [])
+
+    def test_missing_attr(self):
+        # We should fall back to the parent, and still pass the args.
+        val, errs = self.ctx.format('missing-attr-ref', {})
+        self.assertEqual(val, 'ABC option')
+        self.assertEqual(errs, [FluentReferenceError('Unknown attribute: -other.missing')])
+
+
+class TestNestedParameterizedTerms(unittest.TestCase):
+
+    def setUp(self):
+        self.ctx = FluentBundle(['en-US'], use_isolating=False)
+        self.ctx.add_messages(dedent_ftl("""
+            -thing = { $article ->
+                *[definite] { $first-letter ->
+                    *[lower] the thing
+                     [upper] The thing
+                 }
+                 [indefinite] { $first-letter ->
+                    *[lower] a thing
+                     [upper] A thing
+                 }
+             }
+
+            both-args = { -thing(first-letter: "upper", article: "indefinite") }.
+            outer-arg = This is { -thing(article: "indefinite") }.
+            inner-arg = { -thing(first-letter: "upper") }.
+            neither-arg = { -thing() }.
+        """))
+
+    def test_both_args(self):
+        val, errs = self.ctx.format('both-args', {})
+        self.assertEqual(val, 'A thing.')
+        self.assertEqual(errs, [])
+
+    def test_outer_arg(self):
+        val, errs = self.ctx.format('outer-arg', {})
+        self.assertEqual(val, 'This is a thing.')
+        self.assertEqual(errs, [])
+
+    def test_inner_arg(self):
+        val, errs = self.ctx.format('inner-arg', {})
+        self.assertEqual(val, 'The thing.')
+        self.assertEqual(errs, [])
+
+    def test_neither_arg(self):
+        val, errs = self.ctx.format('neither-arg', {})
+        self.assertEqual(val, 'the thing.')
+        self.assertEqual(errs, [])
diff --git a/fluent.runtime/tests/format/test_select_expression.py b/fluent.runtime/tests/format/test_select_expression.py
index bc0af933..8a1b77dc 100644
--- a/fluent.runtime/tests/format/test_select_expression.py
+++ b/fluent.runtime/tests/format/test_select_expression.py
@@ -173,3 +173,45 @@ def test_with_argument_float(self):
         val, errs = self.ctx.format('qux', {'num': 1.0})
         self.assertEqual(val, "A")
         self.assertEqual(len(errs), 0)
+
+
+class TestSelectExpressionWithTerms(unittest.TestCase):
+
+    def setUp(self):
+        self.ctx = FluentBundle(['en-US'], use_isolating=False)
+        self.ctx.add_messages(dedent_ftl("""
+            -my-term = term
+                 .attr = termattribute
+
+            ref-term-attr = { -my-term.attr ->
+                    [termattribute]   Term Attribute
+                   *[other]           Other
+            }
+
+            ref-term-attr-other = { -my-term.attr ->
+                    [x]      Term Attribute
+                   *[other]  Other
+            }
+
+            ref-term-attr-missing = { -my-term.missing ->
+                    [x]      Term Attribute
+                   *[other]  Other
+            }
+        """))
+
+    def test_ref_term_attribute(self):
+        val, errs = self.ctx.format('ref-term-attr')
+        self.assertEqual(val, "Term Attribute")
+        self.assertEqual(len(errs), 0)
+
+    def test_ref_term_attribute_fallback(self):
+        val, errs = self.ctx.format('ref-term-attr-other')
+        self.assertEqual(val, "Other")
+        self.assertEqual(len(errs), 0)
+
+    def test_ref_term_attribute_missing(self):
+        val, errs = self.ctx.format('ref-term-attr-missing')
+        self.assertEqual(val, "Other")
+        self.assertEqual(len(errs), 1)
+        self.assertEqual(errs,
+                         [FluentReferenceError('Unknown attribute: -my-term.missing')])
diff --git a/fluent.runtime/tests/test_bundle.py b/fluent.runtime/tests/test_bundle.py
index d11a4819..63710e8a 100644
--- a/fluent.runtime/tests/test_bundle.py
+++ b/fluent.runtime/tests/test_bundle.py
@@ -98,3 +98,15 @@ def test_format_term(self):
         self.assertRaises(LookupError,
                           self.ctx.format,
                           '-foo')
+        self.assertRaises(LookupError,
+                          self.ctx.format,
+                          'foo')
+
+    def test_message_and_term_separate(self):
+        self.ctx.add_messages(dedent_ftl("""
+            foo = Refers to { -foo }
+            -foo = Foo
+        """))
+        val, errs = self.ctx.format('foo', {})
+        self.assertEqual(val, 'Refers to \u2068Foo\u2069')
+        self.assertEqual(errs, [])
diff --git a/fluent.runtime/tox.ini b/fluent.runtime/tox.ini
index 960d76d9..e566ee16 100644
--- a/fluent.runtime/tox.ini
+++ b/fluent.runtime/tox.ini
@@ -6,7 +6,7 @@ skipsdist=True
 setenv =
     PYTHONPATH = {toxinidir}
 deps =
-     fluent>=0.9,<0.10
+     fluent>=0.10
      six
      attrs
      Babel

From b8aa39dc47f982bfdb5857546a04d3def0f294c5 Mon Sep 17 00:00:00 2001
From: Luke Plant <L.Plant.98@cantab.net>
Date: Tue, 29 Jan 2019 16:52:35 +0300
Subject: [PATCH 2/7] resolver - store attributes in _messages_and_terms dict

This has the result of performance optimization in a number of places,
because we don't need to traverse attributes each time (with probably a very
small startup cost), and simplifying quite a bit of logic.

Also, tests for the corner cases found by looking at code coverage for the
changes, and cleanups of places that were handling message IDs
---
 fluent.runtime/fluent/runtime/__init__.py     | 27 ++++-------
 fluent.runtime/fluent/runtime/resolver.py     | 48 ++++++++++++-------
 fluent.runtime/fluent/runtime/utils.py        | 39 ++++++++++++---
 .../tests/format/test_attributes.py           |  6 +++
 4 files changed, 78 insertions(+), 42 deletions(-)

diff --git a/fluent.runtime/fluent/runtime/__init__.py b/fluent.runtime/fluent/runtime/__init__.py
index 9631da9c..befc1bb0 100644
--- a/fluent.runtime/fluent/runtime/__init__.py
+++ b/fluent.runtime/fluent/runtime/__init__.py
@@ -9,6 +9,7 @@
 
 from .builtins import BUILTINS
 from .resolver import resolve
+from .utils import ATTRIBUTE_SEPARATOR, TERM_SIGIL, add_message_and_attrs_to_store, ast_to_id
 
 
 class FluentBundle(object):
@@ -41,35 +42,25 @@ def add_messages(self, source):
         # TODO - warn/error about duplicates
         for item in resource.body:
             if isinstance(item, (Message, Term)):
-                prefix = "-" if isinstance(item, Term) else ""
-                full_id = prefix + item.id.name
+                full_id = ast_to_id(item)
                 if full_id not in self._messages_and_terms:
-                    self._messages_and_terms[full_id] = item
+                    # We add attributes to the store to enable faster looker
+                    # later, and more direct code in some instances.
+                    add_message_and_attrs_to_store(self._messages_and_terms, full_id, item)
 
     def has_message(self, message_id):
-        if message_id.startswith('-'):
+        if message_id.startswith(TERM_SIGIL) or ATTRIBUTE_SEPARATOR in message_id:
             return False
         return message_id in self._messages_and_terms
 
     def format(self, message_id, args=None):
-        message = self._get_message(message_id)
+        if message_id.startswith(TERM_SIGIL):
+            raise LookupError(message_id)
+        message = self._messages_and_terms[message_id]
         if args is None:
             args = {}
         return resolve(self, message, args)
 
-    def _get_message(self, message_id):
-        if message_id.startswith('-'):
-            raise LookupError(message_id)
-        if '.' in message_id:
-            name, attr_name = message_id.split('.', 1)
-            msg = self._messages_and_terms[name]
-            for attribute in msg.attributes:
-                if attribute.id.name == attr_name:
-                    return attribute.value
-            raise LookupError(message_id)
-        else:
-            return self._messages_and_terms[message_id]
-
     def _get_babel_locale(self):
         for l in self.locales:
             try:
diff --git a/fluent.runtime/fluent/runtime/resolver.py b/fluent.runtime/fluent/runtime/resolver.py
index 52bc5935..1d8a9e69 100644
--- a/fluent.runtime/fluent/runtime/resolver.py
+++ b/fluent.runtime/fluent/runtime/resolver.py
@@ -7,7 +7,7 @@
 import attr
 import six
 
-from fluent.syntax.ast import (AttributeExpression, CallExpression, Identifier, Message, MessageReference,
+from fluent.syntax.ast import (Attribute, AttributeExpression, CallExpression, Identifier, Message, MessageReference,
                                NumberLiteral, Pattern, Placeable, SelectExpression, StringLiteral, Term, TermReference,
                                TextElement, VariableReference, VariantExpression, VariantList)
 
@@ -190,24 +190,31 @@ def handle_term_reference(term_reference, env):
 
 
 def lookup_reference(ref, env):
+    """
+    Given a MessageReference, TermReference or AttributeExpression, returns the
+    AST node, or FluentNone if not found, including fallback logic
+    """
     ref_id = reference_to_id(ref)
-    if "." in ref_id:
-        parent_id, attr_name = ref_id.split('.')
-        if parent_id not in env.context._messages_and_terms:
-            env.errors.append(unknown_reference_error_obj(ref_id))
-            return FluentNone(ref_id)
-        else:
-            parent = env.context._messages_and_terms[parent_id]
-            for attribute in parent.attributes:
-                if attribute.id.name == attr_name:
-                    return attribute.value
-            env.errors.append(unknown_reference_error_obj(ref_id))
-            return parent
-    else:
-        if ref_id not in env.context._messages_and_terms:
-            env.errors.append(unknown_reference_error_obj(ref_id))
-            return FluentNone(ref_id)
-        return env.context._messages_and_terms[ref_id]
+
+    message = None
+    try:
+        message = env.context._messages_and_terms[ref_id]
+    except LookupError:
+        env.errors.append(unknown_reference_error_obj(ref_id))
+
+        if isinstance(ref, AttributeExpression):
+            parent_id = reference_to_id(ref.ref)
+            try:
+                message = env.context._messages_and_terms[parent_id]
+            except LookupError:
+                # Don't add error here, because we already added error for the
+                # actual thing we were looking for.
+                pass
+
+    if message is None:
+        message = FluentNone(ref_id)
+
+    return message
 
 
 @handle.register(FluentNone)
@@ -247,6 +254,11 @@ def handle_attribute_expression(attribute_ref, env):
     return handle(lookup_reference(attribute_ref, env), env)
 
 
+@handle.register(Attribute)
+def handle_attribute(attribute, env):
+    return handle(attribute.value, env)
+
+
 @handle.register(VariantList)
 def handle_variant_list(variant_list, env):
     return select_from_variant_list(variant_list, env, None)
diff --git a/fluent.runtime/fluent/runtime/utils.py b/fluent.runtime/fluent/runtime/utils.py
index f76fa7c0..8e90f601 100644
--- a/fluent.runtime/fluent/runtime/utils.py
+++ b/fluent.runtime/fluent/runtime/utils.py
@@ -1,7 +1,27 @@
-from fluent.syntax.ast import AttributeExpression, TermReference
+from fluent.syntax.ast import AttributeExpression, Term, TermReference
 
 from .errors import FluentReferenceError
 
+TERM_SIGIL = '-'
+ATTRIBUTE_SEPARATOR = '.'
+
+
+def ast_to_id(ast):
+    """
+    Returns a string reference for a Term or Message
+    """
+    return (TERM_SIGIL if isinstance(ast, Term) else '') + ast.id.name
+
+
+def add_message_and_attrs_to_store(store, ref_id, item, is_parent=True):
+    store[ref_id] = item
+    if is_parent:
+        for attr in item.attributes:
+            add_message_and_attrs_to_store(store,
+                                           _make_attr_id(ref_id, attr.id.name),
+                                           attr,
+                                           is_parent=False)
+
 
 def numeric_to_native(val):
     """
@@ -28,15 +48,22 @@ def reference_to_id(ref):
        -term.attr
     """
     if isinstance(ref, AttributeExpression):
-        return "{0}.{1}".format(reference_to_id(ref.ref),
-                                ref.name.name)
-    return ('-' if isinstance(ref, TermReference) else '') + ref.id.name
+        return _make_attr_id(reference_to_id(ref.ref),
+                             ref.name.name)
+    return (TERM_SIGIL if isinstance(ref, TermReference) else '') + ref.id.name
 
 
 def unknown_reference_error_obj(ref_id):
-    if '.' in ref_id:
+    if ATTRIBUTE_SEPARATOR in ref_id:
         return FluentReferenceError("Unknown attribute: {0}".format(ref_id))
-    elif ref_id.startswith('-'):
+    elif ref_id.startswith(TERM_SIGIL):
         return FluentReferenceError("Unknown term: {0}".format(ref_id))
     else:
         return FluentReferenceError("Unknown message: {0}".format(ref_id))
+
+
+def _make_attr_id(parent_ref_id, attr_name):
+    """
+    Given a parent id and the attribute name, return the attribute id
+    """
+    return ''.join([parent_ref_id, ATTRIBUTE_SEPARATOR, attr_name])
diff --git a/fluent.runtime/tests/format/test_attributes.py b/fluent.runtime/tests/format/test_attributes.py
index 6e5b63fb..e2eb4e8f 100644
--- a/fluent.runtime/tests/format/test_attributes.py
+++ b/fluent.runtime/tests/format/test_attributes.py
@@ -106,6 +106,7 @@ def setUp(self):
             ref-qux = { qux.missing }
             attr-only =
                      .attr  = Attr Only Attribute
+            ref-double-missing = { missing.attr }
         """))
 
     def test_falls_back_for_msg_with_string_value_and_no_attributes(self):
@@ -147,3 +148,8 @@ def test_attr_only_attribute(self):
         val, errs = self.ctx.format('attr-only.attr', {})
         self.assertEqual(val, 'Attr Only Attribute')
         self.assertEqual(len(errs), 0)
+
+    def test_missing_message_and_attribute(self):
+        val, errs = self.ctx.format('ref-double-missing', {})
+        self.assertEqual(val, 'missing.attr')
+        self.assertEqual(errs, [FluentReferenceError('Unknown attribute: missing.attr')])

From 628ea31f5e1ab3d74afa595183ea951f8c4c35fc Mon Sep 17 00:00:00 2001
From: Luke Plant <L.Plant.98@cantab.net>
Date: Tue, 29 Jan 2019 18:38:33 +0300
Subject: [PATCH 3/7] Cleaned up some return/else control flow to be more
 consistent

---
 fluent.runtime/fluent/runtime/resolver.py | 56 +++++++++++------------
 fluent.runtime/fluent/runtime/utils.py    |  8 ++--
 2 files changed, 29 insertions(+), 35 deletions(-)

diff --git a/fluent.runtime/fluent/runtime/resolver.py b/fluent.runtime/fluent/runtime/resolver.py
index 1d8a9e69..bc137ff8 100644
--- a/fluent.runtime/fluent/runtime/resolver.py
+++ b/fluent.runtime/fluent/runtime/resolver.py
@@ -95,8 +95,8 @@ def fully_resolve(expr, env):
     retval = handle(expr, env)
     if isinstance(retval, text_type):
         return retval
-    else:
-        return fully_resolve(retval, env)
+
+    return fully_resolve(retval, env)
 
 
 @singledispatch
@@ -196,25 +196,22 @@ def lookup_reference(ref, env):
     """
     ref_id = reference_to_id(ref)
 
-    message = None
     try:
-        message = env.context._messages_and_terms[ref_id]
+        return env.context._messages_and_terms[ref_id]
     except LookupError:
         env.errors.append(unknown_reference_error_obj(ref_id))
 
         if isinstance(ref, AttributeExpression):
+            # Fallback
             parent_id = reference_to_id(ref.ref)
             try:
-                message = env.context._messages_and_terms[parent_id]
+                return env.context._messages_and_terms[parent_id]
             except LookupError:
                 # Don't add error here, because we already added error for the
                 # actual thing we were looking for.
                 pass
 
-    if message is None:
-        message = FluentNone(ref_id)
-
-    return message
+    return FluentNone(ref_id)
 
 
 @handle.register(FluentNone)
@@ -285,8 +282,8 @@ def select_from_variant_list(variant_list, env, key):
         found = default
     if found is None:
         return FluentNone()
-    else:
-        return handle(found.value, env)
+
+    return handle(found.value, env)
 
 
 @handle.register(SelectExpression)
@@ -312,8 +309,7 @@ def select_from_select_expression(expression, env, key):
         found = default
     if found is None:
         return FluentNone()
-    else:
-        return handle(found.value, env)
+    return handle(found.value, env)
 
 
 def is_number(val):
@@ -329,9 +325,8 @@ def match(val1, val2, env):
         if not is_number(val2):
             # Could be plural rule match
             return env.context._plural_form(val1) == val2
-    else:
-        if is_number(val2):
-            return match(val2, val1, env)
+    elif is_number(val2):
+        return match(val2, val1, env)
 
     return val1 == val2
 
@@ -369,20 +364,21 @@ def handle_call_expression(expression, env):
                                                 .format(reference_to_id(expression.callee))))
         with env.modified_for_term_reference(args=kwargs):
             return handle(term, env)
-    else:
-        function_name = expression.callee.id.name
-        try:
-            function = env.context._functions[function_name]
-        except LookupError:
-            env.errors.append(FluentReferenceError("Unknown function: {0}"
-                                                   .format(function_name)))
-            return FluentNone(function_name + "()")
-
-        try:
-            return function(*args, **kwargs)
-        except Exception as e:
-            env.errors.append(e)
-            return FluentNone(function_name + "()")
+
+    # builtin or custom function call
+    function_name = expression.callee.id.name
+    try:
+        function = env.context._functions[function_name]
+    except LookupError:
+        env.errors.append(FluentReferenceError("Unknown function: {0}"
+                                               .format(function_name)))
+        return FluentNone(function_name + "()")
+
+    try:
+        return function(*args, **kwargs)
+    except Exception as e:
+        env.errors.append(e)
+        return FluentNone(function_name + "()")
 
 
 @handle.register(FluentNumber)
diff --git a/fluent.runtime/fluent/runtime/utils.py b/fluent.runtime/fluent/runtime/utils.py
index 8e90f601..c2419ff0 100644
--- a/fluent.runtime/fluent/runtime/utils.py
+++ b/fluent.runtime/fluent/runtime/utils.py
@@ -32,8 +32,7 @@ def numeric_to_native(val):
     #  '-'? [0-9]+ ('.' [0-9]+)?
     if '.' in val:
         return float(val)
-    else:
-        return int(val)
+    return int(val)
 
 
 def reference_to_id(ref):
@@ -56,10 +55,9 @@ def reference_to_id(ref):
 def unknown_reference_error_obj(ref_id):
     if ATTRIBUTE_SEPARATOR in ref_id:
         return FluentReferenceError("Unknown attribute: {0}".format(ref_id))
-    elif ref_id.startswith(TERM_SIGIL):
+    if ref_id.startswith(TERM_SIGIL):
         return FluentReferenceError("Unknown term: {0}".format(ref_id))
-    else:
-        return FluentReferenceError("Unknown message: {0}".format(ref_id))
+    return FluentReferenceError("Unknown message: {0}".format(ref_id))
 
 
 def _make_attr_id(parent_ref_id, attr_name):

From 59292d69e693351cd735fe8d968d1c692d76981c Mon Sep 17 00:00:00 2001
From: Luke Plant <L.Plant.98@cantab.net>
Date: Tue, 29 Jan 2019 21:13:49 +0300
Subject: [PATCH 4/7] resolver: code comments for CurrentEnvironment

---
 fluent.runtime/fluent/runtime/resolver.py | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/fluent.runtime/fluent/runtime/resolver.py b/fluent.runtime/fluent/runtime/resolver.py
index bc137ff8..07e03e40 100644
--- a/fluent.runtime/fluent/runtime/resolver.py
+++ b/fluent.runtime/fluent/runtime/resolver.py
@@ -40,7 +40,16 @@
 class CurrentEnvironment(object):
     # The parts of ResolverEnvironment that we want to mutate (and restore)
     # temporarily for some parts of a call chain.
+
+    # The values of attributes here must not be mutated, they must only be
+    # swapped out for different objects using `modified` (see below).
+
+    # For Messages, VariableReference nodes are interpreted as external args,
+    # but for Terms they are the values explicitly passed using CallExpression
+    # syntax. So we have to be able to change 'args' for this purpose.
     args = attr.ib()
+    # This controls whether we need to report an error if a VariableReference
+    # refers to an arg that is not present in the args dict.
     error_for_missing_arg = attr.ib(default=True)
 
 
@@ -58,8 +67,8 @@ def modified(self, **replacements):
         Context manager that modifies the 'current' attribute of the
         environment, restoring the old data at the end.
         """
-        # CurrentEnvironment only has immutable args at the moment, so the
-        # shallow copy returned by attr.evolve is fine.
+        # CurrentEnvironment only has args that we never mutate, so the shallow
+        # copy returned by attr.evolve is fine (at least for now).
         old_current = self.current
         self.current = attr.evolve(old_current, **replacements)
         yield self

From 9914930e14b8c6b3b0a4a87726be1ba3bb40b671 Mon Sep 17 00:00:00 2001
From: Luke Plant <L.Plant.98@cantab.net>
Date: Thu, 31 Jan 2019 20:05:12 +0300
Subject: [PATCH 5/7] format - some additional tests for messages/terms in
 terms

Thanks @stasm
---
 .../tests/format/test_parameterized_terms.py  | 52 +++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/fluent.runtime/tests/format/test_parameterized_terms.py b/fluent.runtime/tests/format/test_parameterized_terms.py
index d127c1bb..3ff31786 100644
--- a/fluent.runtime/tests/format/test_parameterized_terms.py
+++ b/fluent.runtime/tests/format/test_parameterized_terms.py
@@ -16,6 +16,7 @@ def setUp(self):
             -thing = { $article ->
                   *[definite] the thing
                    [indefinite] a thing
+                   [none] thing
             }
             thing-no-arg = { -thing }
             thing-no-arg-alt = { -thing() }
@@ -56,6 +57,11 @@ def test_no_implicit_access_to_external_args(self):
         self.assertEqual(val, 'the thing')
         self.assertEqual(errs, [])
 
+    def test_no_implicit_access_to_external_args_but_term_args_still_passed(self):
+        val, errs = self.ctx.format('thing-with-arg', {'article': 'none'})
+        self.assertEqual(val, 'a thing')
+        self.assertEqual(errs, [])
+
     def test_bad_term(self):
         val, errs = self.ctx.format('bad-term', {})
         self.assertEqual(val, '-missing')
@@ -138,7 +144,53 @@ def test_inner_arg(self):
         self.assertEqual(val, 'The thing.')
         self.assertEqual(errs, [])
 
+    def test_inner_arg_with_external_args(self):
+        val, errs = self.ctx.format('inner-arg', {'article': 'indefinite'})
+        self.assertEqual(val, 'The thing.')
+        self.assertEqual(errs, [])
+
     def test_neither_arg(self):
         val, errs = self.ctx.format('neither-arg', {})
         self.assertEqual(val, 'the thing.')
         self.assertEqual(errs, [])
+
+
+class TestTermsCalledFromTerms(unittest.TestCase):
+
+    def setUp(self):
+        self.ctx = FluentBundle(['en-US'], use_isolating=False)
+        self.ctx.add_messages(dedent_ftl("""
+            -foo = {$a} {$b}
+            -bar = {-foo(b: 2)}
+            -baz = {-foo}
+            ref-bar = {-bar(a: 1)}
+            ref-baz = {-baz(a: 1)}
+        """))
+
+    def test_term_args_isolated_with_call_syntax(self):
+        val, errs = self.ctx.format('ref-bar', {})
+        self.assertEqual(val, 'a 2')
+        self.assertEqual(errs, [])
+
+    def test_term_args_isolated_without_call_syntax(self):
+        val, errs = self.ctx.format('ref-baz', {})
+        self.assertEqual(val, 'a b')
+        self.assertEqual(errs, [])
+
+
+class TestMessagesCalledFromTerms(unittest.TestCase):
+
+    def setUp(self):
+        self.ctx = FluentBundle(['en-US'], use_isolating=False)
+        self.ctx.add_messages(dedent_ftl("""
+            msg = Msg is {$arg}
+            -foo = {msg}
+            ref-foo = {-foo(arg: 1)}
+        """))
+
+    def test_messages_inherit_term_args(self):
+        # This behaviour may change in future, message calls might be
+        # disallowed from inside terms
+        val, errs = self.ctx.format('ref-foo', {'arg': 2})
+        self.assertEqual(val, 'Msg is 1')
+        self.assertEqual(errs, [])

From 1cbb48851121e472cb336c5675a4cca049227158 Mon Sep 17 00:00:00 2001
From: Luke Plant <L.Plant.98@cantab.net>
Date: Thu, 31 Jan 2019 20:07:47 +0300
Subject: [PATCH 6/7] More consistent if/else pattern in ast_to_id,
 reference_to_id

---
 fluent.runtime/fluent/runtime/utils.py | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/fluent.runtime/fluent/runtime/utils.py b/fluent.runtime/fluent/runtime/utils.py
index c2419ff0..e6f793bd 100644
--- a/fluent.runtime/fluent/runtime/utils.py
+++ b/fluent.runtime/fluent/runtime/utils.py
@@ -10,7 +10,9 @@ def ast_to_id(ast):
     """
     Returns a string reference for a Term or Message
     """
-    return (TERM_SIGIL if isinstance(ast, Term) else '') + ast.id.name
+    if isinstance(ast, Term):
+        return TERM_SIGIL + ast.id.name
+    return ast.id.name
 
 
 def add_message_and_attrs_to_store(store, ref_id, item, is_parent=True):
@@ -49,7 +51,9 @@ def reference_to_id(ref):
     if isinstance(ref, AttributeExpression):
         return _make_attr_id(reference_to_id(ref.ref),
                              ref.name.name)
-    return (TERM_SIGIL if isinstance(ref, TermReference) else '') + ref.id.name
+    if isinstance(ref, TermReference):
+        return TERM_SIGIL + ref.id.name
+    return ref.id.name
 
 
 def unknown_reference_error_obj(ref_id):

From 30eb8563e61679d6166bad85041a94cd13e3ca14 Mon Sep 17 00:00:00 2001
From: Luke Plant <L.Plant.98@cantab.net>
Date: Thu, 31 Jan 2019 20:12:06 +0300
Subject: [PATCH 7/7] fluent.runtime - upper bound on fluent.syntax dependency

---
 fluent.runtime/setup.py | 2 +-
 fluent.runtime/tox.ini  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fluent.runtime/setup.py b/fluent.runtime/setup.py
index 326dcc01..7acc0802 100755
--- a/fluent.runtime/setup.py
+++ b/fluent.runtime/setup.py
@@ -26,7 +26,7 @@
       ],
       packages=['fluent', 'fluent.runtime'],
       install_requires=[
-          'fluent.syntax>=0.10',
+          'fluent.syntax>=0.10,<=0.11',
           'attrs',
           'babel',
           'pytz',
diff --git a/fluent.runtime/tox.ini b/fluent.runtime/tox.ini
index e566ee16..c32eb4d2 100644
--- a/fluent.runtime/tox.ini
+++ b/fluent.runtime/tox.ini
@@ -6,7 +6,7 @@ skipsdist=True
 setenv =
     PYTHONPATH = {toxinidir}
 deps =
-     fluent>=0.10
+     fluent.syntax>=0.10,<=0.11
      six
      attrs
      Babel