Skip to content

Commit

Permalink
Prevent incorrect detection of AVA functions when accessing through "…
Browse files Browse the repository at this point in the history
…context" (#231)

Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>
  • Loading branch information
Guillaume Martigny and sindresorhus committed May 14, 2019
1 parent b8549df commit 521d009
Show file tree
Hide file tree
Showing 14 changed files with 48 additions and 25 deletions.
4 changes: 2 additions & 2 deletions rules/assertion-arguments.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,8 @@ const create = context => {
if (
callee.type !== 'MemberExpression' ||
!callee.property ||
util.nameOfRootObject(callee) !== 't' ||
util.isInContext(callee)
util.getNameOfRootNodeObject(callee) !== 't' ||
util.isPropertyUnderContext(callee)
) {
return;
}
Expand Down
2 changes: 1 addition & 1 deletion rules/assertion-message.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const create = context => {
return;
}

if (callee.property && util.nameOfRootObject(callee) === 't') {
if (callee.property && util.getNameOfRootNodeObject(callee) === 't') {
const nArgs = nbArguments(callee);

if (nArgs === -1) {
Expand Down
2 changes: 1 addition & 1 deletion rules/max-asserts.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const create = context => {
if (
callee.property &&
!notAssertionMethods.includes(callee.property.name) &&
util.nameOfRootObject(callee) === 't'
util.getNameOfRootNodeObject(callee) === 't'
) {
const members = util.getMembers(callee).filter(name => name !== 'skip');

Expand Down
4 changes: 2 additions & 2 deletions rules/no-invalid-end.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ const create = context => {
])(node => {
if (
node.property.name === 'end' &&
!ava.hasTestModifier('cb') &&
util.nameOfRootObject(node) === 't'
node.object.name === 't' &&
!ava.hasTestModifier('cb')
) {
context.report({
node,
Expand Down
13 changes: 8 additions & 5 deletions rules/no-skip-assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,14 @@ const create = context => {
ava.isInTestFile,
ava.isInTestNode
])(node => {
if (node.property.name === 'skip' && util.nameOfRootObject(node) === 't') {
context.report({
node,
message: 'No assertions should be skipped.'
});
if (node.property.name === 'skip') {
const root = util.getRootNode(node);
if (root.object.name === 't' && util.assertionMethods.has(root.property.name)) {
context.report({
node,
message: 'No assertions should be skipped.'
});
}
}
})
});
Expand Down
2 changes: 1 addition & 1 deletion rules/use-t-well.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ const create = context => {
ava.isInTestNode
])(node => {
if (node.parent.type === 'MemberExpression' ||
util.nameOfRootObject(node) !== 't') {
util.getNameOfRootNodeObject(node) !== 't') {
return;
}

Expand Down
2 changes: 1 addition & 1 deletion rules/use-true-false.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ const create = context => {
if (
node.callee.type === 'MemberExpression' &&
(node.callee.property.name === 'truthy' || node.callee.property.name === 'falsy') &&
util.nameOfRootObject(node.callee) === 't'
node.callee.object.name === 't'
) {
const arg = node.arguments[0];

Expand Down
2 changes: 2 additions & 0 deletions test/assertion-arguments.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ ruleTester.run('assertion-arguments', rule, {
testCase(false, 't.true(true, \'message\');'),
testCase(false, 't.truthy(\'unicorn\', \'message\');'),
testCase(false, 't.snapshot(value, \'message\');'),
testCase(false, 't.context.plan();'),
testCase(false, 'foo.t.plan();'),
// Shouldn't be triggered since it's not a test file
testCase(false, 't.true(true);', false, false),

Expand Down
2 changes: 2 additions & 0 deletions test/max-asserts.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ ruleTester.run('max-asserts', rule, {
options: [2]
},
`${header} test(t => { t.context.bar(); ${nbAssertions(5)} });`,
`${header} test(t => { ${'t.context.is(1, 1); '.repeat(6)}});`,
`${header} test(t => { ${'foo.t.is(1, 1); '.repeat(6)}});`,
// Shouldn't be triggered since it's not a test file
`test(t => { ${nbAssertions(10)} });`
],
Expand Down
2 changes: 2 additions & 0 deletions test/no-invalid-end.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ ruleTester.run('no-invalid-end', rule, {
header + 'test.cb(t => { t.end.skip(); });',
header + 'test.cb.only(t => { t.end(); });',
header + 'notTest(t => { t.end(); });',
header + 'test(t => { t.context.end(); })',
header + 'test(t => { foo.t.end(); })',
// Shouldn't be triggered since it's not a test file
'test(t => { t.end(); });'
],
Expand Down
16 changes: 12 additions & 4 deletions test/no-skip-assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,21 +15,29 @@ ruleTester.run('no-skip-assert', rule, {
valid: [
header + 'test(t => { t.is(1, 1); });',
header + 'test.skip(t => { t.is(1, 1); });',
// Not an actual AVA skip
header + 'test(t => { notT.skip.is(1, 1); });',
header + 'test(t => { t.context.is.skip(1, 1); });',
header + 'test(t => { foo.t.is.skip(1, 1); });',
header + 'test(t => { t.skip(); });',
// Shouldn't be triggered since it's not a test file
'test(t => { t.skip.is(1, 1); });'
'test(t => { t.is.skip(1, 1); });'
],
invalid: [
{
code: header + 'test(t => { t.skip.is(1, 1); });',
code: header + 'test(t => { t.is.skip(1, 1); });',
errors
},
{
code: header + 'test.cb(t => { t.skip.is(1, 1); t.end(); });',
code: header + 'test(t => { t.true.skip(1); });',
errors
},
{
code: header + 'test.skip(t => { t.skip.is(1, 1); });',
code: header + 'test.cb(t => { t.is.skip(1, 1); t.end(); });',
errors
},
{
code: header + 'test.skip(t => { t.is.skip(1, 1); });',
errors
}
]
Expand Down
2 changes: 2 additions & 0 deletions test/use-t-well.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ ruleTester.run('use-t-well', rule, {
testCase('t.plan(1);'),
testCase('t.log(\'Unicorns\');'),
testCase('a.foo();'),
testCase('t.context.foo(a, a);'),
testCase('foo.t.bar(a, a);'),
// Shouldn't be triggered since it's not a test file
testCase('t.foo(a, a);', false),
testCase('t.foo;', false)
Expand Down
4 changes: 4 additions & 0 deletions test/use-true-false.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ ruleTester.run('use-true-false', rule, {
testCase('t.falsy(value + value)'),
testCase('t.truthy()'),
testCase('t.falsy()'),
testCase('t.context.truthy(true)'),
testCase('t.context.falsy(false)'),
testCase('foo.t.truthy(true)'),
testCase('foo.t.falsy(false)'),
// Shouldn't be triggered since it's not a test file
testCase('t.truthy(value === 1)', false)
],
Expand Down
16 changes: 8 additions & 8 deletions util.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,20 +22,20 @@ const defaultFiles = [
'**/*.test.js'
];

exports.nameOfRootObject = node => {
exports.getRootNode = node => {
if (node.object.type === 'MemberExpression') {
return exports.nameOfRootObject(node.object);
return exports.getRootNode(node.object);
}

return node.object.name;
return node;
};

exports.isInContext = node => {
if (node.object.type === 'MemberExpression') {
return exports.isInContext(node.object);
}
exports.getNameOfRootNodeObject = node => {
return exports.getRootNode(node).object.name;
};

return node.property.name === 'context';
exports.isPropertyUnderContext = node => {
return exports.getRootNode(node).property.name === 'context';
};

const NO_SUCH_FILE = Symbol('no ava.config.js file');
Expand Down

0 comments on commit 521d009

Please sign in to comment.