assert: removed includesStrict and adapted includes

nodejs · Sep 20, 2024 · 8480d4b · 8480d4b
1 parent c1da69c
commit 8480d4b
Show file tree

Hide file tree

Showing 5 changed files with 34 additions and 188 deletions.
diff --git a/doc/api/assert.md b/doc/api/assert.md
@@ -2727,7 +2727,7 @@ added: REPLACEME
 -->
 
 * `actual` {Array | string}
-* `expected` {Array | string}
+* `expected` {any}
 * `message` {string|Error}
 
 [`assert.includes()`][] compares the `actual` and `expected` parameters to determine if the `expected`
@@ -2739,13 +2739,13 @@ the `expected` parameter must be an array and vice versa for strings.
 ```mjs
 import assert from 'node:assert';
 
-assert.includes([1, 2, 3], ['2', 3]);
+assert.includes([1, 2, 3], 2);
 // OK
 
 assert.includes('Hello World!', 'World');
 // OK
 
-assert.includes([1, 2, 3], [2, 4]);
+assert.includes([1, 2, 3], '2');
 // AssertionError
 
 assert.includes('Hello World!', 'Node.js');
@@ -2755,13 +2755,13 @@ assert.includes('Hello World!', 'Node.js');
 ```cjs
 const assert = require('node:assert');
 
-assert.includes([1, 2, 3], [2, 3]);
+assert.includes([1, 2, 3], 2);
 // OK
 
 assert.includes('Hello World!', 'World');
 // OK
 
-assert.includes([1, 2, 3], [2, 4]);
+assert.includes([1, 2, 3], '2');
 // AssertionError
 
 assert.includes('Hello World!', 'Node.js');
@@ -2774,60 +2774,6 @@ parameter is undefined, a default error message is assigned. If the `message`
 parameter is an instance of an [`Error`][] then it will be thrown instead of the
 `AssertionError`.
 
-## `assert.includesStrict(actual, expected[, message])`
-
-<!-- YAML
-added: REPLACEME
--->
-
-* `actual` {Array | string}
-* `expected` {Array | string}
-* `message` {string|Error}
-
-[`assert.includesStrict()`][] compares the `actual` and `expected` parameters to determine if the `expected`
-parameter is included in the `actual` parameter; the comparison is done without type coercion.
-The `actual` and the `expected` parameters can be
-either an array or a string. If the `actual` parameter is an array,
-the `expected` parameter must be an array and vice versa for strings.
-
-```mjs
-import assert from 'node:assert';
-
-assert.includesStrict([1, 2, 3], [2, 3]);
-// OK
-
-assert.includesStrict('Hello World!', 'World');
-// OK
-
-assert.includesStrict([1, 2, 3], [2, 4]);
-// AssertionError
-
-assert.includesStrict('Hello World!', 'Node.js');
-// AssertionError
-```
-
-```cjs
-const assert = require('node:assert');
-
-assert.includesStrict([1, 2, 3], [2, 3]);
-// OK
-
-assert.includesStrict('Hello World!', 'World');
-// OK
-
-assert.includesStrict([1, 2, 3], [2, 4]);
-// AssertionError
-
-assert.includesStrict('Hello World!', 'Node.js');
-// AssertionError
-```
-
-If the assertion fails, an [`AssertionError`][] is thrown with a `message`
-property set equal to the value of the `message` parameter. If the `message`
-parameter is undefined, a default error message is assigned. If the `message`
-parameter is an instance of an [`Error`][] then it will be thrown instead of the
-`AssertionError`.
-
 [Object wrappers]: https://developer.mozilla.org/en-US/docs/Glossary/Primitive#Primitive_wrapper_objects_in_JavaScript
 [Object.prototype.toString()]: https://tc39.github.io/ecma262/#sec-object.prototype.tostring
 [`!=` operator]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Inequality
@@ -2852,7 +2798,6 @@ parameter is an instance of an [`Error`][] then it will be thrown instead of the
 [`assert.doesNotThrow()`]: #assertdoesnotthrowfn-error-message
 [`assert.equal()`]: #assertequalactual-expected-message
 [`assert.includes()`]: #assertincludesactual-expected-message
-[`assert.includesStrict()`]: #assertincludesstrictactual-expected-message
 [`assert.notDeepEqual()`]: #assertnotdeepequalactual-expected-message
 [`assert.notDeepStrictEqual()`]: #assertnotdeepstrictequalactual-expected-message
 [`assert.notEqual()`]: #assertnotequalactual-expected-message

