Skip to content

Commit

Permalink
refactor(no-manual-cleanup): use custom rule creator (#249)
Browse files Browse the repository at this point in the history
* refactor(no-manual-cleanup): use custom rule creator

* refactor: extract detection utils for import module name

* refactor(no-manual-cleanup): use detection helpers for imported modules

* refactor(no-manual-cleanup): small improvements

* test(no-manual-cleanup): include more variants

* feat(no-manual-cleanup): report custom module

* refactor: rename type for import module node

* refactor: use node utils to know node type

* refactor: remove unused imports

* refactor: remove outdated comment line
  • Loading branch information
Belco90 authored Nov 4, 2020
1 parent eb17456 commit 287ca77
Show file tree
Hide file tree
Showing 7 changed files with 205 additions and 139 deletions.
23 changes: 13 additions & 10 deletions lib/detect-testing-library-utils.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils';
import { isLiteral } from './node-utils';
import { getImportModuleName, isLiteral, ImportModuleNode } from './node-utils';

export type TestingLibrarySettings = {
'testing-library/module'?: string;
Expand All @@ -25,14 +25,11 @@ export type EnhancedRuleCreate<
detectionHelpers: Readonly<DetectionHelpers>
) => TRuleListener;

type ModuleImportation =
| TSESTree.ImportDeclaration
| TSESTree.CallExpression
| null;

export type DetectionHelpers = {
getTestingLibraryImportNode: () => ModuleImportation;
getCustomModuleImportNode: () => ModuleImportation;
getTestingLibraryImportNode: () => ImportModuleNode | null;
getCustomModuleImportNode: () => ImportModuleNode | null;
getTestingLibraryImportName: () => string | undefined;
getCustomModuleImportName: () => string | undefined;
getIsTestingLibraryImported: () => boolean;
getIsValidFilename: () => boolean;
canReportErrors: () => boolean;
Expand All @@ -52,8 +49,8 @@ export function detectTestingLibraryUtils<
context: TestingLibraryContext<TOptions, TMessageIds>,
optionsWithDefault: Readonly<TOptions>
): TSESLint.RuleListener => {
let importedTestingLibraryNode: ModuleImportation = null;
let importedCustomModuleNode: ModuleImportation = null;
let importedTestingLibraryNode: ImportModuleNode | null = null;
let importedCustomModuleNode: ImportModuleNode | null = null;

// Init options based on shared ESLint settings
const customModule = context.settings['testing-library/module'];
Expand All @@ -69,6 +66,12 @@ export function detectTestingLibraryUtils<
getCustomModuleImportNode() {
return importedCustomModuleNode;
},
getTestingLibraryImportName() {
return getImportModuleName(importedTestingLibraryNode);
},
getCustomModuleImportName() {
return getImportModuleName(importedCustomModuleNode);
},
/**
* Gets if Testing Library is considered as imported or not.
*
Expand Down
43 changes: 38 additions & 5 deletions lib/node-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ import {
import { RuleContext } from '@typescript-eslint/experimental-utils/dist/ts-eslint';

export function isCallExpression(
node: TSESTree.Node
node: TSESTree.Node | null | undefined
): node is TSESTree.CallExpression {
return node && node.type === AST_NODE_TYPES.CallExpression;
return node?.type === AST_NODE_TYPES.CallExpression;
}

export function isNewExpression(
Expand All @@ -17,6 +17,7 @@ export function isNewExpression(
return node && node.type === 'NewExpression';
}

// TODO: remove this one and use ASTUtils one instead
export function isIdentifier(node: TSESTree.Node): node is TSESTree.Identifier {
return node && node.type === AST_NODE_TYPES.Identifier;
}
Expand Down Expand Up @@ -69,8 +70,10 @@ export function isObjectPattern(
return node && node.type === AST_NODE_TYPES.ObjectPattern;
}

export function isProperty(node: TSESTree.Node): node is TSESTree.Property {
return node && node.type === AST_NODE_TYPES.Property;
export function isProperty(
node: TSESTree.Node | null | undefined
): node is TSESTree.Property {
return node?.type === AST_NODE_TYPES.Property;
}

export function isJSXAttribute(
Expand Down Expand Up @@ -126,6 +129,7 @@ export function hasThenProperty(node: TSESTree.Node): boolean {
);
}

// TODO: remove this one and use ASTUtils one instead
export function isAwaitExpression(
node: TSESTree.Node
): node is TSESTree.AwaitExpression {
Expand All @@ -150,6 +154,12 @@ export function isArrayExpression(
return node?.type === AST_NODE_TYPES.ArrayExpression;
}

export function isImportDeclaration(
node: TSESTree.Node | null | undefined
): node is TSESTree.ImportDeclaration {
return node?.type === AST_NODE_TYPES.ImportDeclaration;
}

export function isAwaited(node: TSESTree.Node): boolean {
return (
isAwaitExpression(node) ||
Expand All @@ -176,7 +186,7 @@ export function getVariableReferences(
): TSESLint.Scope.Reference[] {
return (
(isVariableDeclarator(node) &&
context.getDeclaredVariables(node)[0].references.slice(1)) ||
context.getDeclaredVariables(node)[0]?.references?.slice(1)) ||
[]
);
}
Expand Down Expand Up @@ -220,3 +230,26 @@ export function isRenderVariableDeclarator(

return false;
}

// TODO: extract into types file?
export type ImportModuleNode =
| TSESTree.ImportDeclaration
| TSESTree.CallExpression;

export function getImportModuleName(
node: ImportModuleNode | undefined | null
): string | undefined {
// import node of shape: import { foo } from 'bar'
if (isImportDeclaration(node) && typeof node.source.value === 'string') {
return node.source.value;
}

// import node of shape: const { foo } = require('bar')
if (
isCallExpression(node) &&
isLiteral(node.arguments[0]) &&
typeof node.arguments[0].value === 'string'
) {
return node.arguments[0].value;
}
}
2 changes: 1 addition & 1 deletion lib/rules/no-container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
}
},

CallExpression(node: TSESTree.CallExpression) {
CallExpression(node) {
if (isMemberExpression(node.callee)) {
showErrorIfChainedContainerMethod(node.callee);
} else {
Expand Down
46 changes: 13 additions & 33 deletions lib/rules/no-dom-import.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
import {
AST_NODE_TYPES,
TSESTree,
} from '@typescript-eslint/experimental-utils';
import { isIdentifier, isLiteral } from '../node-utils';
import { TSESTree } from '@typescript-eslint/experimental-utils';
import { createTestingLibraryRule } from '../create-testing-library-rule';
import { isCallExpression } from '../node-utils';

export const RULE_NAME = 'no-dom-import';
export type MessageIds = 'noDomImport' | 'noDomImportFramework';
Expand Down Expand Up @@ -40,11 +37,10 @@ export default createTestingLibraryRule<Options, MessageIds>({

create(context, [framework], helpers) {
function report(
node: TSESTree.ImportDeclaration | TSESTree.Identifier,
node: TSESTree.ImportDeclaration | TSESTree.CallExpression,
moduleName: string
) {
if (framework) {
const isRequire = isIdentifier(node) && node.name === 'require';
const correctModuleName = moduleName.replace('dom', framework);
context.report({
node,
Expand All @@ -53,18 +49,16 @@ export default createTestingLibraryRule<Options, MessageIds>({
module: correctModuleName,
},
fix(fixer) {
if (isRequire) {
const callExpression = node.parent as TSESTree.CallExpression;
const name = callExpression.arguments[0] as TSESTree.Literal;
if (isCallExpression(node)) {
const name = node.arguments[0] as TSESTree.Literal;

// Replace the module name with the raw module name as we can't predict which punctuation the user is going to use
return fixer.replaceText(
name,
name.raw.replace(moduleName, correctModuleName)
);
} else {
const importDeclaration = node as TSESTree.ImportDeclaration;
const name = importDeclaration.source;
const name = node.source;
return fixer.replaceText(
name,
name.raw.replace(moduleName, correctModuleName)
Expand All @@ -82,36 +76,22 @@ export default createTestingLibraryRule<Options, MessageIds>({

return {
'Program:exit'() {
const importName = helpers.getTestingLibraryImportName();
const importNode = helpers.getTestingLibraryImportNode();

if (!importNode) {
return;
}

// import node of shape: import { foo } from 'bar'
if (importNode.type === AST_NODE_TYPES.ImportDeclaration) {
const domModuleName = DOM_TESTING_LIBRARY_MODULES.find(
(module) => module === importNode.source.value
);
const domModuleName = DOM_TESTING_LIBRARY_MODULES.find(
(module) => module === importName
);

domModuleName && report(importNode, domModuleName);
if (!domModuleName) {
return;
}

// import node of shape: const { foo } = require('bar')
if (importNode.type === AST_NODE_TYPES.CallExpression) {
const literalNodeDomModuleName = importNode.arguments.find(
(arg) =>
isLiteral(arg) &&
typeof arg.value === 'string' &&
DOM_TESTING_LIBRARY_MODULES.includes(arg.value)
) as TSESTree.Literal;

literalNodeDomModuleName &&
report(
importNode.callee as TSESTree.Identifier,
literalNodeDomModuleName.value as string
);
}
report(importNode, domModuleName);
},
};
},
Expand Down
Loading

0 comments on commit 287ca77

Please sign in to comment.