Skip to content

Commit

Permalink
fix error report location for graphql-js rules (#837)
Browse files Browse the repository at this point in the history
  • Loading branch information
dimaMachina committed Dec 14, 2021
1 parent 2c73cb7 commit 503dd9f
Show file tree
Hide file tree
Showing 8 changed files with 105 additions and 25 deletions.
5 changes: 5 additions & 0 deletions .changeset/cold-vans-help.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@graphql-eslint/eslint-plugin': patch
---

fix error report location for `graphql-js` rules
57 changes: 36 additions & 21 deletions packages/plugin/src/rules/graphql-js-validation.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { AST } from 'eslint';
import { validate, GraphQLSchema, DocumentNode, ASTNode, ValidationRule } from 'graphql';
import { validateSDL } from 'graphql/validation/validate';
import { parseImportLine, processImport } from '@graphql-tools/import';
Expand All @@ -7,36 +8,50 @@ import { GraphQLESLintRule, GraphQLESLintRuleContext } from '../types';
import { getLocation, requireGraphQLSchemaFromContext, requireSiblingsOperations } from '../utils';
import { GraphQLESTreeNode } from '../estree-parser';

function extractRuleName(stack?: string): string | null {
const match = (stack || '').match(/validation[/\\]rules[/\\](.*?)\.js:/) || [];
return match[1] || null;
}

export function validateDoc(
sourceNode: GraphQLESTreeNode<ASTNode>,
context: GraphQLESLintRuleContext,
schema: GraphQLSchema | null,
documentNode: DocumentNode,
rules: ReadonlyArray<ValidationRule>,
ruleName: string | null = null
rules: ReadonlyArray<ValidationRule>
): void {
if (documentNode?.definitions?.length > 0) {
try {
const validationErrors = schema ? validate(schema, documentNode, rules) : validateSDL(documentNode, null, rules as any);
if (documentNode.definitions.length === 0) {
return;
}
try {
const validationErrors = schema
? validate(schema, documentNode, rules)
: validateSDL(documentNode, null, rules as any);

for (const error of validationErrors) {
/*
* TODO: Fix ESTree-AST converter because currently it's incorrectly convert loc.end
* Example: loc.end always equal loc.start
* {
* token: {
* type: 'Name',
* loc: { start: { line: 4, column: 13 }, end: { line: 4, column: 13 } },
* value: 'veryBad',
* range: [ 40, 47 ]
* }
* }
*/
const { line, column } = error.locations[0];
const ancestors = context.getAncestors();
const token = (ancestors[0] as AST.Program).tokens.find(
token => token.loc.start.line === line && token.loc.start.column === column
);

for (const error of validationErrors) {
const validateRuleName = ruleName || `[${extractRuleName(error.stack)}]`;
context.report({
loc: getLocation({ start: error.locations[0] }),
message: ruleName ? error.message : `${validateRuleName} ${error.message}`,
});
}
} catch (e) {
context.report({
node: sourceNode,
message: e.message,
loc: getLocation({ start: error.locations[0] }, token?.value),
message: error.message,
});
}
} catch (e) {
context.report({
node: sourceNode,
message: e.message,
});
}
}

Expand Down Expand Up @@ -94,7 +109,7 @@ const validationToRule = (
if (isRealFile && getDocumentNode) {
documentNode = getDocumentNode(context);
}
validateDoc(node, context, schema, documentNode || node.rawNode(), [ruleFn], ruleName);
validateDoc(node, context, schema, documentNode || node.rawNode(), [ruleFn]);
},
};
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@

exports[` 1`] = `
> 1 | type Query { t: String }
| ^ The "Query" definition is not executable.
| ^^^^ The "Query" definition is not executable.
`;
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[` 1`] = `
> 1 | fragment UserFields on User { id bad age }
| ^^^ Cannot query field "bad" on type "User". Did you mean "id"?
`;

exports[` 2`] = `
1 |
2 | {
3 | user {
4 | id
> 5 | veryBad
| ^^^^^^^ Cannot query field "veryBad" on type "User".
6 | age
7 | }
8 | }
9 |
`;
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ exports[` 1`] = `
21 | }
22 |
> 23 | schema {
| ^ Must provide only one schema definition.
| ^^^^^^ Must provide only one schema definition.
24 | query: RootQuery
25 | mutation: RootMutation
26 | }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ exports[` 1`] = `
4 | }
5 |
> 6 | extend type OtherUser {
| ^ Cannot extend type "OtherUser" because it is not defined.
| ^^^^^^^^^ Cannot extend type "OtherUser" because it is not defined.
7 | name: String!
8 | }
9 |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
exports[` 1`] = `
1 |
> 2 | type Query {
| ^ There can be only one type named "Query".
| ^^^^^ There can be only one type named "Query".
3 | foo: String
4 | }
5 |
Expand Down
41 changes: 41 additions & 0 deletions packages/plugin/tests/fields-on-correct-type.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { GraphQLRuleTester, rules, ParserOptions } from '../src';

const parserOptions: ParserOptions = {
schema: /* GraphQL */ `
type User {
id: ID
age: Int
}
type Query {
user: User
}
`,
};
const ruleTester = new GraphQLRuleTester();

ruleTester.runGraphQLTests('fields-on-correct-type', rules['fields-on-correct-type'], {
valid: [],
invalid: [
{
name: 'should highlight selection on single line',
code: 'fragment UserFields on User { id bad age }',
parserOptions,
errors: [{ message: 'Cannot query field "bad" on type "User". Did you mean "id"?' }],
},
{
name: 'should highlight selection on multi line',
code: /* GraphQL */ `
{
user {
id
veryBad
age
}
}
`,
parserOptions,
errors: [{ message: 'Cannot query field "veryBad" on type "User".' }],
},
],
});

0 comments on commit 503dd9f

Please sign in to comment.