diff --git a/lib/assert.js b/lib/assert.js
@@ -23,6 +23,7 @@
 const {
   ArrayIsArray,
   ArrayPrototypeEvery,
+  ArrayPrototypeIncludes,
   ArrayPrototypeIndexOf,
   ArrayPrototypeJoin,
   ArrayPrototypePush,
@@ -63,7 +64,6 @@ const {
     ERR_INVALID_RETURN_VALUE,
     ERR_MISSING_ARGS,
   },
-  hideStackFrames,
 } = require('internal/errors');
 const AssertionError = require('internal/assert/assertion_error');
 const { inspect } = require('internal/util/inspect');
@@ -366,20 +366,6 @@ function isPlainObject(obj) {
   return proto === ObjectPrototype || proto === null || ObjectPrototypeToString(obj) === '[object Object]';
 }
 
-function assertIncludes(actual, expected, loose) {
-  if (expected.length > actual.length) {
-    return false;
-  }
-  if (typeof actual === 'string') {
-    return StringPrototypeIncludes(actual, expected);
-  }
-
-  return ArrayPrototypeEvery(expected, (item) => ArrayPrototypeSome(actual, (actualItem) => {
-    // eslint-disable-next-line eqeqeq
-    return loose ? actualItem == item : actualItem === item;
-  }));
-}
-
 /**
  * Compares two objects or values recursively to check if they are equal.
  * @param {any} actual - The actual value to compare.
@@ -435,7 +421,21 @@ function compareBranch(
 
   // Check if expected array is a subset of actual array
   if (ArrayIsArray(actual) && ArrayIsArray(expected)) {
-    return assertIncludes(actual, expected, loose);
+    if (expected.length > actual.length) {
+      return false;
+    }
+    if (typeof actual === 'string') {
+      return StringPrototypeIncludes(actual, expected);
+    }
+
+    if (loose) {
+      return ArrayPrototypeEvery(expected, (item) => ArrayPrototypeSome(actual, (actualItem) => {
+        // eslint-disable-next-line eqeqeq
+        return actualItem == item;
+      }));
+    }
+
+    return ArrayPrototypeEvery(expected, (item) => ArrayPrototypeIncludes(actual, item));
   }
 
   // Check non object types equality
@@ -520,20 +520,10 @@ assert.partialDeepEqual = function partialDeepEqual(actual, expected, message) {
   }
 };
 
-const throwIfInvalidIncludesParams = hideStackFrames((actual, expected) => {
-  if (typeof actual !== 'string' && !ArrayIsArray(actual)) {
-    throw new ERR_INVALID_ARG_TYPE('actual', ['string', 'Array'], actual);
-  }
-
-  if (typeof expected !== 'string' && !ArrayIsArray(expected)) {
-    throw new ERR_INVALID_ARG_TYPE('expected', ['string', 'Array'], expected);
-  }
-});
-
 /**
  * The inclusion assertion test between two arrays or strings
  * @param {Array | string} actual
- * @param {Array | string} expected
+ * @param {any} expected
  * @param {string | Error} [message]
  * @returns {void}
  */
@@ -542,41 +532,19 @@ assert.includes = function includes(actual, expected, message) {
     throw new ERR_MISSING_ARGS('actual', 'expected');
   }
 
-  throwIfInvalidIncludesParams(actual, expected);
+  const isString = typeof actual === 'string';
 
