Repository: kamilkisiela/graphql-inspector
Pull request #506 (2019-08-09 12:06:23)

Create a rule to supress removal of deprecated fields


Patch:
diff --git a/docs/essentials/diff.md b/docs/essentials/diff.md
index 7e4d5dca..e1cd59e2 100644
--- a/docs/essentials/diff.md
+++ b/docs/essentials/diff.md
@@ -43,3 +43,13 @@ Compare your local schema against a remote server:
 Compare your local schema against a schema on a master branch (github):
 
     graphql-inspector diff github:user/repo#master:schema.graphql schema.graphql
+
+## Rules
+
+In order to customize the diff's behavior, you're able to use a set of rules:
+
+**suppressRemovalOfDeprecatedField**
+
+Every removal of a deprecated field is considered as a breaking change. With that flag you can turn it into a dangerous change so it won't fail a process or a CI check.
+
+    graphql-inspector diff https://api.com/graphql schema.graphql --rule suppressRemovalOfDeprecatedField
diff --git a/docs/essentials/validate.md b/docs/essentials/validate.md
index ce3fd280..ae182dfc 100644
--- a/docs/essentials/validate.md
+++ b/docs/essentials/validate.md
@@ -21,6 +21,7 @@ Run the following command:
 
 - `-d, --deprecated` - Fail on deprecated usage (default: _false_)
 - `--noStrictFragments` - Do not fail on duplicated fragment names (default: _false_)
+- `--apollo` - Support Apollo directives (@client and @connection) (default: _false_)
 - `--maxDepth <n>` - Fail when operation depth exceeds maximum depth (default: _false_)
 - `-r, --require 
` - require a module
 - `-t, --token 
` - an access token
diff --git a/packages/cli/src/commands/diff.ts b/packages/cli/src/commands/diff.ts
index 36b180db..8a60112f 100644
--- a/packages/cli/src/commands/diff.ts
+++ b/packages/cli/src/commands/diff.ts
@@ -1,5 +1,6 @@
 import {
   diff as diffSchema,
+  DiffRule,
   Change,
   CriticalityLevel,
 } from '@graphql-inspector/core';
@@ -18,6 +19,7 @@ export async function diff(
     token?: string;
     renderer?: Renderer;
     require?: string[];
+    rule?: Array<keyof typeof DiffRule>;
     headers?: Record<string, string>;
   },
 ) {
@@ -33,15 +35,25 @@ export async function diff(
       headers: options.headers,
     });
 
