From f2a6635593af505d6937299a3743ed9103514115 Mon Sep 17 00:00:00 2001 From: Dimitri POSTOLOV Date: Fri, 24 Sep 2021 08:43:07 +0200 Subject: [PATCH] =?UTF-8?q?2=EF=B8=8F=E2=83=A3=20=F0=9F=8E=89=20feat(new?= =?UTF-8?q?=20rule):=20`require-field-of-type-query-in-mutation-result`=20?= =?UTF-8?q?(#643)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .changeset/chilly-seals-explain.md | 5 + docs/README.md | 3 +- docs/rules/require-deprecation-date.md | 2 +- ...-field-of-type-query-in-mutation-result.md | 42 +++++++ packages/plugin/src/configs/all.ts | 1 + .../avoid-scalar-result-type-on-mutation.ts | 2 +- packages/plugin/src/rules/index.ts | 2 + .../src/rules/require-deprecation-date.ts | 10 +- ...-field-of-type-query-in-mutation-result.ts | 79 ++++++++++++ ...oid-scalar-result-type-on-mutation.spec.ts | 12 +- .../tests/require-deprecation-date.spec.ts | 8 +- ...d-of-type-query-in-mutation-result.spec.ts | 113 ++++++++++++++++++ 12 files changed, 261 insertions(+), 18 deletions(-) create mode 100644 .changeset/chilly-seals-explain.md create mode 100644 docs/rules/require-field-of-type-query-in-mutation-result.md create mode 100644 packages/plugin/src/rules/require-field-of-type-query-in-mutation-result.ts create mode 100644 packages/plugin/tests/require-field-of-type-query-in-mutation-result.spec.ts diff --git a/.changeset/chilly-seals-explain.md b/.changeset/chilly-seals-explain.md new file mode 100644 index 00000000000..592e7f8eca8 --- /dev/null +++ b/.changeset/chilly-seals-explain.md @@ -0,0 +1,5 @@ +--- +'@graphql-eslint/eslint-plugin': minor +--- + +add new rule `require-field-of-type-query-in-mutation-result` diff --git a/docs/README.md b/docs/README.md index d07bfc87f23..73513a1af51 100644 --- a/docs/README.md +++ b/docs/README.md @@ -44,9 +44,10 @@ Name            &nbs [possible-fragment-spread](rules/possible-fragment-spread.md)|A fragment spread is only valid if the type condition could ever possibly be true: if there is a non-empty intersection of the possible parent types, and possible types which pass the type condition.|๐Ÿ”ฎ||โœ… [possible-type-extension](rules/possible-type-extension.md)|A type extension is only valid if the type is defined and has the same kind.|๐Ÿ”ฎ||โœ… [provided-required-arguments](rules/provided-required-arguments.md)|A field or directive is only valid if all required (non-null without a default value) field arguments have been provided.|๐Ÿ”ฎ||โœ… -[require-deprecation-date](rules/require-deprecation-date.md)|Require deletion date on @deprecated directive. Suggest to remove deprecated things after deprecated date.|๐Ÿš€|| +[require-deprecation-date](rules/require-deprecation-date.md)|Require deletion date on `@deprecated` directive. Suggest removing deprecated things after deprecated date.|๐Ÿš€|| [require-deprecation-reason](rules/require-deprecation-reason.md)|Require all deprecation directives to specify a reason.|๐Ÿš€||โœ… [require-description](rules/require-description.md)|Enforce descriptions in your type definitions.|๐Ÿš€|| +[require-field-of-type-query-in-mutation-result](rules/require-field-of-type-query-in-mutation-result.md)|Allow the client in one round-trip not only to call mutation but also to get a wagon of data to update their application.|๐Ÿš€|| [require-id-when-available](rules/require-id-when-available.md)|Enforce selecting specific fields when they are available on the GraphQL type.|๐Ÿš€|| [scalar-leafs](rules/scalar-leafs.md)|A GraphQL document is valid only if all leaf fields (fields without sub selections) are of scalar or enum types.|๐Ÿ”ฎ||โœ… [selection-set-depth](rules/selection-set-depth.md)|Limit the complexity of the GraphQL operations solely by their depth. Based on [graphql-depth-limit](https://github.com/stems/graphql-depth-limit).|๐Ÿš€|| diff --git a/docs/rules/require-deprecation-date.md b/docs/rules/require-deprecation-date.md index 8749ac0150b..0f58c74633c 100644 --- a/docs/rules/require-deprecation-date.md +++ b/docs/rules/require-deprecation-date.md @@ -5,7 +5,7 @@ - Requires GraphQL Schema: `false` [โ„น๏ธ](../../README.md#extended-linting-rules-with-graphql-schema) - Requires GraphQL Operations: `false` [โ„น๏ธ](../../README.md#extended-linting-rules-with-siblings-operations) -Require deletion date on @deprecated directive. Suggest to remove deprecated things after deprecated date. +Require deletion date on `@deprecated` directive. Suggest removing deprecated things after deprecated date. ## Usage Examples diff --git a/docs/rules/require-field-of-type-query-in-mutation-result.md b/docs/rules/require-field-of-type-query-in-mutation-result.md new file mode 100644 index 00000000000..dc0800ec3c9 --- /dev/null +++ b/docs/rules/require-field-of-type-query-in-mutation-result.md @@ -0,0 +1,42 @@ +# `require-field-of-type-query-in-mutation-result` + +- Category: `Best Practices` +- Rule name: `@graphql-eslint/require-field-of-type-query-in-mutation-result` +- Requires GraphQL Schema: `true` [โ„น๏ธ](../../README.md#extended-linting-rules-with-graphql-schema) +- Requires GraphQL Operations: `false` [โ„น๏ธ](../../README.md#extended-linting-rules-with-siblings-operations) + +Allow the client in one round-trip not only to call mutation but also to get a wagon of data to update their application. +> Currently, no errors are reported for result type `union`, `interface` and `scalar`. + +## Usage Examples + +### Incorrect + +```graphql +# eslint @graphql-eslint/require-field-of-type-query-in-mutation-result: 'error' + +type User { ... } + +type Mutation { + createUser: User! +} +``` + +### Correct + +```graphql +# eslint @graphql-eslint/require-field-of-type-query-in-mutation-result: 'error' + +type User { ... } + +type Query { ... } + +type CreateUserPayload { + user: User! + query: Query! +} + +type Mutation { + createUser: CreateUserPayload! +} +``` \ No newline at end of file diff --git a/packages/plugin/src/configs/all.ts b/packages/plugin/src/configs/all.ts index ea532580947..8333bfdee87 100644 --- a/packages/plugin/src/configs/all.ts +++ b/packages/plugin/src/configs/all.ts @@ -20,6 +20,7 @@ export const allConfig = { '@graphql-eslint/no-unused-fields': 'error', '@graphql-eslint/require-deprecation-date': 'error', '@graphql-eslint/require-description': 'error', + '@graphql-eslint/require-field-of-type-query-in-mutation-result': 'error', '@graphql-eslint/require-id-when-available': 'error', '@graphql-eslint/selection-set-depth': 'error', '@graphql-eslint/unique-fragment-name': 'error', diff --git a/packages/plugin/src/rules/avoid-scalar-result-type-on-mutation.ts b/packages/plugin/src/rules/avoid-scalar-result-type-on-mutation.ts index c4603893dba..977d1ec71c3 100644 --- a/packages/plugin/src/rules/avoid-scalar-result-type-on-mutation.ts +++ b/packages/plugin/src/rules/avoid-scalar-result-type-on-mutation.ts @@ -47,7 +47,7 @@ const rule: GraphQLESLintRule = { if (isScalarType(graphQLType)) { context.report({ node, - message: `Unexpected scalar result type "${typeName}"`, + message: `Unexpected scalar result type "${typeName}".`, }); } }, diff --git a/packages/plugin/src/rules/index.ts b/packages/plugin/src/rules/index.ts index 95a5993a1f4..21167b1c236 100644 --- a/packages/plugin/src/rules/index.ts +++ b/packages/plugin/src/rules/index.ts @@ -21,6 +21,7 @@ import noUnusedFields from './no-unused-fields'; import requireDeprecationDate from './require-deprecation-date'; import requireDeprecationReason from './require-deprecation-reason'; import requireDescription from './require-description'; +import requireFieldOfTypeQueryInMutationResult from './require-field-of-type-query-in-mutation-result'; import requireIdWhenAvailable from './require-id-when-available'; import selectionSetDepth from './selection-set-depth'; import strictIdInTypes from './strict-id-in-types'; @@ -47,6 +48,7 @@ export const rules = { 'require-deprecation-date': requireDeprecationDate, 'require-deprecation-reason': requireDeprecationReason, 'require-description': requireDescription, + 'require-field-of-type-query-in-mutation-result': requireFieldOfTypeQueryInMutationResult, 'require-id-when-available': requireIdWhenAvailable, 'selection-set-depth': selectionSetDepth, 'strict-id-in-types': strictIdInTypes, diff --git a/packages/plugin/src/rules/require-deprecation-date.ts b/packages/plugin/src/rules/require-deprecation-date.ts index 8385a4095cf..eb1ed3b26fc 100644 --- a/packages/plugin/src/rules/require-deprecation-date.ts +++ b/packages/plugin/src/rules/require-deprecation-date.ts @@ -14,7 +14,7 @@ const rule: GraphQLESLintRule<[{ argumentName?: string }]> = { docs: { category: 'Best Practices', description: - 'Require deletion date on @deprecated directive. Suggest to remove deprecated things after deprecated date.', + 'Require deletion date on `@deprecated` directive. Suggest removing deprecated things after deprecated date.', url: 'https://github.com/dotansimha/graphql-eslint/blob/master/docs/rules/require-deprecation-date.md', examples: [ { @@ -47,10 +47,10 @@ const rule: GraphQLESLintRule<[{ argumentName?: string }]> = { ], }, messages: { - [MESSAGE_REQUIRE_DATE]: 'Directive "@deprecated" must have a deletion date', - [MESSAGE_INVALID_FORMAT]: 'Deletion date must be in format "DD/MM/YYYY"', - [MESSAGE_INVALID_DATE]: 'Invalid "{{ deletionDate }}" deletion date', - [MESSAGE_CAN_BE_REMOVED]: '"{{ nodeName }}" ัan be removed', + [MESSAGE_REQUIRE_DATE]: 'Directive "@deprecated" must have a deletion date.', + [MESSAGE_INVALID_FORMAT]: 'Deletion date must be in format "DD/MM/YYYY".', + [MESSAGE_INVALID_DATE]: 'Invalid "{{ deletionDate }}" deletion date.', + [MESSAGE_CAN_BE_REMOVED]: '"{{ nodeName }}" ัan be removed.', }, schema: [ { diff --git a/packages/plugin/src/rules/require-field-of-type-query-in-mutation-result.ts b/packages/plugin/src/rules/require-field-of-type-query-in-mutation-result.ts new file mode 100644 index 00000000000..ef20716c099 --- /dev/null +++ b/packages/plugin/src/rules/require-field-of-type-query-in-mutation-result.ts @@ -0,0 +1,79 @@ +import { Kind, FieldDefinitionNode, isObjectType } from 'graphql'; +import { requireGraphQLSchemaFromContext, getTypeName } from '../utils'; +import { GraphQLESLintRule } from '../types'; +import { GraphQLESTreeNode } from '../estree-parser'; + +const RULE_NAME = 'require-field-of-type-query-in-mutation-result'; + +const rule: GraphQLESLintRule = { + meta: { + type: 'suggestion', + docs: { + category: 'Best Practices', + description: + 'Allow the client in one round-trip not only to call mutation but also to get a wagon of data to update their application.\n> Currently, no errors are reported for result type `union`, `interface` and `scalar`.', + url: `https://github.com/dotansimha/graphql-eslint/blob/master/docs/rules/${RULE_NAME}.md`, + requiresSchema: true, + examples: [ + { + title: 'Incorrect', + code: /* GraphQL */ ` + type User { ... } + + type Mutation { + createUser: User! + } + `, + }, + { + title: 'Correct', + code: /* GraphQL */ ` + type User { ... } + + type Query { ... } + + type CreateUserPayload { + user: User! + query: Query! + } + + type Mutation { + createUser: CreateUserPayload! + } + `, + }, + ], + }, + }, + create(context) { + const schema = requireGraphQLSchemaFromContext(RULE_NAME, context); + const mutationType = schema.getMutationType(); + const queryType = schema.getQueryType(); + + if (!mutationType || !queryType) { + return {}; + } + const selector = `:matches(${Kind.OBJECT_TYPE_DEFINITION}, ${Kind.OBJECT_TYPE_EXTENSION})[name.value=${mutationType.name}] > ${Kind.FIELD_DEFINITION}`; + + return { + [selector](node: GraphQLESTreeNode) { + const rawNode = node.rawNode(); + const typeName = getTypeName(rawNode); + const graphQLType = schema.getType(typeName); + + if (isObjectType(graphQLType)) { + const { fields } = graphQLType.astNode; + const hasQueryType = fields.some(field => getTypeName(field) === queryType.name); + if (!hasQueryType) { + context.report({ + node, + message: `Mutation result type "${graphQLType.name}" must contain field of type "${queryType.name}".`, + }); + } + } + }, + }; + }, +}; + +export default rule; diff --git a/packages/plugin/tests/avoid-scalar-result-type-on-mutation.spec.ts b/packages/plugin/tests/avoid-scalar-result-type-on-mutation.spec.ts index 1681087552c..bd8a06b4d1a 100644 --- a/packages/plugin/tests/avoid-scalar-result-type-on-mutation.spec.ts +++ b/packages/plugin/tests/avoid-scalar-result-type-on-mutation.spec.ts @@ -45,7 +45,7 @@ ruleTester.runGraphQLTests('avoid-scalar-result-type-on-mutation', rule, { createUser: Boolean } `), - errors: [{ message: 'Unexpected scalar result type "Boolean"' }], + errors: [{ message: 'Unexpected scalar result type "Boolean".' }], }, { ...useSchema(/* GraphQL */ ` @@ -55,7 +55,7 @@ ruleTester.runGraphQLTests('avoid-scalar-result-type-on-mutation', rule, { createUser: Boolean } `), - errors: [{ message: 'Unexpected scalar result type "Boolean"' }], + errors: [{ message: 'Unexpected scalar result type "Boolean".' }], }, { ...useSchema(/* GraphQL */ ` @@ -67,7 +67,7 @@ ruleTester.runGraphQLTests('avoid-scalar-result-type-on-mutation', rule, { mutation: RootMutation } `), - errors: [{ message: 'Unexpected scalar result type "Boolean"' }], + errors: [{ message: 'Unexpected scalar result type "Boolean".' }], }, { ...useSchema(/* GraphQL */ ` @@ -80,7 +80,7 @@ ruleTester.runGraphQLTests('avoid-scalar-result-type-on-mutation', rule, { mutation: RootMutation } `), - errors: [{ message: 'Unexpected scalar result type "Boolean"' }], + errors: [{ message: 'Unexpected scalar result type "Boolean".' }], }, { ...useSchema(/* GraphQL */ ` @@ -91,8 +91,8 @@ ruleTester.runGraphQLTests('avoid-scalar-result-type-on-mutation', rule, { } `), errors: [ - { message: 'Unexpected scalar result type "Int"' }, - { message: 'Unexpected scalar result type "Boolean"' }, + { message: 'Unexpected scalar result type "Int".' }, + { message: 'Unexpected scalar result type "Boolean".' }, ], }, ], diff --git a/packages/plugin/tests/require-deprecation-date.spec.ts b/packages/plugin/tests/require-deprecation-date.spec.ts index 448f41488f5..e93b3f197ea 100644 --- a/packages/plugin/tests/require-deprecation-date.spec.ts +++ b/packages/plugin/tests/require-deprecation-date.spec.ts @@ -30,20 +30,20 @@ ruleTester.runGraphQLTests('require-deprecation-date', rule, { invalid: [ { code: 'scalar Old @deprecated(deletionDate: "22/08/2021")', - errors: [{ message: '"Old" ัan be removed' }], + errors: [{ message: '"Old" ัan be removed.' }], }, { code: 'scalar Old @deprecated(untilDate: "22/08/2021")', options: [{ argumentName: 'untilDate' }], - errors: [{ message: '"Old" ัan be removed' }], + errors: [{ message: '"Old" ัan be removed.' }], }, { code: 'scalar Old @deprecated(deletionDate: "bad")', - errors: [{ message: 'Deletion date must be in format "DD/MM/YYYY"' }], + errors: [{ message: 'Deletion date must be in format "DD/MM/YYYY".' }], }, { code: 'scalar Old @deprecated(deletionDate: "32/08/2021")', - errors: [{ message: 'Invalid "32/08/2021" deletion date' }], + errors: [{ message: 'Invalid "32/08/2021" deletion date.' }], }, ], }); diff --git a/packages/plugin/tests/require-field-of-type-query-in-mutation-result.spec.ts b/packages/plugin/tests/require-field-of-type-query-in-mutation-result.spec.ts new file mode 100644 index 00000000000..9f7025bb22f --- /dev/null +++ b/packages/plugin/tests/require-field-of-type-query-in-mutation-result.spec.ts @@ -0,0 +1,113 @@ +import { GraphQLRuleTester, ParserOptions } from '../src'; +import rule from '../src/rules/require-field-of-type-query-in-mutation-result'; + +const useSchema = (code: string): { code: string; parserOptions: ParserOptions } => ({ + code, + parserOptions: { + schema: /* GraphQL */ ` + type User { + id: ID! + } + + ${code} + `, + }, +}); + +const ruleTester = new GraphQLRuleTester(); + +ruleTester.runGraphQLTests('require-field-of-type-query-in-mutation-result', rule, { + valid: [ + useSchema(/* GraphQL */ ` + type Query { + user: User + } + `), + useSchema(/* GraphQL */ ` + # type Query is not defined and no error is reported + type Mutation { + createUser: User! + } + `), + useSchema(/* GraphQL */ ` + type Query + type CreateUserPayload { + user: User! + query: Query! + } + + type Mutation { + createUser: CreateUserPayload! + } + `), + useSchema(/* GraphQL */ ` + # No errors are reported for type union, interface and scalar + type Admin { + id: ID! + } + union Union = User | Admin + + interface Interface { + id: ID! + } + + type Query + type Mutation { + foo: Boolean + bar: Union + baz: Interface + } + `), + ], + invalid: [ + { + ...useSchema(/* GraphQL */ ` + type Query + type Mutation { + createUser: User! + } + `), + errors: [{ message: 'Mutation result type "User" must contain field of type "Query".' }], + }, + { + ...useSchema(/* GraphQL */ ` + type Query + type Mutation + + extend type Mutation { + createUser: User! + } + `), + errors: [{ message: 'Mutation result type "User" must contain field of type "Query".' }], + }, + { + ...useSchema(/* GraphQL */ ` + type RootQuery + type RootMutation { + createUser: User! + } + + schema { + mutation: RootMutation + query: RootQuery + } + `), + errors: [{ message: 'Mutation result type "User" must contain field of type "RootQuery".' }], + }, + { + ...useSchema(/* GraphQL */ ` + type RootQuery + type RootMutation + extend type RootMutation { + createUser: User! + } + + schema { + mutation: RootMutation + query: RootQuery + } + `), + errors: [{ message: 'Mutation result type "User" must contain field of type "RootQuery".' }], + }, + ], +});