Skip to content

Move rules settings to ESLint shared config: part 4 - refactor no-dom-import rule #247

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.

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 13 commits into from
Nov 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
coverage/
dist/
4 changes: 2 additions & 2 deletions .lintstagedrc
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
{
"*.{js,ts}": [
"*.{js,ts}": [
"eslint --fix",
"prettier --write",
"jest --findRelatedTests",
"jest --findRelatedTests"
],
"*.md": ["prettier --write"]
}
101 changes: 75 additions & 26 deletions lib/detect-testing-library-utils.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils';
import { isLiteral } from './node-utils';

export type TestingLibrarySettings = {
'testing-library/module'?: string;
'testing-library/file-name'?: string;
'testing-library/filename-pattern'?: string;
Copy link
Member Author

Choose a reason for hiding this comment

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

Renamed to filename-pattern as discussed in previous PR.

};

export type TestingLibraryContext<
Expand All @@ -24,13 +25,20 @@ export type EnhancedRuleCreate<
detectionHelpers: Readonly<DetectionHelpers>
) => TRuleListener;

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

export type DetectionHelpers = {
getTestingLibraryImportNode: () => ModuleImportation;
getCustomModuleImportNode: () => ModuleImportation;
getIsTestingLibraryImported: () => boolean;
getIsValidFileName: () => boolean;
getIsValidFilename: () => boolean;
canReportErrors: () => boolean;
};

const DEFAULT_FILE_NAME_PATTERN = '^.*\\.(test|spec)\\.[jt]sx?$';
const DEFAULT_FILENAME_PATTERN = '^.*\\.(test|spec)\\.[jt]sx?$';

/**
* Enhances a given rule `create` with helpers to detect Testing Library utils.
Expand All @@ -44,17 +52,23 @@ export function detectTestingLibraryUtils<
context: TestingLibraryContext<TOptions, TMessageIds>,
optionsWithDefault: Readonly<TOptions>
): TSESLint.RuleListener => {
let isImportingTestingLibraryModule = false;
let isImportingCustomModule = false;
let importedTestingLibraryNode: ModuleImportation = null;
let importedCustomModuleNode: ModuleImportation = null;

// Init options based on shared ESLint settings
const customModule = context.settings['testing-library/module'];
const fileNamePattern =
context.settings['testing-library/file-name'] ??
DEFAULT_FILE_NAME_PATTERN;
const filenamePattern =
context.settings['testing-library/filename-pattern'] ??
DEFAULT_FILENAME_PATTERN;

// Helpers for Testing Library detection.
const helpers: DetectionHelpers = {
getTestingLibraryImportNode() {
return importedTestingLibraryNode;
},
getCustomModuleImportNode() {
return importedCustomModuleNode;
},
/**
* Gets if Testing Library is considered as imported or not.
*
Expand All @@ -72,50 +86,85 @@ export function detectTestingLibraryUtils<
return true;
}

return isImportingTestingLibraryModule || isImportingCustomModule;
return !!importedTestingLibraryNode || !!importedCustomModuleNode;
},

/**
* Gets if name of the file being analyzed is valid or not.
* Gets if filename being analyzed is valid or not.
*
* This is based on "testing-library/file-name" setting.
* This is based on "testing-library/filename-pattern" setting.
*/
getIsValidFileName() {
getIsValidFilename() {
const fileName = context.getFilename();
return !!fileName.match(fileNamePattern);
return !!fileName.match(filenamePattern);
},

/**
* Wraps all conditions that must be met to report rules.
*/
canReportErrors() {
return this.getIsTestingLibraryImported() && this.getIsValidFileName();
return this.getIsTestingLibraryImported() && this.getIsValidFilename();
},
};

