Skip to content

Commit

Permalink
chore: minor tidy up of code (#603)
Browse files Browse the repository at this point in the history
* test: ensure `cwd` is restored after each test

* chore: add `eslint-plugin-eslint-config`

* chore: replace `ban-ts-ignore` with `ban-ts-comment` rule

* chore: enable `padding-line-between-statements` rule

* chore: remove some unneeded conditional checks

* chore: use correct jsdoc syntax for optional params
  • Loading branch information
G-Rath committed Jun 20, 2020
1 parent 0456d2e commit 2a8ac30
Show file tree
Hide file tree
Showing 25 changed files with 87 additions and 18 deletions.
1 change: 1 addition & 0 deletions .eslintignore
@@ -1,2 +1,3 @@
coverage/
lib/
!.eslintrc.js
16 changes: 15 additions & 1 deletion .eslintrc.js
Expand Up @@ -5,6 +5,7 @@ const globals = require('./src/globals.json');
module.exports = {
parser: require.resolve('@typescript-eslint/parser'),
extends: [
'plugin:eslint-config/rc',
'plugin:eslint-plugin/recommended',
'plugin:eslint-comments/recommended',
'plugin:node/recommended',
Expand All @@ -13,6 +14,7 @@ module.exports = {
'prettier/@typescript-eslint',
],
plugins: [
'eslint-config',
'eslint-plugin',
'eslint-comments',
'node',
Expand All @@ -30,7 +32,7 @@ module.exports = {
rules: {
'@typescript-eslint/array-type': ['error', { default: 'array-simple' }],
'@typescript-eslint/no-require-imports': 'error',
'@typescript-eslint/ban-ts-ignore': 'warn',
'@typescript-eslint/ban-ts-comment': 'warn',
'@typescript-eslint/ban-types': 'error',
'@typescript-eslint/no-unused-vars': 'error',
'eslint-comments/no-unused-disable': 'error',
Expand Down Expand Up @@ -58,6 +60,18 @@ module.exports = {
'error',
{ alphabetize: { order: 'asc' }, 'newlines-between': 'never' },
],
'padding-line-between-statements': [
'error',
{ blankLine: 'always', prev: '*', next: 'return' },
{ blankLine: 'always', prev: ['const', 'let', 'var'], next: '*' },
{
blankLine: 'any',
prev: ['const', 'let', 'var'],
next: ['const', 'let', 'var'],
},
{ blankLine: 'always', prev: 'directive', next: '*' },
{ blankLine: 'any', prev: 'directive', next: 'directive' },
],
},
overrides: [
{
Expand Down
1 change: 1 addition & 0 deletions package.json
Expand Up @@ -109,6 +109,7 @@
"eslint": "^5.1.0 || ^6.0.0",
"eslint-config-prettier": "^6.5.0",
"eslint-plugin-eslint-comments": "^3.1.2",
"eslint-plugin-eslint-config": "^1.0.2",
"eslint-plugin-eslint-plugin": "^2.0.0",
"eslint-plugin-import": "^2.20.2",
"eslint-plugin-node": "^11.0.0",
Expand Down
2 changes: 2 additions & 0 deletions src/__tests__/rules.test.ts
Expand Up @@ -36,6 +36,7 @@ describe('rules', () => {

it('should have the correct amount of rules', () => {
const { length } = ruleNames;

if (length !== numberOfRules) {
throw new Error(
`There should be exactly ${numberOfRules} rules, but there are ${length}. If you've added a new rule, please update this number.`,
Expand Down Expand Up @@ -65,6 +66,7 @@ describe('rules', () => {
allConfigRules.forEach(rule => {
const ruleNamePrefix = 'jest/';
const ruleName = rule.slice(ruleNamePrefix.length);

expect(rule.startsWith(ruleNamePrefix)).toBe(true);
expect(ruleNames).toContain(ruleName);
// eslint-disable-next-line @typescript-eslint/no-require-imports
Expand Down
2 changes: 2 additions & 0 deletions src/rules/expect-expect.ts
Expand Up @@ -31,6 +31,7 @@ function matchesAssertFunctionName(
.split('.')
.map(x => {
if (x === '**') return '[a-z\\.]*';
return x.replace(/\*/gu, '[a-z]*');
})
.join('\\.')}(\\.|$)`,
Expand Down Expand Up @@ -97,6 +98,7 @@ export default createRule<
return {
CallExpression(node) {
const name = getNodeName(node.callee);

if (name === TestCaseName.it || name === TestCaseName.test) {
unchecked.push(node);
} else if (
Expand Down
1 change: 1 addition & 0 deletions src/rules/lowercase-name.ts
Expand Up @@ -43,6 +43,7 @@ const jestFunctionName = (
allowedPrefixes: readonly string[],
) => {
const description = getStringValue(node.arguments[0]);

if (allowedPrefixes.some(name => description.startsWith(name))) {
return null;
}
Expand Down
1 change: 1 addition & 0 deletions src/rules/no-alias-methods.ts
Expand Up @@ -45,6 +45,7 @@ export default createRule({
}

const alias = matcher.name;

if (alias in methodNames) {
const canonical = methodNames[alias];

Expand Down
2 changes: 2 additions & 0 deletions src/rules/no-duplicate-hooks.ts
Expand Up @@ -24,6 +24,7 @@ export default createRule({
defaultOptions: [],
create(context) {
const hookContexts = [newHookContext()];

return {
CallExpression(node) {
if (isDescribe(node)) {
Expand All @@ -32,6 +33,7 @@ export default createRule({

if (isHook(node)) {
const currentLayer = hookContexts[hookContexts.length - 1];

currentLayer[node.callee.name] += 1;
if (currentLayer[node.callee.name] > 1) {
context.report({
Expand Down
4 changes: 3 additions & 1 deletion src/rules/no-focused-tests.ts
Expand Up @@ -24,7 +24,6 @@ interface ConcurrentExpression extends TSESTree.MemberExpressionComputedName {
const isConcurrentExpression = (
expression: TSESTree.MemberExpression,
): expression is ConcurrentExpression =>
expression.type === AST_NODE_TYPES.MemberExpression &&
isSupportedAccessor(expression.property, TestCaseProperty.concurrent) &&
!!expression.parent &&
expression.parent.type === AST_NODE_TYPES.MemberExpression;
Expand Down Expand Up @@ -72,6 +71,7 @@ export default createRule({
isCallToFocusedTestFunction(callee.object)
) {
context.report({ messageId: 'focusedTest', node: callee.object });

return;
}

Expand All @@ -83,11 +83,13 @@ export default createRule({
messageId: 'focusedTest',
node: callee.object.property,
});

return;
}

if (isCallToTestOnlyFunction(callee)) {
context.report({ messageId: 'focusedTest', node: callee.property });

return;
}
}
Expand Down
3 changes: 3 additions & 0 deletions src/rules/no-identical-title.ts
Expand Up @@ -36,13 +36,16 @@ export default createRule({
defaultOptions: [],
create(context) {
const contexts = [newDescribeContext()];

return {
CallExpression(node) {
const currentLayer = contexts[contexts.length - 1];

if (isDescribe(node)) {
contexts.push(newDescribeContext());
}
const [argument] = node.arguments;

if (!argument || !isStringNode(argument)) {
return;
}
Expand Down
5 changes: 5 additions & 0 deletions src/rules/no-jasmine-globals.ts
Expand Up @@ -67,6 +67,7 @@ export default createRule({
context.report({ node, messageId: 'illegalPending' });
break;
}

return;
}

Expand All @@ -92,6 +93,7 @@ export default createRule({
replacement: `expect.${functionName}`,
},
});

return;
}

Expand All @@ -104,6 +106,7 @@ export default createRule({
replacement: 'expect.extend',
},
});

return;
}

Expand All @@ -116,6 +119,7 @@ export default createRule({
replacement: 'jest.fn',
},
});

return;
}

Expand All @@ -141,6 +145,7 @@ export default createRule({
node,
messageId: 'illegalJasmine',
});

return;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/rules/no-large-snapshots.ts
Expand Up @@ -43,7 +43,6 @@ const reportOnViolation = (
let isWhitelisted = false;

if (
whitelistedSnapshots &&
node.type === AST_NODE_TYPES.ExpressionStatement &&
'left' in node.expression &&
isExpectMember(node.expression.left)
Expand All @@ -53,6 +52,7 @@ const reportOnViolation = (

if (whitelistedSnapshotsInFile) {
const snapshotName = getAccessorValue(node.expression.left.property);

isWhitelisted = whitelistedSnapshotsInFile.some(name => {
if (name instanceof RegExp) {
return name.test(snapshotName);
Expand Down
2 changes: 1 addition & 1 deletion src/rules/no-mocks-import.ts
Expand Up @@ -28,7 +28,7 @@ export default createRule({
create(context) {
return {
ImportDeclaration(node: TSESTree.ImportDeclaration) {
if (node.source && isMockImportLiteral(node.source)) {
if (isMockImportLiteral(node.source)) {
context.report({ node, messageId: 'noManualImport' });
}
},
Expand Down
7 changes: 7 additions & 0 deletions src/rules/no-standalone-expect.ts
Expand Up @@ -29,6 +29,7 @@ const getBlockType = (
}
if (isFunction(func) && func.parent) {
const expr = func.parent;

// arrowfunction or function expr
if (expr.type === AST_NODE_TYPES.VariableDeclarator) {
return 'function';
Expand All @@ -38,6 +39,7 @@ const getBlockType = (
return DescribeAlias.describe;
}
}

return null;
};

Expand Down Expand Up @@ -78,9 +80,11 @@ export default createRule({
CallExpression(node) {
if (isExpectCall(node)) {
const parent = callStack[callStack.length - 1];

if (!parent || parent === DescribeAlias.describe) {
context.report({ node, messageId: 'unexpectedExpect' });
}

return;
}
if (isTestCase(node)) {
Expand All @@ -92,6 +96,7 @@ export default createRule({
},
'CallExpression:exit'(node: TSESTree.CallExpression) {
const top = callStack[callStack.length - 1];

if (
(((isTestCase(node) &&
node.callee.type !== AST_NODE_TYPES.MemberExpression) ||
Expand All @@ -105,12 +110,14 @@ export default createRule({
},
BlockStatement(stmt) {
const blockType = getBlockType(stmt);

if (blockType) {
callStack.push(blockType);
}
},
'BlockStatement:exit'(stmt: TSESTree.BlockStatement) {
const blockType = getBlockType(stmt);

if (blockType && blockType === callStack[callStack.length - 1]) {
callStack.pop();
}
Expand Down
4 changes: 4 additions & 0 deletions src/rules/no-test-return-statement.ts
Expand Up @@ -20,6 +20,7 @@ const getBody = (args: TSESTree.Expression[]) => {
) {
return secondArg.body.body;
}

return [];
};

Expand All @@ -46,6 +47,7 @@ export default createRule({
const returnStmt = body.find(
t => t.type === AST_NODE_TYPES.ReturnStatement,
);

if (!returnStmt) return;

context.report({ messageId: 'noReturnValue', node: returnStmt });
Expand All @@ -55,11 +57,13 @@ export default createRule({
const testCallExpressions = getTestCallExpressionsFromDeclaredVariables(
declaredVariables,
);

if (testCallExpressions.length === 0) return;

const returnStmt = node.body.body.find(
t => t.type === AST_NODE_TYPES.ReturnStatement,
);

if (!returnStmt) return;

context.report({ messageId: 'noReturnValue', node: returnStmt });
Expand Down
1 change: 1 addition & 0 deletions src/rules/no-truthy-falsy.ts
Expand Up @@ -26,6 +26,7 @@ export default createRule({
}

const { matcher } = parseExpectCall(node);

if (!matcher || !['toBeTruthy', 'toBeFalsy'].includes(matcher.name)) {
return;
}
Expand Down
6 changes: 2 additions & 4 deletions src/rules/prefer-expect-assertions.ts
Expand Up @@ -35,9 +35,6 @@ const isExpectAssertionsOrHasAssertionsCall = (expression: TSESTree.Node) => {
);
};

const getFunctionFirstLine = (functionBody: [TSESTree.ExpressionStatement]) =>
functionBody[0] && functionBody[0].expression;

const isFirstLineExprStmt = (
functionBody: TSESTree.Statement[],
): functionBody is [TSESTree.ExpressionStatement] =>
Expand Down Expand Up @@ -81,7 +78,8 @@ export default createRule({
return;
}

const testFuncFirstLine = getFunctionFirstLine(testFuncBody);
const testFuncFirstLine = testFuncBody[0].expression;

if (!isExpectAssertionsOrHasAssertionsCall(testFuncFirstLine)) {
context.report({ messageId: 'haveExpectAssertions', node });
}
Expand Down
1 change: 1 addition & 0 deletions src/rules/prefer-hooks-on-top.ts
Expand Up @@ -17,6 +17,7 @@ export default createRule({
defaultOptions: [],
create(context) {
const hooksContext = [false];

return {
CallExpression(node) {
if (!isHook(node) && isTestCase(node)) {
Expand Down
4 changes: 3 additions & 1 deletion src/rules/prefer-to-contain.ts
Expand Up @@ -153,6 +153,7 @@ const getCommonFixes = (
openParenthesis,
];
};

// expect(array.includes(<value>)[not.]{toBe,toEqual}(<boolean>)
export default createRule({
name: __filename,
Expand Down Expand Up @@ -206,7 +207,7 @@ export default createRule({
fileName,
).map(target => fixer.remove(target));

if (modifier && modifier.name === ModifierName.not) {
if (modifier) {
return getNegationFixes(
includesCall,
modifier,
Expand All @@ -232,6 +233,7 @@ export default createRule({
sourceCode.getText(containArg),
),
);

return fixArr;
},
messageId: 'useToContain',
Expand Down
1 change: 1 addition & 0 deletions src/rules/prefer-todo.ts
Expand Up @@ -40,6 +40,7 @@ function createTodoFixer(
const testName = getNodeName(node.callee)
.split('.')
.shift();

return fixer.replaceText(node.callee, `${testName}.todo`);
}

Expand Down

0 comments on commit 2a8ac30

Please sign in to comment.