Skip to content

ts-migration/no-if #324

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 20, 2019
Merged

ts-migration/no-if #324

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bfa0bb4
chore(no-if): convert to typescript
G-Rath Jul 20, 2019
e2d8fa2
chore(no-if): narrow parameter type of `validate`
G-Rath Jul 20, 2019
9ff0da2
chore(no-if): remove unneeded checks from `isTestArrowFunction` fn
G-Rath Jul 20, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/rules/__tests__/no-if.js → src/rules/__tests__/no-if.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { RuleTester } from 'eslint';
import { TSESLint } from '@typescript-eslint/experimental-utils';
import rule from '../no-if';

const ruleTester = new RuleTester({
const ruleTester = new TSESLint.RuleTester({
parserOptions: {
ecmaVersion: 6,
},
Expand Down
42 changes: 30 additions & 12 deletions src/rules/no-if.js → src/rules/no-if.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,36 +1,54 @@
import { getDocsUrl, isTestCase } from './util';
import { TestCaseName, createRule, getNodeName, isTestCase } from './tsUtils';
import {
AST_NODE_TYPES,
TSESTree,
} from '@typescript-eslint/experimental-utils';

const isTestArrowFunction = node =>
node !== undefined &&
node.type === 'ArrowFunctionExpression' &&
isTestCase(node.parent);
const testCaseNames = new Set<string | null>([
Copy link
Member

@SimenB SimenB Jul 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

null? seems wrong

Copy link
Collaborator Author

@G-Rath G-Rath Jul 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that's just b/c of how the typings work in relation to #has.

Basically, it's defined like this:

interface Set<T> {
  has(item: T): boolean;
}

That means if T is just string, you can't do has checks against things like null, which is a possible return of getNodeName :)

Originally I had getNodeName(/* ... */) || '', but since that's a runtime solution, it's expected to be tested for by branch coverage 😂

...Object.keys(TestCaseName),
'it.only',
'it.skip',
'test.only',
'test.skip',
]);

export default {
const isTestArrowFunction = (node: TSESTree.ArrowFunctionExpression) =>
node.parent !== undefined &&
node.parent.type === AST_NODE_TYPES.CallExpression &&
testCaseNames.has(getNodeName(node.parent.callee));

export default createRule({
name: __filename,
meta: {
docs: {
description: 'Disallow conditional logic',
category: 'Best Practices',
recommended: false,
uri: getDocsUrl('jest/no-if'),
},
messages: {
noIf: 'Tests should not contain if statements.',
noConditional: 'Tests should not contain conditional statements.',
},
schema: [],
type: 'suggestion',
},

defaultOptions: [],
create(context) {
const stack = [];
const stack: Array<boolean> = [];

function validate(node) {
function validate(
node: TSESTree.ConditionalExpression | TSESTree.IfStatement,
) {
const lastElementInStack = stack[stack.length - 1];

if (stack.length === 0 || lastElementInStack === false) {
return;
}

const messageId =
node.type === 'ConditionalExpression' ? 'noConditional' : 'noIf';
node.type === AST_NODE_TYPES.ConditionalExpression
? 'noConditional'
: 'noIf';

context.report({
messageId,
Expand Down Expand Up @@ -67,4 +85,4 @@ export default {
},
};
},
};
});