diff --git a/.npmignore b/.npmignore index 21e3149..aa723ca 100644 --- a/.npmignore +++ b/.npmignore @@ -6,3 +6,4 @@ src/ .editorconfig runTests.js tsconfig.json +yarn.lock diff --git a/CHANGELOG.md b/CHANGELOG.md index 60b1dcb..40794ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,17 @@ Change Log === +v2.0.1 +--- +* [enhancement] Added preliminary checks for `jasmine-no-lambda-expression-callbacks` rule to skip walking file if there are no jasmine +top-level statements. + v2.0.0 --- -* [enhancement] Upgrades to tslint@4 +* **BREAKING CHANGES** + * [enhancement] Upgraded to tslint@4 + +v1.0.0 +--- +* [new-rule] `import-barrels` rule added +* [new-rule] `jasmine-no-lambda-expression-callbacks` rule added diff --git a/package.json b/package.json index c8a18dd..a660b57 100644 --- a/package.json +++ b/package.json @@ -33,16 +33,16 @@ }, "homepage": "https://github.com/BendingBender/tslint-rules#readme", "dependencies": { - "lodash": "^4.16.4" + "lodash": "^4.17.3" }, "devDependencies": { - "@types/lodash": "^4.14.37", - "@types/node": "^6.0.45", + "@types/lodash": "^4.14.45", + "@types/node": "^6.0.55", "coveralls": "^2.11.14", "glob": "^7.1.1", "istanbul": "^0.4.5", "rimraf": "^2.5.4", - "tslint": "^4.0.2", + "tslint": "^4.2.0", "tslint-microsoft-contrib": "^4.0.0", "typescript": "~2.1.4", "yargs": "^6.0.0" diff --git a/runTests.js b/runTests.js index 53f497c..fe319d4 100644 --- a/runTests.js +++ b/runTests.js @@ -1,3 +1,4 @@ +#!/usr/bin/env node 'use strict'; const argv = require('yargs') @@ -27,17 +28,17 @@ glob('src/tests/**/*.ts.lint', (error, files) => { throw error; } - files - .map(file => path.dirname(file)) - .forEach(dir => { - const pathToBuildDir = path.resolve(dir, buildDir); - const pathToCoverageDir = path.resolve(dir, coverageDir); + const uniqueDirs = new Set(files.map(path.dirname)); - child_process.execSync(getTestCommand(pathToBuildDir, pathToCoverageDir), { - cwd: dir, - stdio: 'inherit' - }); + uniqueDirs.forEach(dir => { + const pathToBuildDir = path.resolve(dir, buildDir); + const pathToCoverageDir = path.resolve(dir, coverageDir); + + child_process.execSync(getTestCommand(pathToBuildDir, pathToCoverageDir), { + cwd: dir, + stdio: 'inherit' }); + }); }); function getTestCommand(buildDir, coverageDir) { diff --git a/src/importBarrelsRule.ts b/src/importBarrelsRule.ts index ae02bb7..02dc455 100644 --- a/src/importBarrelsRule.ts +++ b/src/importBarrelsRule.ts @@ -59,7 +59,7 @@ class ImportBarrelsWalker extends RuleWalker { const moduleExpressionError = this.getModuleExpressionErrorMessage(moduleExpression); if (moduleExpressionError) { - this.addFailure(this.createFailure(moduleExpression.getStart(), moduleExpression.getWidth(), moduleExpressionError)); + this.addFailureAtNode(moduleExpression, moduleExpressionError); } super.visitImportDeclaration(node); diff --git a/src/jasmineNoLambdaExpressionCallbacksRule.ts b/src/jasmineNoLambdaExpressionCallbacksRule.ts index 39a2383..725930f 100644 --- a/src/jasmineNoLambdaExpressionCallbacksRule.ts +++ b/src/jasmineNoLambdaExpressionCallbacksRule.ts @@ -1,6 +1,6 @@ import { IRuleMetadata, RuleFailure, Rules, RuleWalker, Utils } from 'tslint/lib'; -import { ArrowFunction, CallExpression, Expression, Identifier, SourceFile, SyntaxKind } from 'typescript'; -import includes = require('lodash/includes'); +import { ArrowFunction, CallExpression, Expression, SourceFile, SyntaxKind } from 'typescript'; +import { isJasmineDescribe, isJasmineIt, isJasmineSetupTeardown, isJasmineTest } from './utils/jasmineUtils'; import find = require('lodash/find'); export class Rule extends Rules.AbstractRule { @@ -33,20 +33,6 @@ export class Rule extends Rules.AbstractRule { }; public static FAILURE_STRING = "Don't use lambda expressions as callbacks to jasmine functions"; - public static JASMINE_BDD_API = [ - 'describe', - 'it', - 'fdescribe', - 'fit', - 'xdescribe', - 'xit', - ]; - public static JASMINE_SETUP_TEARDOWN = [ - 'beforeEach', - 'afterEach', - 'beforeAll', - 'afterAll', - ]; public apply(sourceFile:SourceFile):RuleFailure[] { return this.applyWithWalker(new JasmineNoLambdaExpressionCallbacksWalker(sourceFile, this.getOptions())); @@ -54,31 +40,37 @@ export class Rule extends Rules.AbstractRule { } class JasmineNoLambdaExpressionCallbacksWalker extends RuleWalker { + protected visitSourceFile(node:SourceFile) { + if (isJasmineTest(node)) { + super.visitSourceFile(node); + } + } + protected visitCallExpression(node:CallExpression) { const invalidLambdaExpression = this.getInvalidLambdaExpression(node); if (invalidLambdaExpression) { - this.addFailure(this.createFailure(invalidLambdaExpression.getStart(), invalidLambdaExpression.getWidth(), Rule.FAILURE_STRING)); + this.addFailureAtNode(invalidLambdaExpression, Rule.FAILURE_STRING); } super.visitCallExpression(node); } - private getInvalidLambdaExpression(node:CallExpression):ArrowFunction|null { + private getInvalidLambdaExpression(node:CallExpression):ArrowFunction|false { if (node.expression.kind !== SyntaxKind.Identifier) { - return null; + return false; } - const functionIdentifier = (node.expression).text; + const functionIdentifier = node.expression.getText(); const functionArgs = node.arguments; - if (includes(Rule.JASMINE_BDD_API, functionIdentifier) && functionArgs.length >= 2) { - return this.getLambdaExpressionFromArg(functionArgs[1]); - } else if (includes(Rule.JASMINE_SETUP_TEARDOWN, functionIdentifier) && functionArgs.length >= 1) { - return this.getLambdaExpressionFromArg(functionArgs[0]); + if ((isJasmineDescribe(functionIdentifier) || isJasmineIt(functionIdentifier)) && functionArgs.length > 1) { + return this.getLambdaExpressionFromArg(functionArgs[1]) || false; + } else if (isJasmineSetupTeardown(functionIdentifier) && functionArgs.length > 0) { + return this.getLambdaExpressionFromArg(functionArgs[0]) || false; } - return null; + return false; } private getLambdaExpressionFromArg(apiArg:Expression):ArrowFunction|null { diff --git a/src/tests/jasmineNoLambdaExpressionCallback/default/jasmineNoLambdaExpressionCallbackRuleDefault.ts.lint b/src/tests/jasmineNoLambdaExpressionCallback/default/jasmineNoLambdaExpressionCallbackRuleDefault.ts.lint index f16f992..d082305 100644 --- a/src/tests/jasmineNoLambdaExpressionCallback/default/jasmineNoLambdaExpressionCallbackRuleDefault.ts.lint +++ b/src/tests/jasmineNoLambdaExpressionCallback/default/jasmineNoLambdaExpressionCallbackRuleDefault.ts.lint @@ -1,3 +1,16 @@ +describe("this is ok", function() { + it("this is ok", function() {}); + it("this is ok", wrap(function() {})); + + describe("this is still ok", wrapAgain(function() {})); +}); + +// still ok +(() => {})(); + +// still ok +someOtherCallWithLambda(() => {}) + describe("this", () => { ~~~~~~~ it("is an error", function() { @@ -54,8 +67,6 @@ describe("this, too", function() { ~~~ [no-lambda-callback] }); -someOtherCallWithLambda(() => {}) - fdescribe("this, too", wrapCallback(() => { ~~~~~~~ })); @@ -73,8 +84,4 @@ describe("this, too", wrapCallback(function() { ~~~ [no-lambda-callback] })); -(() => {})(); - - - [no-lambda-callback]: Don't use lambda expressions as callbacks to jasmine functions diff --git a/src/tests/jasmineNoLambdaExpressionCallback/default/jasmineNoLambdaExpressionCallbackRuleDefaultIIFE1.ts.lint b/src/tests/jasmineNoLambdaExpressionCallback/default/jasmineNoLambdaExpressionCallbackRuleDefaultIIFE1.ts.lint new file mode 100644 index 0000000..4846ee1 --- /dev/null +++ b/src/tests/jasmineNoLambdaExpressionCallback/default/jasmineNoLambdaExpressionCallbackRuleDefaultIIFE1.ts.lint @@ -0,0 +1,6 @@ +(function test() { + describe('some test in an IIFE', () => {}); + ~~~~~~~~ [no-lambda-callback] +})(); + +[no-lambda-callback]: Don't use lambda expressions as callbacks to jasmine functions diff --git a/src/tests/jasmineNoLambdaExpressionCallback/default/jasmineNoLambdaExpressionCallbackRuleDefaultIIFE2.ts.lint b/src/tests/jasmineNoLambdaExpressionCallback/default/jasmineNoLambdaExpressionCallbackRuleDefaultIIFE2.ts.lint new file mode 100644 index 0000000..91614c3 --- /dev/null +++ b/src/tests/jasmineNoLambdaExpressionCallback/default/jasmineNoLambdaExpressionCallbackRuleDefaultIIFE2.ts.lint @@ -0,0 +1,6 @@ +(() => { + describe('some test in an IIFE', () => {}); + ~~~~~~~~ [no-lambda-callback] +})(); + +[no-lambda-callback]: Don't use lambda expressions as callbacks to jasmine functions diff --git a/src/tests/jasmineNoLambdaExpressionCallback/default/jasmineNoLambdaExpressionCallbackRuleDefaultIIFE3.ts.lint b/src/tests/jasmineNoLambdaExpressionCallback/default/jasmineNoLambdaExpressionCallbackRuleDefaultIIFE3.ts.lint new file mode 100644 index 0000000..f53b8b7 --- /dev/null +++ b/src/tests/jasmineNoLambdaExpressionCallback/default/jasmineNoLambdaExpressionCallbackRuleDefaultIIFE3.ts.lint @@ -0,0 +1,6 @@ +(function test() { + describe('some test in an other IIFE', () => {}); + ~~~~~~~~ [no-lambda-callback] +}()); + +[no-lambda-callback]: Don't use lambda expressions as callbacks to jasmine functions diff --git a/src/tests/jasmineNoLambdaExpressionCallback/default/jasmineNoLambdaExpressionCallbackRuleDefaultWrapped.ts.lint b/src/tests/jasmineNoLambdaExpressionCallback/default/jasmineNoLambdaExpressionCallbackRuleDefaultWrapped.ts.lint new file mode 100644 index 0000000..f10191d --- /dev/null +++ b/src/tests/jasmineNoLambdaExpressionCallback/default/jasmineNoLambdaExpressionCallbackRuleDefaultWrapped.ts.lint @@ -0,0 +1,6 @@ +export function test() { + describe('some test in a callback', () => {}); + ~~~~~~~~ [no-lambda-callback] +} + +[no-lambda-callback]: Don't use lambda expressions as callbacks to jasmine functions diff --git a/src/tslint.json b/src/tslint.json index a576647..aa8995c 100644 --- a/src/tslint.json +++ b/src/tslint.json @@ -12,13 +12,22 @@ } ], "no-any": false, + "no-empty-interface": true, "no-inferrable-types": true, "no-internal-module": true, + "no-magic-numbers": [ + true, + 0, + -1, + 1, + 1000 + ], "no-namespace": true, "no-reference": true, "no-var-requires": true, "only-arrow-functions": false, "prefer-for-of": true, + "promise-function-async": false, "typedef": false, "typedef-whitespace": [ true, @@ -40,6 +49,7 @@ "ban": false, "curly": true, "forin": true, + "import-blacklist": false, "label-position": true, "no-arg": true, "no-bitwise": true, @@ -57,8 +67,10 @@ "no-duplicate-variable": true, "no-empty": true, "no-eval": true, - // no-for-in-array requires type info + // "no-for-in-array" requires type info "no-for-in-array": false, + // "no-inferred-empty-object-type" requires type info + "no-inferred-empty-object-type": false, "no-invalid-this": [ true, "check-function-in-method" @@ -66,14 +78,18 @@ "no-null-keyword": false, "no-shadowed-variable": false, "no-string-literal": false, + "no-string-throw": true, "no-switch-case-fall-through": true, "no-unsafe-finally": true, "no-unused-expression": true, "no-unused-new": true, "no-use-before-declare": true, "no-var-keyword": true, + // "no-void-expression" requires type info + "no-void-expression": false, "radix": true, "restrict-plus-operands": false, + "strict-boolean-expressions": false, "switch-default": true, "triple-equals": [ true, @@ -106,13 +122,18 @@ "no-require-imports": false, "no-trailing-whitespace": true, "object-literal-sort-keys": false, + "prefer-const": true, "trailing-comma": true, "align": false, "array-type": [ true, "array" ], - "arrow-parens": false, + "arrow-parens": [ + true, + "ban-single-arg-parens" + ], + "callable-types": true, "class-name": true, "comment-format": [ true, @@ -124,6 +145,7 @@ true, "never-prefix" ], + "interface-over-type-literal": true, "jsdoc-format": true, "new-parens": true, "no-angle-bracket-type-assertion": false, @@ -237,7 +259,6 @@ ], "possible-timing-attack": false, "prefer-array-literal": true, - "prefer-const": true, "prefer-type-cast": true, "promise-must-complete": true, "react-iframe-missing-sandbox": false, diff --git a/src/utils/jasmineUtils.ts b/src/utils/jasmineUtils.ts new file mode 100644 index 0000000..95e73a4 --- /dev/null +++ b/src/utils/jasmineUtils.ts @@ -0,0 +1,79 @@ +import { + Block, + CallExpression, + Expression, + ExpressionStatement, + FunctionDeclaration, + SourceFile, + Statement, + SyntaxKind +} from 'typescript'; +import includes = require('lodash/includes'); +import { getIIFEExpressionBody } from './languageUtils'; + +const JASMINE_DESCRIBE = [ + 'describe', + 'fdescribe', + 'xdescribe', +]; +const JASMINE_IT = [ + 'it', + 'fit', + 'xit', +]; +const JASMINE_SETUP_TEARDOWN = [ + 'beforeEach', + 'afterEach', + 'beforeAll', + 'afterAll', +]; + +export function isJasmineTest(node:SourceFile):boolean { + return includesJasmineTopLevelCalls(node.statements) || node.statements.find(isStatementWithWrappedJasmineCalls) != null; +} + +export function isJasmineDescribe(callExpressionText:string):boolean { + return includes(JASMINE_DESCRIBE, callExpressionText); +} + +export function isJasmineIt(callExpressionText:string):boolean { + return includes(JASMINE_IT, callExpressionText); +} + +export function isJasmineSetupTeardown(callExpressionText:string):boolean { + return includes(JASMINE_SETUP_TEARDOWN, callExpressionText); +} + +function isStatementWithWrappedJasmineCalls(statement:Statement):boolean { + let functionBody:Block|null|undefined; + + if (statement.kind === SyntaxKind.FunctionDeclaration) { + functionBody = (statement).body; + } else if (statement.kind === SyntaxKind.ExpressionStatement) { + functionBody = getIIFEExpressionBody((statement).expression); + } + + if (functionBody != null) { + return includesJasmineTopLevelCalls(functionBody.statements); + } + + return false; +} + +function includesJasmineTopLevelCalls(statements:Statement[]):boolean { + return statements.find(statement => isStatementJasmineTopLevelCall(statement)) != null; +} + +function isStatementJasmineTopLevelCall(statement:Statement):boolean { + if (statement.kind === SyntaxKind.ExpressionStatement) { + const expression:Expression = (statement).expression; + + if (expression.kind === SyntaxKind.CallExpression) { + const call:CallExpression = expression; + const expressionText:string = call.expression.getText(); + return isJasmineDescribe(expressionText) || isJasmineSetupTeardown(expressionText); + } + } + + return false; +} diff --git a/src/utils/languageUtils.ts b/src/utils/languageUtils.ts new file mode 100644 index 0000000..95f9cfe --- /dev/null +++ b/src/utils/languageUtils.ts @@ -0,0 +1,30 @@ +import { unwrapParentheses } from 'tslint'; +import { + Block, + CallExpression, + Expression, + FunctionExpression, + SyntaxKind +} from 'typescript'; + +export function getIIFEExpressionBody(expression:Expression):Block|null { + if (expression.kind === SyntaxKind.CallExpression) { + const callExpression = expression; + if (callExpression.expression.kind === SyntaxKind.ParenthesizedExpression) { + const parensContentExpression = unwrapParentheses(callExpression.expression); + if (parensContentExpression.kind === SyntaxKind.FunctionExpression || parensContentExpression.kind === SyntaxKind.ArrowFunction) { + return (parensContentExpression).body; + } + } + } else if (expression.kind === SyntaxKind.ParenthesizedExpression) { + const parensContentExpression = unwrapParentheses(expression); + if (parensContentExpression.kind === SyntaxKind.CallExpression) { + const calleeExpression = (parensContentExpression).expression; + if (calleeExpression.kind === SyntaxKind.FunctionExpression) { + return (calleeExpression).body; + } + } + } + + return null; +} diff --git a/tsconfig.json b/tsconfig.json index 3d53790..68cfc94 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -14,6 +14,7 @@ "noEmitHelpers": false, "noFallthroughCasesInSwitch": true, "noImplicitAny": true, + "noImplicitThis": true, "noImplicitReturns": true, "noUnusedLocals": true, "noUnusedParameters": true, diff --git a/yarn.lock b/yarn.lock index 6af8577..b1181df 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2,13 +2,13 @@ # yarn lockfile v1 -"@types/lodash@^4.14.37": - version "4.14.43" - resolved "https://registry.yarnpkg.com/@types/lodash/-/lodash-4.14.43.tgz#0373de422e70f5bce12c15340103a3c126c81086" +"@types/lodash@^4.14.45": + version "4.14.45" + resolved "https://registry.yarnpkg.com/@types/lodash/-/lodash-4.14.45.tgz#db20238f23fa195d6fb8f9ff7988e8651fc8c302" -"@types/node@^6.0.45": - version "6.0.52" - resolved "https://registry.yarnpkg.com/@types/node/-/node-6.0.52.tgz#1ac3a99b42320f9e463482f25af4c2359473aaa6" +"@types/node@^6.0.55": + version "6.0.55" + resolved "https://registry.yarnpkg.com/@types/node/-/node-6.0.55.tgz#e5cb679a43561f42afd1bd6d58d3992ec8f31720" abbrev@1, abbrev@1.0.x: version "1.0.9" @@ -78,6 +78,14 @@ aws4@^1.2.1: version "1.5.0" resolved "https://registry.yarnpkg.com/aws4/-/aws4-1.5.0.tgz#0a29ffb79c31c9e712eeb087e8e7a64b4a56d755" +babel-code-frame@^6.20.0: + version "6.20.0" + resolved "https://registry.yarnpkg.com/babel-code-frame/-/babel-code-frame-6.20.0.tgz#b968f839090f9a8bc6d41938fb96cb84f7387b26" + dependencies: + chalk "^1.1.0" + esutils "^2.0.2" + js-tokens "^2.0.0" + balanced-match@^0.4.1: version "0.4.2" resolved "https://registry.yarnpkg.com/balanced-match/-/balanced-match-0.4.2.tgz#cb3f3e3c732dc0f01ee70b403f302e61d7709838" @@ -152,7 +160,7 @@ center-align@^0.1.1: align-text "^0.1.3" lazy-cache "^1.0.3" -chalk@^1.0.0, chalk@^1.1.1: +chalk@^1.0.0, chalk@^1.1.0, chalk@^1.1.1: version "1.1.3" resolved "https://registry.yarnpkg.com/chalk/-/chalk-1.1.3.tgz#a8115c55e4a702fe4d150abd3872822a7e09fc98" dependencies: @@ -618,6 +626,10 @@ jodid25519@^1.0.0: dependencies: jsbn "~0.1.0" +js-tokens@^2.0.0: + version "2.0.0" + resolved "https://registry.yarnpkg.com/js-tokens/-/js-tokens-2.0.0.tgz#79903f5563ee778cc1162e6dcf1a0027c97f9cb5" + js-yaml@3.6.1, js-yaml@3.x: version "3.6.1" resolved "https://registry.yarnpkg.com/js-yaml/-/js-yaml-3.6.1.tgz#6e5fe67d8b205ce4d22fad05b7781e8dadcc4b30" @@ -696,9 +708,9 @@ load-json-file@^1.0.0: pinkie-promise "^2.0.0" strip-bom "^2.0.0" -lodash@^4.16.4: - version "4.17.2" - resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.2.tgz#34a3055babe04ce42467b607d700072c7ff6bf42" +lodash@^4.17.3: + version "4.17.3" + resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.3.tgz#557ed7d2a9438cac5fd5a43043ca60cb455e01f7" log-driver@1.2.5: version "1.2.5" @@ -1128,18 +1140,15 @@ tough-cookie@~2.3.0: dependencies: punycode "^1.4.1" -tslib@^1.2.0: - version "1.2.0" - resolved "https://registry.yarnpkg.com/tslib/-/tslib-1.2.0.tgz#074fb171034594bdba4c4dbb197c26e4a0b88383" - tslint-microsoft-contrib@^4.0.0: version "4.0.0" resolved "https://registry.yarnpkg.com/tslint-microsoft-contrib/-/tslint-microsoft-contrib-4.0.0.tgz#7e00a56c1417251d3f91a880efcc43361a69fb2a" -tslint@^4.0.2: - version "4.0.2" - resolved "https://registry.yarnpkg.com/tslint/-/tslint-4.0.2.tgz#d43f24c0c1f826de7f3a097bb7808a8b4325feac" +tslint@^4.2.0: + version "4.2.0" + resolved "https://registry.yarnpkg.com/tslint/-/tslint-4.2.0.tgz#b9f5c5b871b784ab2f4809e704ade42d62f523ad" dependencies: + babel-code-frame "^6.20.0" colors "^1.1.2" diff "^3.0.1" findup-sync "~0.3.0"