From f305a48391acbff29ca1cd2786e67c7fef6626e4 Mon Sep 17 00:00:00 2001
From: William Chargin <wchargin@gmail.com>
Date: Fri, 2 Mar 2018 13:47:13 -0800
Subject: [PATCH] Check for `null`/`undefined` in graph functions (#55)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Summary:
These will make nicer error functions in cases where static analysis
doesn’t detect the pollution: e.g., a user isn’t using Flow, or an
expression like `arr[0]` introduces an `undefined`.

Paired with @dandelionmane.

Test Plan:
New unit tests added. Run `yarn test`.

wchargin-branch: null-undefined-check
---
 src/backend/graph.js      | 18 ++++++++++++++++++
 src/backend/graph.test.js | 39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+)

diff --git a/src/backend/graph.js b/src/backend/graph.js
index 1fabbc3..0d392eb 100644
--- a/src/backend/graph.js
+++ b/src/backend/graph.js
@@ -38,6 +38,9 @@ export class Graph {
   }
 
   addNode(node: Node<mixed>) {
+    if (node == null) {
+      throw new Error(`node is ${String(node)}`);
+    }
     if (this.getNode(node.address) !== undefined) {
       throw new Error(
         `node at address ${JSON.stringify(node.address)} already exists`
@@ -51,6 +54,9 @@ export class Graph {
   }
 
   addEdge(edge: Edge<mixed>) {
+    if (edge == null) {
+      throw new Error(`edge is ${String(edge)}`);
+    }
     if (this.getEdge(edge.address) !== undefined) {
       throw new Error(
         `edge at address ${JSON.stringify(edge.address)} already exists`
@@ -81,6 +87,9 @@ export class Graph {
    * The order of the resulting array is unspecified.
    */
   getOutEdges(nodeAddress: Address): Edge<mixed>[] {
+    if (nodeAddress == null) {
+      throw new Error(`address is ${String(nodeAddress)}`);
+    }
     const addresses = this._outEdges[addressToString(nodeAddress)];
     if (addresses === undefined) {
       throw new Error(`no node for address ${JSON.stringify(nodeAddress)}`);
@@ -93,6 +102,9 @@ export class Graph {
    * The order of the resulting array is unspecified.
    */
   getInEdges(nodeAddress: Address): Edge<mixed>[] {
+    if (nodeAddress == null) {
+      throw new Error(`address is ${String(nodeAddress)}`);
+    }
     const addresses = this._inEdges[addressToString(nodeAddress)];
     if (addresses === undefined) {
       throw new Error(`no node for address ${JSON.stringify(nodeAddress)}`);
@@ -116,6 +128,9 @@ export class Graph {
 }
 
 export function addressToString(address: Address) {
+  if (address == null) {
+    throw new Error(`address is ${String(address)}`);
+  }
   if (address.repositoryName.includes("$")) {
     const escaped = JSON.stringify(address.repositoryName);
     throw new Error(`address.repositoryName must not include "\$": ${escaped}`);
@@ -132,6 +147,9 @@ export function addressToString(address: Address) {
 }
 
 export function stringToAddress(string: string) {
+  if (string == null) {
+    throw new Error(`address string is ${String(string)}`);
+  }
   const parts = string.split("$");
   if (parts.length !== 3) {
     const escaped = JSON.stringify(string);
diff --git a/src/backend/graph.test.js b/src/backend/graph.test.js
index a06c119..cc810dc 100644
--- a/src/backend/graph.test.js
+++ b/src/backend/graph.test.js
@@ -159,6 +159,45 @@ describe("graph", () => {
       });
     });
 
+    describe("has nice error messages for", () => {
+      [null, undefined].forEach((bad) => {
+        // The following tests have `(bad: any)` because Flow
+        // correctly detects that using `null` and `undefined` here is
+        // bad. Thanks, Flow---but we want to simulate runtime
+        // undefined-pollution, so we'll override you here.
+        it(`adding ${String(bad)} nodes`, () => {
+          expect(() => new Graph().addNode((bad: any))).toThrow(
+            `node is ${String(bad)}`
+          );
+        });
+        it(`adding ${String(bad)} edges`, () => {
+          expect(() => new Graph().addEdge((bad: any))).toThrow(
+            `edge is ${String(bad)}`
+          );
+        });
+        it(`getting ${String(bad)} nodes`, () => {
+          expect(() => new Graph().getNode((bad: any))).toThrow(
+            `address is ${String(bad)}`
+          );
+        });
+        it(`getting ${String(bad)} edges`, () => {
+          expect(() => new Graph().getEdge((bad: any))).toThrow(
+            `address is ${String(bad)}`
+          );
+        });
+        it(`getting ${String(bad)} in-edges`, () => {
+          expect(() => new Graph().getInEdges((bad: any))).toThrow(
+            `address is ${String(bad)}`
+          );
+        });
+        it(`getting ${String(bad)} out-edges`, () => {
+          expect(() => new Graph().getOutEdges((bad: any))).toThrow(
+            `address is ${String(bad)}`
+          );
+        });
+      });
+    });
+
     describe("getting nodes and edges", () => {
       it("correctly gets nodes in the simple graph", () => {
         const g = simpleMealGraph();