Create GitHub reference edges (#182)

This commit adds the `addReferenceEdges()` method to the GitHub parser,
which examines all of the posts in the parsed graph and adds References
edges when it detects references between posts. As an example, `Hey
@wchargin, take a look at #1337` would generate two references.

We currently parse the following kinds of references:
- Numeric references to issues/PRs.
- Explicit in-repository url references (to any entity)
- @-author references

We do not parse:
- Cross-repository urls
- Cross-repository shortform (e.g. `sourcecred/sourcecred#100`)

`Parser.parse` calls `addReferenceEdges()`, so no change is required by
consumers to have reference edges added to their graphs.

The GitHub porcelain API layer now includes methods for retreiving the
entities referenced by a post.

Test plan:
This commit is tested both via snapshot tests, and explicit testing at
api layer. (Actually, the creation of the porcelain API layer was
prompted by wanting a cleaner way to test this commit.) I recommend
inspecting the snapshot tests for sanity, but mostly relying on the
tested behavior in api.test.js.

src/plugins/github/api.js

@@ -28,6 +28,7 @@ import {
   PULL_REQUEST_NODE_TYPE,
   PULL_REQUEST_REVIEW_NODE_TYPE,
   PULL_REQUEST_REVIEW_COMMENT_NODE_TYPE,
+  REFERENCES_EDGE_TYPE,
 } from "./types";
 
 export type Entity =
@@ -135,6 +136,45 @@ class Post<
   body(): string {
     return this.node().payload.body;
   }
+
+  references(): Entity[] {
+    const result: Entity[] = [];
+    this.graph
+      .neighborhood(this.nodeAddress, {
+        edgeType: REFERENCES_EDGE_TYPE,
+        direction: "OUT",
+      })
+      .forEach(({neighbor}) => {
+        const type: NodeType = (neighbor.type: any);
+        switch (type) {
+          case "ISSUE":
+            result.push(new Issue(this.graph, neighbor));
+            break;
+          case "PULL_REQUEST":
+            result.push(new PullRequest(this.graph, neighbor));
+            break;
+          case "COMMENT":
+            result.push(new Comment(this.graph, neighbor));
+            break;
+          case "AUTHOR":
+            result.push(new Author(this.graph, neighbor));
+            break;
+          case "PULL_REQUEST_REVIEW":
+            result.push(new PullRequestReview(this.graph, neighbor));
+            break;
+          case "PULL_REQUEST_REVIEW_COMMENT":
+            result.push(new PullRequestReviewComment(this.graph, neighbor));
+            break;
+          default:
+            // eslint-disable-next-line no-unused-expressions
+            (type: empty);
+            throw new Error(
+              `Attempted to parse reference to unknown entity type ${type}`
+            );
+        }
+      });
+    return result;
+  }
 }
 
 class Commentable<T: IssueNodePayload | PullRequestNodePayload> extends Post<

src/plugins/github/api.test.js

@@ -8,6 +8,8 @@ import {
   COMMENT_NODE_TYPE,
   ISSUE_NODE_TYPE,
   PULL_REQUEST_NODE_TYPE,
+  PULL_REQUEST_REVIEW_NODE_TYPE,
+  PULL_REQUEST_REVIEW_COMMENT_NODE_TYPE,
 } from "./types";
 describe("GitHub porcelain API", () => {
   const graph = parse("sourcecred/example-repo", exampleRepoData);
@@ -127,4 +129,100 @@ describe("GitHub porcelain API", () => {
       ).toThrowError("to have type AUTHOR");
     });
   });
