feat: add definitions option for alphabetize rule (#965)

* feat: add definitions options for `alphabetize` rule

* simplify require-id-when-available

* improve location report in description-style

* change arguments for getLocation

* fix runGraphQLTests generic arguments

* fix GraphQLESTreeNode expand tooltip

* update eslint patch

* update eslint in examples to trigger yarn.loc changes

* add changeset

Author: dimaMachina

.changeset/beige-bats-happen.md

@@ -0,0 +1,5 @@
+---
+'@graphql-eslint/eslint-plugin': minor
+---
+
+feat: add `definitions` option for `alphabetize` rule

docs/rules/alphabetize.md

@@ -97,7 +97,7 @@ The schema defines the following properties:
 
 ### `fields` (array)
 
-Fields of `type`, `interface`, and `input`
+Fields of `type`, `interface`, and `input`.
 
 The elements of the array can contain the following enum values:
 
@@ -112,7 +112,7 @@ Additional restrictions:
 
 ### `values` (array)
 
-Values of `enum`
+Values of `enum`.
 
 The elements of the array can contain the following enum values:
 
@@ -125,7 +125,7 @@ Additional restrictions:
 
 ### `selections` (array)
 
-Selections of operations (`query`, `mutation` and `subscription`) and `fragment`
+Selections of `fragment` and operations `query`, `mutation` and `subscription`.
 
 The elements of the array can contain the following enum values:
 
@@ -139,7 +139,7 @@ Additional restrictions:
 
 ### `variables` (array)
 
-Variables of operations (`query`, `mutation` and `subscription`)
+Variables of operations `query`, `mutation` and `subscription`.
 
 The elements of the array can contain the following enum values:
 
@@ -152,7 +152,7 @@ Additional restrictions:
 
 ### `arguments` (array)
 
-Arguments of fields and directives
+Arguments of fields and directives.
 
 The elements of the array can contain the following enum values:
 
@@ -166,6 +166,12 @@ Additional restrictions:
 * Minimum items: `1`
 * Unique items: `true`
 
+### `definitions` (boolean)
+
+Definitions – `type`, `interface`, `enum`, `scalar`, `input`, `union` and `directive`.
+
+Default: `false`
+
 ## Resources
 
 - [Rule source](../../packages/plugin/src/rules/alphabetize.ts)

examples/basic/package.json

@@ -13,6 +13,6 @@
   },
   "devDependencies": {
     "@graphql-eslint/eslint-plugin": "3.8.0",
-    "eslint": "8.7.0"
+    "eslint": "8.9.0"
   }
 }

examples/code-file/package.json

@@ -13,6 +13,6 @@
   },
   "devDependencies": {
     "@graphql-eslint/eslint-plugin": "3.8.0",
-    "eslint": "8.7.0"
+    "eslint": "8.9.0"
   }
 }

examples/graphql-config-code-file/package.json

@@ -14,6 +14,6 @@
   },
   "devDependencies": {
     "@graphql-eslint/eslint-plugin": "3.8.0",
-    "eslint": "8.7.0"
+    "eslint": "8.9.0"
   }
 }

examples/graphql-config/package.json

@@ -13,6 +13,6 @@
   },
   "devDependencies": {
     "@graphql-eslint/eslint-plugin": "3.8.0",
-    "eslint": "8.7.0"
+    "eslint": "8.9.0"
   }
 }

examples/prettier/package.json

@@ -13,7 +13,7 @@
   },
   "devDependencies": {
     "@graphql-eslint/eslint-plugin": "3.8.0",
-    "eslint": "8.7.0",
+    "eslint": "8.9.0",
     "eslint-config-prettier": "8.3.0",
     "eslint-plugin-prettier": "4.0.0",
     "prettier": "2.4.1"

packages/plugin/src/estree-parser/estree-ast.ts

@@ -26,7 +26,8 @@ type SingleESTreeNode<T extends ASTNode | ValueNode, WithTypeInfo extends boolea
     rawNode: () => T;
   };
 
-export type GraphQLESTreeNode<T, WithTypeInfo extends boolean = false> = T extends ASTNode | ValueNode
+// eslint-disable-next-line @typescript-eslint/ban-types -- only empty object makes the tooltips expand without recursion
+export type GraphQLESTreeNode<T, WithTypeInfo extends boolean = false> = {} & (T extends ASTNode | ValueNode
   ? {
       [K in keyof SingleESTreeNode<T, WithTypeInfo>]: SingleESTreeNode<T, WithTypeInfo>[K] extends ReadonlyArray<
         infer Nested
@@ -36,4 +37,4 @@ export type GraphQLESTreeNode<T, WithTypeInfo extends boolean = false> = T exten
         ? GraphQLESTreeNode<SingleESTreeNode<T, WithTypeInfo>[K], WithTypeInfo>
         : SingleESTreeNode<T, WithTypeInfo>[K];
     }
-  : T;
+  : T);

