Make writing "within a test" and "t.XXX" matchers easier:

[jamestalmage]

We have this code construct everywhere:

CallExpression: function (node) {
  // only check if we are within a test file, and if this CallExpression is within a test.
  if (!ava.isTestFile || !ava.currentTestNode) {
    return;
  }
}

I propose we add some sugar to ava.merge() that allows something like this:

'CallExpression:inTest': function (node) {

},
'VariableDeclaration:inTestFile': function (node) {

},
'CallExpression:inTest:inTestFile': function (node) {

},

While we are at it, we should define one (specifically for CallExpression), that checks the callee is a MemberExpression of the form t.XXX:

'AssertionCallExpression:inTest': function (node) {

}

[jamestalmage]

Counterpoint: Does our API change often enough that these sorts of helpers would be worth the effort it took to create?

Err wait - this is the linter, not codemods. Can't keep all the AVA related AST projects straight.

[sindresorhus]

// @twada

[jfmengels]

Agree with the idea, but I'm not sure it's a good idea. I noticed that we don't handle cases like this

function implementation(t) {
  t.doWhatever();
}

test(implementation);

though we might want to, especially with the talk about "macros" in avajs/ava#695. In this case, the "in test" part will pretty much always be wrong.

Otherwise, I think we don't need to add the inTest suffix, we could make ava.isTestFile && ava.currentTestNode a necessary condition for the visitor to be called (they all check that as far as I can tell).

We can definitely add the ava.isTestFile condition everywhere though (not ava.currentTestNode), and the AssertionCallExpression seems like a good idea too.

[twada]

Hmm, I think that's out of scope of static analysis tool, it's better and easier to detect and report at runtime.

[sindresorhus]

@twada What specifically is out of scope?

[twada]

What specifically is out of scope?

Chasing some kind of macros, renamed variables, destructuring, etc.
They tend to make static analysis tools fragile.
(It's just my opinion)

[jfmengels]

What do you guys think of specifying a simple filter function instead?

From

CallExpression: function (node) {
  // only check if we are within a test file, and if this CallExpression is within a test.
  if (!ava.isTestFile || !ava.currentTestNode) {
    return;
  }
}

to

// exported by create-ava-rule for instance
function ifInAVATest(fn) {
  return function(node) {
    if (ava.isTestFile && ava.currentTestNode) {
      return fn(node);
    }
  };
}

// test file
CallExpression: ifInAVATest(function (node) {
  // do stuff
})

No new sugar, just explicit functions, and they're chainable too if needed.

[jfmengels]

Even better: just have some predicate functions and a visitor filter function

// create-ava-rule.js
function isAssertion(node) {
  return node.type === 'CallExpression' && ...
}

function callIf(...fns) {
  return function(visitor) {
    return function (node) {
      // some kind of
      if (fns.every((fn) => fn(node)) {
        visitor(node);
      }
    };
  };
}

// test file
CallExpression: callIf(ava.isAssertion, ava.fooBar)(function (node) {
  // do stuff
  // I can still use ava.isAssertion programmatically
})

[sindresorhus]

@jfmengels I like it. @jamestalmage ?

[twada]

@jfmengels Wow it looks nice :)

[jamestalmage]

👍