Skip to content

Add consistent-assert rule #2535

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 21 commits into from
Jan 24, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 51 additions & 0 deletions docs/rules/consistent-assert.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
# Enforce consistent assertion style with `node:assert`

💼 This rule is enabled in the ✅ `recommended` [config](https://github.com/sindresorhus/eslint-plugin-unicorn#preset-configs-eslintconfigjs).

🔧 This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix).

<!-- end auto-generated rule header -->
<!-- Do not manually modify this header. Run: `npm run fix:eslint-docs` -->

Prefer `assert.ok()` over `assert()` for its explicit intent and better readability. It aligns with other assert methods, ensuring consistency and making code easier to maintain and understand.

## Examples

```js
import assert from 'node:assert/strict';

assert.strictEqual(actual, expected);
assert.deepStrictEqual(actual, expected);

// ❌
assert(divide(10, 2) === 5); // Inconsistent with other API styles

// ✅
assert.ok(divide(10, 2) === 5);
```

```js
import assert from 'node:assert';

assert.strictEqual(actual, expected);
assert.deepStrictEqual(actual, expected);

// ❌
assert(divide(10, 2) === 5); // Inconsistent with other API styles

// ✅
assert.ok(divide(10, 2) === 5);
```

```js
import {strict as assert} from 'node:assert';

assert.strictEqual(actual, expected);
assert.deepStrictEqual(actual, expected);

// ❌
assert(divide(10, 2) === 5); // Inconsistent with other API styles

// ✅
assert.ok(divide(10, 2) === 5);
```
1 change: 1 addition & 0 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ export default [
| :----------------------------------------------------------------------------------------------- | :---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | :- | :- | :- |
| [better-regex](docs/rules/better-regex.md) | Improve regexes by making them shorter, consistent, and safer. | | 🔧 | |
| [catch-error-name](docs/rules/catch-error-name.md) | Enforce a specific parameter name in catch clauses. | ✅ | 🔧 | |
| [consistent-assert](docs/rules/consistent-assert.md) | Enforce consistent assertion style with `node:assert`. | ✅ | 🔧 | |
| [consistent-destructuring](docs/rules/consistent-destructuring.md) | Use destructured variables over properties. | | 🔧 | 💡 |
| [consistent-empty-array-spread](docs/rules/consistent-empty-array-spread.md) | Prefer consistent types when spreading a ternary in an array literal. | ✅ | 🔧 | |
| [consistent-existence-index-check](docs/rules/consistent-existence-index-check.md) | Enforce consistent style for element existence checks with `indexOf()`, `lastIndexOf()`, `findIndex()`, and `findLastIndex()`. | ✅ | 🔧 | |
Expand Down
98 changes: 98 additions & 0 deletions rules/consistent-assert.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
const MESSAGE_ID_ERROR = 'consistent-assert/error';
const messages = {
[MESSAGE_ID_ERROR]: 'Prefer `{{name}}.ok(…)` over `{{name}}(…)`.',
};

/**
@param {import('estree').ImportSpecifier | import('estree').ImportDefaultSpecifier | import('estree').ImportSpecifier | import('estree').ImportDeclaration} node
*/
const isValueImport = node => !node.importKind || node.importKind === 'value';

/**
Check if a specifier is `assert` function.

@param {import('estree').ImportSpecifier | import('estree').ImportDefaultSpecifier} specifier
@param {string} moduleName
*/
const isAssertFunction = (specifier, moduleName) =>
// `import assert from 'node:assert';`
// `import assert from 'node:assert/strict';`
specifier.type === 'ImportDefaultSpecifier'
// `import {default as assert} from 'node:assert';`
// `import {default as assert} from 'node:assert/strict';`
|| (
specifier.type === 'ImportSpecifier'
&& specifier.imported.name === 'default'
)
// `import {strict as assert} from 'node:assert';`
|| (
moduleName === 'assert'
&& specifier.type === 'ImportSpecifier'
&& specifier.imported.name === 'strict'
);

const NODE_PROTOCOL = 'node:';

/** @type {import('eslint').Rule.RuleModule['create']} */
const create = context => ({
* ImportDeclaration(importDeclaration) {
if (!isValueImport(importDeclaration)) {
return;
}

let moduleName = importDeclaration.source.value;

if (moduleName.startsWith(NODE_PROTOCOL)) {
moduleName = moduleName.slice(NODE_PROTOCOL.length);
}

if (moduleName !== 'assert' && moduleName !== 'assert/strict') {
return;
}

for (const specifier of importDeclaration.specifiers) {
if (!isValueImport(specifier) || !isAssertFunction(specifier, moduleName)) {
continue;
}

const variables = context.sourceCode.getDeclaredVariables(specifier);

/* c8 ignore next 3 */
if (!Array.isArray(variables) && variables.length === 1) {
continue;
}

const [variable] = variables;

for (const {identifier} of variable.references) {
if (!(identifier.parent.type === 'CallExpression' && identifier.parent.callee === identifier)) {
continue;
}

yield {
node: identifier,
messageId: MESSAGE_ID_ERROR,
data: {name: identifier.name},
/** @param {import('eslint').Rule.RuleFixer} fixer */
fix: fixer => fixer.insertTextAfter(identifier, '.ok'),
};
}
}
},
});

/** @type {import('eslint').Rule.RuleModule} */
const config = {
create,
meta: {
type: 'problem',
docs: {
description: 'Enforce consistent assertion style with `node:assert`.',
recommended: true,
},
fixable: 'code',
messages,
},
};

export default config;
2 changes: 2 additions & 0 deletions rules/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {createRule} from './utils/rule.js';

import betterRegex from './better-regex.js';
import catchErrorName from './catch-error-name.js';
import consistentAssert from './consistent-assert.js';
import consistentDestructuring from './consistent-destructuring.js';
import consistentEmptyArraySpread from './consistent-empty-array-spread.js';
import consistentExistenceIndexCheck from './consistent-existence-index-check.js';
Expand Down Expand Up @@ -128,6 +129,7 @@ import throwNewError from './throw-new-error.js';
const rules = {
'better-regex': createRule(betterRegex, 'better-regex'),
'catch-error-name': createRule(catchErrorName, 'catch-error-name'),
'consistent-assert': createRule(consistentAssert, 'consistent-assert'),
'consistent-destructuring': createRule(consistentDestructuring, 'consistent-destructuring'),
'consistent-empty-array-spread': createRule(consistentEmptyArraySpread, 'consistent-empty-array-spread'),
'consistent-existence-index-check': createRule(consistentExistenceIndexCheck, 'consistent-existence-index-check'),
Expand Down
138 changes: 138 additions & 0 deletions test/consistent-assert.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
import outdent from 'outdent';
import {getTester, parsers} from './utils/test.js';

const {test} = getTester(import.meta);

test.snapshot({
valid: [
'assert(foo)',
'import assert from "assert";',
// Import but not invoke
outdent`
import assert from 'node:assert';
assert;
`,
outdent`
import customAssert from 'node:assert';
assert(foo);
`,
outdent`
function foo (assert) {
assert(bar);
}
`,
outdent`
import assert from 'node:assert';

function foo (assert) {
assert(bar);
}
`,
// Invalid named import
outdent`
import {strict} from 'node:assert/strict';

strict(foo);
`,
outdent`
import * as assert from 'node:assert';
assert(foo);
`,
outdent`
export * as assert from 'node:assert';
assert(foo);
`,
outdent`
export {default as assert} from 'node:assert';
export {assert as strict} from 'node:assert';
assert(foo);
`,
outdent`
import assert from 'node:assert/strict';
console.log(assert)
`,
...[
'import type assert from "node:assert/strict";',
'import {type strict as assert} from "node:assert/strict";',
'import type {strict as assert} from "node:assert/strict";',
].flatMap(code => [code, `${code}\nassert();`]).map(code => ({code, languageOptions: {parser: parsers.typescript}})),
],
invalid: [
// Default import
outdent`
import assert from 'assert';
assert(foo)
`,
outdent`
import assert from 'node:assert';
assert(foo)
`,
outdent`
import assert from 'assert/strict';
assert(foo)
`,
outdent`
import assert from 'node:assert/strict';
assert(foo)
`,
outdent`
import customAssert from 'assert';
customAssert(foo)
`,
outdent`
import customAssert from 'node:assert';
customAssert(foo)
`,
// Multiple references
outdent`
import assert from 'assert';
assert(foo)
assert(bar)
assert(baz)
`,
// Named import
outdent`
import {strict} from 'assert';
strict(foo)
`,
// Named import with alias
outdent`
import {strict as assert} from 'assert';
assert(foo)
`,
// All cases
outdent`
import a, {strict as b, default as c} from 'node:assert';
import d, {strict as e, default as f} from 'assert';
import g, {default as h} from 'node:assert/strict';
import i, {default as j} from 'assert/strict';
a(foo);
b(foo);
c(foo);
d(foo);
e(foo);
f(foo);
g(foo);
h(foo);
i(foo);
j(foo);
`,
// Optional call, not really matters
outdent`
import assert from 'node:assert';
assert?.(foo)
`,
outdent`
import assert from 'assert';

((
/* comment */ ((
/* comment */
assert
/* comment */
)) /* comment */
(/* comment */ typeof foo === 'string', 'foo must be a string' /** after comment */)
));
`,
],
});
1 change: 1 addition & 0 deletions test/package.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ const RULES_WITHOUT_PASS_FAIL_SECTIONS = new Set([
'consistent-existence-index-check',
'prefer-global-this',
'no-instanceof-builtin-object',
'consistent-assert',
]);

test('Every rule is defined in index file in alphabetical order', t => {
Expand Down
Loading
Loading