Skip to content

Commit

Permalink
refactor(no-node-access): use new testing library rule maker (#237)
Browse files Browse the repository at this point in the history
* build: add npmrc file

Adding .npmrc file to indicate we don't want to generate package-lock
properly.

* refactor: first approach for testing library detection

* refactor: move testing library detection to high-order function

* refactor: include create-testing-library-rule

* refactor(no-node-access): use create-testing-library-rule

* test: decrease coverage threshold for utils detection

* test: decrease coverage threshold for utils detection branches

* style: add missing return type on function

* style: format with prettier properly

Apparently the regexp for formatting the files within npm command must
be passed with double quotes. More details here:
https://dev.to/gruckion/comment/b665

* docs: copied types clarification
  • Loading branch information
Belco90 committed Oct 20, 2020
1 parent 1dbb513 commit ce4770f
Show file tree
Hide file tree
Showing 11 changed files with 157 additions and 55 deletions.
1 change: 1 addition & 0 deletions .npmrc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
package-lock=false
10 changes: 8 additions & 2 deletions jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,17 @@ module.exports = {
statements: 100,
},
// TODO drop this custom threshold in v4
"./lib/node-utils.ts": {
'./lib/detect-testing-library-utils.ts': {
branches: 50,
functions: 90,
lines: 90,
statements: 90,
},
'./lib/node-utils.ts': {
branches: 90,
functions: 90,
lines: 90,
statements: 90,
}
},
},
};
38 changes: 38 additions & 0 deletions lib/create-testing-library-rule.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { ESLintUtils, TSESLint } from '@typescript-eslint/experimental-utils';
import { getDocsUrl } from './utils';
import {
detectTestingLibraryUtils,
DetectionHelpers,
} from './detect-testing-library-utils';

// These 2 types are copied from @typescript-eslint/experimental-utils
type CreateRuleMetaDocs = Omit<TSESLint.RuleMetaDataDocs, 'url'>;
type CreateRuleMeta<TMessageIds extends string> = {
docs: CreateRuleMetaDocs;
} & Omit<TSESLint.RuleMetaData<TMessageIds>, 'docs'>;

export function createTestingLibraryRule<
TOptions extends readonly unknown[],
TMessageIds extends string,
TRuleListener extends TSESLint.RuleListener = TSESLint.RuleListener
>(
config: Readonly<{
name: string;
meta: CreateRuleMeta<TMessageIds>;
defaultOptions: Readonly<TOptions>;
create: (
context: Readonly<TSESLint.RuleContext<TMessageIds, TOptions>>,
optionsWithDefault: Readonly<TOptions>,
detectionHelpers: Readonly<DetectionHelpers>
) => TRuleListener;
}>
): TSESLint.RuleModule<TMessageIds, TOptions, TRuleListener> {
const { create, ...remainingConfig } = config;

return ESLintUtils.RuleCreator(getDocsUrl)({
...remainingConfig,
create: detectTestingLibraryUtils<TOptions, TMessageIds, TRuleListener>(
create
),
});
}
65 changes: 65 additions & 0 deletions lib/detect-testing-library-utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils';

export type DetectionHelpers = {
getIsImportingTestingLibrary: () => boolean;
};

