fix false negative cases for no-unreachable-types rule (#555)

dimaMachina · Aug 3, 2021 · 44f0d73 · 44f0d73
1 parent 9232f09
commit 44f0d73
Show file tree

Hide file tree

Showing 6 changed files with 203 additions and 318 deletions.
diff --git a/.changeset/great-beans-design.md b/.changeset/great-beans-design.md
@@ -0,0 +1,5 @@
+---
+'@graphql-eslint/eslint-plugin': patch
+---
+
+fix false negative cases for `no-unreachable-types` rule
diff --git a/packages/plugin/src/graphql-ast.ts b/packages/plugin/src/graphql-ast.ts
@@ -1,25 +1,5 @@
-import {
-  GraphQLSchema,
-  GraphQLFieldMap,
-  GraphQLInputFieldMap,
-  GraphQLField,
-  GraphQLInputField,
-  GraphQLInputType,
-  GraphQLOutputType,
-  GraphQLNamedType,
-  GraphQLInterfaceType,
-  GraphQLArgument,
-  GraphQLDirective,
-  isObjectType,
-  isInterfaceType,
-  isUnionType,
-  isInputObjectType,
-  isListType,
-  isNonNullType,
-  TypeInfo,
-  visit,
-  visitWithTypeInfo,
-} from 'graphql';
+import { GraphQLSchema, TypeInfo, ASTKindToNode, Visitor, visit, visitWithTypeInfo, parse } from 'graphql';
+import { printSchemaWithDirectives } from '@graphql-tools/utils';
 import { SiblingOperations } from './sibling-operations';
 
 export type ReachableTypes = Set<string>;
@@ -30,100 +10,52 @@ export function getReachableTypes(schema: GraphQLSchema): ReachableTypes {
   // We don't want cache reachableTypes on test environment
   // Otherwise reachableTypes will be same for all tests
   if (process.env.NODE_ENV !== 'test' && reachableTypesCache) {
-    return reachableTypesCache
+    return reachableTypesCache;
   }
+  // 👀 `printSchemaWithDirectives` keep all custom directives and `printSchema` from `graphql` not
+  const printedSchema = printSchemaWithDirectives(schema); // Returns a string representation of the schema
+  const astNode = parse(printedSchema); // Transforms the string into ASTNode
+  const cache = Object.create(null);
 
-  const reachableTypes: ReachableTypes = new Set();
-
-  collectFromDirectives(schema.getDirectives());
-  collectFrom(schema.getQueryType());
-  collectFrom(schema.getMutationType());
-  collectFrom(schema.getSubscriptionType());
-
-  reachableTypesCache = reachableTypes;
-  return reachableTypesCache;
-
-  function collectFromDirectives(directives: readonly GraphQLDirective[]): void {
-    for (const directive of directives || []) {
-      reachableTypes.add(directive.name);
-      directive.args.forEach(collectFromArgument);
-    }
-  }
-
-  function collectFrom(type?: GraphQLNamedType): void {
-    if (type && shouldCollect(type.name)) {
-      if (isObjectType(type)) {
-        collectFromFieldMap(type.getFields());
-        collectFromInterfaces(type.getInterfaces());
-      } else if (isInterfaceType(type)) {
-        collectFromFieldMap(type.getFields());
-        collectFromInterfaces(type.getInterfaces());
-        collectFromImplementations(type);
-      } else if (isUnionType(type)) {
-        type.getTypes().forEach(collectFrom);
-      } else if (isInputObjectType(type)) {
-        collectFromInputFieldMap(type.getFields());
-      }
-    }
-  }
-
-  function collectFromFieldMap(fieldMap: GraphQLFieldMap<any, any>): void {
-    for (const fieldName in fieldMap) {
-      collectFromField(fieldMap[fieldName]);
-    }
-  }
-
-  function collectFromField(field: GraphQLField<any, any>): void {
-    collectFromOutputType(field.type);
-    field.args.forEach(collectFromArgument);
-  }
-
-  function collectFromArgument(arg: GraphQLArgument): void {
-    collectFromInputType(arg.type);
-  }
-
-  function collectFromInputFieldMap(fieldMap: GraphQLInputFieldMap): void {
-    for (const fieldName in fieldMap) {
-      collectFromInputField(fieldMap[fieldName]);
-    }
-  }
-
-  function collectFromInputField(field: GraphQLInputField): void {
-    collectFromInputType(field.type);
-  }
-
-  function collectFromInterfaces(interfaces: GraphQLInterfaceType[]): void {
-    if (interfaces) {
-      interfaces.forEach(collectFrom);
+  const collect = (nodeType: any): void => {
+    let node = nodeType;
+    while (node.type) {
+      node = node.type;
     }
-  }
-
-  function collectFromOutputType(output: GraphQLOutputType): void {
-    collectFrom(schema.getType(resolveName(output)));
-  }
+    const typeName = node.name.value;
+    cache[typeName] ??= 0;
+    cache[typeName] += 1;
+  };
 
-  function collectFromInputType(input: GraphQLInputType): void {
-    collectFrom(schema.getType(resolveName(input)));
-  }
+  const visitor: Visitor<ASTKindToNode> = {
+    SchemaDefinition(node) {
+      node.operationTypes.forEach(collect);
+    },
+    ObjectTypeDefinition(node) {
+      [node, ...node.interfaces].forEach(collect);
+    },
+    UnionTypeDefinition(node) {
+      [node, ...node.types].forEach(collect);
+    },
+    InputObjectTypeDefinition: collect,
+    InterfaceTypeDefinition: collect,
+    ScalarTypeDefinition: collect,
+    InputValueDefinition: collect,
+    DirectiveDefinition: collect,
+    EnumTypeDefinition: collect,
+    FieldDefinition: collect,
+    Directive: collect,
+  };
 
-  function collectFromImplementations(type: GraphQLInterfaceType): void {
-    schema.getPossibleTypes(type).forEach(collectFrom);
-  }
+  visit(astNode, visitor);
 
-  function resolveName(type: GraphQLOutputType | GraphQLInputType) {
-    if (isListType(type) || isNonNullType(type)) {
-      return resolveName(type.ofType);
-    }
-    return type.name;
-  }
+  const operationTypeNames = new Set(['Query', 'Mutation', 'Subscription']);
+  const usedTypes = Object.entries(cache)
+    .filter(([typeName, usedCount]) => usedCount > 1 || operationTypeNames.has(typeName))
+    .map(([typeName]) => typeName);
 
-  function shouldCollect(name: string): boolean {
-    if (reachableTypes.has(name)) {
-      return false;
-    }
-    reachableTypes.add(name);
-    return true;
-  }
+  reachableTypesCache = new Set(usedTypes);
+  return reachableTypesCache;
 }
 
 export type UsedFields = Record<string, Set<string>>;
@@ -136,41 +68,30 @@ export function getUsedFields(schema: GraphQLSchema, operations: SiblingOperatio
   if (process.env.NODE_ENV !== 'test' && usedFieldsCache) {
     return usedFieldsCache;
   }
-
   const usedFields: UsedFields = {};
-
-  const addField = (typeName, fieldName) => {
-    const fieldType = usedFields[typeName] ?? (usedFields[typeName] = new Set());
-    fieldType.add(fieldName);
-  };
-
   const typeInfo = new TypeInfo(schema);
+  const allDocuments = [...operations.getOperations(), ...operations.getFragments()];
 
   const visitor = visitWithTypeInfo(typeInfo, {
     Field: {
-      enter(node) {
+      enter(node): false | void {
         const fieldDef = typeInfo.getFieldDef();
-
         if (!fieldDef) {
           // skip visiting this node if field is not defined in schema
           return false;
         }
-
-        const parent = typeInfo.getParentType();
+        const parentTypeName = typeInfo.getParentType().name;
         const fieldName = node.name.value;
-        addField(parent.name, fieldName);
 
-        return undefined;
+        usedFields[parentTypeName] ??= new Set();
+        usedFields[parentTypeName].add(fieldName);
       },
     },
   });
 
-  const allDocuments = [...operations.getOperations(), ...operations.getFragments()];
-
   for (const { document } of allDocuments) {
     visit(document, visitor);
   }
-
   usedFieldsCache = usedFields;
   return usedFieldsCache;
 }
diff --git a/packages/plugin/src/rules/no-unreachable-types.ts b/packages/plugin/src/rules/no-unreachable-types.ts
@@ -2,6 +2,7 @@ import { GraphQLESLintRule } from '../types';
 import { requireReachableTypesFromContext } from '../utils';
 
 const UNREACHABLE_TYPE = 'UNREACHABLE_TYPE';
+const RULE_NAME = 'no-unreachable-types';
 
 const rule: GraphQLESLintRule = {
   meta: {
@@ -11,7 +12,7 @@ const rule: GraphQLESLintRule = {
     docs: {
       description: `Requires all types to be reachable at some level by root level fields.`,
       category: 'Best Practices',
-      url: `https://github.com/dotansimha/graphql-eslint/blob/master/docs/rules/no-unreachable-types.md`,
+      url: `https://github.com/dotansimha/graphql-eslint/blob/master/docs/rules/${RULE_NAME}.md`,
       requiresSchema: true,
       examples: [
         {
@@ -46,18 +47,17 @@ const rule: GraphQLESLintRule = {
     type: 'suggestion',
   },
   create(context) {
-    function ensureReachability(node) {
+    const reachableTypes = requireReachableTypesFromContext(RULE_NAME, context);
+
+    function ensureReachability(node): void {
       const typeName = node.name.value;
-      const reachableTypes = requireReachableTypesFromContext('no-unreachable-types', context);
 
       if (!reachableTypes.has(typeName)) {
         context.report({
           node,
           messageId: UNREACHABLE_TYPE,
-          data: {
-            typeName,
-          },
-          fix: fixer => fixer.removeRange(node.range),
+          data: { typeName },
+          fix: fixer => fixer.remove(node),
         });
       }
     }

diff --git a/packages/plugin/src/sibling-operations.ts b/packages/plugin/src/sibling-operations.ts
@@ -31,14 +31,14 @@ export type SiblingOperations = {
 };
 
 const handleVirtualPath = (documents: Source[]): Source[] => {
-  const filepathMap: Record<string, number> = {};
+  const filepathMap: Record<string, number> = Object.create(null);
 
   return documents.map(source => {
     const { location } = source;
     if (['.gql', '.graphql'].some(extension => location.endsWith(extension))) {
       return source;
     }
-    filepathMap[location] = filepathMap[location] ?? -1;
+    filepathMap[location] ??= -1;
     const index = filepathMap[location] += 1;
     return {
       ...source,
@@ -65,16 +65,17 @@ const getSiblings = (filePath: string, gqlConfig: GraphQLConfig): Source[] => {
     return operationsCache.get(documentsKey);
   }
 
-  const siblings = projectForFile.loadDocumentsSync(projectForFile.documents, {
+  const documents = projectForFile.loadDocumentsSync(projectForFile.documents, {
     skipGraphQLImport: true,
   });
+  const siblings = handleVirtualPath(documents)
   operationsCache.set(documentsKey, siblings);
 
   return siblings;
 };
 
 export function getSiblingOperations(options: ParserOptions, gqlConfig: GraphQLConfig): SiblingOperations {
-  const siblings = handleVirtualPath(getSiblings(options.filePath, gqlConfig));
+  const siblings = getSiblings(options.filePath, gqlConfig);
 
   if (siblings.length === 0) {
     let printed = false;

diff --git a/packages/plugin/tests/mocks/graphql-server.ts b/packages/plugin/tests/mocks/graphql-server.ts
@@ -45,7 +45,7 @@ class TestGraphQLServer {
     }
   }
 
-  parseData(req: IncomingMessage): Promise<any | string> {
+  private parseData(req: IncomingMessage): Promise<any | string> {
     return new Promise(resolve => {
       let data = '';
       req.on('data', chunk => {