From c8ae059dcdd7325e930fdd3d63b7217c806cd2c2 Mon Sep 17 00:00:00 2001 From: ST-DDT Date: Wed, 6 Apr 2022 14:11:52 +0200 Subject: [PATCH 1/4] feat: immutable options in random.alpha methods --- src/random.ts | 47 ++++++++++++++++++----------------------------- 1 file changed, 18 insertions(+), 29 deletions(-) diff --git a/src/random.ts b/src/random.ts index 448cf4f8696..45742f17de2 100644 --- a/src/random.ts +++ b/src/random.ts @@ -400,28 +400,22 @@ 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?: string[] } = { + count: 1, + upcase: false, + bannedChars: [], + } ): 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 = options.count ?? 1; + const upcase = options.upcase ?? false; + const bannedChars = options.bannedChars ?? []; let wholeString = ''; let charsArray = [ @@ -452,15 +446,14 @@ 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); + + for (let i = 0; i < count; i++) { wholeString += this.faker.random.arrayElement(charsArray); } - return options.upcase ? wholeString.toUpperCase() : wholeString; + return upcase ? wholeString.toUpperCase() : wholeString; } /** @@ -477,11 +470,9 @@ export class Random { */ alphaNumeric( count: number = 1, - options: { bannedChars?: string[] } = {} + options: { bannedChars?: string[] } = { bannedChars: [] } ): string { - if (options.bannedChars == null) { - options.bannedChars = []; - } + const bannedChars = options.bannedChars ?? []; let wholeString = ''; let charsArray = [ @@ -523,9 +514,7 @@ export class Random { 'z', ]; - if (options.bannedChars) { - charsArray = arrayRemove(charsArray, options.bannedChars); - } + charsArray = arrayRemove(charsArray, bannedChars); if (charsArray.length === 0) { throw new FakerError( From 665f82ccb99053bf754a12ac1b13bf4cbca8f373 Mon Sep 17 00:00:00 2001 From: ST-DDT Date: Wed, 6 Apr 2022 15:04:41 +0200 Subject: [PATCH 2/4] chore: accept readonly arrays --- src/random.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/random.ts b/src/random.ts index 45742f17de2..e85ae47832f 100644 --- 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(arr: T[], values: T[]): T[] { +function arrayRemove(arr: T[], values: readonly T[]): T[] { values.forEach((value) => { arr = arr.filter((ele) => ele !== value); }); @@ -402,7 +402,11 @@ export class Random { alpha( options: | number - | { count?: number; upcase?: boolean; bannedChars?: string[] } = { + | { + count?: number; + upcase?: boolean; + bannedChars?: readonly string[]; + } = { count: 1, upcase: false, bannedChars: [], @@ -470,7 +474,7 @@ export class Random { */ alphaNumeric( count: number = 1, - options: { bannedChars?: string[] } = { bannedChars: [] } + options: { bannedChars?: readonly string[] } = { bannedChars: [] } ): string { const bannedChars = options.bannedChars ?? []; From faf6425d5812cf008e82db349ebf6c7292b70c31 Mon Sep 17 00:00:00 2001 From: ST-DDT Date: Fri, 8 Apr 2022 18:15:55 +0200 Subject: [PATCH 3/4] chore: use "fancy" destructors and test options immutablility --- src/random.ts | 10 ++++------ test/random.spec.ts | 26 ++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/src/random.ts b/src/random.ts index e85ae47832f..134afe6e23b 100644 --- a/src/random.ts +++ b/src/random.ts @@ -417,11 +417,8 @@ export class Random { count: options, }; } - const count = options.count ?? 1; - const upcase = options.upcase ?? false; - const bannedChars = options.bannedChars ?? []; + const { count = 1, upcase = false, bannedChars = [] } = options; - let wholeString = ''; let charsArray = [ 'a', 'b', @@ -453,6 +450,7 @@ export class Random { charsArray = arrayRemove(charsArray, bannedChars); + let wholeString = ''; for (let i = 0; i < count; i++) { wholeString += this.faker.random.arrayElement(charsArray); } @@ -476,9 +474,8 @@ export class Random { count: number = 1, options: { bannedChars?: readonly string[] } = { bannedChars: [] } ): string { - const bannedChars = options.bannedChars ?? []; + const { bannedChars = [] } = options; - let wholeString = ''; let charsArray = [ '0', '1', @@ -526,6 +523,7 @@ export class Random { ); } + let wholeString = ''; for (let i = 0; i < count; i++) { wholeString += this.faker.random.arrayElement(charsArray); } diff --git a/test/random.spec.ts b/test/random.spec.ts index 79dbf18d5ef..886284cde74 100644 --- 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', () => { From 6d84908950458d8f398b1b4b0e5ba5afc8bc5e88 Mon Sep 17 00:00:00 2001 From: ST-DDT Date: Sat, 9 Apr 2022 03:29:43 +0200 Subject: [PATCH 4/4] chore: simplify defaults --- src/random.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/random.ts b/src/random.ts index 208599a8e08..96128dd5c40 100644 --- a/src/random.ts +++ b/src/random.ts @@ -406,11 +406,7 @@ export class Random { count?: number; upcase?: boolean; bannedChars?: readonly string[]; - } = { - count: 1, - upcase: false, - bannedChars: [], - } + } = {} ): string { if (typeof options === 'number') { options = { @@ -472,7 +468,7 @@ export class Random { */ alphaNumeric( count: number = 1, - options: { bannedChars?: readonly string[] } = { bannedChars: [] } + options: { bannedChars?: readonly string[] } = {} ): string { const { bannedChars = [] } = options;