// Instructions for Testing Library detection.
const detectionInstructions: TSESLint.RuleListener = {
/**
* This ImportDeclaration rule listener will check if Testing Library related
* modules are loaded. Since imports happen first thing in a file, it's
* modules are imported. Since imports happen first thing in a file, it's
* safe to use `isImportingTestingLibraryModule` and `isImportingCustomModule`
* since they will have corresponding value already updated when reporting other
* parts of the file.
*/
ImportDeclaration(node: TSESTree.ImportDeclaration) {
if (!isImportingTestingLibraryModule) {
// check only if testing library import not found yet so we avoid
// to override isImportingTestingLibraryModule after it's found
isImportingTestingLibraryModule = /testing-library/g.test(
node.source.value as string
);
// check only if testing library import not found yet so we avoid
// to override importedTestingLibraryNode after it's found
if (
!importedTestingLibraryNode &&
/testing-library/g.test(node.source.value as string)
) {
importedTestingLibraryNode = node;
}

// check only if custom module import not found yet so we avoid
// to override importedCustomModuleNode after it's found
if (
!importedCustomModuleNode &&
String(node.source.value).endsWith(customModule)
) {
importedCustomModuleNode = node;
}
},

// Check if Testing Library related modules are loaded with required.
[`CallExpression > Identifier[name="require"]`](
node: TSESTree.Identifier
) {
const callExpression = node.parent as TSESTree.CallExpression;
const { arguments: args } = callExpression;

if (
!importedTestingLibraryNode &&
args.some(
(arg) =>
isLiteral(arg) &&
typeof arg.value === 'string' &&
/testing-library/g.test(arg.value)
)
) {
importedTestingLibraryNode = callExpression;
}

if (!isImportingCustomModule) {
// check only if custom module import not found yet so we avoid
// to override isImportingCustomModule after it's found
const importName = String(node.source.value);
isImportingCustomModule = importName.endsWith(customModule);
if (
!importedCustomModuleNode &&
args.some(
(arg) =>
isLiteral(arg) &&
typeof arg.value === 'string' &&
arg.value.endsWith(customModule)
)
) {
importedCustomModuleNode = callExpression;
}
},
};
Expand Down
6 changes: 4 additions & 2 deletions lib/node-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,10 @@ export function isMemberExpression(
return node && node.type === AST_NODE_TYPES.MemberExpression;
}

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

export function isImportSpecifier(
Expand Down
60 changes: 34 additions & 26 deletions lib/rules/no-dom-import.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils';
import { getDocsUrl } from '../utils';
import { isLiteral, isIdentifier } from '../node-utils';
import {
AST_NODE_TYPES,
TSESTree,
} from '@typescript-eslint/experimental-utils';
import { isIdentifier, isLiteral } from '../node-utils';
import { createTestingLibraryRule } from '../create-testing-library-rule';

export const RULE_NAME = 'no-dom-import';
export type MessageIds = 'noDomImport' | 'noDomImportFramework';
Expand All @@ -11,7 +14,7 @@ const DOM_TESTING_LIBRARY_MODULES = [
'@testing-library/dom',
];

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

create(context, [framework]) {
create(context, [framework], helpers) {
function report(
node: TSESTree.ImportDeclaration | TSESTree.Identifier,
moduleName: string
Expand Down Expand Up @@ -76,33 +79,38 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
});
}
}

return {
ImportDeclaration(node) {
const value = node.source.value;
const domModuleName = DOM_TESTING_LIBRARY_MODULES.find(
(module) => module === value
);
'Program:exit'() {
const importNode = helpers.getTestingLibraryImportNode();

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

[`CallExpression > Identifier[name="require"]`](
node: TSESTree.Identifier
) {
const callExpression = node.parent as TSESTree.CallExpression;
const { arguments: args } = callExpression;
// 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
);

domModuleName && report(importNode, domModuleName);
}

const literalNodeDomModuleName = args.find(
(args) =>
isLiteral(args) &&
typeof args.value === 'string' &&
DOM_TESTING_LIBRARY_MODULES.includes(args.value)
) as TSESTree.Literal;
// 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;

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