Skip to content

Commit

Permalink
Add require-nullable-result-in-root lint rule (#1657)
Browse files Browse the repository at this point in the history
Co-authored-by: Dimitri POSTOLOV <[email protected]>
  • Loading branch information
nishtahir and dimaMachina authored May 22, 2023
1 parent 496fc36 commit 0a571bb
Show file tree
Hide file tree
Showing 10 changed files with 263 additions and 0 deletions.
5 changes: 5 additions & 0 deletions .changeset/dull-tables-mix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@graphql-eslint/eslint-plugin': minor
---

Add `require-nullable-result-in-root` rule to report on non-null fields in root types
1 change: 1 addition & 0 deletions packages/plugin/src/configs/schema-all.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export default {
'@graphql-eslint/require-deprecation-date': 'error',
'@graphql-eslint/require-field-of-type-query-in-mutation-result': 'error',
'@graphql-eslint/require-nullable-fields-with-oneof': 'error',
'@graphql-eslint/require-nullable-result-in-root': 'error',
'@graphql-eslint/require-type-pattern-with-oneof': 'error',
},
};
2 changes: 2 additions & 0 deletions packages/plugin/src/rules/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import { rule as requireFieldOfTypeQueryInMutationResult } from './require-field
import { rule as requireIdWhenAvailable } from './require-id-when-available.js';
import { rule as requireImportFragment } from './require-import-fragment.js';
import { rule as requireNullableFieldsWithOneof } from './require-nullable-fields-with-oneof.js';
import { rule as requireNullableResultInRoot } from './require-nullable-result-in-root.js';
import { rule as requireTypePatternWithOneof } from './require-type-pattern-with-oneof.js';
import { rule as selectionSetDepth } from './selection-set-depth.js';
import { rule as strictIdInTypes } from './strict-id-in-types.js';
Expand Down Expand Up @@ -67,6 +68,7 @@ export const rules = {
'require-id-when-available': requireIdWhenAvailable,
'require-import-fragment': requireImportFragment,
'require-nullable-fields-with-oneof': requireNullableFieldsWithOneof,
'require-nullable-result-in-root': requireNullableResultInRoot,
'require-type-pattern-with-oneof': requireTypePatternWithOneof,
'selection-set-depth': selectionSetDepth,
'strict-id-in-types': strictIdInTypes,
Expand Down
90 changes: 90 additions & 0 deletions packages/plugin/src/rules/require-nullable-result-in-root.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
import { Kind, ObjectTypeDefinitionNode } from 'graphql';
import { GraphQLESLintRule } from '../types.js';
import { getNodeName, requireGraphQLSchemaFromContext, truthy } from '../utils.js';
import { GraphQLESTreeNode } from '../estree-converter/index.js';

const RULE_ID = 'require-nullable-result-in-root';

export const rule: GraphQLESLintRule = {
meta: {
type: 'suggestion',
hasSuggestions: true,
docs: {
category: 'Schema',
description: 'Require nullable fields in root types.',
url: `https://github.com/B2o5T/graphql-eslint/blob/master/docs/rules/${RULE_ID}.md`,
requiresSchema: true,
examples: [
{
title: 'Incorrect',
code: /* GraphQL */ `
type Query {
user: User!
}
`,
},
{
title: 'Correct',
code: /* GraphQL */ `
type Query {
foo: User
baz: [User]!
bar: [User!]!
}
`,
},
],
},
messages: {
[RULE_ID]: 'Unexpected non-null result {{ resultType }} in {{ rootType }}',
},
schema: [],
},
create(context) {
const schema = requireGraphQLSchemaFromContext(RULE_ID, context);
const rootTypeNames = new Set(
[schema.getQueryType(), schema.getMutationType(), schema.getSubscriptionType()]
.filter(truthy)
.map(type => type.name),
);
const sourceCode = context.getSourceCode();

return {
'ObjectTypeDefinition,ObjectTypeExtension'(
node: GraphQLESTreeNode<ObjectTypeDefinitionNode>,
) {
if (!rootTypeNames.has(node.name.value)) return;

for (const field of node.fields || []) {
if (
field.gqlType.type !== Kind.NON_NULL_TYPE ||
field.gqlType.gqlType.type !== Kind.NAMED_TYPE
)
continue;
const name = field.gqlType.gqlType.name.value;
const type = schema.getType(name);
const resultType = type ? getNodeName(type.astNode as any) : '';

context.report({
node: field.gqlType,
messageId: RULE_ID,
data: {
resultType,
rootType: getNodeName(node),
},
suggest: [
{
desc: `Make ${resultType} nullable`,
fix(fixer) {
const text = sourceCode.getText(field.gqlType as any);

return fixer.replaceText(field.gqlType as any, text.replace('!', ''));
},
},
],
});
}
},
};
},
};
1 change: 1 addition & 0 deletions packages/plugin/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ export function displayNodeName(node: GraphQLESTreeNode<ASTNode>): string {
export function getNodeName(node: GraphQLESTreeNode<ASTNode>): string {
switch (node.kind) {
case Kind.OBJECT_TYPE_DEFINITION:
case Kind.OBJECT_TYPE_EXTENSION:
case Kind.INTERFACE_TYPE_DEFINITION:
case Kind.ENUM_TYPE_DEFINITION:
case Kind.SCALAR_TYPE_DEFINITION:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html

exports[`Invalid #1 1`] = `
#### ⌨️ Code

1 | type Query {
2 | user: User!
3 | }
4 | type User {
5 | id: ID!
6 | }

#### ❌ Error

1 | type Query {
> 2 | user: User!
| ^^^^ Unexpected non-null result type "User" in type "Query"
3 | }

#### 💡 Suggestion: Make type "User" nullable

1 | type Query {
2 | user: User
3 | }
4 | type User {
5 | id: ID!
6 | }
`;

exports[`should work with extend query 1`] = `
#### ⌨️ Code

1 | type MyMutation
2 | extend type MyMutation {
3 | user: User!
4 | }
5 | interface User {
6 | id: ID!
7 | }
8 | schema {
9 | mutation: MyMutation
10 | }

#### ❌ Error

2 | extend type MyMutation {
> 3 | user: User!
| ^^^^ Unexpected non-null result interface "User" in type "MyMutation"
4 | }

#### 💡 Suggestion: Make interface "User" nullable

1 | type MyMutation
2 | extend type MyMutation {
3 | user: User
4 | }
5 | interface User {
6 | id: ID!
7 | }
8 | schema {
9 | mutation: MyMutation
10 | }
`;
57 changes: 57 additions & 0 deletions packages/plugin/tests/require-nullable-result-in-root.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import { GraphQLRuleTester, ParserOptions } from '../src';
import { rule } from '../src/rules/require-nullable-result-in-root';

const ruleTester = new GraphQLRuleTester();

function useSchema(code: string): { code: string; parserOptions: Omit<ParserOptions, 'filePath'> } {
return {
code,
parserOptions: { schema: code },
};
}

ruleTester.runGraphQLTests('require-nullable-result-in-root', rule, {
valid: [
{
...useSchema(/* GraphQL */ `
type Query {
foo: User
baz: [User]!
bar: [User!]!
}
type User {
id: ID!
}
`),
},
],
invalid: [
{
...useSchema(/* GraphQL */ `
type Query {
user: User!
}
type User {
id: ID!
}
`),
errors: 1,
},
{
name: 'should work with extend query',
...useSchema(/* GraphQL */ `
type MyMutation
extend type MyMutation {
user: User!
}
interface User {
id: ID!
}
schema {
mutation: MyMutation
}
`),
errors: 1,
},
],
});
1 change: 1 addition & 0 deletions website/src/pages/rules/_meta.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
"require-description": "",
"require-field-of-type-query-in-mutation-result": "",
"require-nullable-fields-with-oneof": "",
"require-nullable-result-in-root": "",
"require-type-pattern-with-oneof": "",
"strict-id-in-types": "",
"unique-directive-names": "",
Expand Down
1 change: 1 addition & 0 deletions website/src/pages/rules/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ Each rule has emojis denoting:
| [require-id-when-available](/rules/require-id-when-available) | Enforce selecting specific fields when they are available on the GraphQL type. | ![recommended][] | 📦 | 🚀 | 💡 |
| [require-import-fragment](/rules/require-import-fragment) | Require fragments to be imported via an import expression. | | 📦 | 🚀 | 💡 |
| [require-nullable-fields-with-oneof](/rules/require-nullable-fields-with-oneof) | Require `input` or `type` fields to be non-nullable with `@oneOf` directive. | ![all][] | 📄 | 🚀 |
| [require-nullable-result-in-root](/rules/require-nullable-result-in-root) | Require nullable fields in root types. | ![all][] | 📄 | 🚀 | 💡 |
| [require-type-pattern-with-oneof](/rules/require-type-pattern-with-oneof) | Enforce types with `@oneOf` directive have `error` and `ok` fields. | ![all][] | 📄 | 🚀 |
| [scalar-leafs](/rules/scalar-leafs) | A GraphQL document is valid only if all leaf fields (fields without sub selections) are of scalar or enum types. | ![recommended][] | 📦 | 🔮 | 💡 |
| [selection-set-depth](/rules/selection-set-depth) | Limit the complexity of the GraphQL operations solely by their depth. Based on [graphql-depth-limit](https://npmjs.com/package/graphql-depth-limit). | ![recommended][] | 📦 | 🚀 | 💡 |
Expand Down
42 changes: 42 additions & 0 deletions website/src/pages/rules/require-nullable-result-in-root.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# `require-nullable-result-in-root`

💡 This rule provides
[suggestions](https://eslint.org/docs/developer-guide/working-with-rules#providing-suggestions)

- Category: `Schema`
- Rule name: `@graphql-eslint/require-nullable-result-in-root`
- Requires GraphQL Schema: `true`
[ℹ️](/docs/getting-started#extended-linting-rules-with-graphql-schema)
- Requires GraphQL Operations: `false`
[ℹ️](/docs/getting-started#extended-linting-rules-with-siblings-operations)

Require nullable fields in root types.

## Usage Examples

### Incorrect

```graphql
# eslint @graphql-eslint/require-nullable-result-in-root: 'error'

type Query {
user: User!
}
```

### Correct

```graphql
# eslint @graphql-eslint/require-nullable-result-in-root: 'error'

type Query {
foo: User
baz: [User]!
bar: [User!]!
}
```

## Resources

- [Rule source](https://github.com/B2o5T/graphql-eslint/tree/master/packages/plugin/src/rules/require-nullable-result-in-root.ts)
- [Test source](https://github.com/B2o5T/graphql-eslint/tree/master/packages/plugin/tests/require-nullable-result-in-root.spec.ts)

0 comments on commit 0a571bb

Please sign in to comment.