-    const changes = diffSchema(oldSchema, newSchema);
+    const rules = options.rule
+      ? options.rule
+          .map(rule => {
+            if (!DiffRule[rule]) {
+              renderer.error(`\Rule '${rule}' does not exist!\n`);
+              process.exit(1);
+            }
+
+            return DiffRule[rule];
+          })
+          .filter(f => f)
+      : [];
+    const changes = diffSchema(oldSchema, newSchema, rules);
 
     if (!changes.length) {
       renderer.success('No changes detected');
     } else {
       renderer.emit(
-        `\nDetected the following changes (${
-          changes.length
-        }) between schemas:\n`,
+        `\nDetected the following changes (${changes.length}) between schemas:\n`,
       );
 
       changes.forEach(change => {
diff --git a/packages/cli/src/commands/validate.ts b/packages/cli/src/commands/validate.ts
index 4d5dcd69..e3c3a5be 100644
--- a/packages/cli/src/commands/validate.ts
+++ b/packages/cli/src/commands/validate.ts
@@ -18,6 +18,7 @@ export async function validate(
     require?: string[];
     deprecated: boolean;
     noStrictFragments: boolean;
+    apollo?: boolean;
     maxDepth?: number;
     renderer?: Renderer;
     headers?: Record<string, string>;
@@ -34,6 +35,7 @@ export async function validate(
     const invalidDocuments = validateDocuments(schema, documents, {
       strictFragments: !options.noStrictFragments,
       maxDepth: options.maxDepth || undefined,
+      apollo: options.apollo || false,
     });
 
     if (!invalidDocuments.length) {
diff --git a/packages/cli/src/index.ts b/packages/cli/src/index.ts
index 03f38193..cb7c187e 100644
--- a/packages/cli/src/index.ts
+++ b/packages/cli/src/index.ts
@@ -48,6 +48,7 @@ commander
 commander
   .command('diff <old> <new>')
   .description('Diff two schemas')
+  .option('--rule [name]', 'Add rules', collect, [])
   .action((oldSchema: string, newSchema: string, cmd: commander.Command) =>
     diff(oldSchema, newSchema, normalizeOptions(cmd)),
   );
@@ -63,6 +64,7 @@ commander
   .option('--maxDepth <n>', 'Fail on deep operations', (val: string) =>
     parseInt(val, 10),
   )
+  .option('--apollo', 'Support Apollo directives', false)
   .description('Validate documents against a schema')
   .action((documents: string, schema: string, cmd: commander.Command) =>
     validate(documents, schema, normalizeOptions(cmd)),
diff --git a/packages/core/src/diff/changes/field.ts b/packages/core/src/diff/changes/field.ts
index ae4f3ad8..0e294de1 100644
--- a/packages/core/src/diff/changes/field.ts
+++ b/packages/core/src/diff/changes/field.ts
@@ -23,7 +23,9 @@ export function fieldRemoved(
         : `Removing a field is a breaking change. It is preferable to deprecate the field before removing it.`,
     },
     type: ChangeType.FieldRemoved,
-    message: `Field '${field.name}' was removed from ${entity} '${type.name}'`,
+    message: `Field '${field.name}' ${
+      field.isDeprecated ? '(deprecated) ' : ''
+    }was removed from ${entity} '${type.name}'`,
     path: [type.name, field.name].join('.'),
   };
 }
@@ -53,9 +55,7 @@ export function fieldDescriptionChanged(
       level: CriticalityLevel.NonBreaking,
     },
     type: ChangeType.FieldDescriptionChanged,
-    message: `Field '${type.name}.${oldField.name}' description changed from '${
-      oldField.description
-    }' to '${newField.description}'`,
+    message: `Field '${type.name}.${oldField.name}' description changed from '${oldField.description}' to '${newField.description}'`,
     path: [type.name, oldField.name].join('.'),
   };
 }
@@ -69,9 +69,7 @@ export function fieldDescriptionAdded(
       level: CriticalityLevel.NonBreaking,
     },
     type: ChangeType.FieldDescriptionAdded,
-    message: `Field '${type.name}.${field.name}' has description '${
-      field.description
-    }'`,
+    message: `Field '${type.name}.${field.name}' has description '${field.description}'`,
     path: [type.name, field.name].join('.'),
   };
 }
@@ -128,11 +126,7 @@ export function fieldDeprecationReasonChanged(
       level: CriticalityLevel.NonBreaking,
     },
     type: ChangeType.FieldDeprecationReasonChanged,
-    message: `Deprecation reason on field '${type.name}.${
-      newField.name
-    }' has changed from '${oldField.deprecationReason}' to '${
-      newField.deprecationReason
-    }'`,
+    message: `Deprecation reason on field '${type.name}.${newField.name}' has changed from '${oldField.deprecationReason}' to '${newField.deprecationReason}'`,
     path: [type.name, oldField.name].join('.'),
   };
 }
@@ -146,9 +140,7 @@ export function fieldDeprecationReasonAdded(
       level: CriticalityLevel.NonBreaking,
     },
     type: ChangeType.FieldDeprecationReasonAdded,
-    message: `Field '${type.name}.${field.name}' has deprecation reason '${
-      field.deprecationReason
-    }'`,
+    message: `Field '${type.name}.${field.name}' has deprecation reason '${field.deprecationReason}'`,
     path: [type.name, field.name].join('.'),
   };
 }
@@ -162,9 +154,7 @@ export function fieldDeprecationReasonRemoved(
       level: CriticalityLevel.NonBreaking,
     },
     type: ChangeType.FieldDeprecationReasonRemoved,
-    message: `Deprecation reason was removed from field '${type.name}.${
-      field.name
-    }'`,
+    message: `Deprecation reason was removed from field '${type.name}.${field.name}'`,
     path: [type.name, field.name].join('.'),
   };
 }
@@ -181,9 +171,7 @@ export function fieldTypeChanged(
         : CriticalityLevel.Breaking,
     },
     type: ChangeType.FieldTypeChanged,
-    message: `Field '${type}.${oldField.name}' changed type from '${
-      oldField.type
-    }' to '${newField.type}'`,
+    message: `Field '${type}.${oldField.name}' changed type from '${oldField.type}' to '${newField.type}'`,
     path: [type.name, oldField.name].join('.'),
   };
 }
@@ -203,9 +191,7 @@ export function fieldArgumentAdded(
           level: CriticalityLevel.Dangerous,
         },
     type: ChangeType.FieldArgumentAdded,