/**
* Enhances a given rule `create` with helpers to detect Testing Library utils.
*/
export function detectTestingLibraryUtils<
TOptions extends readonly unknown[],
TMessageIds extends string,
TRuleListener extends TSESLint.RuleListener = TSESLint.RuleListener
>(
ruleCreate: (
context: Readonly<TSESLint.RuleContext<TMessageIds, TOptions>>,
optionsWithDefault: Readonly<TOptions>,
detectionHelpers: Readonly<DetectionHelpers>
) => TRuleListener
) {
return (
context: Readonly<TSESLint.RuleContext<TMessageIds, TOptions>>,
optionsWithDefault: Readonly<TOptions>
): TRuleListener => {
let isImportingTestingLibrary = false;

// TODO: init here options based on shared ESLint config

// helpers for Testing Library detection
const helpers: DetectionHelpers = {
getIsImportingTestingLibrary() {
return isImportingTestingLibrary;
},
};

// instructions for Testing Library detection
const detectionInstructions: TSESLint.RuleListener = {
ImportDeclaration(node: TSESTree.ImportDeclaration) {
isImportingTestingLibrary = /testing-library/g.test(
node.source.value as string
);
},
};

// update given rule to inject Testing Library detection
const ruleInstructions = ruleCreate(context, optionsWithDefault, helpers);
const enhancedRuleInstructions = Object.assign({}, ruleInstructions);

Object.keys(detectionInstructions).forEach((instruction) => {
(enhancedRuleInstructions as TSESLint.RuleListener)[instruction] = (
node
) => {
if (instruction in detectionInstructions) {
detectionInstructions[instruction](node);
}

if (ruleInstructions[instruction]) {
return ruleInstructions[instruction](node);
}
};
});

return enhancedRuleInstructions;
};
}
4 changes: 2 additions & 2 deletions lib/node-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export function isImportSpecifier(
export function isImportNamespaceSpecifier(
node: TSESTree.Node
): node is TSESTree.ImportNamespaceSpecifier {
return node?.type === AST_NODE_TYPES.ImportNamespaceSpecifier
return node?.type === AST_NODE_TYPES.ImportNamespaceSpecifier;
}

export function isImportDefaultSpecifier(
Expand Down Expand Up @@ -145,7 +145,7 @@ export function isReturnStatement(
export function isArrayExpression(
node: TSESTree.Node
): node is TSESTree.ArrayExpression {
return node?.type === AST_NODE_TYPES.ArrayExpression
return node?.type === AST_NODE_TYPES.ArrayExpression;
}

export function isAwaited(node: TSESTree.Node): boolean {
Expand Down
20 changes: 6 additions & 14 deletions lib/rules/no-node-access.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils';
import { getDocsUrl, ALL_RETURNING_NODES } from '../utils';
import { TSESTree } from '@typescript-eslint/experimental-utils';
import { ALL_RETURNING_NODES } from '../utils';
import { isIdentifier } from '../node-utils';
import { createTestingLibraryRule } from '../create-testing-library-rule';

export const RULE_NAME = 'no-node-access';
export type MessageIds = 'noNodeAccess';
type Options = [];

export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
export default createTestingLibraryRule<Options, MessageIds>({
name: RULE_NAME,
meta: {
type: 'problem',
Expand All @@ -24,19 +25,11 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
},
defaultOptions: [],

create(context) {
let isImportingTestingLibrary = false;

function checkTestingEnvironment(node: TSESTree.ImportDeclaration) {
isImportingTestingLibrary = /testing-library/g.test(
node.source.value as string
);
}

create: (context, _, helpers) => {
function showErrorForNodeAccess(node: TSESTree.MemberExpression) {
isIdentifier(node.property) &&
ALL_RETURNING_NODES.includes(node.property.name) &&
isImportingTestingLibrary &&
helpers.getIsImportingTestingLibrary() &&
context.report({
node: node,
loc: node.property.loc.start,
Expand All @@ -45,7 +38,6 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
}

return {
['ImportDeclaration']: checkTestingEnvironment,
['ExpressionStatement MemberExpression']: showErrorForNodeAccess,
['VariableDeclarator MemberExpression']: showErrorForNodeAccess,
};
Expand Down
2 changes: 1 addition & 1 deletion lib/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,5 +128,5 @@ export {
METHODS_RETURNING_NODES,
ALL_RETURNING_NODES,
PRESENCE_MATCHERS,
ABSENCE_MATCHERS
ABSENCE_MATCHERS,
};
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@
"postbuild": "cpy README.md ./dist && cpy package.json ./dist && cpy LICENSE ./dist",
"lint": "eslint . --ext .js,.ts",
"lint:fix": "npm run lint -- --fix",
"format": "prettier --write README.md {lib,docs,tests}/**/*.{js,ts,md}",
"format:check": "prettier --check README.md {lib,docs,tests}/**/*.{js,json,yml,ts,md}",
"format": "prettier --write README.md \"{lib,docs,tests}/**/*.{js,ts,md}\"",
"format:check": "prettier --check README.md \"{lib,docs,tests}/**/*.{js,json,yml,ts,md}\"",
"test:local": "jest",
"test:ci": "jest --coverage",
"test:update": "npm run test:local -- --u",
Expand Down
12 changes: 6 additions & 6 deletions tests/lib/rules/await-async-utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ ruleTester.run(RULE_NAME, rule, {
});
`,
})),
...ASYNC_UTILS.map(asyncUtil => ({
...ASYNC_UTILS.map((asyncUtil) => ({
code: `
import { ${asyncUtil} } from '@testing-library/dom';
test('${asyncUtil} util used in with Promise.all() does not trigger an error', async () => {
Expand All @@ -131,7 +131,7 @@ ruleTester.run(RULE_NAME, rule, {
});
`,
})),
...ASYNC_UTILS.map(asyncUtil => ({
...ASYNC_UTILS.map((asyncUtil) => ({
code: `
import { ${asyncUtil} } from '@testing-library/dom';
test('${asyncUtil} util used in with Promise.all() with an await does not trigger an error', async () => {
Expand All @@ -142,7 +142,7 @@ ruleTester.run(RULE_NAME, rule, {
});
`,
})),
...ASYNC_UTILS.map(asyncUtil => ({
...ASYNC_UTILS.map((asyncUtil) => ({
code: `
import { ${asyncUtil} } from '@testing-library/dom';
test('${asyncUtil} util used in with Promise.all() with ".then" does not trigger an error', async () => {
Expand All @@ -162,7 +162,7 @@ ruleTester.run(RULE_NAME, rule, {
waitForElementToBeRemoved(() => document.querySelector('div.getOuttaHere')),
])
});
`
`,
},
{
code: `
Expand Down Expand Up @@ -191,8 +191,8 @@ ruleTester.run(RULE_NAME, rule, {
await foo().then(() => baz())
])
})
`
}
`,
},
],
invalid: [
...ASYNC_UTILS.map((asyncUtil) => ({
Expand Down
Loading

0 comments on commit ce4770f

Please sign in to comment.