+  describe("References", () => {
+    it("via #-number", () => {
+      const srcIssue = issueOrPRByNumber(2);
+      const references = srcIssue.references();
+      expect(references).toHaveLength(1);
+      // Note: this verifies that we are not counting in-references, as
+      // https://github.com/sourcecred/example-repo/issues/6#issuecomment-385223316
+      // references #2.
+
+      const referenced = Issue.from(references[0]);
+      expect(referenced.number()).toBe(1);
+    });
+
+    describe("by exact url", () => {
+      function expectCommentToHaveSingleReference({commentNumber, type, url}) {
+        const comments = issueOrPRByNumber(2).comments();
+        const references = comments[commentNumber].references();
+        expect(references).toHaveLength(1);
+        expect(references[0].url()).toBe(url);
+        expect(references[0].type()).toBe(type);
+      }
+
+      it("to an issue", () => {
+        expectCommentToHaveSingleReference({
+          commentNumber: 0,
+          type: ISSUE_NODE_TYPE,
+          url: "https://github.com/sourcecred/example-repo/issues/6",
+        });
+      });
+
+      it("to a comment", () => {
+        expectCommentToHaveSingleReference({
+          commentNumber: 1,
+          type: COMMENT_NODE_TYPE,
+          url:
+            "https://github.com/sourcecred/example-repo/issues/6#issuecomment-373768538",
+        });
+      });
+
+      it("to a pull request", () => {
+        expectCommentToHaveSingleReference({
+          commentNumber: 2,
+          type: PULL_REQUEST_NODE_TYPE,
+          url: "https://github.com/sourcecred/example-repo/pull/5",
+        });
+      });
+
+      it("to a pull request review", () => {
+        expectCommentToHaveSingleReference({
+          commentNumber: 3,
+          type: PULL_REQUEST_REVIEW_NODE_TYPE,
+          url:
+            "https://github.com/sourcecred/example-repo/pull/5#pullrequestreview-100313899",
+        });
+      });
+
+      it("to a pull request review comment", () => {
+        expectCommentToHaveSingleReference({
+          commentNumber: 4,
+          type: PULL_REQUEST_REVIEW_COMMENT_NODE_TYPE,
+          url:
+            "https://github.com/sourcecred/example-repo/pull/5#discussion_r171460198",
+        });
+      });
+
+      it("to an author", () => {
+        expectCommentToHaveSingleReference({
+          commentNumber: 5,
+          type: AUTHOR_NODE_TYPE,
+          url: "https://github.com/wchargin",
+        });
+      });
+
+      it("to multiple entities", () => {
+        const references = issueOrPRByNumber(2)
+          .comments()[6]
+          .references();
+        expect(references).toHaveLength(5);
+      });
+
+      it("to no entities", () => {
+        const references = issueOrPRByNumber(2)
+          .comments()[7]
+          .references();
+        expect(references).toHaveLength(0);
+      });
+    });
+
+    it("References by @-author", () => {
+      const pr = issueOrPRByNumber(5);
+      const references = pr.references();
+      expect(references).toHaveLength(1);
+      const referenced = Author.from(references[0]);
+      expect(referenced.login()).toBe("wchargin");
+    });
+  });
 });

src/plugins/github/parser.js

@@ -1,5 +1,7 @@
 // @flow
 
