chore: move title type checking from valid-describe to valid-title

BREAKING CHANGE: `valid-describe` no longer validates the title - use `valid-title` for that purpose
jest-community · Oct 27, 2019 · 1275471 · 1275471
1 parent bb2d09e
commit 1275471
Show file tree

Hide file tree

Showing 5 changed files with 119 additions and 28 deletions.
diff --git a/docs/rules/valid-title.md b/docs/rules/valid-title.md
@@ -3,6 +3,7 @@
 Checks that the title of Jest blocks are valid by ensuring that titles are:
 
 - not empty,
+- is a string,
 - not prefixed with their block name,
 - have no leading or trailing spaces
 
@@ -40,6 +41,31 @@ xit('foo', () => {});
 xtest('foo', () => {});
 ```
 
+**titleMustBeString**
+
+Titles should always be a string literal or expression.
+
+Examples of **incorrect** code for this rule:
+
+```js
+it(123, () => {});
+describe(String(/.+/), () => {});
+describe(myFunction, () => {});
+xdescribe(myFunction, () => {});
+describe(6, function () {})
+```
+
+Examples of **correct** code for this rule:
+
+```js
+it("is a string", () => {});
+test("is a string", () => {});
+xtest("is a string", () => {});
+describe("is a string", () => {});
+describe.skip("is a string", () => {});
+fdescribe("is a string", () => {});
+```
+
 **duplicatePrefix**
 
 A describe/ test block should not start with duplicatePrefix

diff --git a/src/rules/__tests__/valid-describe.test.ts b/src/rules/__tests__/valid-describe.test.ts
@@ -55,10 +55,7 @@ ruleTester.run('valid-describe', rule, {
   invalid: [
     {
       code: 'describe(() => {})',
-      errors: [
-        { messageId: 'firstArgumentMustBeName', line: 1, column: 10 },
-        { messageId: 'nameAndCallback', line: 1, column: 10 },
-      ],
+      errors: [{ messageId: 'nameAndCallback', line: 1, column: 10 }],
     },
     {
       code: 'describe("foo")',

diff --git a/src/rules/__tests__/valid-title.test.ts b/src/rules/__tests__/valid-title.test.ts
@@ -7,11 +7,83 @@ const ruleTester = new TSESLint.RuleTester({
   },
 });
 
+ruleTester.run('title-must-be-string', rule, {
+  valid: [
+    'it("is a string", () => {});',
+    'test("is a string", () => {});',
+    'xtest("is a string", () => {});',
+    'describe("is a string", () => {});',
+    'describe.skip("is a string", () => {});',
+    'fdescribe("is a string", () => {});',
+  ],
+  invalid: [
+    {
+      code: 'it(123, () => {});',
+      errors: [
+        {
+          messageId: 'titleMustBeString',
+          column: 4,
+          line: 1,
+        },
+      ],
+    },
+    {
+      code: 'describe(String(/.+/), () => {});',
+      errors: [
+        {
+          messageId: 'titleMustBeString',
+          column: 10,
+          line: 1,
+        },
+      ],
+    },
+    {
+      code: 'describe(myFunction, () => {});',
+      errors: [
+        {
+          messageId: 'titleMustBeString',
+          column: 10,
+          line: 1,
+        },
+      ],
+    },
+    {
+      code: 'xdescribe(myFunction, () => {});',
+      errors: [
+        {
+          messageId: 'titleMustBeString',
+          column: 11,
+          line: 1,
+        },
+      ],
+    },
+    {
+      code: 'describe(6, function () {})',
+      errors: [
+        {
+          messageId: 'titleMustBeString',
+          column: 10,
+          line: 1,
+        },
+      ],
+    },
+    {
+      code: 'describe.skip(123, () => {});',
+      errors: [
+        {
+          messageId: 'titleMustBeString',
+          column: 15,
+          line: 1,
+        },
+      ],
+    },
+  ],
+});
+
 ruleTester.run('no-empty-title', rule, {
   valid: [
     'describe()',
     'someFn("", function () {})',
-    'describe(1, function () {})',
     'describe("foo", function () {})',
     'describe("foo", function () { it("bar", function () {}) })',
     'test("foo", function () {})',
@@ -120,7 +192,6 @@ ruleTester.run('no-accidental-space', rule, {
     'describe()',
     'it.each()()',
     'describe("foo", function () {})',
-    'describe(6, function () {})',
     'fdescribe("foo", function () {})',
     'xdescribe("foo", function () {})',
     'it("foo", function () {})',

diff --git a/src/rules/valid-describe.ts b/src/rules/valid-describe.ts
@@ -6,7 +6,6 @@ import {
   createRule,
   isDescribe,
   isFunction,
-  isStringNode,
   isSupportedAccessor,
 } from './utils';
 
@@ -38,7 +37,6 @@ export default createRule({
     },
     messages: {
       nameAndCallback: 'Describe requires name and callback arguments',
-      firstArgumentMustBeName: 'First argument must be name',
       secondArgumentMustBeFunction: 'Second argument must be function',
       noAsyncDescribeCallback: 'No async describe callback',
       unexpectedDescribeArgument: 'Unexpected argument(s) in describe callback',
@@ -55,21 +53,14 @@ export default createRule({
           return;
         }
 
-        if (node.arguments.length === 0) {
+        if (node.arguments.length < 1) {
           return context.report({
             messageId: 'nameAndCallback',
             loc: node.loc,
           });
         }
 
-        const [name, callback] = node.arguments;
-
-        if (!isStringNode(name)) {
-          context.report({
-            messageId: 'firstArgumentMustBeName',
-            loc: paramsLocation(node.arguments),
-          });
-        }
+        const [, callback] = node.arguments;
 
         if (!callback) {
           context.report({

diff --git a/src/rules/valid-title.ts b/src/rules/valid-title.ts
@@ -25,6 +25,7 @@ export default createRule({
       recommended: false,
     },
     messages: {
+      titleMustBeString: 'Title must be a string',
       emptyTitle: '{{ jestFunctionName }} should not have an empty title',
       duplicatePrefix: 'should not have duplicate prefix',
       accidentalSpace: 'should not have leading or trailing spaces',
@@ -44,23 +45,28 @@ export default createRule({
         const [argument] = node.arguments;
 
         if (!isStringNode(argument)) {
+          if (argument.type !== AST_NODE_TYPES.TemplateLiteral) {
+            context.report({
+              messageId: 'titleMustBeString',
+              loc: argument.loc,
+            });
+          }
+
           return;
         }
 
         const title = getStringValue(argument);
 
         if (!title) {
-          if (typeof title === 'string') {
-            context.report({
-              messageId: 'emptyTitle',
-              data: {
-                jestFunctionName: isDescribe(node)
-                  ? DescribeAlias.describe
-                  : TestCaseName.test,
-              },
-              node,
-            });
-          }
+          context.report({
+            messageId: 'emptyTitle',
+            data: {
+              jestFunctionName: isDescribe(node)
+                ? DescribeAlias.describe
+                : TestCaseName.test,
+            },
+            node,
+          });
 
           return;
         }