Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
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
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-disabled-tests';

const ruleTester = new RuleTester({
const ruleTester = new TSESLint.RuleTester({
parserOptions: {
sourceType: 'module',
},
Expand All @@ -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()',
Expand Down Expand Up @@ -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 }],
Expand Down
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-jasmine-globals';

const ruleTester = new RuleTester();
const ruleTester = new TSESLint.RuleTester();

ruleTester.run('no-jasmine-globals', rule, {
valid: [
Expand Down Expand Up @@ -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() {}',
Copy link
Member

Choose a reason for hiding this comment

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

is this valid jasmine code?

Copy link
Member

Choose a reason for hiding this comment

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

I'm happy to warn on it either way

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't think so, but it's technically something that could be done. Eitherway, it triggers the previously uncovered branch :)

errors: [{ messageId: 'illegalJasmine', column: 1, line: 1 }],
},
{
code: 'jasmine.addMatchers(matchers)',
errors: [
Expand Down
14 changes: 10 additions & 4 deletions src/rules/no-disabled-tests.js → src/rules/no-disabled-tests.ts
Original file line number Diff line number Diff line change
@@ -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',
Expand All @@ -16,7 +20,9 @@ export default {
disabledTest: 'Disabled test',
},
schema: [],
type: 'suggestion',
},
defaultOptions: [],
create(context) {
let suiteDepth = 0;
let testDepth = 0;
Expand Down Expand Up @@ -72,4 +78,4 @@ export default {
},
};
},
};
});
66 changes: 41 additions & 25 deletions src/rules/no-jasmine-globals.js → src/rules/no-jasmine-globals.ts
Original file line number Diff line number Diff line change
@@ -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 }}`',
Expand All @@ -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' ||
Expand Down Expand Up @@ -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 (
Expand All @@ -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: {
Expand All @@ -87,7 +96,7 @@ export default {
messageId: 'illegalMethod',
data: {
method: calleeName,
replacement: `expect.extend`,
replacement: 'expect.extend',
},
});
return;
Expand All @@ -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' });
Expand All @@ -133,4 +149,4 @@ export default {
},
};
},
};
});
62 changes: 62 additions & 0 deletions src/rules/tsUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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;
Expand All @@ -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)
);
};
38 changes: 0 additions & 38 deletions src/rules/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand Down