+import stringify from "json-stable-stringify";
+
 import type {Node, Edge} from "../../core/graph";
 import type {
   NodeType,
@@ -12,6 +14,7 @@ import type {
   PullRequestReviewCommentNodePayload,
   CommentNodePayload,
   PullRequestNodePayload,
+  ReferencesEdgePayload,
   IssueNodePayload,
   AuthorSubtype,
 } from "./types";
@@ -28,7 +31,7 @@ import type {
 import type {Address} from "../../core/address";
 import {PLUGIN_NAME} from "./pluginName";
 import {Graph, edgeID} from "../../core/graph";
-const stringify = require("json-stable-stringify");
+import {findReferences} from "./findReferences";
 
 export function parse(
   repositoryName: string,
@@ -36,6 +39,7 @@ export function parse(
 ): Graph<NodePayload, EdgePayload> {
   const parser = new GithubParser(repositoryName);
   parser.addData(repositoryJSON);
+  parser.addReferenceEdges();
   return parser.graph;
 }
 
@@ -239,6 +243,70 @@ class GithubParser {
     reviewJson.comments.nodes.forEach((c) => this.addComment(reviewNode, c));
   }
 
+  /** Add all the in-repo GitHub reference edges detected.
+   *
+   * Parse all the nodes added to the GitHubParser, detect any
+   * GitHub references (e.g. url, #num, or @login), and add corresponding
+   * REFERENCE type edges.
+   *
+   * Needs to be called after adding data (or it will no-op).
+   * @returns {string[]}: All of the dangling (unparsed) reference strings.
+   */
+  addReferenceEdges(): string[] {
+    const referenceToNode = {};
+    this.graph.nodes().forEach((node) => {
+      referenceToNode[node.payload.url] = node;
+      const anyNode: Node<any> = node;
+      const type: NodeType = (node.address.type: any);
+      switch (type) {
+        case "ISSUE":
+        case "PULL_REQUEST":
+          const thisPayload: IssueNodePayload | PullRequestNodePayload =
+            anyNode.payload;
+          referenceToNode[`#${thisPayload.number}`] = node;
+          break;
+        case "AUTHOR":
+          let authorPayload: AuthorNodePayload = anyNode.payload;
+          referenceToNode[`@${authorPayload.login}`] = node;
+          break;
+        case "COMMENT":
+        case "PULL_REQUEST_REVIEW":
+        case "PULL_REQUEST_REVIEW_COMMENT":
+          break;
+        default:
+          // eslint-disable-next-line no-unused-expressions
+          (type: empty);
+          throw new Error(`unknown node type: ${type}`);
+      }
+    });
+
+    const danglingReferences = [];
+    this.graph.nodes().forEach((srcNode) => {
+      if (srcNode.payload.body !== undefined) {
+        const references = findReferences(srcNode.payload.body);
+        references.forEach((ref) => {
+          const dstNode = referenceToNode[ref];
+          if (dstNode === undefined) {
+            danglingReferences.push(ref);
+          } else {
+            const referenceEdge: Edge<ReferencesEdgePayload> = {
+              address: this.makeEdgeAddress(
+                "REFERENCES",
+                srcNode.address,
+                dstNode.address
+              ),
+              payload: {},
+              src: srcNode.address,
+              dst: dstNode.address,
+            };
+            this.graph.addEdge(referenceEdge);
+          }
+        });
+      }
+    });
+    return danglingReferences;
+  }
+
   addData(dataJson: RepositoryJSON) {
     dataJson.repository.issues.nodes.forEach((i) => this.addIssue(i));
     dataJson.repository.pullRequests.nodes.forEach((pr) =>

src/plugins/github/parser.test.js

@@ -8,6 +8,23 @@ import {Graph} from "../../core/graph";
 import exampleRepoData from "./demoData/example-repo.json";
 
 describe("GithubParser", () => {
+  function getIssue(n): IssueJSON {
+    const issues = exampleRepoData.repository.issues.nodes;
+    const selected = issues.filter((x) => x.number === n);
+    if (selected.length !== 1) {
+      throw new Error(`Failure finding issue #${n}`);
+    }
+    return selected[0];
+  }
+  function getPR(n): PullRequestJSON {
+    const pulls = exampleRepoData.repository.pullRequests.nodes;
+    const selected = pulls.filter((x) => x.number === n);
+    if (selected.length !== 1) {
+      throw new Error(`Failure finding PR #${n}`);
+    }
+    return selected[0];
+  }
+
   describe("whole repo parsing", () => {
     const graph = parse("sourcecred/example-repo", exampleRepoData);
 
@@ -68,22 +85,6 @@ describe("GithubParser", () => {
     });
   });
 
-  function getIssue(n: number): IssueJSON {
-    const issues = exampleRepoData.repository.issues.nodes;
-    const selected = issues.filter((x) => x.number === n);
-    if (selected.length !== 1) {
-      throw new Error(`Failure finding issue #${n}`);
-    }
-    return selected[0];
-  }
-  function getPR(n: number): PullRequestJSON {
-    const pulls = exampleRepoData.repository.pullRequests.nodes;
-    const selected = pulls.filter((x) => x.number === n);
-    if (selected.length !== 1) {
-      throw new Error(`Failure finding PR #${n}`);
-    }
-    return selected[0];
-  }
   type ExampleInput = {
     issues?: number[],
     prs?: number[],
@@ -134,4 +135,26 @@ describe("GithubParser", () => {
       expect(parseExample({prs: [5]})).toMatchSnapshot();
     });
   });
+
+  describe("reference detection", () => {
+    // These tests are included mostly for regression testing. To be persuaded that the references
+    // were added correctly, see the reference api tests in api.test.js. Those tests are much
+    // easier to read and to be persuaded that the behavior is working as intended.
+    it("discovers a simple reference", () => {
+      expect(parseExample({issues: [1, 2, 6]})).toMatchSnapshot();
+    });
+
+    it("discovers references even when parsing issues out of order", () => {
+      // Ensure that we will detect a reference from A to B, even if B hasn't
+      // been discovered at the time that we parse A.
+      const graphA = parseExample({issues: [1, 2, 6]});
+      const graphB = parseExample({issues: [6, 2, 1]});
+      expect(graphA.equals(graphB)).toBe(true);
+    });
+
+    it("handles dangling references gracefully", () => {
+      const graph = parseExample({issues: [2]});
+      expect(graph).toMatchSnapshot();
+    });
+  });
 });