eslint-community / eslint-plugin-n

Additional ESLint rules for Node.js
MIT License
231 stars 41 forks source link

Rule idea: enforce first arg of `assert.x` cannot be a string #69

Open ErisDS opened 1 year ago

ErisDS commented 1 year ago

Opening this up to discuss the idea and naming, not to request someone else implement it - I'm happy to do that.

Today I noticed across several projects that I maintain that the order of the arguments passed to Node's assert method are often confused.

Take assert.equal(actual, expected[, message]) for example... the docs clearly show the first argument should be the actual value, and the second the expected value.

The impact of getting them the wrong way around is very minor - but it breaks my brain when trying to debug a test and staring at this output where the prompt says the expected value should start with a plus and be green, but in reality that's the actual value:

image

I think this is very easy to enforce by ensuring that the first param passed to the assert functions is not a string.

Soooo what to call this rule? I'm thinking: params-order-assert Slightly confusing because assert is referring to the module, not enforcing the rule...

aladdin-add commented 1 year ago

I think this is very easy to enforce by ensuring that the first param passed to the assert functions is not a string.

hi, sorry for the late! "first param passed....is not a string" - do you mean it cannot be a literal value - e.g. true/false/numbers/strings/....?

I'm open to the idea, let's see what others think! /cc @eslint-community/eslint-plugin-node

scagood commented 1 year ago

It may be worth checking that the first value is an Identifier, (or a CallExpression).

In terms of checks, I mostly use:

import { equal } from 'node:assert/strict';

equal(
  funcToTest(),
  'expected',
);

I am happy to work on whatever is decided :)