Check for `null`/`undefined` in graph functions (#55)

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
This commit is contained in:
William Chargin 2018-03-02 13:47:13 -08:00 committed by GitHub
parent ca3502009b
commit f305a48391
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 57 additions and 0 deletions

View File

@ -38,6 +38,9 @@ export class Graph {
} }
addNode(node: Node<mixed>) { addNode(node: Node<mixed>) {
if (node == null) {
throw new Error(`node is ${String(node)}`);
}
if (this.getNode(node.address) !== undefined) { if (this.getNode(node.address) !== undefined) {
throw new Error( throw new Error(
`node at address ${JSON.stringify(node.address)} already exists` `node at address ${JSON.stringify(node.address)} already exists`
@ -51,6 +54,9 @@ export class Graph {
} }
addEdge(edge: Edge<mixed>) { addEdge(edge: Edge<mixed>) {
if (edge == null) {
throw new Error(`edge is ${String(edge)}`);
}
if (this.getEdge(edge.address) !== undefined) { if (this.getEdge(edge.address) !== undefined) {
throw new Error( throw new Error(
`edge at address ${JSON.stringify(edge.address)} already exists` `edge at address ${JSON.stringify(edge.address)} already exists`
@ -81,6 +87,9 @@ export class Graph {
* The order of the resulting array is unspecified. * The order of the resulting array is unspecified.
*/ */
getOutEdges(nodeAddress: Address): Edge<mixed>[] { getOutEdges(nodeAddress: Address): Edge<mixed>[] {
if (nodeAddress == null) {
throw new Error(`address is ${String(nodeAddress)}`);
}
const addresses = this._outEdges[addressToString(nodeAddress)]; const addresses = this._outEdges[addressToString(nodeAddress)];
if (addresses === undefined) { if (addresses === undefined) {
throw new Error(`no node for address ${JSON.stringify(nodeAddress)}`); 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. * The order of the resulting array is unspecified.
*/ */
getInEdges(nodeAddress: Address): Edge<mixed>[] { getInEdges(nodeAddress: Address): Edge<mixed>[] {
if (nodeAddress == null) {
throw new Error(`address is ${String(nodeAddress)}`);
}
const addresses = this._inEdges[addressToString(nodeAddress)]; const addresses = this._inEdges[addressToString(nodeAddress)];
if (addresses === undefined) { if (addresses === undefined) {
throw new Error(`no node for address ${JSON.stringify(nodeAddress)}`); throw new Error(`no node for address ${JSON.stringify(nodeAddress)}`);
@ -116,6 +128,9 @@ export class Graph {
} }
export function addressToString(address: Address) { export function addressToString(address: Address) {
if (address == null) {
throw new Error(`address is ${String(address)}`);
}
if (address.repositoryName.includes("$")) { if (address.repositoryName.includes("$")) {
const escaped = JSON.stringify(address.repositoryName); const escaped = JSON.stringify(address.repositoryName);
throw new Error(`address.repositoryName must not include "\$": ${escaped}`); throw new Error(`address.repositoryName must not include "\$": ${escaped}`);
@ -132,6 +147,9 @@ export function addressToString(address: Address) {
} }
export function stringToAddress(string: string) { export function stringToAddress(string: string) {
if (string == null) {
throw new Error(`address string is ${String(string)}`);
}
const parts = string.split("$"); const parts = string.split("$");
if (parts.length !== 3) { if (parts.length !== 3) {
const escaped = JSON.stringify(string); const escaped = JSON.stringify(string);

View File

@ -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", () => { describe("getting nodes and edges", () => {
it("correctly gets nodes in the simple graph", () => { it("correctly gets nodes in the simple graph", () => {
const g = simpleMealGraph(); const g = simpleMealGraph();