Rule proposal: Enforce the use of either .exec() or .then() on DB queries

[jfmengels]

Rule proposal: Enforce the use of either .exec() or .then() on DB queries

Objective:

• Avoid mistakes where you use Bluebird methods like tap/nodeify on queries

To figure out that e.g. User is a database model, and thus that User.find() is a query call, we'll consider anything to be a database model if it is the result of a call to db.model('...') or <anything>.db.model('...'), in order to take into account calls like context.db.model('user') or job.db.model('user').

Bad

const User = context.db.model('user'); // User is a db model

User.find({}); // Result is not used, and not executed

function foo() {
  return User.find({}); // Result is returned but not executed. Forcing the caller to execute the Promise, which is dangereous
}

// Use of a non-query method before a `then` / `exec` call
const userP = User.find({}).tap(fn);
const userP = User.find({}).nodeify(fn);

// Detect temporary assignment in variable (much harder to enforce)
const userP = User.find({});
const sortedUserP.sort(order);
sortedUserP.tap(fn);

Good

// Stat is not considered as a DB model, so it does not get reported
const Stat = {};
Stat.find({}).nodeify(fn);

const Stat = model('stat');
Stat.find({}).nodeify(fn);

const User = context.db.model('user'); // User is a db model

User.find({}); // Result is not used, and not executed

function foo() {
  return User.find({}).exec();
}
function foo() {
  return User.find({}).then(fn);
}

const userP = User.find({}).exec().tap(fn);
const userP = User.find({}).then(fn1).nodeify(fn2);

// Use of known query modifiers before execution
const userP = User.find({}).sort(order).exec().tap(fn);

// Allow temporary assignment in variable (much harder to enforce)
const userP = User.find({});
const sortedUserP.sort(order);
sortedUserP.exec().tap(fn);

cc @lcalvy @godu