From 2a7d08bb6b7ebbdcc1a18c66936bf09907fb7dc4 Mon Sep 17 00:00:00 2001 From: Dotan Simha Date: Wed, 23 Dec 2020 10:17:29 +0200 Subject: [PATCH 1/2] NEW RULE: avoid-duplicate-fields --- .changeset/clean-apricots-attend.md | 5 + docs/README.md | 3 +- docs/rules/avoid-duplicate-fields.md | 54 +++++++ .../src/rules/avoid-duplicate-fields.ts | 133 ++++++++++++++++++ packages/plugin/src/rules/index.ts | 4 +- .../tests/avoid-duplicate-fields.spec.ts | 41 ++++++ 6 files changed, 238 insertions(+), 2 deletions(-) create mode 100644 .changeset/clean-apricots-attend.md create mode 100644 docs/rules/avoid-duplicate-fields.md create mode 100644 packages/plugin/src/rules/avoid-duplicate-fields.ts create mode 100644 packages/plugin/tests/avoid-duplicate-fields.spec.ts diff --git a/.changeset/clean-apricots-attend.md b/.changeset/clean-apricots-attend.md new file mode 100644 index 00000000000..161d8b290a1 --- /dev/null +++ b/.changeset/clean-apricots-attend.md @@ -0,0 +1,5 @@ +--- +'@graphql-eslint/eslint-plugin': minor +--- + +NEW RULE: avoid-duplicate-fields diff --git a/docs/README.md b/docs/README.md index 6fd4388a7cd..d0d5d63185c 100644 --- a/docs/README.md +++ b/docs/README.md @@ -15,9 +15,10 @@ - [`require-description`](./rules/require-description.md) - [`require-id-when-available`](./rules/require-id-when-available.md) - [`description-style`](./rules/description-style.md) -- [`prettier`](./rules/prettier.md) +- [`avoid-duplicate-fields`](./rules/avoid-duplicate-fields.md) - [`naming-convention`](./rules/naming-convention.md) - [`input-name`](./rules/input-name.md) +- [`prettier`](./rules/prettier.md) - [`executable-definitions`](./rules/executable-definitions.md) - [`fields-on-correct-type`](./rules/fields-on-correct-type.md) - [`fragments-on-composite-type`](./rules/fragments-on-composite-type.md) diff --git a/docs/rules/avoid-duplicate-fields.md b/docs/rules/avoid-duplicate-fields.md new file mode 100644 index 00000000000..82acd47dcfd --- /dev/null +++ b/docs/rules/avoid-duplicate-fields.md @@ -0,0 +1,54 @@ +# `avoid-duplicate-fields` + +- Category: `Stylistic Issues` +- Rule name: `@graphql-eslint/avoid-duplicate-fields` +- Requires GraphQL Schema: `false` [ℹ️](../../README.md#extended-linting-rules-with-graphql-schema) +- Requires GraphQL Operations: `false` [ℹ️](../../README.md#extended-linting-rules-with-siblings-operations) + +Checks for duplicate fields in selection set, variables in operation definition, or in arguments set of a field. + +## Usage Examples + +### Incorrect + +```graphql +# eslint @graphql-eslint/avoid-duplicate-fields: ["error"] + +query getUserDetails { + user { + name # first + email + name # second + } +} +``` + +### Incorrect + +```graphql +# eslint @graphql-eslint/avoid-duplicate-fields: ["error"] + +query getUsers { + users( + first: 100 + skip: 50 + after: "cji629tngfgou0b73kt7vi5jo" + first: 100 # duplicate argument + ) { + id + } +} +``` + +### Incorrect + +```graphql +# eslint @graphql-eslint/avoid-duplicate-fields: ["error"] + +query getUsers($first: Int!, $first: Int!) { + # Duplicate variable + users(first: 100, skip: 50, after: "cji629tngfgou0b73kt7vi5jo") { + id + } +} +``` \ No newline at end of file diff --git a/packages/plugin/src/rules/avoid-duplicate-fields.ts b/packages/plugin/src/rules/avoid-duplicate-fields.ts new file mode 100644 index 00000000000..561ff5e708c --- /dev/null +++ b/packages/plugin/src/rules/avoid-duplicate-fields.ts @@ -0,0 +1,133 @@ +import { Kind } from 'graphql'; +import { GraphQLESLintRule } from '../types'; + +const AVOID_DUPLICATE_FIELDS = 'AVOID_DUPLICATE_FIELDS'; + +const ensureUnique = () => { + const set = new Set(); + + return { + add: (item: string, onError: () => void) => { + if (set.has(item)) { + onError(); + } else { + set.add(item); + } + }, + }; +}; + +const rule: GraphQLESLintRule<[], false> = { + meta: { + type: 'suggestion', + docs: { + requiresSchema: false, + requiresSiblings: false, + description: + 'Checks for duplicate fields in selection set, variables in operation definition, or in arguments set of a field.', + category: 'Stylistic Issues', + url: 'https://github.com/dotansimha/graphql-eslint/blob/master/docs/rules/avoid-duplicate-fields.md', + examples: [ + { + title: 'Incorrect', + code: /* GraphQL */ ` + query getUserDetails { + user { + name # first + email + name # second + } + } + `, + }, + { + title: 'Incorrect', + code: /* GraphQL */ ` + query getUsers { + users( + first: 100 + skip: 50 + after: "cji629tngfgou0b73kt7vi5jo" + first: 100 # duplicate argument + ) { + id + } + } + `, + }, + { + title: 'Incorrect', + code: /* GraphQL */ ` + query getUsers($first: Int!, $first: Int!) { + # Duplicate variable + users(first: 100, skip: 50, after: "cji629tngfgou0b73kt7vi5jo") { + id + } + } + `, + }, + ], + }, + messages: { + [AVOID_DUPLICATE_FIELDS]: `{{ type }} "{{ fieldName }}" defined multiple times.`, + }, + }, + create(context) { + return { + OperationDefinition(node) { + const uniqueCheck = ensureUnique(); + + for (const arg of node.variableDefinitions || []) { + uniqueCheck.add(arg.variable.name.value, () => { + context.report({ + messageId: AVOID_DUPLICATE_FIELDS, + data: { + type: 'Operation variable', + fieldName: arg.variable.name.value, + }, + node: arg, + }); + }); + } + }, + Field(node) { + const uniqueCheck = ensureUnique(); + + for (const arg of node.arguments || []) { + uniqueCheck.add(arg.name.value, () => { + context.report({ + messageId: AVOID_DUPLICATE_FIELDS, + data: { + type: 'Field argument', + fieldName: arg.name.value, + }, + node: arg, + }); + }); + } + }, + SelectionSet(node) { + const uniqueCheck = ensureUnique(); + + for (const selection of node.selections || []) { + if (selection.kind === Kind.FIELD) { + const nameToCheck = selection.alias?.value || selection.name.value; + + uniqueCheck.add(nameToCheck, () => { + context.report({ + messageId: AVOID_DUPLICATE_FIELDS, + data: { + type: 'Field', + fieldName: nameToCheck, + }, + node: selection, + }); + }); + } + } + }, + }; + }, +}; + +export default rule; diff --git a/packages/plugin/src/rules/index.ts b/packages/plugin/src/rules/index.ts index 6b9a0b8ebd1..96abba08a6c 100644 --- a/packages/plugin/src/rules/index.ts +++ b/packages/plugin/src/rules/index.ts @@ -15,6 +15,7 @@ import uniqueOperationName from './unique-operation-name'; import noDeprecated from './no-deprecated'; import noHashtagDescription from './no-hashtag-description'; import selectionSetDepth from './selection-set-depth'; +import avoidDuplicateFields from './avoid-duplicate-fields'; import { GraphQLESLintRule } from '../types'; import { GRAPHQL_JS_VALIDATIONS } from './graphql-js-validation'; @@ -33,8 +34,9 @@ export const rules: Record = { 'require-description': requireDescription, 'require-id-when-available': requireIdWhenAvailable, 'description-style': descriptionStyle, - prettier: prettier, + 'avoid-duplicate-fields': avoidDuplicateFields, 'naming-convention': namingConvention, 'input-name': inputName, + prettier, ...GRAPHQL_JS_VALIDATIONS, }; diff --git a/packages/plugin/tests/avoid-duplicate-fields.spec.ts b/packages/plugin/tests/avoid-duplicate-fields.spec.ts new file mode 100644 index 00000000000..d1ba3f2ff3a --- /dev/null +++ b/packages/plugin/tests/avoid-duplicate-fields.spec.ts @@ -0,0 +1,41 @@ +import { GraphQLRuleTester } from '../src/testkit'; +import rule from '../src/rules/avoid-duplicate-fields'; + +const ruleTester = new GraphQLRuleTester(); + +ruleTester.runGraphQLTests('avoid-duplicate-fields', rule, { + valid: [], + invalid: [ + { + code: /* GraphQL */ ` + query test($v: String, $t: String, $v: String) { + id + } + `, + errors: [{ message: 'Operation variable "v" defined multiple times.' }], + }, + { + code: /* GraphQL */ ` + query test { + users(first: 100, after: 10, filter: "test", first: 50) { + id + } + } + `, + errors: [{ message: 'Field argument "first" defined multiple times.' }], + }, + { + code: /* GraphQL */ ` + query test { + users { + id + name + email + name + } + } + `, + errors: [{ message: 'Field "name" defined multiple times.' }], + }, + ], +}); From e187bfcf2a4a1d97251113b4ea9bd8cdc6559a53 Mon Sep 17 00:00:00 2001 From: Dotan Simha Date: Wed, 23 Dec 2020 10:19:07 +0200 Subject: [PATCH 2/2] added more tests --- .../plugin/tests/avoid-duplicate-fields.spec.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/packages/plugin/tests/avoid-duplicate-fields.spec.ts b/packages/plugin/tests/avoid-duplicate-fields.spec.ts index d1ba3f2ff3a..6f09eb48827 100644 --- a/packages/plugin/tests/avoid-duplicate-fields.spec.ts +++ b/packages/plugin/tests/avoid-duplicate-fields.spec.ts @@ -37,5 +37,18 @@ ruleTester.runGraphQLTests('avoid-duplicate-fields', rule, { `, errors: [{ message: 'Field "name" defined multiple times.' }], }, + { + code: /* GraphQL */ ` + query test { + users { + id + name + email + email: somethingElse + } + } + `, + errors: [{ message: 'Field "email" defined multiple times.' }], + }, ], });