-  if (!assertIncludes(actual, expected, true)) {
-    innerFail({
-      actual,
-      expected,
-      message,
-      operator: 'includes',
-      stackStartFn: includes,
-    });
-  }
-};
-
-
-/**
- * The strict inclusion assertion test between two arrays or strings
- * @param {Array | string} actual
- * @param {Array | string} expected
- * @param {string | Error} [message]
- * @returns {void}
- */
-assert.includesStrict = function includesStrict(actual, expected, message) {
-  if (arguments.length < 2) {
-    throw new ERR_MISSING_ARGS('actual', 'expected');
+  if (!isString && !ArrayIsArray(actual)) {
+    throw new ERR_INVALID_ARG_TYPE('actual', ['string', 'Array'], actual);
   }
 
-  throwIfInvalidIncludesParams(actual, expected);
-
-  if (!assertIncludes(actual, expected, false)) {
+  if (!(isString ? StringPrototypeIncludes : ArrayPrototypeIncludes)(actual, expected)) {
     innerFail({
       actual,
       expected,
       message,
-      operator: 'includesStrict',
-      stackStartFn: includesStrict,
+      operator: 'includes',
+      stackStartFn: includes,
     });
   }
 };
@@ -1048,7 +1016,6 @@ assert.strict = ObjectAssign(strict, assert, {
   notEqual: assert.notStrictEqual,
   notDeepEqual: assert.notDeepStrictEqual,
   partialDeepEqual: assert.partialDeepStrictEqual,
-  includes: assert.includesStrict,
 });
 
 assert.strict.strict = assert.strict;
diff --git a/lib/internal/test_runner/test.js b/lib/internal/test_runner/test.js
@@ -110,7 +110,6 @@ function lazyAssertObject(harness) {
       'fail',
       'ifError',
       'includes',
-      'includesStrict',
       'match',
       'notDeepEqual',
       'notDeepStrictEqual',

diff --git a/test/parallel/test-assert-objects.js b/test/parallel/test-assert-objects.js
@@ -542,69 +542,6 @@ describe('Object Comparison Tests', () => {
       });
     });
   });
-  describe('includesStrict', () => {
-    describe('throws an error', () => {
-      [
-        {
-          description: 'throws because actual is nor an Array or a string',
-          actual: { a: 1 },
-          expected: [1],
-          errorCode: 'ERR_INVALID_ARG_TYPE'
-        },
-        {
-          description: 'throws because expected is nor an Array or a string',
-          actual: [1],
-          expected: { a: 1 },
-          errorCode: 'ERR_INVALID_ARG_TYPE'
-        },
-        {
-          description:
-            'throws when comparing one subset array with another',
-          actual: [1, 2, 3],
-          expected: ['2'],
-        },
-        {
-          description:
-            'throws when comparing one subset string with another',
-          actual: 'abc',
-          expected: '2',
-        },
-        {
-          description:
-            'throws because the expected value is longer than the actual value',
-          actual: 'abc',
-          expected: 'abcd',
-        },
-      ].forEach(({ description, actual, expected, errorCode }) => {
-        it(description, () => {
-          assert.throws(() => assert.includesStrict(actual, expected), errorCode ?
-            { code: errorCode } :
-            assert.AssertionError);
-        });
-      });
-    });
-
-    describe('does not throw an error', () => {
-      [
-        {
-          description:
-            'compares one subset array with another',
-          actual: [1, 2, 3],
-          expected: [2],
-        },
-        {
-          description:
-            'compares one subset string with another',
-          actual: 'abc',
-          expected: 'b',
-        },
-      ].forEach(({ description, actual, expected }) => {
-        it(description, () => {
-          assert.includesStrict(actual, expected);
-        });
-      });
-    });
-  });
 
   describe('includes', () => {
     describe('throws an error', () => {
@@ -615,12 +552,6 @@ describe('Object Comparison Tests', () => {
           expected: [1],
           errorCode: 'ERR_INVALID_ARG_TYPE'
         },
-        {
-          description: 'throws because expected is nor an Array or a string',
-          actual: [1],
-          expected: { a: 1 },
-          errorCode: 'ERR_INVALID_ARG_TYPE'
-        },
         {
           description:
             'throws because using includes with a string and an array',
@@ -647,13 +578,18 @@ describe('Object Comparison Tests', () => {
         {
           description: 'compares one subset array with another',
           actual: [1, 2, 3],
-          expected: ['2'],
+          expected: 2,
         },
         {
           description: 'compares one subset string with another',
           actual: 'abc',
           expected: 'b',
         },
+        {
+          description: 'compares a null-prototype array with an element',
+          actual: Object.setPrototypeOf([1, 2, 3], null),
+          expected: 2,
+        },
       ].forEach(({ description, actual, expected }) => {
         it(description, () => {
           assert.includes(actual, expected);

diff --git a/test/parallel/test-runner-assert.js b/test/parallel/test-runner-assert.js
@@ -14,7 +14,6 @@ test('only methods from node:assert are on t.assert', (t) => {
     'fail',
     'ifError',
     'includes',
-    'includesStrict',
     'match',
     'notDeepEqual',
     'notDeepStrictEqual',