packages/plugin/src/rules/alphabetize.ts

@@ -7,26 +7,23 @@ import {
   InputObjectTypeDefinitionNode,
   InputObjectTypeExtensionNode,
   FieldDefinitionNode,
-  InputValueDefinitionNode,
   EnumTypeDefinitionNode,
   EnumTypeExtensionNode,
-  EnumValueDefinitionNode,
   DirectiveDefinitionNode,
-  ArgumentNode,
-  VariableDefinitionNode,
   OperationDefinitionNode,
   FieldNode,
   DirectiveNode,
   SelectionSetNode,
-  FragmentSpreadNode,
+  ASTNode,
 } from 'graphql';
 import type { SourceLocation, Comment } from 'estree';
 import type { AST } from 'eslint';
+import lowerCase from 'lodash.lowercase';
 import { GraphQLESLintRule } from '../types';
 import { GraphQLESTreeNode } from '../estree-parser';
 import { GraphQLESLintRuleListener } from '../testkit';
 
-const ALPHABETIZE = 'ALPHABETIZE';
+const RULE_ID = 'alphabetize';
 
 const fieldsEnum: ('ObjectTypeDefinition' | 'InterfaceTypeDefinition' | 'InputObjectTypeDefinition')[] = [
   Kind.OBJECT_TYPE_DEFINITION,
@@ -52,6 +49,7 @@ export type AlphabetizeConfig = {
   selections?: typeof selectionsEnum;
   variables?: typeof variablesEnum;
   arguments?: typeof argumentsEnum;
+  definitions?: boolean;
 };
 
 const rule: GraphQLESLintRule<[AlphabetizeConfig]> = {
@@ -61,7 +59,7 @@ const rule: GraphQLESLintRule<[AlphabetizeConfig]> = {
     docs: {
       category: ['Schema', 'Operations'],
       description: `Enforce arrange in alphabetical order for type fields, enum values, input object fields, operation selections and more.`,
-      url: 'https://github.com/dotansimha/graphql-eslint/blob/master/docs/rules/alphabetize.md',
+      url: `https://github.com/dotansimha/graphql-eslint/blob/master/docs/rules/${RULE_ID}.md`,
       examples: [
         {
           title: 'Incorrect',
@@ -144,6 +142,8 @@ const rule: GraphQLESLintRule<[AlphabetizeConfig]> = {
             fields: fieldsEnum,
             values: valuesEnum,
             arguments: argumentsEnum,
+            // TODO: add in graphql-eslint v4
+            // definitions: true,
           },
         ],
         operations: [
@@ -156,7 +156,7 @@ const rule: GraphQLESLintRule<[AlphabetizeConfig]> = {
       },
     },
     messages: {
-      [ALPHABETIZE]: '"{{ currName }}" should be before "{{ prevName }}"',
+      [RULE_ID]: '`{{ currName }}` should be before {{ prevName }}.',
     },
     schema: {
       type: 'array',
@@ -174,7 +174,7 @@ const rule: GraphQLESLintRule<[AlphabetizeConfig]> = {
             items: {
               enum: fieldsEnum,
             },
-            description: 'Fields of `type`, `interface`, and `input`',
+            description: 'Fields of `type`, `interface`, and `input`.',
           },
           values: {
             type: 'array',
@@ -183,7 +183,7 @@ const rule: GraphQLESLintRule<[AlphabetizeConfig]> = {
             items: {
               enum: valuesEnum,
             },
-            description: 'Values of `enum`',
+            description: 'Values of `enum`.',
           },
           selections: {
             type: 'array',
@@ -192,7 +192,7 @@ const rule: GraphQLESLintRule<[AlphabetizeConfig]> = {
             items: {
               enum: selectionsEnum,
             },
-            description: 'Selections of operations (`query`, `mutation` and `subscription`) and `fragment`',
+            description: 'Selections of `fragment` and operations `query`, `mutation` and `subscription`.',
           },
           variables: {
             type: 'array',
@@ -201,7 +201,7 @@ const rule: GraphQLESLintRule<[AlphabetizeConfig]> = {
             items: {
               enum: variablesEnum,
             },
-            description: 'Variables of operations (`query`, `mutation` and `subscription`)',
+            description: 'Variables of operations `query`, `mutation` and `subscription`.',
           },
           arguments: {
             type: 'array',
@@ -210,7 +210,12 @@ const rule: GraphQLESLintRule<[AlphabetizeConfig]> = {
             items: {
               enum: argumentsEnum,
             },
-            description: 'Arguments of fields and directives',
+            description: 'Arguments of fields and directives.',
+          },
+          definitions: {
+            type: 'boolean',
+            description: 'Definitions – `type`, `interface`, `enum`, `scalar`, `input`, `union` and `directive`.',
+            default: false,
           },
         },
       },
@@ -232,7 +237,17 @@ const rule: GraphQLESLintRule<[AlphabetizeConfig]> = {
       if (tokenBefore) {
         return commentsBefore.filter(comment => !isNodeAndCommentOnSameLine(tokenBefore, comment));
       }
-      return commentsBefore;
+      const filteredComments = [];
+      const nodeLine = node.loc.start.line;
+      // Break on comment that not attached to node
+      for (let i = commentsBefore.length - 1; i >= 0; i -= 1) {
+        const comment = commentsBefore[i];
+        if (nodeLine - comment.loc.start.line - filteredComments.length > 1) {
+          break;
+        }
+        filteredComments.unshift(comment);
+      }
+      return filteredComments;
     }
 
     function getRangeWithComments(node): AST.Range {
@@ -243,37 +258,37 @@ const rule: GraphQLESLintRule<[AlphabetizeConfig]> = {
       return [from.range[0], to.range[1]];
     }
 
-    function checkNodes(
-      nodes: GraphQLESTreeNode<
-        | FieldDefinitionNode
-        | InputValueDefinitionNode
-        | EnumValueDefinitionNode
-        | FieldNode
-        | FragmentSpreadNode
-        | ArgumentNode
-        | VariableDefinitionNode['variable']
-      >[]
-    ) {
+    function checkNodes(nodes: GraphQLESTreeNode<ASTNode>[]) {
       // Starts from 1, ignore nodes.length <= 1
       for (let i = 1; i < nodes.length; i += 1) {
-        const prevNode = nodes[i - 1];
         const currNode = nodes[i];
-        const prevName = prevNode.name.value;
-        const currName = currNode.name.value;
-        // Compare with lexicographic order
-        if (prevName.localeCompare(currName) !== 1) {
+        const currName = 'name' in currNode && currNode.name?.value;
+        if (!currName) {
+          // we don't move unnamed current nodes
           continue;
         }
-        const isVariableNode = currNode.kind === Kind.VARIABLE;
+
+        const prevNode = nodes[i - 1];
+        const prevName = 'name' in prevNode && prevNode.name?.value;
+        if (prevName) {
+          // Compare with lexicographic order
+          const compareResult = prevName.localeCompare(currName);
+          const shouldSort = compareResult === 1;
+          if (!shouldSort) {
+            const isSameName = compareResult === 0;
+            if (!isSameName || !prevNode.kind.endsWith('Extension') || currNode.kind.endsWith('Extension')) {
+              continue;
+            }
+          }
+        }
+
         context.report({
           node: currNode.name,
-          messageId: ALPHABETIZE,
-          data: isVariableNode
-            ? {
-                currName: `$${currName}`,
-                prevName: `$${prevName}`,
-              }
-            : { currName, prevName },
+          messageId: RULE_ID,
+          data: {
+            currName,
+            prevName: prevName ? `\`${prevName}\`` : lowerCase(prevNode.kind),
+          },
           *fix(fixer) {
             const prevRange = getRangeWithComments(prevNode);
             const currRange = getRangeWithComments(currNode);
@@ -331,13 +346,10 @@ const rule: GraphQLESLintRule<[AlphabetizeConfig]> = {
     if (selectionsSelector) {
       listeners[`:matches(${selectionsSelector}) SelectionSet`] = (node: GraphQLESTreeNode<SelectionSetNode>) => {
         checkNodes(
-          node.selections
-            // inline fragment don't have name, so we skip them
-            .filter(selection => selection.kind !== Kind.INLINE_FRAGMENT)
-            .map(selection =>
-              // sort by alias is field is renamed
-              'alias' in selection && selection.alias ? ({ name: selection.alias } as any) : selection
-            )
+          node.selections.map(selection =>
+            // sort by alias is field is renamed
+            'alias' in selection && selection.alias ? ({ name: selection.alias } as any) : selection
+          )
         );
       };
     }
@@ -356,6 +368,12 @@ const rule: GraphQLESLintRule<[AlphabetizeConfig]> = {
       };
     }
 
+    if (opts.definitions) {
+      listeners.Document = node => {
+        checkNodes(node.definitions);
+      };
+    }
+
     return listeners;
   },
 };

packages/plugin/src/rules/description-style.ts

@@ -1,6 +1,5 @@
 import { StringValueNode } from 'graphql';
 import { GraphQLESLintRule } from '../types';
-import { getLocation } from '../utils';
 import { GraphQLESTreeNode } from '../estree-parser';
 
 type DescriptionStyleRuleConfig = { style: 'inline' | 'block' };
@@ -55,7 +54,7 @@ const rule: GraphQLESLintRule<[DescriptionStyleRuleConfig]> = {
     return {
       [`.description[type=StringValue][block!=${isBlock}]`](node: GraphQLESTreeNode<StringValueNode>) {
         context.report({
-          loc: getLocation(node.loc),
+          loc: isBlock ? node.loc : node.loc.start,
           message: `Unexpected ${isBlock ? 'inline' : 'block'} description`,
         });
       },