chore(valid-describe): use shared guards (#379)

src/rules/__tests__/valid-describe.test.ts

@@ -10,6 +10,15 @@ const ruleTester = new TSESLint.RuleTester({
 ruleTester.run('valid-describe', rule, {
   valid: [
     'describe.each()()',
+    'describe.each(() => {})()',
+    'describe["each"]()()',
+    'describe["each"](() => {})()',
+    'describe["each"](() => {})("foo")',
+    'describe.each(() => {})("foo")',
+    'describe["each"]()(() => {})',
+    'describe.each()(() => {})',
+    'describe["each"]("foo")(() => {})',
+    'describe.each("foo")(() => {})',
     'describe("foo", function() {})',
     'describe("foo", () => {})',
     'describe(`foo`, () => {})',
@@ -101,7 +110,8 @@ ruleTester.run('valid-describe', rule, {
             throw new Error('Fail');
           });
         });
-      });`,
+      });
+      `,
       errors: [{ messageId: 'noAsyncDescribeCallback', line: 6, column: 27 }],
     },
     {

src/rules/tsUtils.ts

@@ -108,6 +108,61 @@ export const isStringNode = <V extends string>(
 export const getStringValue = <S extends string>(node: StringNode<S>): S =>
   isTemplateLiteral(node) ? node.quasis[0].value.raw : node.value;
 
+/**
+ * An `Identifier` with a known `name` value - i.e `expect`.
+ */
+interface KnownIdentifier<Name extends string> extends TSESTree.Identifier {
+  name: Name;
+}
+
+/**
+ * Checks if the given `node` is an `Identifier`.
+ *
+ * If a `name` is provided, & the `node` is an `Identifier`,
+ * the `name` will be compared to that of the `identifier`.
+ *
+ * @param {Node} node
+ * @param {V?} name
+ *
+ * @return {node is KnownIdentifier<Name>}
+ *
+ * @template V
+ */
+const isIdentifier = <V extends string>(
+  node: TSESTree.Node,
+  name?: V,
+): node is KnownIdentifier<V> =>
+  node.type === AST_NODE_TYPES.Identifier &&
+  (name === undefined || node.name === name);
+
+/**
+ * Checks if the given `node` is a "supported accessor".
+ *
+ * This means that it's a node can be used to access properties,
+ * and who's "value" can be statically determined.
+ *
+ * `MemberExpression` nodes most commonly contain accessors,
+ * but it's possible for other nodes to contain them.
+ *
+ * If a `value` is provided & the `node` is an `AccessorNode`,
+ * the `value` will be compared to that of the `AccessorNode`.
+ *
+ * Note that `value` here refers to the normalised value.
+ * The property that holds the value is not always called `name`.
+ *
+ * @param {Node} node
+ * @param {V?} value
+ *
+ * @return {node is AccessorNode<V>}
+ *
+ * @template V
+ */
+export const isSupportedAccessor = <V extends string>(
+  node: TSESTree.Node,
+  value?: V,
+): node is AccessorNode<V> =>
+  isIdentifier(node, value) || isStringNode(node, value);
+
 /**
  * Represents a `CallExpression` with a single argument.
  */
@@ -141,9 +196,15 @@ export const hasOnlyOneArgument = (
  */
 export const getAccessorValue = <S extends string = string>(
   accessor: AccessorNode<S>,
-): S => getStringValue(accessor);
-
-type AccessorNode<Specifics extends string = string> = StringNode<Specifics>;
+): S =>
+  /* istanbul ignore next */
+  accessor.type === AST_NODE_TYPES.Identifier
+    ? accessor.name
+    : getStringValue(accessor);
+
+type AccessorNode<Specifics extends string = string> =
+  | StringNode<Specifics>
+  | KnownIdentifier<Specifics>;
 
 interface JestExpectIdentifier extends TSESTree.Identifier {
   name: 'expect';

src/rules/valid-describe.ts

@@ -3,45 +3,37 @@ import {
   TSESTree,
 } from '@typescript-eslint/experimental-utils';
 import {
-  FunctionExpression,
-  JestFunctionCallExpression,
   createRule,
   isDescribe,
   isFunction,
+  isStringNode,
+  isSupportedAccessor,
 } from './tsUtils';
 
-const isAsync = (node: FunctionExpression): boolean => node.async;
-
-const isString = (node: TSESTree.Node): boolean =>
-  (node.type === AST_NODE_TYPES.Literal && typeof node.value === 'string') ||
-  node.type === AST_NODE_TYPES.TemplateLiteral;
-
-const hasParams = (node: FunctionExpression): boolean => node.params.length > 0;
-
 const paramsLocation = (
   params: TSESTree.Expression[] | TSESTree.Parameter[],
 ) => {
   const [first] = params;
   const last = params[params.length - 1];
+
   return {
     start: first.loc.start,
     end: last.loc.end,
   };
 };
 
-const isDescribeEach = (node: JestFunctionCallExpression) =>
+const isDescribeEach = (node: TSESTree.CallExpression) =>
   node.callee.type === AST_NODE_TYPES.MemberExpression &&
-  node.callee.property.type === AST_NODE_TYPES.Identifier &&
-  node.callee.property.name === 'each';
+  isSupportedAccessor(node.callee.property, 'each');
 
 export default createRule({
   name: __filename,
   meta: {
     type: 'problem',
     docs: {
+      category: 'Possible Errors',
       description:
         'Using an improper `describe()` callback function can lead to unexpected test errors.',
-      category: 'Possible Errors',
       recommended: 'warn',
     },
     messages: {
@@ -54,7 +46,7 @@ export default createRule({
         'Unexpected return statement in describe callback',
     },
     schema: [],
-  } as const,
+  },
   defaultOptions: [],
   create(context) {
     return {
@@ -69,55 +61,61 @@ export default createRule({
             loc: node.loc,
           });
         }
-        const [name] = node.arguments;
-        const [, callbackFunction] = node.arguments;
-        if (!isString(name)) {
+
+        const [name, callback] = node.arguments;
+
+        if (!isStringNode(name)) {
           context.report({
             messageId: 'firstArgumentMustBeName',
             loc: paramsLocation(node.arguments),
           });
         }
-        if (!callbackFunction) {
+
+        if (!callback) {
           context.report({
             messageId: 'nameAndCallback',
             loc: paramsLocation(node.arguments),
           });
 
           return;
         }
-        if (isFunction(callbackFunction)) {
-          if (isAsync(callbackFunction)) {
-            context.report({
-              messageId: 'noAsyncDescribeCallback',
-              node: callbackFunction,
-            });
-          }
-          if (hasParams(callbackFunction)) {
-            context.report({
-              messageId: 'unexpectedDescribeArgument',
-              loc: paramsLocation(callbackFunction.params),
-            });
-          }
-          if (
-            callbackFunction.body &&
-            callbackFunction.body.type === AST_NODE_TYPES.BlockStatement
-          ) {
-            callbackFunction.body.body.forEach(node => {
-              if (node.type === 'ReturnStatement') {
-                context.report({
-                  messageId: 'unexpectedReturnInDescribe',
-                  node,
-                });
-              }
-            });
-          }
-        } else {
+
+        if (!isFunction(callback)) {
           context.report({
             messageId: 'secondArgumentMustBeFunction',
             loc: paramsLocation(node.arguments),
           });
+
           return;
         }
+
+        if (callback.async) {
+          context.report({
+            messageId: 'noAsyncDescribeCallback',
+            node: callback,
+          });
+        }
+
+        if (callback.params.length) {
+          context.report({
+            messageId: 'unexpectedDescribeArgument',
+            loc: paramsLocation(callback.params),
+          });
+        }
+
+        if (
+          callback.body &&
+          callback.body.type === AST_NODE_TYPES.BlockStatement
+        ) {
+          callback.body.body.forEach(node => {
+            if (node.type === AST_NODE_TYPES.ReturnStatement) {
+              context.report({
+                messageId: 'unexpectedReturnInDescribe',
+                node,
+              });
+            }
+          });
+        }
       },
     };
   },