Fix compare/equal/less on booleans (#1191)

The codegen tests were hitting this not implemented path.

js/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@attic/noms",
-  "version": "7.6.0",
+  "version": "7.6.1",
   "main": "dist/commonjs/noms.js",
   "jsnext:main": "dist/es6/noms.js",
   "dependencies": {

js/src/compare-test.js

@@ -0,0 +1,112 @@
+// @flow
+
+import {suite, test} from 'mocha';
+import {assert} from 'chai';
+import {getCompareFunction, compare, equals} from './compare.js';
+import {
+  boolType,
+  float64Type,
+  int8Type,
+  makeListType,
+  stringType,
+} from './type.js';
+import {newList} from './list.js';
+
+suite('compare', () => {
+  suite('getCompareFunction', () => {
+    test('int8', () => {
+      const compare = getCompareFunction(int8Type);
+      assert.equal(compare(1, 1), 0);
+      assert.equal(compare(1, 3), -2);
+      assert.equal(compare(4, 2), 2);
+    });
+
+    test('string', () => {
+      const compare = getCompareFunction(stringType);
+      assert.equal(compare('a', 'a'), 0);
+      assert.equal(compare('a', 'b'), -1);
+      assert.equal(compare('c', 'a'), 1);
+    });
+
+    test('bool', () => {
+      const compare = getCompareFunction(boolType);
+      assert.equal(compare(true, true), 0);
+      assert.equal(compare(true, false), -1);
+      assert.equal(compare(false, true), 1);
+    });
+
+    test('list', async () => {
+      const listOfFloat64Type = makeListType(float64Type);
+      const compare = getCompareFunction(listOfFloat64Type);
+      const listA = await newList([0, 1, 2, 3], listOfFloat64Type);
+      const listB = await newList([0, 1, 2, 3], listOfFloat64Type);
+      const listC = await newList([4, 5, 6, 7], listOfFloat64Type);
+      assert.equal(compare(listA, listA), 0);
+      assert.equal(compare(listA, listB), 0);
+      assert.equal(compare(listA, listC), 1);
+      assert.equal(compare(listC, listA), -1);
+    });
+  });
+
+  suite('compare', () => {
+    test('int8', () => {
+      assert.equal(compare(1, 1), 0);
+      assert.equal(compare(1, 3), -1);
+      assert.equal(compare(4, 2), 1);
+    });
+
+    test('string', () => {
+      assert.equal(compare('a', 'a'), 0);
+      assert.equal(compare('a', 'b'), -1);
+      assert.equal(compare('c', 'a'), 1);
+    });
+
+    test('bool', () => {
+      assert.equal(compare(true, true), 0);
+      assert.equal(compare(true, false), -1);
+      assert.equal(compare(false, true), 1);
+    });
+
+    test('list', async () => {
+      const listOfFloat64Type = makeListType(float64Type);
+      const listA = await newList([0, 1, 2, 3], listOfFloat64Type);
+      const listB = await newList([0, 1, 2, 3], listOfFloat64Type);
+      const listC = await newList([4, 5, 6, 7], listOfFloat64Type);
+      assert.equal(compare(listA, listA), 0);
+      assert.equal(compare(listA, listB), 0);
+      assert.equal(compare(listA, listC), 1);
+      assert.equal(compare(listC, listA), -1);
+    });
+  });
+
+  suite('equal', () => {
+    test('int8', () => {
+      assert.isTrue(equals(1, 1));
+      assert.isFalse(equals(1, 3));
+      assert.isFalse(equals(4, 2));
+    });
+
+    test('string', () => {
+      assert.isTrue(equals('a', 'a'));
+      assert.isFalse(equals('a', 'b'));
+      assert.isFalse(equals('c', 'a'));
+    });
+
+    test('bool', () => {
+      assert.isTrue(equals(true, true));
+      assert.isFalse(equals(true, false));
+      assert.isFalse(equals(false, true));
+    });
+
+    test('list', async () => {
+      const listOfFloat64Type = makeListType(float64Type);
+      const listA = await newList([0, 1, 2, 3], listOfFloat64Type);
+      const listB = await newList([0, 1, 2, 3], listOfFloat64Type);
+      const listC = await newList([4, 5, 6, 7], listOfFloat64Type);
+      assert.isTrue(equals(listA, listA));
+      assert.isTrue(equals(listA, listB));
+      assert.isFalse(equals(listA, listC));
+      assert.isFalse(equals(listC, listA));
+    });
+  });
+});

js/src/compare.js

@@ -3,44 +3,51 @@
 import type {valueOrPrimitive, Value} from './value.js';
 import {invariant} from './assert.js';
 import {Kind} from './noms-kind.js';
+import {getRef} from './get-ref.js';
+import {boolType} from './type.js';
 import type {Type} from './type.js';
 
-export function less(v1: any, v2: any): boolean {
-  invariant(v1 !== null && v1 !== undefined && v2 !== null && v2 !== undefined);
+export function less(v1: valueOrPrimitive, v2: valueOrPrimitive): boolean {
+  invariant(typeof v1 === typeof v2);
 
   if (typeof v1 === 'object') {
-    invariant(typeof v2 === 'object');
-    return (v1:Value).less(v2);
+    return v1.less(v2);
   }
 
-  if (typeof v1 === 'string') {
-    invariant(typeof v2 === 'string');
-    return v1 < v2;
+  if (typeof v1 === 'boolean') {
+    // $FlowIssue: Flow does not realize that v1 and v2 have the same type.
+    return compareBools(v1, v2) === -1;
   }
 
-  invariant(typeof v1 === 'number');
-  invariant(typeof v2 === 'number');
+  invariant(typeof v1 === 'number' || typeof v1 === 'string');
+  // $FlowIssue: Flow does not realize that v1 and v2 have the same type.
   return v1 < v2;
 }
 
 export function equals(v1: valueOrPrimitive, v2: valueOrPrimitive): boolean {
-  invariant(v1 !== null && v1 !== undefined && v2 !== null && v2 !== undefined);
+  if (v1 === v2) {
+    return true;
+  }
+
+  invariant(typeof v1 === typeof v2);
 
   if (typeof v1 === 'object') {
-    invariant(typeof v2 === 'object');
-    return (v1: Value).equals((v2: Value));
+    return v1.equals(v2);
   }
-  invariant(typeof v1 === 'string' || typeof v2 === 'number');
-  invariant(typeof v1 === typeof v2);
+
+  invariant(typeof v1 === 'string' || typeof v1 === 'number' || typeof v1 === 'boolean');
   return v1 === v2;
 }
 
 export function compare(v1: valueOrPrimitive, v2: valueOrPrimitive): number {
-  if (less(v1, v2)) {
-    return -1;
+  if (v1 === v2) {
+    return 0;
   }
 
-  return equals(v1, v2) ? 0 : 1;
+  if (equals(v1, v2)) {
+    return 0;
+  }
+  return less(v1, v2) ? -1 : 1;
 }
 
 function compareNumbers(v1: number, v2: number) {
@@ -48,13 +55,27 @@ function compareNumbers(v1: number, v2: number) {
 }
 
 function compareObjects(v1: Value, v2: Value) {
+  if (v1 === v2 || v1.equals(v2)) {
+    return 0;
+  }
+
   return v1.less(v2) ? -1 : 1;
 }
 
 function compareStrings(v1: string, v2: string): number {
+  if (v1 === v2) {
+    return 0;
+  }
   return v1 < v2 ? -1 : 1;
 }
 
+function compareBools(v1: boolean, v2: boolean): number {
+  if (v1 === v2) {
+    return 0;
+  }
+  return getRef(v1, boolType).less(getRef(v2 ,boolType)) ? -1 : 1;
+}
+
 /**
  * Returns a compare function that can be used with `Array.prototype.sort` based on the type.
  */
@@ -86,8 +107,10 @@ export function getCompareFunction(t: Type): (v1: any, v2: any) => number {
     case Kind.Package:
       return compareObjects;
 
-    case Kind.Value:
     case Kind.Bool:
+      return compareBools;
+
+    case Kind.Value:
       throw new Error('not implemented');
 
     default:

js/src/set-test.js

@@ -7,7 +7,15 @@ import DataStore from './data-store.js';
 import MemoryStore from './memory-store.js';
 import RefValue from './ref-value.js';
 import {newStruct} from './struct.js';
-import {Field, makeCompoundType, makePrimitiveType, makeStructType, makeType} from './type.js';
+import {
+  Field,
+  makeCompoundType,
+  makePrimitiveType,
+  makeStructType,
+  makeType,
+  makeSetType,
+  boolType,
+} from './type.js';
 import {flatten, flattenParallel} from './test-util.js';
 import {invariant, notNull} from './assert.js';
 import {Kind} from './noms-kind.js';
@@ -457,4 +465,11 @@ suite('CompoundSet', () => {
     await test([1, 2], [-2, -1, 1, 2]);
     await test([0, 1, 2], [-2, -1, 0, 1, 2]);
   });
+
+  test('set of bool', async () => {
+    const tr = makeSetType(boolType);
+    const set = await newSet([true], tr);
+    assert.isTrue(await set.has(true));
+    assert.isFalse(await set.has(false));
+  });
 });