refactor: clean up AST traversal and report building (#26)

* Refactor visitor code

* Improve naming of various funcs

* Add more explicit structure for report building and split up logic

Author: CDThomas

src/findJSGraphQLTags.test.ts

@@ -1,5 +1,8 @@
 import findJSGraphQLTags from "./findJSGraphQLTags";
 
+// TODO: add test for queries/mutations/subscriptions without names
+// TODO: add test for subscriptions
+
 describe("findJSGraphQLTags", () => {
   test("returns GraphQL tags given JS source code", () => {
     const js = `

src/findJSGraphQLTags.ts

@@ -1,4 +1,4 @@
-import parser = require("@babel/parser");
+import { parse, ParserOptions, ParserPlugin } from "@babel/parser";
 import traverse, { NodePath } from "@babel/traverse";
 import {
   Expression,
@@ -10,7 +10,7 @@ import { GraphQLTag } from "./types";
 
 // https://github.com/facebook/relay/blob/master/packages/relay-compiler/language/javascript/FindGraphQLTags.js
 
-const plugins: parser.ParserPlugin[] = [
+const plugins: ParserPlugin[] = [
   "asyncGenerators",
   "classProperties",
   ["decorators", { decoratorsBeforeExport: true }],
@@ -26,7 +26,7 @@ const plugins: parser.ParserPlugin[] = [
   "optionalCatchBinding"
 ];
 
-const PARSER_OPTIONS: parser.ParserOptions = {
+const PARSER_OPTIONS: ParserOptions = {
   allowImportExportEverywhere: true,
   allowReturnOutsideFunction: true,
   allowSuperOutsideMethod: true,
@@ -35,16 +35,16 @@ const PARSER_OPTIONS: parser.ParserOptions = {
   strictMode: false
 };
 
-function find(text: string, filePath: string): Array<GraphQLTag> {
-  const result: Array<GraphQLTag> = [];
-  const ast = parser.parse(text, PARSER_OPTIONS);
+function findGraphQLTags(code: string, filePath: string): GraphQLTag[] {
+  const results: GraphQLTag[] = [];
+  const ast = parse(code, PARSER_OPTIONS);
 
   const visitors = {
     TaggedTemplateExpression: ({
       node
     }: NodePath<TaggedTemplateExpression>) => {
       if (isGraphQLTag(node.tag)) {
-        result.push({
+        results.push({
           template: node.quasi.quasis[0].value.raw,
           sourceLocationOffset: getSourceLocationOffset(node.quasi),
           filePath
@@ -54,7 +54,7 @@ function find(text: string, filePath: string): Array<GraphQLTag> {
   };
 
   traverse(ast, visitors);
-  return result;
+  return results;
 }
 
 function isGraphQLTag(tag: Expression): boolean {
@@ -81,4 +81,4 @@ function getSourceLocationOffset(
   };
 }
 
-export default find;
+export default findGraphQLTags;

src/getFieldInfo.ts

@@ -1,7 +1,8 @@
 import {
-  ASTNode,
   FieldNode,
+  FragmentDefinitionNode,
   getLocation,
+  OperationDefinitionNode,
   parse,
   Source,
   TypeInfo,
@@ -20,59 +21,41 @@ export interface FieldInfo {
   filePath: string;
 }
 
-function getFeildInfo(
-  { template, sourceLocationOffset, filePath }: GraphQLTag,
+function findFields(
+  graphQLTag: GraphQLTag,
   typeInfo: TypeInfo,
   cb: (fieldInfo: FieldInfo) => void
 ) {
-  const ast = parse(template);
+  const ast = parse(graphQLTag.template);
 
   visit(
     ast,
     visitWithTypeInfo(typeInfo, {
-      OperationDefinition(graphqlNode) {
-        if (graphqlNode.name) {
-          visitFields(
-            graphqlNode,
-            graphqlNode.name.value,
-            typeInfo,
-            template,
-            sourceLocationOffset,
-            filePath,
-            cb
-          );
-        } else {
-          throw new Error(`No name for OperationDefinition`);
-        }
+      OperationDefinition(node) {
+        visitFields(node, graphQLTag, typeInfo, cb);
       },
-      FragmentDefinition(graphqlNode) {
-        if (graphqlNode.name) {
-          visitFields(
-            graphqlNode,
-            graphqlNode.name.value,
-            typeInfo,
-            template,
-            sourceLocationOffset,
-            filePath,
-            cb
-          );
-        } else {
-          throw new Error(`No name for FragmentDefinition`);
-        }
+      FragmentDefinition(node) {
+        visitFields(node, graphQLTag, typeInfo, cb);
       }
     })
   );
 }
 
 function visitFields(
-  node: ASTNode,
-  operationOrFragmentName: string,
+  node: OperationDefinitionNode | FragmentDefinitionNode,
+  graphQLTag: GraphQLTag,
   typeInfo: TypeInfo,
-  template: string,
-  sourceLocationOffset: { line: number; column: number },
-  filePath: string,
   cb: (fieldInfo: FieldInfo) => void
 ) {
+  if (!node.name) {
+    throw new Error(
+      "visitFields expects OperationDefinitions and FragmentDefinitions to be named"
+    );
+  }
+
+  const { filePath, sourceLocationOffset, template } = graphQLTag;
+  const operationOrFragmentName = node.name.value;
+
   visit(
     node,
     visitWithTypeInfo(typeInfo, {
@@ -85,15 +68,21 @@ function visitFields(
         const nodeName = graphqlNode.name.value;
 
         if (!parentType) {
-          throw new Error(`No parent type for ${nodeName}`);
+          throw new Error(
+            `visitFields expects fields to have a parent type. No parent type for ${nodeName}`
+          );
         }
 
         if (!nodeType) {
-          throw new Error(`No type for ${nodeName}`);
+          throw new Error(
+            `visitFields expects fields to have a type. No type for ${nodeName}`
+          );
         }
 
         if (!graphqlNode.loc) {
-          throw new Error(`No location for ${nodeName}`);
+          throw new Error(
+            `visitFields expects fields to have a location. No location for ${nodeName}`
+          );
         }
 
         const loc = graphqlNode.loc;
@@ -124,4 +113,4 @@ function isClientOnlyField(field: FieldNode): boolean {
   return !!clientOnlyDirective;
 }
 
-export default getFeildInfo;
+export default findFields;

src/index.test.ts

@@ -243,8 +243,8 @@ function assertOutputMatchesSnapshot(output: {
     type.fields.map((field: any) => {
       expect(omit(["occurrences"], field)).toMatchSnapshot();
 
-      field.occurrences.map((occurence: any) => {
-        expect(occurence).toMatchSnapshot({
+      field.occurrences.map((occurrence: any) => {
+        expect(occurrence).toMatchSnapshot({
           filename: expect.stringMatching(
             /^https:\/\/github.com\/CDThomas\/graphql-usage\/tree\/.*\.(js|jsx|ts|tsx)#L\d$/
           )

src/index.ts

@@ -18,7 +18,7 @@ import findTSGraphQLTags from "./findTSGraphQLTags";
 import flatten from "./flatten";
 import getFeildInfo, { FieldInfo } from "./getFieldInfo";
 import { getGitHubBaseURL, getGitProjectRoot } from "./gitUtils";
-import { buildReport, Report } from "./report";
+import { addOccurrence, buildInitialState, format, Report } from "./report";
 import createServer from "./server";
 import { GraphQLTag } from "./types";
 
@@ -125,15 +125,29 @@ async function analyzeFiles(
   });
 
   const data = await Promise.all<SourceFile>(
-    files.map(unary(partialRight(readFiles, [sourceDir])))
+    files.map(unary(partialRight(readFile, [sourceDir])))
   );
   let tags = flatten(data.map(findGraphQLTags));
 
-  const info: FieldInfo[] = [];
-  const cb = (fieldInfo: FieldInfo) => info.push(fieldInfo);
-  findFields(schema, tags, cb);
+  const state = buildInitialState(schema);
+  findFields(
+    schema,
+    tags,
+    ({ parentType, name, filePath, line, rootNodeName }: FieldInfo) => {
+      const gitHubFileURL = filePath.replace(
+        path.resolve(gitDir),
+        gitHubBaseURL
+      );
+      const link = `${gitHubFileURL}#L${line}`;
+
+      addOccurrence(state, parentType, name, {
+        filename: link,
+        rootNodeName
+      });
+    }
+  );
 
-  return buildReport(info, schema, gitDir, gitHubBaseURL);
+  return format(state);
 }
 
 async function readSchema(schemaFile: string): Promise<GraphQLSchema> {
@@ -181,7 +195,7 @@ interface SourceFile {
   fullPath: string;
 }
 
-async function readFiles(
+async function readFile(
   filePath: string,
   sourceDir: string
 ): Promise<SourceFile> {

src/report.test.ts

@@ -0,0 +1,116 @@
+import { buildSchema, isObjectType } from "graphql";
+
+import { addOccurrence, buildInitialState, format } from "./report";
+
+const testSchema = buildSchema(`
+  type Query {
+    books: [Book]
+  }
+
+  type Book {
+    title: String!
+    pageCount: Int
+    isPublished: Boolean
+  }
+`);
+
+describe("buildInitialState", () => {
+  test("includes a property for each named type in the schema", () => {
+    // TODO: non-object types
+    const expectedTypes = testSchema.toConfig().types.filter(isObjectType);
+
+    const { types } = buildInitialState(testSchema);
+
+    expectedTypes.forEach(type => {
+      expect(types).toHaveProperty(type.name);
+    });
+  });
+
+  test("builds the correct properties for Object types", () => {
+    const { types } = buildInitialState(testSchema);
+
+    expect(types.Book).toEqual({
+      fields: expect.any(Object),
+      // kind: "Object",
+      name: "Book"
+    });
+  });
+
+  test("includes a property for each field of an Object type", () => {
+    const initialState = buildInitialState(testSchema);
+    const fields = initialState.types.Book.fields;
+
+    expect(fields).toEqual({
+      title: expect.any(Object),
+      pageCount: expect.any(Object),
+      isPublished: expect.any(Object)
+    });
+  });
+
+  test("includes the correct properties for individual object fields", () => {
+    const initialState = buildInitialState(testSchema);
+    const field = initialState.types.Book.fields.title;
+
+    expect(field).toEqual({
+      name: "title",
+      occurrences: [],
+      type: {
+        kind: "NonNull",
+        name: null,
+        ofType: {
+          kind: "Scalar",
+          name: "String",
+          ofType: null
+        }
+      }
+    });
+  });
+});
+
+describe("addOccurrence", () => {
+  test("adds an occurrence to the given field", () => {
+    const state = buildInitialState(testSchema);
+
+    addOccurrence(state, "Book", "title", {
+      filename: "src/Component.js",
+      rootNodeName: "ComponentQuery"
+    });
+
+    expect(state.types.Book.fields.title.occurrences).toEqual([
+      {
+        filename: "src/Component.js",
+        rootNodeName: "ComponentQuery"
+      }
+    ]);
+  });
+});
+
+describe("format", () => {
+  test("formats types as a list sorted alphabetically", () => {
+    const state = buildInitialState(testSchema);
+    const report = format(state);
+
+    expect(report.data.types.map(({ name }) => name)).toEqual([
+      "Book",
+      "Query",
+      "__Directive",
+      "__EnumValue",
+      "__Field",
+      "__InputValue",
+      "__Schema",
+      "__Type"
+    ]);
+  });
+
+  test("formats fields as a list sorted alphabetically", () => {
+    const state = buildInitialState(testSchema);
+    const report = format(state);
+
+    const bookType = report.data.types.find(type => type.name === "Book")!;
+    expect(bookType.fields.map(({ name }) => name)).toEqual([
+      "isPublished",
+      "pageCount",
+      "title"
+    ]);
+  });
+});