Merge pull request #398 from stasm/report-errors-from-functions

stasm · web-flow · commit c40eb5ee1af1 · 2019-07-23T09:38:02.000+02:00
Report errors from Functions and Intl objects
diff --git a/fluent/src/builtins.js b/fluent/src/builtins.js
@@ -28,21 +28,25 @@ function values(opts) {
 export
 function NUMBER([arg], opts) {
   if (arg instanceof FluentNone) {
-    return arg;
+    return new FluentNone(`NUMBER(${arg.valueOf()})`);
   }
+
   if (arg instanceof FluentNumber) {
     return new FluentNumber(arg.valueOf(), merge(arg.opts, opts));
   }
-  return new FluentNone("NUMBER()");
+
+  throw new TypeError("Invalid argument type to NUMBER");
 }
 
 export
 function DATETIME([arg], opts) {
   if (arg instanceof FluentNone) {
-    return arg;
+    return new FluentNone(`DATETIME(${arg.valueOf()})`);
   }
+
   if (arg instanceof FluentDateTime) {
     return new FluentDateTime(arg.valueOf(), merge(arg.opts, opts));
   }
-  return new FluentNone("DATETIME()");
+
+  throw new TypeError("Invalid argument type to DATETIME");
 }
diff --git a/fluent/src/resolver.js b/fluent/src/resolver.js
@@ -127,7 +127,7 @@ function resolveExpression(scope, expr) {
 function VariableReference(scope, {name}) {
   if (!scope.args || !scope.args.hasOwnProperty(name)) {
     if (scope.insideTermReference === false) {
-      scope.reportError(new ReferenceError(`Unknown variable: ${name}`));
+      scope.reportError(new ReferenceError(`Unknown variable: $${name}`));
     }
     return new FluentNone(`$${name}`);
   }
@@ -151,7 +151,7 @@ function VariableReference(scope, {name}) {
       }
     default:
       scope.reportError(
-        new TypeError(`Unsupported variable type: ${name}, ${typeof arg}`)
+        new TypeError(`Variable type not supported: $${name}, ${typeof arg}`)
       );
       return new FluentNone(`$${name}`);
   }
@@ -224,8 +224,8 @@ function FunctionReference(scope, {name, args}) {
 
   try {
     return func(...getArguments(scope, args));
-  } catch (e) {
-    // XXX Report errors.
+  } catch (err) {
+    scope.reportError(err);
     return new FluentNone(`${name}()`);
   }
 }
diff --git a/fluent/src/types.js b/fluent/src/types.js
@@ -45,12 +45,12 @@ export class FluentType {
 }
 
 export class FluentNone extends FluentType {
-  valueOf() {
-    return null;
+  constructor(value = "???") {
+    super(value);
   }
 
   toString() {
-    return `{${this.value || "???"}}`;
+    return `{${this.value}}`;
   }
 }
 
@@ -63,8 +63,8 @@ export class FluentNumber extends FluentType {
     try {
       const nf = scope.memoizeIntlObject(Intl.NumberFormat, this.opts);
       return nf.format(this.value);
-    } catch (e) {
-      // XXX Report the error.
+    } catch (err) {
+      scope.reportError(err);
       return this.value;
     }
   }
@@ -79,8 +79,8 @@ export class FluentDateTime extends FluentType {
     try {
       const dtf = scope.memoizeIntlObject(Intl.DateTimeFormat, this.opts);
       return dtf.format(this.value);
-    } catch (e) {
-      // XXX Report the error.
+    } catch (err) {
+      scope.reportError(err);
       return this.value;
     }
   }
diff --git a/fluent/test/functions_builtin_test.js b/fluent/test/functions_builtin_test.js
@@ -25,20 +25,26 @@ suite('Built-in functions', function() {
     test('missing argument', function() {
       let msg;
 
+      errors = [];
       msg = bundle.getMessage('num-decimal');
-      assert.strictEqual(bundle.formatPattern(msg.value, {}, errors), '{$arg}');
+      assert.strictEqual(bundle.formatPattern(msg.value, {}, errors), '{NUMBER($arg)}');
       assert.strictEqual(errors.length, 1);
-      assert.ok(errors.pop() instanceof ReferenceError);
+      assert.ok(errors[0] instanceof ReferenceError);
+      assert.strictEqual(errors[0].message, "Unknown variable: $arg");
 
+      errors = [];
       msg = bundle.getMessage('num-percent');
-      assert.strictEqual(bundle.formatPattern(msg.value, {}, errors), '{$arg}');
+      assert.strictEqual(bundle.formatPattern(msg.value, {}, errors), '{NUMBER($arg)}');
       assert.strictEqual(errors.length, 1);
-      assert.ok(errors.pop() instanceof ReferenceError);
+      assert.ok(errors[0] instanceof ReferenceError);
+      assert.strictEqual(errors[0].message, "Unknown variable: $arg");
 
+      errors = [];
       msg = bundle.getMessage('num-bad-opt');
-      assert.strictEqual(bundle.formatPattern(msg.value, {}, errors), '{$arg}');
+      assert.strictEqual(bundle.formatPattern(msg.value, {}, errors), '{NUMBER($arg)}');
       assert.strictEqual(errors.length, 1);
-      assert.ok(errors.pop() instanceof ReferenceError);
+      assert.ok(errors[0] instanceof ReferenceError);
+      assert.strictEqual(errors[0].message, "Unknown variable: $arg");
     });
 
     test('number argument', function() {
@@ -55,66 +61,87 @@ suite('Built-in functions', function() {
 
       msg = bundle.getMessage('num-bad-opt');
       assert.strictEqual(bundle.formatPattern(msg.value, args, errors), '1');
-      assert.strictEqual(errors.length, 0);
+      assert.strictEqual(errors.length, 1);
+      assert.ok(errors[0] instanceof RangeError); // Invalid option value
     });
 
-    // XXX Functions should report errors.
-    // https://github.com/projectfluent/fluent.js/issues/106
     test('string argument', function() {
       const args = {arg: "Foo"};
       let msg;
 
+      errors = [];
       msg = bundle.getMessage('num-decimal');
       assert.strictEqual(bundle.formatPattern(msg.value, args, errors), '{NUMBER()}');
-      assert.strictEqual(errors.length, 0);
+      assert.strictEqual(errors.length, 1);
+      assert.ok(errors[0] instanceof TypeError);
+      assert.strictEqual(errors[0].message, "Invalid argument type to NUMBER");
 
+      errors = [];
       msg = bundle.getMessage('num-percent');
       assert.strictEqual(bundle.formatPattern(msg.value, args, errors), '{NUMBER()}');
-      assert.strictEqual(errors.length, 0);
+      assert.strictEqual(errors.length, 1);
+      assert.ok(errors[0] instanceof TypeError);
+      assert.strictEqual(errors[0].message, "Invalid argument type to NUMBER");
 
+      errors = [];
       msg = bundle.getMessage('num-bad-opt');
       assert.strictEqual(bundle.formatPattern(msg.value, args, errors), '{NUMBER()}');
-      assert.strictEqual(errors.length, 0);
+      assert.strictEqual(errors.length, 1);
+      assert.ok(errors[0] instanceof TypeError);
+      assert.strictEqual(errors[0].message, "Invalid argument type to NUMBER");
     });
 
-    // XXX Functions should report errors.
-    // https://github.com/projectfluent/fluent.js/issues/106
     test('date argument', function() {
       const date = new Date('2016-09-29');
       const args = {arg: date};
       let msg;
 
+      errors = [];
       msg = bundle.getMessage('num-decimal');
       assert.strictEqual(bundle.formatPattern(msg.value, args, errors), '{NUMBER()}');
-      assert.strictEqual(errors.length, 0);
+      assert.strictEqual(errors.length, 1);
+      assert.ok(errors[0] instanceof TypeError);
+      assert.strictEqual(errors[0].message, "Invalid argument type to NUMBER");
 
+      errors = [];
       msg = bundle.getMessage('num-percent');
       assert.strictEqual(bundle.formatPattern(msg.value, args, errors), '{NUMBER()}');
-      assert.strictEqual(errors.length, 0);
+      assert.strictEqual(errors.length, 1);
+      assert.ok(errors[0] instanceof TypeError);
+      assert.strictEqual(errors[0].message, "Invalid argument type to NUMBER");
 
+      errors = [];
       msg = bundle.getMessage('num-bad-opt');
       assert.strictEqual(bundle.formatPattern(msg.value, args, errors), '{NUMBER()}');
-      assert.strictEqual(errors.length, 0);
+      assert.strictEqual(errors.length, 1);
+      assert.ok(errors[0] instanceof TypeError);
+      assert.strictEqual(errors[0].message, "Invalid argument type to NUMBER");
     });
 
     test('invalid argument', function() {
       const args = {arg: []};
       let msg;
 
+      errors = [];
       msg = bundle.getMessage('num-decimal');
-      assert.strictEqual(bundle.formatPattern(msg.value, args, errors), '{$arg}');
+      assert.strictEqual(bundle.formatPattern(msg.value, args, errors), '{NUMBER($arg)}');
       assert.strictEqual(errors.length, 1);
-      assert.ok(errors.pop() instanceof TypeError);
+      assert.ok(errors[0] instanceof TypeError);
+      assert.strictEqual(errors[0].message, "Variable type not supported: $arg, object");
 
+      errors = [];
       msg = bundle.getMessage('num-percent');
-      assert.strictEqual(bundle.formatPattern(msg.value, args, errors), '{$arg}');
+      assert.strictEqual(bundle.formatPattern(msg.value, args, errors), '{NUMBER($arg)}');
       assert.strictEqual(errors.length, 1);
-      assert.ok(errors.pop() instanceof TypeError);
+      assert.ok(errors[0] instanceof TypeError);
+      assert.strictEqual(errors[0].message, "Variable type not supported: $arg, object");
 
+      errors = [];
       msg = bundle.getMessage('num-bad-opt');
-      assert.strictEqual(bundle.formatPattern(msg.value, args, errors), '{$arg}');
+      assert.strictEqual(bundle.formatPattern(msg.value, args, errors), '{NUMBER($arg)}');
       assert.strictEqual(errors.length, 1);
-      assert.ok(errors.pop() instanceof TypeError);
+      assert.ok(errors[0] instanceof TypeError);
+      assert.strictEqual(errors[0].message, "Variable type not supported: $arg, object");
     });
   });
 
@@ -131,20 +158,26 @@ suite('Built-in functions', function() {
     test('missing argument', function() {
       let msg;
 
+      errors = [];
       msg = bundle.getMessage('dt-default');
-      assert.strictEqual(bundle.formatPattern(msg.value, {}, errors), '{$arg}');
+      assert.strictEqual(bundle.formatPattern(msg.value, {}, errors), '{DATETIME($arg)}');
       assert.strictEqual(errors.length, 1);
-      assert.ok(errors.pop() instanceof ReferenceError);
+      assert.ok(errors[0] instanceof ReferenceError);
+      assert.strictEqual(errors[0].message, "Unknown variable: $arg");
 
+      errors = [];
       msg = bundle.getMessage('dt-month');
-      assert.strictEqual(bundle.formatPattern(msg.value, {}, errors), '{$arg}');
+      assert.strictEqual(bundle.formatPattern(msg.value, {}, errors), '{DATETIME($arg)}');
       assert.strictEqual(errors.length, 1);
-      assert.ok(errors.pop() instanceof ReferenceError);
+      assert.ok(errors[0] instanceof ReferenceError);
+      assert.strictEqual(errors[0].message, "Unknown variable: $arg");
 
+      errors = [];
       msg = bundle.getMessage('dt-bad-opt');
-      assert.strictEqual(bundle.formatPattern(msg.value, {}, errors), '{$arg}');
+      assert.strictEqual(bundle.formatPattern(msg.value, {}, errors), '{DATETIME($arg)}');
       assert.strictEqual(errors.length, 1);
-      assert.ok(errors.pop() instanceof ReferenceError);
+      assert.ok(errors[0] instanceof ReferenceError);
+      assert.strictEqual(errors[0].message, "Unknown variable: $arg");
     });
 
     test('Date argument', function () {
@@ -172,65 +205,86 @@ suite('Built-in functions', function() {
       // may vary depending on the TZ:
       //     Thu Sep 29 2016 02:00:00 GMT+0200 (CEST)
       assert.strictEqual(bundle.formatPattern(msg.value, args, errors), date.toString());
-      assert.strictEqual(errors.length, 0);
+      assert.strictEqual(errors.length, 1);
+      assert.ok(errors[0] instanceof RangeError); // Invalid option value
     });
 
-    // XXX Functions should report errors.
-    // https://github.com/projectfluent/fluent.js/issues/106
     test('number argument', function() {
       let args = {arg: 1};
       let msg;
 
+      errors = [];
       msg = bundle.getMessage('dt-default');
       assert.strictEqual(bundle.formatPattern(msg.value, args, errors), '{DATETIME()}');
-      assert.strictEqual(errors.length, 0);
+      assert.strictEqual(errors.length, 1);
+      assert.ok(errors[0] instanceof TypeError);
+      assert.strictEqual(errors[0].message, "Invalid argument type to DATETIME");
 
+      errors = [];
       msg = bundle.getMessage('dt-month');
       assert.strictEqual(bundle.formatPattern(msg.value, args, errors), '{DATETIME()}');
-      assert.strictEqual(errors.length, 0);
+      assert.strictEqual(errors.length, 1);
+      assert.ok(errors[0] instanceof TypeError);
+      assert.strictEqual(errors[0].message, "Invalid argument type to DATETIME");
 
+      errors = [];
       msg = bundle.getMessage('dt-bad-opt');
       assert.strictEqual(bundle.formatPattern(msg.value, args, errors), '{DATETIME()}');
-      assert.strictEqual(errors.length, 0);
+      assert.strictEqual(errors.length, 1);
+      assert.ok(errors[0] instanceof TypeError);
+      assert.strictEqual(errors[0].message, "Invalid argument type to DATETIME");
     });
 
-    // XXX Functions should report errors.
-    // https://github.com/projectfluent/fluent.js/issues/106
     test('string argument', function() {
       let args = {arg: 'Foo'};
       let msg;
 
+      errors = [];
       msg = bundle.getMessage('dt-default');
       assert.strictEqual(bundle.formatPattern(msg.value, args, errors), '{DATETIME()}');
-      assert.strictEqual(errors.length, 0);
+      assert.strictEqual(errors.length, 1);
+      assert.ok(errors[0] instanceof TypeError);
+      assert.strictEqual(errors[0].message, "Invalid argument type to DATETIME");
 
+      errors = [];
       msg = bundle.getMessage('dt-month');
       assert.strictEqual(bundle.formatPattern(msg.value, args, errors), '{DATETIME()}');
-      assert.strictEqual(errors.length, 0);
+      assert.strictEqual(errors.length, 1);
+      assert.ok(errors[0] instanceof TypeError);
+      assert.strictEqual(errors[0].message, "Invalid argument type to DATETIME");
 
+      errors = [];
       msg = bundle.getMessage('dt-bad-opt');
       assert.strictEqual(bundle.formatPattern(msg.value, args, errors), '{DATETIME()}');
-      assert.strictEqual(errors.length, 0);
+      assert.strictEqual(errors.length, 1);
+      assert.ok(errors[0] instanceof TypeError);
+      assert.strictEqual(errors[0].message, "Invalid argument type to DATETIME");
     });
 
     test('invalid argument', function() {
       let args = {arg: []};
       let msg;
 
+      errors = [];
       msg = bundle.getMessage('dt-default');
-      assert.strictEqual(bundle.formatPattern(msg.value, args, errors), '{$arg}');
+      assert.strictEqual(bundle.formatPattern(msg.value, args, errors), '{DATETIME($arg)}');
       assert.strictEqual(errors.length, 1);
-      assert.ok(errors.pop() instanceof TypeError);
+      assert.ok(errors[0] instanceof TypeError);
+      assert.strictEqual(errors[0].message, "Variable type not supported: $arg, object");
 
+      errors = [];
       msg = bundle.getMessage('dt-month');
-      assert.strictEqual(bundle.formatPattern(msg.value, args, errors), '{$arg}');
+      assert.strictEqual(bundle.formatPattern(msg.value, args, errors), '{DATETIME($arg)}');
       assert.strictEqual(errors.length, 1);
-      assert.ok(errors.pop() instanceof TypeError);
+      assert.ok(errors[0] instanceof TypeError);
+      assert.strictEqual(errors[0].message, "Variable type not supported: $arg, object");
 
+      errors = [];
       msg = bundle.getMessage('dt-bad-opt');
-      assert.strictEqual(bundle.formatPattern(msg.value, args, errors), '{$arg}');
+      assert.strictEqual(bundle.formatPattern(msg.value, args, errors), '{DATETIME($arg)}');
       assert.strictEqual(errors.length, 1);
-      assert.ok(errors.pop() instanceof TypeError);
+      assert.ok(errors[0] instanceof TypeError);
+      assert.strictEqual(errors[0].message, "Variable type not supported: $arg, object");
     });
   });
 });

Original file line number	Diff line number	Diff line change
`@@ -28,21 +28,25 @@ function values(opts) {`
`28`	`28`	`export`
`29`	`29`	`function NUMBER([arg], opts) {`
`30`	`30`	`if (arg instanceof FluentNone) {`
`31`		`- return arg;`
	`31`	+ return new FluentNone(`NUMBER(${arg.valueOf()})`);
`32`	`32`	`}`
	`33`	`+`
`33`	`34`	`if (arg instanceof FluentNumber) {`
`34`	`35`	`return new FluentNumber(arg.valueOf(), merge(arg.opts, opts));`
`35`	`36`	`}`
`36`		`- return new FluentNone("NUMBER()");`
	`37`	`+`
	`38`	`+ throw new TypeError("Invalid argument type to NUMBER");`
`37`	`39`	`}`
`38`	`40`
`39`	`41`	`export`
`40`	`42`	`function DATETIME([arg], opts) {`
`41`	`43`	`if (arg instanceof FluentNone) {`
`42`		`- return arg;`
	`44`	+ return new FluentNone(`DATETIME(${arg.valueOf()})`);
`43`	`45`	`}`
	`46`	`+`
`44`	`47`	`if (arg instanceof FluentDateTime) {`
`45`	`48`	`return new FluentDateTime(arg.valueOf(), merge(arg.opts, opts));`
`46`	`49`	`}`
`47`		`- return new FluentNone("DATETIME()");`
	`50`	`+`
	`51`	`+ throw new TypeError("Invalid argument type to DATETIME");`
`48`	`52`	`}`
Original file line number	Diff line number	Diff line change
`@@ -127,7 +127,7 @@ function resolveExpression(scope, expr) {`
`127`	`127`	`function VariableReference(scope, {name}) {`
`128`	`128`	`if (!scope.args \|\| !scope.args.hasOwnProperty(name)) {`
`129`	`129`	`if (scope.insideTermReference === false) {`
`130`		- scope.reportError(new ReferenceError(`Unknown variable: ${name}`));
	`130`	+ scope.reportError(new ReferenceError(`Unknown variable: $${name}`));
`131`	`131`	`}`
`132`	`132`	return new FluentNone(`$${name}`);
`133`	`133`	`}`
`@@ -151,7 +151,7 @@ function VariableReference(scope, {name}) {`
`151`	`151`	`}`
`152`	`152`	`default:`
`153`	`153`	`scope.reportError(`
`154`		- new TypeError(`Unsupported variable type: ${name}, ${typeof arg}`)
	`154`	+ new TypeError(`Variable type not supported: $${name}, ${typeof arg}`)
`155`	`155`	`);`
`156`	`156`	return new FluentNone(`$${name}`);
`157`	`157`	`}`
`@@ -224,8 +224,8 @@ function FunctionReference(scope, {name, args}) {`
`224`	`224`
`225`	`225`	`try {`
`226`	`226`	`return func(...getArguments(scope, args));`
`227`		`- } catch (e) {`
`228`		`- // XXX Report errors.`
	`227`	`+ } catch (err) {`
	`228`	`+ scope.reportError(err);`
`229`	`229`	return new FluentNone(`${name}()`);
`230`	`230`	`}`
`231`	`231`	`}`
Original file line number	Diff line number	Diff line change
`@@ -45,12 +45,12 @@ export class FluentType {`
`45`	`45`	`}`
`46`	`46`
`47`	`47`	`export class FluentNone extends FluentType {`
`48`		`- valueOf() {`
`49`		`- return null;`
	`48`	`+ constructor(value = "???") {`
	`49`	`+ super(value);`
`50`	`50`	`}`
`51`	`51`
`52`	`52`	`toString() {`
`53`		- return `{${this.value \|\| "???"}}`;
	`53`	+ return `{${this.value}}`;
`54`	`54`	`}`
`55`	`55`	`}`
`56`	`56`
`@@ -63,8 +63,8 @@ export class FluentNumber extends FluentType {`
`63`	`63`	`try {`
`64`	`64`	`const nf = scope.memoizeIntlObject(Intl.NumberFormat, this.opts);`
`65`	`65`	`return nf.format(this.value);`
`66`		`- } catch (e) {`
`67`		`- // XXX Report the error.`
	`66`	`+ } catch (err) {`
	`67`	`+ scope.reportError(err);`
`68`	`68`	`return this.value;`
`69`	`69`	`}`
`70`	`70`	`}`
`@@ -79,8 +79,8 @@ export class FluentDateTime extends FluentType {`
`79`	`79`	`try {`
`80`	`80`	`const dtf = scope.memoizeIntlObject(Intl.DateTimeFormat, this.opts);`
`81`	`81`	`return dtf.format(this.value);`
`82`		`- } catch (e) {`
`83`		`- // XXX Report the error.`
	`82`	`+ } catch (err) {`
	`83`	`+ scope.reportError(err);`
`84`	`84`	`return this.value;`
`85`	`85`	`}`
`86`	`86`	`}`