Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ignore config to no-unused-fields rule #2281

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/lovely-emus-shake.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@graphql-eslint/eslint-plugin": major
---

Add ignore config to `no-unused-fields` rule
21 changes: 21 additions & 0 deletions packages/plugin/__tests__/no-unused-fields.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,27 @@ ruleTester.run('no-unused-fields', rule, {
},
},
},
{
name: 'should allow fields if they are ignored',
options: [{ ignoredFieldSelectors: ['FieldDefinition[name.value=firstName]'] }],
code: /* GraphQL */ `
type User {
id: ID!
firstName: String
}
`,
parserOptions: {
graphQLConfig: {
documents: /* GraphQL */ `
{
user(id: 1) {
id
}
}
`,
},
},
},
],
invalid: [
{
Expand Down
120 changes: 89 additions & 31 deletions packages/plugin/src/rules/no-unused-fields.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,39 @@
import { GraphQLSchema, TypeInfo, visit, visitWithTypeInfo } from 'graphql';
import { FieldDefinitionNode, GraphQLSchema, TypeInfo, visit, visitWithTypeInfo } from 'graphql';
import { FromSchema } from 'json-schema-to-ts';
import { GraphQLESTreeNode } from '../estree-converter/types.js';
import { SiblingOperations } from '../siblings.js';
import { GraphQLESLintRule } from '../types.js';
import { GraphQLESLintRule, GraphQLESLintRuleListener } from '../types.js';
import { requireGraphQLSchemaFromContext, requireSiblingsOperations } from '../utils.js';

const RULE_ID = 'no-unused-fields';

const schema = {
type: 'array',
maxItems: 1,
items: {
type: 'object',
additionalProperties: false,
properties: {
ignoredFieldSelectors: {
Copy link
Author

@maciesielka maciesielka May 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few other names I considered here were:

  • allowedUnusedFields
  • allowedUnusedFieldSelectors
  • ignoredFields

I think ignoredFieldSelectors avoids some of the oxymoronic qualities of allowedUnused* and provides useful context that this configuration uses ESLint selectors, but am open to other options.

type: 'array',
uniqueItems: true,
minItems: 1,
description: [
'Fields that will be ignored and are allowed to be unused.',
'',
'> These fields are defined by ESLint [`selectors`](https://eslint.org/docs/developer-guide/selectors). Paste or drop code into the editor in [ASTExplorer](https://astexplorer.net) and inspect the generated AST to compose your selector.',
Copy link
Author

@maciesielka maciesielka May 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The naming-convention rule already makes use of ESLint selectors within configurations and it felt good to follow suit here. While this adds some clunkiness to the configuration (allowing an individual field looks like "FieldDefinition[name.value=field]" instead of "field"), there are benefits in that allowing fields with wildcards, based on their parent object, etc. are implicitly supported.

].join('\n'),
items: {
type: 'string',
pattern: '^FieldDefinition(.+)$',
},
},
},
},
} as const;

export type RuleOptions = FromSchema<typeof schema>;

type UsedFields = Record<string, Set<string>>;

let usedFieldsCache: UsedFields;
Expand Down Expand Up @@ -41,7 +70,7 @@ function getUsedFields(schema: GraphQLSchema, operations: SiblingOperations): Us
return usedFieldsCache;
}

export const rule: GraphQLESLintRule = {
export const rule: GraphQLESLintRule<RuleOptions> = {
meta: {
messages: {
[RULE_ID]: 'Field "{{fieldName}}" is unused',
Expand Down Expand Up @@ -96,45 +125,74 @@ export const rule: GraphQLESLintRule = {
}
`,
},
{
title: 'Correct (ignoring fields)',
usage: [{ ignoredFieldSelectors: ['FieldDefinition[name.value=lastName]'] }],
code: /* GraphQL */ `
type User {
id: ID!
firstName: String
lastName: String
}

type Query {
me: User
}

query {
me {
id
firstName
}
}
`,
},
],
},
type: 'suggestion',
schema: [],
schema,
hasSuggestions: true,
},
create(context) {
const schema = requireGraphQLSchemaFromContext(RULE_ID, context);
const siblingsOperations = requireSiblingsOperations(RULE_ID, context);
const usedFields = getUsedFields(schema, siblingsOperations);
const { ignoredFieldSelectors } = context.options[0] || {};
const selector = (ignoredFieldSelectors || []).reduce(
(acc, selector) => `${acc}:not(${selector})`,
'FieldDefinition',
);

return {
FieldDefinition(node) {
const fieldName = node.name.value;
const parentTypeName = node.parent.name.value;
const isUsed = usedFields[parentTypeName]?.has(fieldName);

if (isUsed) {
return;
}

context.report({
node: node.name,
messageId: RULE_ID,
data: { fieldName },
suggest: [
{
desc: `Remove \`${fieldName}\` field`,
fix(fixer) {
const sourceCode = context.getSourceCode() as any;
const tokenBefore = sourceCode.getTokenBefore(node);
const tokenAfter = sourceCode.getTokenAfter(node);
const isEmptyType = tokenBefore.type === '{' && tokenAfter.type === '}';
return fixer.remove((isEmptyType ? node.parent : node) as any);
},
const listener: GraphQLESLintRuleListener = (node: GraphQLESTreeNode<FieldDefinitionNode>) => {
const fieldName = node.name.value;
const parentTypeName = node.parent.name.value;
const isUsed = usedFields[parentTypeName]?.has(fieldName);

if (isUsed) {
return;
}

context.report({
node: node.name,
messageId: RULE_ID,
data: { fieldName },
suggest: [
{
desc: `Remove \`${fieldName}\` field`,
fix(fixer) {
const sourceCode = context.getSourceCode() as any;
const tokenBefore = sourceCode.getTokenBefore(node);
const tokenAfter = sourceCode.getTokenAfter(node);
const isEmptyType = tokenBefore.type === '{' && tokenAfter.type === '}';
return fixer.remove((isEmptyType ? node.parent : node) as any);
},
],
});
},
},
],
});
};

return {
[selector]: listener,
};
},
};
43 changes: 43 additions & 0 deletions website/src/pages/rules/no-unused-fields.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,49 @@ query {
}
```

### Correct (ignoring fields)

```graphql
# eslint @graphql-eslint/no-unused-fields: ['error', { ignoredFieldSelectors: ['FieldDefinition[name.value=lastName]'] }]

type User {
id: ID!
firstName: String
lastName: String
}

type Query {
me: User
}

query {
me {
id
firstName
}
}
```

## Config Schema

The schema defines the following properties:

### `ignoredFieldSelectors` (array)

Fields that will be ignored and are allowed to be unused.

> These fields are defined by ESLint
> [`selectors`](https://eslint.org/docs/developer-guide/selectors) . Paste or drop code into the
> editor in [ASTExplorer](https://astexplorer.net) and inspect the generated AST to compose your
> selector.

The object is an array with all elements of the type `string`.

Additional restrictions:

- Minimum items: `1`
- Unique items: `true`

## Resources

- [Rule source](https://github.com/B2o5T/graphql-eslint/tree/master/packages/plugin/src/rules/no-unused-fields.ts)
Expand Down