feat: immutable options in random.alpha methods (#790)

faker-js · Apr 10, 2022 · dd11846 · dd11846
1 parent 612fc38
commit dd11846
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 32 deletions.
diff --git a/src/random.ts b/src/random.ts
@@ -9,7 +9,7 @@ import { deprecated } from './internal/deprecated';
  * @param values array of characters which should be removed
  * @returns new array without banned characters
  */
-function arrayRemove<T>(arr: T[], values: T[]): T[] {
+function arrayRemove<T>(arr: T[], values: readonly T[]): T[] {
   values.forEach((value) => {
     arr = arr.filter((ele) => ele !== value);
   });
@@ -400,30 +400,21 @@ export class Random {
    */
   // TODO @Shinigami92 2022-02-14: Tests covered `(count, options)`, but they were never typed like that
   alpha(
-    options?:
+    options:
       | number
-      | { count?: number; upcase?: boolean; bannedChars?: string[] }
+      | {
+          count?: number;
+          upcase?: boolean;
+          bannedChars?: readonly string[];
+        } = {}
   ): string {
-    if (options == null) {
-      options = {
-        count: 1,
-      };
-    } else if (typeof options === 'number') {
+    if (typeof options === 'number') {
       options = {
         count: options,
       };
-    } else if (options.count == null) {
-      options.count = 1;
-    }
-
-    if (options.upcase == null) {
-      options.upcase = false;
-    }
-    if (options.bannedChars == null) {
-      options.bannedChars = [];
     }
+    const { count = 1, upcase = false, bannedChars = [] } = options;
 
-    let wholeString = '';
     let charsArray = [
       'a',
       'b',
@@ -452,15 +443,15 @@ export class Random {
       'y',
       'z',
     ];
-    // TODO @Shinigami92 2022-01-11: A default empty array gets assigned above, we should check the length against 0 or not here
-    if (options.bannedChars) {
-      charsArray = arrayRemove(charsArray, options.bannedChars);
-    }
-    for (let i = 0; i < options.count; i++) {
+
+    charsArray = arrayRemove(charsArray, bannedChars);
+
+    let wholeString = '';
+    for (let i = 0; i < count; i++) {
       wholeString += this.arrayElement(charsArray);
     }
 
-    return options.upcase ? wholeString.toUpperCase() : wholeString;
+    return upcase ? wholeString.toUpperCase() : wholeString;
   }
 
   /**
@@ -477,13 +468,10 @@ export class Random {
    */
   alphaNumeric(
     count: number = 1,
-    options: { bannedChars?: string[] } = {}
+    options: { bannedChars?: readonly string[] } = {}
   ): string {
-    if (options.bannedChars == null) {
-      options.bannedChars = [];
-    }
+    const { bannedChars = [] } = options;
 
-    let wholeString = '';
     let charsArray = [
       '0',
       '1',
@@ -523,16 +511,15 @@ export class Random {
       'z',
     ];
 
-    if (options.bannedChars) {
-      charsArray = arrayRemove(charsArray, options.bannedChars);
-    }
+    charsArray = arrayRemove(charsArray, bannedChars);
 
     if (charsArray.length === 0) {
       throw new FakerError(
         'Unable to generate string, because all possible characters are banned.'
       );
     }
 
+    let wholeString = '';
     for (let i = 0; i < count; i++) {
       wholeString += this.arrayElement(charsArray);
     }

diff --git a/test/random.spec.ts b/test/random.spec.ts
@@ -220,6 +220,21 @@ describe('random', () => {
       expect(alphaText).toHaveLength(5);
       expect(alphaText).match(/^[b-oq-z]{5}$/);
     });
+
+    it('should not mutate the input object', () => {
+      const input: {
+        count: number;
+        upcase: boolean;
+        bannedChars: string[];
+      } = Object.freeze({
+        count: 5,
+        upcase: true,
+        bannedChars: ['a', '%'],
+      });
+
+      expect(() => faker.random.alpha(input)).not.toThrow();
+      expect(input.bannedChars).toEqual(['a', '%']);
+    });
   });
 
   describe('alphaNumeric', () => {
@@ -276,6 +291,17 @@ describe('random', () => {
         })
       ).toThrowError();
     });
+
+    it('should not mutate the input object', () => {
+      const input: {
+        bannedChars: string[];
+      } = Object.freeze({
+        bannedChars: ['a', '0', '%'],
+      });
+
+      expect(() => faker.random.alphaNumeric(5, input)).not.toThrow();
+      expect(input.bannedChars).toEqual(['a', '0', '%']);
+    });
   });
 
   describe('deprecation warnings', () => {