-    message: `Argument '${arg.name}: ${arg.type}' added to field '${
-      type.name
-    }.${field.name}'`,
+    message: `Argument '${arg.name}: ${arg.type}' added to field '${type.name}.${field.name}'`,
     path: [type.name, field.name, arg.name].join('.'),
   };
 }
@@ -221,9 +207,7 @@ export function fieldArgumentRemoved(
       reason: `Removing a field argument is a breaking change because it will cause existing queries that use this argument to error.`,
     },
     type: ChangeType.FieldArgumentRemoved,
-    message: `Argument '${arg.name}: ${arg.type}' was removed from field '${
-      type.name
-    }.${field.name}'`,
+    message: `Argument '${arg.name}: ${arg.type}' was removed from field '${type.name}.${field.name}'`,
     path: [type.name, field.name, arg.name].join('.'),
   };
 }
diff --git a/packages/core/src/diff/common/path.ts b/packages/core/src/diff/common/path.ts
new file mode 100644
index 00000000..53560e07
--- /dev/null
+++ b/packages/core/src/diff/common/path.ts
@@ -0,0 +1,3 @@
+export function parsePath(path: string): string[] {
+  return path.split('.');
+}
diff --git a/packages/core/src/diff/index.ts b/packages/core/src/diff/index.ts
new file mode 100644
index 00000000..b56c1b0e
--- /dev/null
+++ b/packages/core/src/diff/index.ts
@@ -0,0 +1,26 @@
+import {GraphQLSchema} from 'graphql';
+
+import {diffSchema} from './schema';
+import {Change} from './changes/change';
+import {Rule} from './rules/types';
+import * as rules from './rules';
+
+export const DiffRule = rules;
+
+export function diff(
+  oldSchema: GraphQLSchema,
+  newSchema: GraphQLSchema,
+  rules: Rule[] = [],
+): Change[] {
+  const changes = diffSchema(oldSchema, newSchema);
+
+  return rules.reduce(
+    (prev, rule) =>
+      rule({
+        changes: prev,
+        oldSchema,
+        newSchema,
+      }),
+    changes,
+  );
+}
diff --git a/packages/core/src/diff/rules/index.ts b/packages/core/src/diff/rules/index.ts
new file mode 100644
index 00000000..0b13df95
--- /dev/null
+++ b/packages/core/src/diff/rules/index.ts
@@ -0,0 +1,1 @@
+export * from './suppress-removal-of-deprecated-field';
diff --git a/packages/core/src/diff/rules/suppress-removal-of-deprecated-field.ts b/packages/core/src/diff/rules/suppress-removal-of-deprecated-field.ts
new file mode 100644
index 00000000..a6ebe89d
--- /dev/null
+++ b/packages/core/src/diff/rules/suppress-removal-of-deprecated-field.ts
@@ -0,0 +1,37 @@
+import {isObjectType, isInterfaceType} from 'graphql';
+
+import {CriticalityLevel, ChangeType} from './../changes/change';
+import {Rule} from './types';
+import {parsePath} from '../common/path';
+
+export const suppressRemovalOfDeprecatedField: Rule = ({
+  changes,
+  oldSchema,
+}) => {
+  return changes.map(change => {
+    if (
+      change.type === ChangeType.FieldRemoved &&
+      change.criticality.level === CriticalityLevel.Breaking &&
+      change.path
+    ) {
+      const [typeName, fieldName] = parsePath(change.path);
+      const type = oldSchema.getType(typeName);
+
+      if (isObjectType(type) || isInterfaceType(type)) {
+        const field = type.getFields()[fieldName];
+
+        if (field.isDeprecated) {
+          return {
+            ...change,
+            criticality: {
+              ...change.criticality,
+              level: CriticalityLevel.Dangerous,
+            },
+          };
+        }
+      }
+    }
+
+    return change;
+  });
+};
diff --git a/packages/core/src/diff/rules/types.ts b/packages/core/src/diff/rules/types.ts
new file mode 100644
index 00000000..4f4b67c7
--- /dev/null
+++ b/packages/core/src/diff/rules/types.ts
@@ -0,0 +1,8 @@
+import {GraphQLSchema} from 'graphql';
+import {Change} from '../changes/change';
+
+export type Rule = (input: {
+  changes: Change[];
+  oldSchema: GraphQLSchema;
+  newSchema: GraphQLSchema;
+}) => Change[];
diff --git a/packages/core/src/diff/schema.ts b/packages/core/src/diff/schema.ts
index 0d1b11ca..cbf786d9 100644
--- a/packages/core/src/diff/schema.ts
+++ b/packages/core/src/diff/schema.ts
@@ -34,7 +34,7 @@ import {changesInObject} from './object';
 import {changesInInterface} from './interface';
 import {changesInDirective} from './directive';
 
