From f846fae54f937f9be52e9deb5dfff63f2cc80190 Mon Sep 17 00:00:00 2001 From: Dotan Simha Date: Mon, 5 Oct 2020 16:20:57 +0300 Subject: [PATCH 1/5] improve reported error from validate-against-schema ignore NoUnusedFragmentsRule when it's not set --- .../src/rules/validate-against-schema.ts | 18 ++++++++++++++---- .../tests/validate-against-schema.spec.ts | 18 +++++++++++------- 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/packages/plugin/src/rules/validate-against-schema.ts b/packages/plugin/src/rules/validate-against-schema.ts index ab30f9b05d2..955211e3346 100644 --- a/packages/plugin/src/rules/validate-against-schema.ts +++ b/packages/plugin/src/rules/validate-against-schema.ts @@ -3,6 +3,16 @@ import { GraphQLESTreeNode } from '../estree-parser'; import { GraphQLESLintRule, GraphQLESlintRuleContext } from '../types'; import { requireGraphQLSchemaFromContext } from '../utils'; +function extractRuleName(stack: string | undefined): string | null { + const match = (stack || '').match(/validation\/rules\/(.*?)\.js\:/); + + if (!match) { + return null; + } + + return match[1] || null; +} + function validateDoc( sourceNode: GraphQLESTreeNode, context: GraphQLESlintRuleContext, @@ -15,11 +25,11 @@ function validateDoc( const validationErrors = validate(schema, documentNode, rules); for (const error of validationErrors) { - const node = (error.nodes[0] as any) as GraphQLESTreeNode; + const validateRuleName = extractRuleName(error.stack); context.report({ - loc: node.loc, - message: error.message, + loc: error.locations[0], + message: validateRuleName ? `[${validateRuleName}] ${error.message}` : error.message, }); } } catch (e) { @@ -112,7 +122,7 @@ const rule: GraphQLESLintRule = { kind: Kind.DOCUMENT, definitions: [node.rawNode()], }, - rulesArr + rulesArr.filter(r => r.name !== 'NoUnusedFragmentsRule') ); }, }; diff --git a/packages/plugin/tests/validate-against-schema.spec.ts b/packages/plugin/tests/validate-against-schema.spec.ts index 1e103c3db91..f48dca8ab70 100644 --- a/packages/plugin/tests/validate-against-schema.spec.ts +++ b/packages/plugin/tests/validate-against-schema.spec.ts @@ -30,27 +30,31 @@ ruleTester.runGraphQLTests('validate-against-schema', rule, { options: [{ overrideRules: ['NoUnusedVariablesRule'] }], code: `query named ($id: ID!) { user(id: $id) { id @client } }`, }, + { + ...WITH_SCHEMA, + code: `fragment UserFields on User { id }`, + }, ], invalid: [ { ...WITH_SCHEMA, code: `query { user(id: 1) { notExists } }`, - errors: ['Cannot query field "notExists" on type "User".'], + errors: ['[FieldsOnCorrectTypeRule] Cannot query field "notExists" on type "User".'], }, { ...WITH_SCHEMA, options: [{ overrideRules: ['NoUnusedVariablesRule'] }], code: `query named ($id: ID!) { user(id: 2) { id @client } }`, - errors: ['Variable "$id" is never used in operation "named".'], + errors: ['[NoUnusedVariablesRule] Variable "$id" is never used in operation "named".'], }, { ...WITH_SCHEMA, - errors: ['Unknown directive "@client".'], + errors: ['[KnownDirectivesRule] Unknown directive "@client".'], code: `query named ($id: ID!) { user(id: $id) { id @client } }`, }, { ...WITH_SCHEMA, - errors: ['Unknown directive "@client".'], + errors: ['[KnownDirectivesRule] Unknown directive "@client".'], options: [{ overrideRules: ['KnownDirectivesRule'] }], code: `query named ($id: ID!) { user(id: $id) { id @client } }`, }, @@ -58,9 +62,9 @@ ruleTester.runGraphQLTests('validate-against-schema', rule, { ...WITH_SCHEMA, code: `query test($id: ID!) { user(invalid: $id) { test } }`, errors: [ - 'Unknown argument "invalid" on field "Query.user".', - 'Cannot query field "test" on type "User".', - 'Field "user" argument "id" of type "ID!" is required, but it was not provided.', + '[ProvidedRequiredArgumentsRule] Field "user" argument "id" of type "ID!" is required, but it was not provided.', + '[KnownArgumentNamesRule] Unknown argument "invalid" on field "Query.user".', + '[FieldsOnCorrectTypeRule] Cannot query field "test" on type "User".', ], }, ], From c5368f1f1e6f0a1d5fcfe7516b72001c9e6b7f4b Mon Sep 17 00:00:00 2001 From: Dotan Simha Date: Mon, 5 Oct 2020 16:21:54 +0300 Subject: [PATCH 2/5] added changeset --- .changeset/eleven-turkeys-smash.md | 5 +++++ .changeset/spotty-bottles-whisper.md | 5 +++++ 2 files changed, 10 insertions(+) create mode 100644 .changeset/eleven-turkeys-smash.md create mode 100644 .changeset/spotty-bottles-whisper.md diff --git a/.changeset/eleven-turkeys-smash.md b/.changeset/eleven-turkeys-smash.md new file mode 100644 index 00000000000..c5b4e473132 --- /dev/null +++ b/.changeset/eleven-turkeys-smash.md @@ -0,0 +1,5 @@ +--- +'@graphql-eslint/eslint-plugin': patch +--- + +Ignore NoUnusedFragmentsRule validation for fragments operations diff --git a/.changeset/spotty-bottles-whisper.md b/.changeset/spotty-bottles-whisper.md new file mode 100644 index 00000000000..3041f772614 --- /dev/null +++ b/.changeset/spotty-bottles-whisper.md @@ -0,0 +1,5 @@ +--- +'@graphql-eslint/eslint-plugin': patch +--- + +Improve error reported from validate-against-schema (added the rule name to the error) From c226b73288482e1f6e1b9ea5eabda8e6eefb9155 Mon Sep 17 00:00:00 2001 From: Dotan Simha Date: Mon, 5 Oct 2020 16:24:37 +0300 Subject: [PATCH 3/5] fixed lint issues --- packages/plugin/src/rules/validate-against-schema.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/plugin/src/rules/validate-against-schema.ts b/packages/plugin/src/rules/validate-against-schema.ts index 955211e3346..d44d6a553e5 100644 --- a/packages/plugin/src/rules/validate-against-schema.ts +++ b/packages/plugin/src/rules/validate-against-schema.ts @@ -4,7 +4,7 @@ import { GraphQLESLintRule, GraphQLESlintRuleContext } from '../types'; import { requireGraphQLSchemaFromContext } from '../utils'; function extractRuleName(stack: string | undefined): string | null { - const match = (stack || '').match(/validation\/rules\/(.*?)\.js\:/); + const match = (stack || '').match(/validation\/rules\/(.*?)\.js:/); if (!match) { return null; From d4a87a93d140f6a5ed4667a4061511270b6c35c4 Mon Sep 17 00:00:00 2001 From: Dotan Simha Date: Mon, 5 Oct 2020 16:28:06 +0300 Subject: [PATCH 4/5] fixes --- packages/plugin/src/rules/validate-against-schema.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/plugin/src/rules/validate-against-schema.ts b/packages/plugin/src/rules/validate-against-schema.ts index d44d6a553e5..eae2e621c18 100644 --- a/packages/plugin/src/rules/validate-against-schema.ts +++ b/packages/plugin/src/rules/validate-against-schema.ts @@ -4,7 +4,7 @@ import { GraphQLESLintRule, GraphQLESlintRuleContext } from '../types'; import { requireGraphQLSchemaFromContext } from '../utils'; function extractRuleName(stack: string | undefined): string | null { - const match = (stack || '').match(/validation\/rules\/(.*?)\.js:/); + const match = (stack || '').match(/validation[/\\\\]rules[/\\\\](.*?)\.js:/); if (!match) { return null; From 22d2a662b58345aa941fec1dc69f84f4378b4345 Mon Sep 17 00:00:00 2001 From: Dotan Simha Date: Mon, 5 Oct 2020 16:38:11 +0300 Subject: [PATCH 5/5] fixes for unused --- packages/plugin/src/rules/validate-against-schema.ts | 2 +- packages/plugin/tests/validate-against-schema.spec.ts | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/plugin/src/rules/validate-against-schema.ts b/packages/plugin/src/rules/validate-against-schema.ts index eae2e621c18..53b00a146d6 100644 --- a/packages/plugin/src/rules/validate-against-schema.ts +++ b/packages/plugin/src/rules/validate-against-schema.ts @@ -109,7 +109,7 @@ const rule: GraphQLESLintRule = { kind: Kind.DOCUMENT, definitions: [node.rawNode()], }, - rulesArr + rulesArr.filter(r => r.name !== 'KnownFragmentNamesRule') ); }, FragmentDefinition(node) { diff --git a/packages/plugin/tests/validate-against-schema.spec.ts b/packages/plugin/tests/validate-against-schema.spec.ts index f48dca8ab70..aaf4d049956 100644 --- a/packages/plugin/tests/validate-against-schema.spec.ts +++ b/packages/plugin/tests/validate-against-schema.spec.ts @@ -34,6 +34,14 @@ ruleTester.runGraphQLTests('validate-against-schema', rule, { ...WITH_SCHEMA, code: `fragment UserFields on User { id }`, }, + { + ...WITH_SCHEMA, + code: `fragment UserFields on User { id }`, + }, + { + ...WITH_SCHEMA, + code: `query { user(id: 1) { ...UserFields } }`, + }, ], invalid: [ {