Add consistent-assert rule (#2535)

Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>
Co-authored-by: fisker <lionkay@gmail.com>

Author: 3 people

docs/rules/consistent-assert.md

@@ -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);
+```

readme.md

@@ -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()`.                                                                                    | ✅  | 🔧 |    |

rules/consistent-assert.js

@@ -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;

rules/index.js

@@ -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';
@@ -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'),

test/consistent-assert.js

@@ -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 */)
+			));
+		`,
+	],
+});

test/package.js

@@ -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 => {