Print null checks with the precedence of ===

changes/null-check-parentheses.fix.md

@@ -0,0 +1 @@
+- Fix `Nullable.isNullable` of a conditional, `||` or bitwise expression testing only part of it, as in `Nullable.isNullable(c ? a : b)`.

compiler/core/js_dump.ml

@@ -754,8 +754,10 @@ and expression_desc cxt ~(level : int) f x : cxt =
     if need_paren then P.paren f action else action ();
     cxt
   | Is_null_or_undefined e ->
-    P.cond_paren_group f (level > 0) (fun _ ->
-        let cxt = expression ~level:1 cxt f e in
+    (* Printed as [e == null], which binds like [===] *)
+    let out, lft, _ = Js_op_util.op_prec EqEqEq in
+    P.cond_paren_group f (level > out) (fun _ ->
+        let cxt = expression ~level:lft cxt f e in
         P.space f;
         P.string f "==";
         P.space f;

packages/@rescript/runtime/lib/es6/Primitive_option.mjs

@@ -46,7 +46,7 @@ function fromNull(x) {
 }
 
 function valFromOption(x) {
-  if ((x == null) || x.BS_PRIVATE_NESTED_SOME_NONE === undefined) {
+  if (x == null || x.BS_PRIVATE_NESTED_SOME_NONE === undefined) {
     return x;
   }
   let depth = x.BS_PRIVATE_NESTED_SOME_NONE;

packages/@rescript/runtime/lib/es6/Stdlib_Nullable.mjs

@@ -10,11 +10,11 @@ function fromOption(option) {
 }
 
 function equal(a, b, eq) {
-  return Stdlib_Option.equal((a == null) ? undefined : Primitive_option.some(a), (b == null) ? undefined : Primitive_option.some(b), eq);
+  return Stdlib_Option.equal(a == null ? undefined : Primitive_option.some(a), b == null ? undefined : Primitive_option.some(b), eq);
 }
 
 function compare(a, b, cmp) {
-  return Stdlib_Option.compare((a == null) ? undefined : Primitive_option.some(a), (b == null) ? undefined : Primitive_option.some(b), cmp);
+  return Stdlib_Option.compare(a == null ? undefined : Primitive_option.some(a), b == null ? undefined : Primitive_option.some(b), cmp);
 }
 
 function getOr(value, $$default) {

packages/@rescript/runtime/lib/js/Primitive_option.cjs

@@ -46,7 +46,7 @@ function fromNull(x) {
 }
 
 function valFromOption(x) {
-  if ((x == null) || x.BS_PRIVATE_NESTED_SOME_NONE === undefined) {
+  if (x == null || x.BS_PRIVATE_NESTED_SOME_NONE === undefined) {
     return x;
   }
   let depth = x.BS_PRIVATE_NESTED_SOME_NONE;

packages/@rescript/runtime/lib/js/Stdlib_Nullable.cjs

@@ -10,11 +10,11 @@ function fromOption(option) {
 }
 
 function equal(a, b, eq) {
-  return Stdlib_Option.equal((a == null) ? undefined : Primitive_option.some(a), (b == null) ? undefined : Primitive_option.some(b), eq);
+  return Stdlib_Option.equal(a == null ? undefined : Primitive_option.some(a), b == null ? undefined : Primitive_option.some(b), eq);
 }
 
 function compare(a, b, cmp) {
-  return Stdlib_Option.compare((a == null) ? undefined : Primitive_option.some(a), (b == null) ? undefined : Primitive_option.some(b), cmp);
+  return Stdlib_Option.compare(a == null ? undefined : Primitive_option.some(a), b == null ? undefined : Primitive_option.some(b), cmp);
 }
 
 function getOr(value, $$default) {

tests/tests/src/is_nullable_paren_test.mjs

@@ -0,0 +1,104 @@
+// Generated by ReScript, PLEASE EDIT WITH CARE
+
+import * as Mocha from "mocha";
+import * as Test_utils from "./test_utils.mjs";
+
+let opaque = (x => x);
+
+function ofCond(c, a, b) {
+  return (
+    c ? a : b
+  ) == null;
+}
+
+function ofIf(c, a, b) {
+  return (
+    c ? a : b
+  ) == null;
+}
+
+function ofOr(a, b) {
+  return (a || b) == null;
+}
+
+function ofSum(a, b) {
+  return (a + b | 0) == null;
+}
+
+function ofObject(a) {
+  return () => ({
+    a: a
+  }) == null;
+}
+
+function inOr(x, y) {
+  if (x == null) {
+    return true;
+  } else {
+    return y;
+  }
+}
+
+function inCond(x) {
+  if (x == null) {
+    return "none";
+  } else {
+    return "some";
+  }
+}
+
+function inEq(x, y) {
+  return y === (x == null);
+}
+
+Mocha.describe("Is_nullable_paren_test", () => {
+  Mocha.test("isNullable of a conditional", () => {
+    let t = opaque(true);
+    let f = opaque(false);
+    let $$null = opaque(null);
+    let one = opaque(1);
+    Test_utils.eq("File \"is_nullable_paren_test.res\", line 33, characters 7-14", ofCond(t, $$null, one), true);
+    Test_utils.eq("File \"is_nullable_paren_test.res\", line 34, characters 7-14", ofCond(t, one, $$null), false);
+    Test_utils.eq("File \"is_nullable_paren_test.res\", line 35, characters 7-14", ofCond(f, one, opaque(undefined)), true);
+    Test_utils.eq("File \"is_nullable_paren_test.res\", line 36, characters 7-14", ofIf(t, $$null, one), true);
+    Test_utils.eq("File \"is_nullable_paren_test.res\", line 37, characters 7-14", ofIf(f, $$null, one), false);
+  });
+  Mocha.test("isNullable of a logical or", () => {
+    let a = opaque(true);
+    let b = opaque(false);
+    Test_utils.eq("File \"is_nullable_paren_test.res\", line 40, characters 46-53", (a || b) == null, false);
+  });
+  Mocha.test("isNullable of an int sum", () => {
+    let a = opaque(1);
+    Test_utils.eq("File \"is_nullable_paren_test.res\", line 42, characters 44-51", (a + 2 | 0) == null, false);
+  });
+  Mocha.test("isNullable of an object in an arrow body", () => {
+    let a = opaque(1);
+    Test_utils.eq("File \"is_nullable_paren_test.res\", line 44, characters 60-67", ({
+      a: a
+    }) == null, false);
+  });
+  Mocha.test("isNullable as an operand", () => {
+    let $$null = opaque(null);
+    let one = opaque(1);
+    Test_utils.eq("File \"is_nullable_paren_test.res\", line 48, characters 7-14", $$null == null, true);
+    Test_utils.eq("File \"is_nullable_paren_test.res\", line 49, characters 7-14", one == null, false);
+    Test_utils.eq("File \"is_nullable_paren_test.res\", line 50, characters 7-14", inCond($$null), "none");
+    Test_utils.eq("File \"is_nullable_paren_test.res\", line 51, characters 7-14", inCond(one), "some");
+    Test_utils.eq("File \"is_nullable_paren_test.res\", line 52, characters 7-14", true === ($$null == null), true);
+    Test_utils.eq("File \"is_nullable_paren_test.res\", line 53, characters 7-14", true === (one == null), false);
+  });
+});
+
+export {
+  opaque,
+  ofCond,
+  ofIf,
+  ofOr,
+  ofSum,
+  ofObject,
+  inOr,
+  inCond,
+  inEq,
+}
+/*  Not a pure module */

tests/tests/src/is_nullable_paren_test.res

@@ -0,0 +1,55 @@
+open Mocha
+open Test_utils
+
+// `Nullable.isNullable(e)` is printed as `e == null`, which binds like `===`:
+// `e` keeps the parentheses it needs, and the check needs none as an operand
+// of `||` or as the test of `?:`.
+
+// Hides values from the optimizer, so the calls below are not folded away
+let opaque: 'a => 'a = %raw(`x => x`)
+
+let ofCond = (c, a: Nullable.t<int>, b: Nullable.t<int>) => Nullable.isNullable(c ? a : b)
+
+let ofIf = (c, a: Nullable.t<int>, b: Nullable.t<int>) =>
+  Nullable.isNullable(
+    if c {
+      a
+    } else {
+      b
+    },
+  )
+
+let ofOr = (a: bool, b: bool) => Nullable.isNullable(Nullable.make(a || b))
+let ofSum = (a: int, b: int) => Nullable.isNullable(Nullable.make(a + b))
+let ofObject = (a: int) => () => Nullable.isNullable(Nullable.make({"a": a}))
+let inOr = (x: Nullable.t<int>, y: bool) => Nullable.isNullable(x) || y
+let inCond = (x: Nullable.t<int>) => Nullable.isNullable(x) ? "none" : "some"
+let inEq = (x: Nullable.t<int>, y: bool) => y == Nullable.isNullable(x)
+
+describe(__MODULE__, () => {
+  test("isNullable of a conditional", () => {
+    let (t, f) = (opaque(true), opaque(false))
+    let (null, one) = (opaque(Nullable.null), opaque(Nullable.make(1)))
+    eq(__LOC__, ofCond(t, null, one), true)
+    eq(__LOC__, ofCond(t, one, null), false)
+    eq(__LOC__, ofCond(f, one, opaque(Nullable.undefined)), true)
+    eq(__LOC__, ofIf(t, null, one), true)
+    eq(__LOC__, ofIf(f, null, one), false)
+  })
+
+  test("isNullable of a logical or", () => eq(__LOC__, ofOr(opaque(true), opaque(false)), false))
+
+  test("isNullable of an int sum", () => eq(__LOC__, ofSum(opaque(1), 2), false))
+
+  test("isNullable of an object in an arrow body", () => eq(__LOC__, ofObject(opaque(1))(), false))
+
+  test("isNullable as an operand", () => {
+    let (null, one) = (opaque(Nullable.null), opaque(Nullable.make(1)))
+    eq(__LOC__, inOr(null, false), true)
+    eq(__LOC__, inOr(one, false), false)
+    eq(__LOC__, inCond(null), "none")
+    eq(__LOC__, inCond(one), "some")
+    eq(__LOC__, inEq(null, true), true)
+    eq(__LOC__, inEq(one, true), false)
+  })
+})

tests/tests/src/optional_external_arg_once_test.mjs

@@ -85,7 +85,7 @@ Mocha.describe("Optional_external_arg_once_test", () => {
       "first",
       1
     ], [
-      (key == null) ? undefined : Primitive_option.some(key),
+      key == null ? undefined : Primitive_option.some(key),
       s$2.reads
     ]);
     let s$3 = makeSource();

tests/tests/src/stdlib/Stdlib_TempTests.mjs

@@ -202,7 +202,7 @@ console.log(regex.test(string));
 
 let result = regex.exec(string);
 
-let result$1 = (result == null) ? undefined : Primitive_option.some(result);
+let result$1 = result == null ? undefined : Primitive_option.some(result);
 
 console.log(Stdlib_Option.map(result$1, prim => prim.input));