-export function diff(
+export function diffSchema(
   oldSchema: GraphQLSchema,
   newSchema: GraphQLSchema,
 ): Change[] {
diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts
index eb8ecaf7..b70e6250 100644
--- a/packages/core/src/index.ts
+++ b/packages/core/src/index.ts
@@ -1,4 +1,4 @@
-export {diff} from './diff/schema';
+export {diff, DiffRule} from './diff';
 export {validate, InvalidDocument} from './validate';
 export {similar, SimilarMap} from './similar';
 export * from './coverage';
diff --git a/packages/core/src/utils/apollo.ts b/packages/core/src/utils/apollo.ts
new file mode 100644
index 00000000..860b2a10
--- /dev/null
+++ b/packages/core/src/utils/apollo.ts
@@ -0,0 +1,22 @@
+import {DocumentNode, visit, parse, GraphQLSchema, extendSchema} from 'graphql';
+
+import {removeFieldIfDirectives} from './graphql';
+
+export function transformDocumentWithApollo(doc: DocumentNode): DocumentNode {
+  return visit(doc, {
+    Field(node) {
+      return removeFieldIfDirectives(node, ['client']);
+    },
+  });
+}
+
+export function transformSchemaWithApollo(
+  schema: GraphQLSchema,
+): GraphQLSchema {
+  return extendSchema(
+    schema,
+    parse(/* GraphQL */ `
+      directive @connection(key: String!, filter: [String]) on FIELD
+    `),
+  );
+}
diff --git a/packages/core/src/utils/graphql.ts b/packages/core/src/utils/graphql.ts
index 2d50ce7b..14c2705c 100644
--- a/packages/core/src/utils/graphql.ts
+++ b/packages/core/src/utils/graphql.ts
@@ -14,6 +14,7 @@ import {
   visit,
   visitWithTypeInfo,
   getNamedType,
+  FieldNode,
 } from 'graphql';
 
 export function safeChangeForField(
@@ -154,3 +155,18 @@ export function findDeprecatedUsages(
 
   return errors;
 }
+
+export function removeFieldIfDirectives(
+  node: FieldNode,
+  directiveNames: string[],
+): FieldNode | null {
+  if (node.directives) {
+    if (
+      node.directives.some(d => directiveNames.indexOf(d.name.value) !== -1)
+    ) {
+      return null;
+    }
+  }
+
+  return node;
+}
diff --git a/packages/core/src/validate/index.ts b/packages/core/src/validate/index.ts
index 9906ba5c..472093d9 100644
--- a/packages/core/src/validate/index.ts
+++ b/packages/core/src/validate/index.ts
@@ -13,6 +13,10 @@ import {DepGraph} from 'dependency-graph';
 import {readDocument} from '../ast/document';
 import {findDeprecatedUsages} from '../utils/graphql';
 import {validateQueryDepth} from './query-depth';
+import {
+  transformSchemaWithApollo,
+  transformDocumentWithApollo,
+} from '../utils/apollo';
 
 export interface InvalidDocument {
   source: Source;
@@ -23,6 +27,7 @@ export interface InvalidDocument {
 export interface ValidateOptions {
   strictFragments?: boolean;
   strictDeprecated?: boolean;
+  apollo?: boolean;
   maxDepth?: number;
 }
 
@@ -34,6 +39,7 @@ export function validate(
   const config: ValidateOptions = {
     strictDeprecated: true,
     strictFragments: true,
+    apollo: false,
     ...options,
   };
   const invalidDocuments: InvalidDocument[] = [];
@@ -89,12 +95,23 @@ export function validate(
         definitions: [...docWithOperations.definitions, ...extractedFragments],
       };
 
-      const errors = (validateDocument(schema, merged) as GraphQLError[]) || [];
+      const transformedSchema = config.apollo
+        ? transformSchemaWithApollo(schema)
+        : schema;
+      const transformedDoc = config.apollo
+        ? transformDocumentWithApollo(merged)
+        : merged;
+
+      const errors =
+        (validateDocument(
+          transformedSchema,
+          transformedDoc,
+        ) as GraphQLError[]) || [];
 
       if (config.maxDepth) {
         const depthError = validateQueryDepth({
           source: doc.source,
-          doc: merged,
+          doc: transformedDoc,
           maxDepth: config.maxDepth,
           fragmentGraph: graph,
         });
@@ -105,7 +122,7 @@ export function validate(
       }
 
       const deprecated = config.strictDeprecated
-        ? findDeprecatedUsages(schema, parse(doc.source.body))
+        ? findDeprecatedUsages(transformedSchema, parse(doc.source.body))
         : [];
       const duplicatedFragments = config.strictFragments
         ? findDuplicatedFragments(fragmentNames)


Test patch:
diff --git a/packages/core/__tests__/diff/object.ts b/packages/core/__tests__/diff/object.ts
index 0ce548dc..6f3215a4 100644
--- a/packages/core/__tests__/diff/object.ts
+++ b/packages/core/__tests__/diff/object.ts
@@ -1,6 +1,6 @@
 import {buildSchema} from 'graphql';
 
-import {diff, CriticalityLevel} from '../../src/index';
+import {diff, DiffRule, CriticalityLevel} from '../../src/index';
 import {findFirstChangeByPath, findChangesByPath} from '../../utils/testing';
 
 describe('object', () => {
@@ -325,6 +325,43 @@ describe('object', () => {
       expect(change.b.type).toEqual('FIELD_DEPRECATION_ADDED');
       expect(change.b.message).toEqual("Field 'Foo.b' is deprecated");
     });
+
+    test('removal of a deprecated field', () => {
+      const a = buildSchema(/* GraphQL */ `
+        type Foo {
+          a: String!
+          b: String! @deprecated
+        }
+      `);
+      const b = buildSchema(/* GraphQL */ `
+        type Foo {
+          a: String!
+        }
+      `);
+
+      const change = findFirstChangeByPath(diff(a, b), 'Foo.b');
+
+      expect(change.criticality.level).toEqual(CriticalityLevel.Breaking);
+      expect(change.type).toEqual('FIELD_REMOVED');
+      expect(change.message).toEqual(
+        "Field 'b' (deprecated) was removed from object type 'Foo'",
+      );
+
+      // suppressRemovalOfDeprecatedField rule should make it only Dangerous
+
+      const changeWithRule = findFirstChangeByPath(
+        diff(a, b, [DiffRule.suppressRemovalOfDeprecatedField]),
+        'Foo.b',
+      );
+
+      expect(changeWithRule.criticality.level).toEqual(
+        CriticalityLevel.Dangerous,
+      );
+      expect(changeWithRule.type).toEqual('FIELD_REMOVED');
+      expect(changeWithRule.message).toEqual(
+        "Field 'b' (deprecated) was removed from object type 'Foo'",
+      );
+    });
   });
 
   describe('arguments', () => {
diff --git a/packages/core/__tests__/validate/apollo.ts b/packages/core/__tests__/validate/apollo.ts
new file mode 100644
index 00000000..709bf236
--- /dev/null
+++ b/packages/core/__tests__/validate/apollo.ts
@@ -0,0 +1,64 @@
+import {buildSchema, Source, print, parse} from 'graphql';
+
+import {validate} from '../../src/validate';
+
+describe('apollo', () => {
+  test('should remove a filed with @client', () => {
+    const schema = buildSchema(/* GraphQL */ `
+      type Post {
+        id: ID
+        title: String
+      }
+
+      type Query {
+        post: Post
+      }
+    `);
+
+    const doc = parse(/* GraphQL */ `
+      query getPost {
+        post {
+          id
+          title
+          extra @client
+        }
+      }
+    `);
+
+    const results = validate(schema, [new Source(print(doc))], {
+      apollo: true,
+    });
+
+    expect(results).toHaveLength(0);
+  });
+
+  test('should include @connection', () => {
+    const schema = buildSchema(/* GraphQL */ `
+      type Post {
+        id: ID
+        title: String
+        comments: [String]
+      }
+
+      type Query {
+        post(id: ID!): Post
+      }
+    `);
+
+    const doc = parse(/* GraphQL */ `
+      query getPost {
+        post(id: 1) {
+          id
+          title
+          comments @connection(key: "comments")
+        }
+      }
+    `);
+
+    const results = validate(schema, [new Source(print(doc))], {
+      apollo: true,
+    });
+
+    expect(results).toHaveLength(0);
+  });
+});