diff --git a/src/rules/__tests__/no-disabled-tests.test.js b/src/rules/__tests__/no-disabled-tests.test.ts similarity index 91% rename from src/rules/__tests__/no-disabled-tests.test.js rename to src/rules/__tests__/no-disabled-tests.test.ts index 0ed3929a6..49fea2bbc 100644 --- a/src/rules/__tests__/no-disabled-tests.test.js +++ b/src/rules/__tests__/no-disabled-tests.test.ts @@ -1,7 +1,7 @@ -import { RuleTester } from 'eslint'; +import { TSESLint } from '@typescript-eslint/experimental-utils'; import rule from '../no-disabled-tests'; -const ruleTester = new RuleTester({ +const ruleTester = new TSESLint.RuleTester({ parserOptions: { sourceType: 'module', }, @@ -15,6 +15,7 @@ ruleTester.run('no-disabled-tests', rule, { 'it.only("foo", function () {})', 'test("foo", function () {})', 'test.only("foo", function () {})', + 'describe[`${"skip"}`]("foo", function () {})', 'var appliedSkip = describe.skip; appliedSkip.apply(describe)', 'var calledSkip = it.skip; calledSkip.call(it)', '({ f: function () {} }).f()', @@ -57,6 +58,10 @@ ruleTester.run('no-disabled-tests', rule, { code: 'describe.skip("foo", function () {})', errors: [{ messageId: 'skippedTestSuite', column: 1, line: 1 }], }, + { + code: 'describe[`skip`]("foo", function () {})', + errors: [{ messageId: 'skippedTestSuite', column: 1, line: 1 }], + }, { code: 'describe["skip"]("foo", function () {})', errors: [{ messageId: 'skippedTestSuite', column: 1, line: 1 }], diff --git a/src/rules/__tests__/no-jasmine-globals.test.js b/src/rules/__tests__/no-jasmine-globals.test.ts similarity index 94% rename from src/rules/__tests__/no-jasmine-globals.test.js rename to src/rules/__tests__/no-jasmine-globals.test.ts index 2d73156ca..254c62d30 100644 --- a/src/rules/__tests__/no-jasmine-globals.test.js +++ b/src/rules/__tests__/no-jasmine-globals.test.ts @@ -1,7 +1,7 @@ -import { RuleTester } from 'eslint'; +import { TSESLint } from '@typescript-eslint/experimental-utils'; import rule from '../no-jasmine-globals'; -const ruleTester = new RuleTester(); +const ruleTester = new TSESLint.RuleTester(); ruleTester.run('no-jasmine-globals', rule, { valid: [ @@ -53,6 +53,10 @@ ruleTester.run('no-jasmine-globals', rule, { errors: [{ messageId: 'illegalJasmine', column: 1, line: 1 }], output: 'jest.setTimeout(5000);', }, + { + code: 'jasmine.DEFAULT_TIMEOUT_INTERVAL = function() {}', + errors: [{ messageId: 'illegalJasmine', column: 1, line: 1 }], + }, { code: 'jasmine.addMatchers(matchers)', errors: [ diff --git a/src/rules/no-disabled-tests.js b/src/rules/no-disabled-tests.ts similarity index 87% rename from src/rules/no-disabled-tests.js rename to src/rules/no-disabled-tests.ts index 38fa7ec80..1937d7fcc 100644 --- a/src/rules/no-disabled-tests.js +++ b/src/rules/no-disabled-tests.ts @@ -1,9 +1,13 @@ -import { getDocsUrl, getNodeName, scopeHasLocalReference } from './util'; +import { createRule } from './tsUtils'; +import { getNodeName, scopeHasLocalReference } from './tsUtils'; -export default { +export default createRule({ + name: __filename, meta: { docs: { - url: getDocsUrl(__filename), + category: 'Best Practices', + description: 'Disallow disabled tests', + recommended: false, }, messages: { missingFunction: 'Test is missing function argument', @@ -16,7 +20,9 @@ export default { disabledTest: 'Disabled test', }, schema: [], + type: 'suggestion', }, + defaultOptions: [], create(context) { let suiteDepth = 0; let testDepth = 0; @@ -72,4 +78,4 @@ export default { }, }; }, -}; +}); diff --git a/src/rules/no-jasmine-globals.js b/src/rules/no-jasmine-globals.ts similarity index 68% rename from src/rules/no-jasmine-globals.js rename to src/rules/no-jasmine-globals.ts index e3a617dce..5cc6a191d 100644 --- a/src/rules/no-jasmine-globals.js +++ b/src/rules/no-jasmine-globals.ts @@ -1,11 +1,14 @@ -import { getDocsUrl, getNodeName, scopeHasLocalReference } from './util'; +import { AST_NODE_TYPES } from '@typescript-eslint/experimental-utils'; +import { createRule, getNodeName, scopeHasLocalReference } from './tsUtils'; -export default { +export default createRule({ + name: __filename, meta: { docs: { - url: getDocsUrl(__filename), + category: 'Best Practices', + description: 'Disallow Jasmine globals', + recommended: 'error', }, - fixable: 'code', messages: { illegalGlobal: 'Illegal usage of global `{{ global }}`, prefer `{{ replacement }}`', @@ -17,16 +20,21 @@ export default { 'Illegal usage of `pending`, prefer explicitly skipping a test using `test.skip`', illegalJasmine: 'Illegal usage of jasmine global', }, + fixable: 'code', schema: [], + type: 'suggestion', }, + defaultOptions: [], create(context) { return { CallExpression(node) { - const calleeName = getNodeName(node.callee); + const { callee } = node; + const calleeName = getNodeName(callee); if (!calleeName) { return; } + if ( calleeName === 'spyOn' || calleeName === 'spyOnProperty' || @@ -57,7 +65,10 @@ export default { return; } - if (calleeName.startsWith('jasmine.')) { + if ( + callee.type === AST_NODE_TYPES.MemberExpression && + calleeName.startsWith('jasmine.') + ) { const functionName = calleeName.replace('jasmine.', ''); if ( @@ -68,9 +79,7 @@ export default { functionName === 'stringMatching' ) { context.report({ - fix(fixer) { - return [fixer.replaceText(node.callee.object, 'expect')]; - }, + fix: fixer => [fixer.replaceText(callee.object, 'expect')], node, messageId: 'illegalMethod', data: { @@ -87,7 +96,7 @@ export default { messageId: 'illegalMethod', data: { method: calleeName, - replacement: `expect.extend`, + replacement: 'expect.extend', }, }); return; @@ -109,22 +118,29 @@ export default { } }, MemberExpression(node) { - if (node.object.name === 'jasmine') { - if (node.parent.type === 'AssignmentExpression') { - if (node.property.name === 'DEFAULT_TIMEOUT_INTERVAL') { - context.report({ - fix(fixer) { - return [ + if ('name' in node.object && node.object.name === 'jasmine') { + const { parent, property } = node; + + if (parent && parent.type === AST_NODE_TYPES.AssignmentExpression) { + if ( + 'name' in property && + property.name === 'DEFAULT_TIMEOUT_INTERVAL' + ) { + const { right } = parent; + + if (right.type === AST_NODE_TYPES.Literal) { + context.report({ + fix: fixer => [ fixer.replaceText( - node.parent, - `jest.setTimeout(${node.parent.right.value})`, + parent, + `jest.setTimeout(${right.value})`, ), - ]; - }, - node, - messageId: 'illegalJasmine', - }); - return; + ], + node, + messageId: 'illegalJasmine', + }); + return; + } } context.report({ node, messageId: 'illegalJasmine' }); @@ -133,4 +149,4 @@ export default { }, }; }, -}; +}); diff --git a/src/rules/tsUtils.ts b/src/rules/tsUtils.ts index 91aaf4779..73d9af889 100644 --- a/src/rules/tsUtils.ts +++ b/src/rules/tsUtils.ts @@ -3,6 +3,7 @@ import { basename } from 'path'; import { AST_NODE_TYPES, ESLintUtils, + TSESLint, TSESTree, } from '@typescript-eslint/experimental-utils'; import { version } from '../../package.json'; @@ -20,6 +21,26 @@ enum DescribeAlias { 'xdescribe', } +export const getNodeName = (node: TSESTree.Node): string | null => { + function joinNames(a?: string | null, b?: string | null): string | null { + return a && b ? `${a}.${b}` : null; + } + + switch (node.type) { + case AST_NODE_TYPES.Identifier: + return node.name; + case AST_NODE_TYPES.Literal: + return `${node.value}`; + case AST_NODE_TYPES.TemplateLiteral: + if (node.expressions.length === 0) return node.quasis[0].value.cooked; + break; + case AST_NODE_TYPES.MemberExpression: + return joinNames(getNodeName(node.object), getNodeName(node.property)); + } + + return null; +}; + export type FunctionExpression = | TSESTree.ArrowFunctionExpression | TSESTree.FunctionExpression; @@ -41,3 +62,44 @@ export const isDescribe = (node: TSESTree.CallExpression): boolean => { export const isLiteralNode = (node: { type: AST_NODE_TYPES; }): node is TSESTree.Literal => node.type === AST_NODE_TYPES.Literal; + +const collectReferences = (scope: TSESLint.Scope.Scope) => { + const locals = new Set(); + const unresolved = new Set(); + + let currentScope: TSESLint.Scope.Scope | null = scope; + + while (currentScope !== null) { + for (const ref of currentScope.variables) { + const isReferenceDefined = ref.defs.some(def => { + return def.type !== 'ImplicitGlobalVariable'; + }); + + if (isReferenceDefined) { + locals.add(ref.name); + } + } + + for (const ref of currentScope.through) { + unresolved.add(ref.identifier.name); + } + + currentScope = currentScope.upper; + } + + return { locals, unresolved }; +}; + +export const scopeHasLocalReference = ( + scope: TSESLint.Scope.Scope, + referenceName: string, +) => { + const references = collectReferences(scope); + return ( + // referenceName was found as a local variable or function declaration. + references.locals.has(referenceName) || + // referenceName was not found as an unresolved reference, + // meaning it is likely not an implicit global reference. + !references.unresolved.has(referenceName) + ); +}; diff --git a/src/rules/util.js b/src/rules/util.js index 4d1eaecd4..19da4db42 100644 --- a/src/rules/util.js +++ b/src/rules/util.js @@ -172,44 +172,6 @@ export const getDocsUrl = filename => { return `${REPO_URL}/blob/v${version}/docs/rules/${ruleName}.md`; }; -const collectReferences = scope => { - const locals = new Set(); - const unresolved = new Set(); - - let currentScope = scope; - - while (currentScope !== null) { - for (const ref of currentScope.variables) { - const isReferenceDefined = ref.defs.some(def => { - return def.type !== 'ImplicitGlobalVariable'; - }); - - if (isReferenceDefined) { - locals.add(ref.name); - } - } - - for (const ref of currentScope.through) { - unresolved.add(ref.identifier.name); - } - - currentScope = currentScope.upper; - } - - return { locals, unresolved }; -}; - -export const scopeHasLocalReference = (scope, referenceName) => { - const references = collectReferences(scope); - return ( - // referenceName was found as a local variable or function declaration. - references.locals.has(referenceName) || - // referenceName was not found as an unresolved reference, - // meaning it is likely not an implicit global reference. - !references.unresolved.has(referenceName) - ); -}; - export function composeFixers(node) { return (...fixers) => { return fixerApi => {