Skip to content
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
Contributor 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.

Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer ignoredFieldSelectors too

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
Contributor 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.

Copy link
Contributor

@dimaMachina dimaMachina Nov 17, 2024

Choose a reason for hiding this comment

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

I think it's fine to use selectors here instead of plain strings because we have the flexibility to choose parent name and parent pattern, see

[
  '[parent.name.value=PageInfo][name.value=endCursor]',
  '[parent.name.value=PageInfo][name.value=startCursor]',
  '[parent.name.value=PageInfo][name.value=hasNextPage]',
  '[parent.name.value=PageInfo][name.value=hasPreviousPage]',
  '[parent.name.value=/Edge$/][name.value=cursor]',
  '[parent.name.value=/Connection$/][name.value=pageInfo]',
]

we can specify ends with Edge and Connection, @maciesielka maybe we should update the default options to include it there?

].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