From 7338362420eb4970f99be2016bb4ded5732797e3 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Sun, 12 Jan 2020 23:50:55 +1300 Subject: [PATCH] feat(valid-expect): refactor `valid-expect` linting messages (#501) --- src/rules/__tests__/valid-expect.test.ts | 42 ++++++------ src/rules/valid-expect.ts | 85 +++++++++++------------- 2 files changed, 60 insertions(+), 67 deletions(-) diff --git a/src/rules/__tests__/valid-expect.test.ts b/src/rules/__tests__/valid-expect.test.ts index b75b9b0eb..becf990af 100644 --- 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 index c5a587981..ff7cd2ecc 100644 --- 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,37 +164,37 @@ 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, }); } @@ -207,8 +202,7 @@ export default createRule<[{ alwaysAwait?: boolean }], MessageIds>({ 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 }); } }, };