Clean up the scope after resolving a term reference

Author: stasm

fluent-bundle/src/resolver.js

@@ -88,7 +88,7 @@ function getArguments(scope, args) {
     }
   }
 
-  return [positional, named];
+  return {positional, named};
 }
 
 // Resolve an expression to a Fluent type.
@@ -117,15 +117,23 @@ function resolveExpression(scope, expr) {
 
 // Resolve a reference to a variable.
 function VariableReference(scope, {name}) {
-  if (!scope.args || !scope.args.hasOwnProperty(name)) {
-    if (scope.insideTermReference === false) {
-      scope.reportError(new ReferenceError(`Unknown variable: $${name}`));
+  let arg;
+  if (scope.params) {
+    // We're inside a TermReference. It's OK to reference undefined parameters.
+    if (scope.params.hasOwnProperty(name)) {
+      arg = scope.params[name];
+    } else {
+      return new FluentNone(`$${name}`);
     }
+  } else if (scope.args && scope.args.hasOwnProperty(name)) {
+    // We're in the top-level Pattern or inside a MessageReference. Missing
+    // variables references produce ReferenceErrors.
+    arg = scope.args[name];
+  } else {
+    scope.reportError(new ReferenceError(`Unknown variable: $${name}`));
     return new FluentNone(`$${name}`);
   }
 
-  const arg = scope.args[name];
-
   // Return early if the argument already is an instance of FluentType.
   if (arg instanceof FluentType) {
     return arg;
@@ -183,20 +191,23 @@ function TermReference(scope, {name, attr, args}) {
     return new FluentNone(id);
   }
 
-  // Every TermReference has its own variables.
-  const [, params] = getArguments(scope, args);
-  const local = scope.cloneForTermReference(params);
-
   if (attr) {
     const attribute = term.attributes[attr];
     if (attribute) {
-      return resolvePattern(local, attribute);
+      // Every TermReference has its own variables.
+      scope.params = getArguments(scope, args).named;
+      const resolved = resolvePattern(scope, attribute);
+      scope.params = null;
+      return resolved;
     }
     scope.reportError(new ReferenceError(`Unknown attribute: ${attr}`));
     return new FluentNone(`${id}.${attr}`);
   }
 
-  return resolvePattern(local, term.value);
+  scope.params = getArguments(scope, args).named;
+  const resolved = resolvePattern(scope, term.value);
+  scope.params = null;
+  return resolved;
 }
 
 // Resolve a call to a Function with positional and key-value arguments.
@@ -215,7 +226,8 @@ function FunctionReference(scope, {name, args}) {
   }
 
   try {
-    return func(...getArguments(scope, args));
+    let resolved = getArguments(scope, args);
+    return func(resolved.positional, resolved.named);
   } catch (err) {
     scope.reportError(err);
     return new FluentNone(`${name}()`);

fluent-bundle/src/scope.js

@@ -1,10 +1,5 @@
 export default class Scope {
-  constructor(
-    bundle,
-    errors,
-    args,
-    dirty = new WeakSet()
-  ) {
+  constructor(bundle, errors, args) {
     /** The bundle for which the given resolution is happening. */
     this.bundle = bundle;
     /** The list of errors collected while resolving. */
@@ -14,21 +9,14 @@ export default class Scope {
 
     /** The Set of patterns already encountered during this resolution.
       * Used to detect and prevent cyclic resolutions. */
-    this.dirty = dirty;
-    /** Term references require different variable lookup logic. */
-    this.insideTermReference = false;
+    this.dirty = new WeakSet();
+    /** A dict of parameters passed to a TermReference. */
+    this.params = null;
     /** The running count of placeables resolved so far. Used to detect the
       * Billion Laughs and Quadratic Blowup attacks. */
     this.placeables = 0;
   }
 
-  cloneForTermReference(args) {
-    let scope = new Scope(this.bundle, this.errors, args, this.dirty);
-    scope.insideTermReference = true;
-    scope.placeables = this.placeables;
-    return scope;
-  }
-
   reportError(error) {
     if (!this.errors) {
       throw error;