diff --git a/package.json b/package.json index 5ddd1ee1..9873f197 100644 --- a/package.json +++ b/package.json @@ -62,10 +62,13 @@ "mocha" ], "dependencies": { + "acorn": "^3.0.4", "deep-strict-equal": "^0.1.0", "espree": "^3.1.3", + "estraverse": "^4.2.0", "espurify": "^1.5.0", "multimatch": "^2.1.0", + "num-lines": "^1.0.0", "object-assign": "^4.0.1", "pkg-up": "^1.0.0" }, diff --git a/rules/no-commented-test.js b/rules/no-commented-test.js new file mode 100644 index 00000000..80a1f0bb --- /dev/null +++ b/rules/no-commented-test.js @@ -0,0 +1,112 @@ +'use strict'; +var acorn = require('acorn'); +var acornLoose = require('acorn/dist/acorn_loose'); +var estraverse = require('estraverse'); +var numLines = require('num-lines'); +var createAvaRule = require('../create-ava-rule'); + +module.exports = function (context) { + var processedUpTo = -1; + + function concatComments(commentBlock) { + var endOfLastComment = commentBlock[commentBlock.length - 1].end; + + // Comments are often attached as "trailing" on one node, and "leading" on the next. + // Avoid double processing the same block. + if (processedUpTo >= endOfLastComment) { + return null; + } + + processedUpTo = endOfLastComment; + + return commentBlock.map(function (commentNode) { + return commentNode.value; + }).join('\n'); + } + + function process(commentBlock) { + if (!commentBlock || !commentBlock.length) { + return; + } + + var src = concatComments(commentBlock); + + // Does a best effort parse of the code. AST will contain `x` Identifiers for chunks it can't parse. + var ast = acornLoose.parse_dammit(src, {locations: true}); + + // Traverse the contents of the comment block, looking for a test call. + estraverse.traverse(ast, { + enter: function (node) { + if (node.type === 'CallExpression' && isTestFunctionCall(node.callee)) { + try { + // Strict parse - don't report if the commented out test contains syntax errors + acorn.parse(src.substring(node.start, node.end)); + + var loc = translateLocation(node.loc.start, commentBlock); + + context.report({ + message: 'use test.skip instead of commenting out tests', + loc: loc + }); + } catch (e) {} + } + } + }); + } + + var ava = createAvaRule(); + + function visitNode(node) { + if (!ava.isTestFile) { + return; + } + + process(node.leadingComments); + process(node.trailingComments); + } + + // build the visitor object, we want to asses the comments around any XxxStatement node. + var visitor = {}; + + Object.keys(estraverse.Syntax).forEach(function (name) { + if (/Statement$/.test(name)) { + visitor[name] = visitNode; + } + }); + + return ava.merge(visitor); +}; + +function translateLocation(locationInComments, commentBlock) { + var linesLeft = locationInComments.line; + + for (var i = 0; i < commentBlock.length; i++) { + var commentNode = commentBlock[i]; + var linesThisComment = numLines(commentNode.value); + + if (linesThisComment >= linesLeft) { + return { + line: commentNode.loc.start.line + linesLeft - 1, + column: linesLeft === 1 ? + commentNode.loc.start.column + locationInComments.column + 2 : + locationInComments.column + }; + } + + linesLeft -= linesThisComment; + } + + // TODO: Probably should not throw here. Maybe return `null` and modify the message warning the exact location could not be found? + // This should be unreachable code, so would like to encourage bug reports if that ever proves untrue. + throw new Error('unable to translate location'); +} + +function isTestFunctionCall(node) { + if (node.type === 'Identifier') { + return node.name === 'test'; + } else if (node.type === 'MemberExpression') { + return isTestFunctionCall(node.object); + } + + return false; +} diff --git a/test/no-commented-tests.js b/test/no-commented-tests.js new file mode 100644 index 00000000..85b6615d --- /dev/null +++ b/test/no-commented-tests.js @@ -0,0 +1,48 @@ +import test from 'ava'; +import {RuleTester} from 'eslint'; +import rule from '../rules/no-commented-test'; + +const ruleTester = new RuleTester({ + env: { + es6: true + } +}); + +function errorAt(line, column) { + return { + ruleId: 'no-commented-tests', + line, + column + }; +} +const header = `const test = require('ava');\n`; + +test(() => { + ruleTester.run('no-commented-tests', rule, { + valid: [ + header + 'test("my test name", t => { t.pass(); });', + header + 'something(); // Is this a test(..)', + header + 'something(); // Some sentence ending with the word test (followed by parens)', + // shouldn't be triggered since it's not a test file + 'test.cb(t => {});\n// test.cb(t=> {})' + ], + invalid: [ + { + code: header + 'test.cb(t => { t.pass(); });\n// test.cb(t=> {})', + errors: [errorAt(3, 4)] + }, + { + code: header + 'test.cb(t => { t.pass(); });\n // test.cb(t=> {})', + errors: [errorAt(3, 5)] + }, + { + code: header + 'test.cb(t => { t.pass(); });\n// test.cb(t=> {})', + errors: [errorAt(3, 5)] + }, + { + code: header + 'test.cb(t => { t.pass(); });\n// This is not javascript!\n// test.cb(t=> {})', + errors: [errorAt(4, 4)] + } + ] + }); +});