feat(valid-expect): refactor valid-expect linting messages (#501)

jest-community · Jan 12, 2020 · 7338362 · 7338362
1 parent 6570863
commit 7338362
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 67 deletions.
diff --git a/src/rules/__tests__/valid-expect.test.ts b/src/rules/__tests__/valid-expect.test.ts
@@ -99,25 +99,31 @@ ruleTester.run('valid-expect', rule, {
      */
     {
       code: 'expect().toBe(true);',
-      errors: [{ endColumn: 8, column: 7, messageId: 'noArgs' }],
+      errors: [
+        { endColumn: 8, column: 7, messageId: 'incorrectNumberOfArguments' },
+      ],
     },
     {
       code: 'expect().toEqual("something");',
-      errors: [{ endColumn: 8, column: 7, messageId: 'noArgs' }],
+      errors: [
+        { endColumn: 8, column: 7, messageId: 'incorrectNumberOfArguments' },
+      ],
     },
     {
       code: 'expect("something", "else").toEqual("something");',
-      errors: [{ endColumn: 26, column: 21, messageId: 'multipleArgs' }],
+      errors: [
+        { endColumn: 26, column: 21, messageId: 'incorrectNumberOfArguments' },
+      ],
     },
     {
       code: 'expect("something");',
-      errors: [{ endColumn: 20, column: 1, messageId: 'noAssertions' }],
+      errors: [{ endColumn: 20, column: 1, messageId: 'matcherNotFound' }],
     },
     {
       code: 'expect();',
       errors: [
-        { endColumn: 9, column: 1, messageId: 'noAssertions' },
-        { endColumn: 8, column: 7, messageId: 'noArgs' },
+        { endColumn: 9, column: 1, messageId: 'matcherNotFound' },
+        { endColumn: 8, column: 7, messageId: 'incorrectNumberOfArguments' },
       ],
     },
     {
@@ -126,8 +132,7 @@ ruleTester.run('valid-expect', rule, {
         {
           endColumn: 25,
           column: 14,
-          messageId: 'matcherOnPropertyNotCalled',
-          data: { propertyName: 'toBeDefined' },
+          messageId: 'matcherNotCalled',
         },
       ],
     },
@@ -137,8 +142,7 @@ ruleTester.run('valid-expect', rule, {
         {
           endColumn: 29,
           column: 18,
-          messageId: 'matcherOnPropertyNotCalled',
-          data: { propertyName: 'toBeDefined' },
+          messageId: 'matcherNotCalled',
         },
       ],
     },
@@ -148,8 +152,8 @@ ruleTester.run('valid-expect', rule, {
         {
           endColumn: 18,
           column: 14,
-          messageId: 'invalidProperty',
-          data: { propertyName: 'nope' },
+          messageId: 'modifierUnknown',
+          data: { modifierName: 'nope' },
         },
       ],
     },
@@ -159,8 +163,7 @@ ruleTester.run('valid-expect', rule, {
         {
           endColumn: 22,
           column: 14,
-          messageId: 'propertyWithoutMatcher',
-          data: { propertyName: 'resolves' },
+          messageId: 'matcherNotFound',
         },
       ],
     },
@@ -170,8 +173,7 @@ ruleTester.run('valid-expect', rule, {
         {
           endColumn: 21,
           column: 14,
-          messageId: 'propertyWithoutMatcher',
-          data: { propertyName: 'rejects' },
+          messageId: 'matcherNotFound',
         },
       ],
     },
@@ -181,8 +183,7 @@ ruleTester.run('valid-expect', rule, {
         {
           endColumn: 17,
           column: 14,
-          messageId: 'propertyWithoutMatcher',
-          data: { propertyName: 'not' },
+          messageId: 'matcherNotFound',
         },
       ],
     },
@@ -538,8 +539,7 @@ ruleTester.run('valid-expect', rule, {
         {
           column: 37,
           endColumn: 41,
-          messageId: 'matcherOnPropertyNotCalled',
-          data: { propertyName: 'toBe' },
+          messageId: 'matcherNotCalled',
         },
       ],
     },
@@ -582,7 +582,7 @@ ruleTester.run('valid-expect', rule, {
       code: `test("valid-expect", async () => {
         await expect(Promise.resolve(1));
       });`,
-      errors: [{ endColumn: 41, column: 15, messageId: 'noAssertions' }],
+      errors: [{ endColumn: 41, column: 15, messageId: 'matcherNotFound' }],
     },
   ],
 });
diff --git a/src/rules/valid-expect.ts b/src/rules/valid-expect.ts
@@ -100,12 +100,10 @@ const promiseArrayExceptionKey = ({ start, end }: TSESTree.SourceLocation) =>
   `${start.line}:${start.column}-${end.line}:${end.column}`;
 
 type MessageIds =
-  | 'multipleArgs'
-  | 'noArgs'
-  | 'noAssertions'
-  | 'invalidProperty'
-  | 'propertyWithoutMatcher'
-  | 'matcherOnPropertyNotCalled'
+  | 'incorrectNumberOfArguments'
+  | 'modifierUnknown'
+  | 'matcherNotFound'
+  | 'matcherNotCalled'
   | 'asyncMustBeAwaited'
   | 'promisesWithAsyncAssertionsMustBeAwaited';
 
@@ -118,13 +116,10 @@ export default createRule<[{ alwaysAwait?: boolean }], MessageIds>({
       recommended: 'error',
     },
     messages: {
-      multipleArgs: 'More than one argument was passed to expect().',
-      noArgs: 'No arguments were passed to expect().',
-      noAssertions: 'No assertion was called on expect().',
-      invalidProperty:
-        '"{{ propertyName }}" is not a valid property of expect.',
-      propertyWithoutMatcher: '"{{ propertyName }}" needs to call a matcher.',
-      matcherOnPropertyNotCalled: '"{{ propertyName }}" was not called.',
+      incorrectNumberOfArguments: 'Expect takes one and only one argument.',
+      modifierUnknown: 'Expect has no modifier named "{{ modifierName }}".',
+      matcherNotFound: 'Expect must have a corresponding matcher call.',
+      matcherNotCalled: 'Matchers must be called to assert.',
       asyncMustBeAwaited: 'Async assertions must be awaited{{ orReturned }}.',
       promisesWithAsyncAssertionsMustBeAwaited:
         'Promises which return async assertions must be awaited{{ orReturned }}.',
@@ -169,46 +164,45 @@ export default createRule<[{ alwaysAwait?: boolean }], MessageIds>({
 
         const { expect, modifier, matcher } = parseExpectCall(node);
 
-        if (expect.arguments.length > 1) {
-          const secondArgumentLocStart = expect.arguments[1].loc.start;
-          const lastArgumentLocEnd =
-            expect.arguments[node.arguments.length - 1].loc.end;
+        if (expect.arguments.length !== 1) {
+          const expectLength = getAccessorValue(expect.callee).length;
 
-          context.report({
-            loc: {
-              end: {
-                column: lastArgumentLocEnd.column - 1,
-                line: lastArgumentLocEnd.line,
-              },
-              start: secondArgumentLocStart,
+          let loc: TSESTree.SourceLocation = {
+            start: {
+              column: node.loc.start.column + expectLength,
+              line: node.loc.start.line,
             },
-            messageId: 'multipleArgs',
-            node,
-          });
-        } else if (expect.arguments.length === 0) {
-          const expectLength = getAccessorValue(expect.callee).length;
-          context.report({
-            loc: {
+            end: {
+              column: node.loc.start.column + expectLength + 1,
+              line: node.loc.start.line,
+            },
+          };
+
+          if (expect.arguments.length !== 0) {
+            const { start } = expect.arguments[1].loc;
+            const { end } = expect.arguments[node.arguments.length - 1].loc;
+
+            loc = {
+              start,
               end: {
-                column: node.loc.start.column + expectLength + 1,
-                line: node.loc.start.line,
-              },
-              start: {
-                column: node.loc.start.column + expectLength,
-                line: node.loc.start.line,
+                column: end.column - 1,
+                line: end.line,
               },
-            },
-            messageId: 'noArgs',
+            };
+          }
+
+          context.report({
+            messageId: 'incorrectNumberOfArguments',
             node,
+            loc,
           });
         }
 
         // something was called on `expect()`
         if (!matcher) {
           if (modifier) {
             context.report({
-              data: { propertyName: modifier.name }, // todo: rename to 'modifierName'
-              messageId: 'propertyWithoutMatcher', // todo: rename to 'modifierWithoutMatcher'
+              messageId: 'matcherNotFound',
               node: modifier.node.property,
             });
           }
@@ -218,8 +212,8 @@ export default createRule<[{ alwaysAwait?: boolean }], MessageIds>({
 
         if (matcher.node.parent && isExpectMember(matcher.node.parent)) {
           context.report({
-            messageId: 'invalidProperty', // todo: rename to 'invalidModifier'
-            data: { propertyName: matcher.name }, // todo: rename to 'matcherName' (or modifierName?)
+            messageId: 'modifierUnknown',
+            data: { modifierName: matcher.name },
             node: matcher.node.property,
           });
 
@@ -228,8 +222,7 @@ export default createRule<[{ alwaysAwait?: boolean }], MessageIds>({
 
         if (!matcher.arguments) {
           context.report({
-            data: { propertyName: matcher.name }, // todo: rename to 'matcherName'
-            messageId: 'matcherOnPropertyNotCalled', // todo: rename to 'matcherNotCalled'
+            messageId: 'matcherNotCalled',
             node: matcher.node.property,
           });
         }
@@ -287,7 +280,7 @@ export default createRule<[{ alwaysAwait?: boolean }], MessageIds>({
       // nothing called on "expect()"
       'CallExpression:exit'(node: TSESTree.CallExpression) {
         if (isExpectCall(node) && isNoAssertionsParentNode(node.parent)) {
-          context.report({ messageId: 'noAssertions', node });
+          context.report({ messageId: 'matcherNotFound', node });
         }
       },
     };