From 8ba6180f58619a46eb95ab3285f029f303c12768 Mon Sep 17 00:00:00 2001 From: Thiago Martins Date: Tue, 25 Oct 2022 09:36:19 -0300 Subject: [PATCH 1/4] chore(graphql): rename test file to match production code file --- ...unction.utils.spec.ts => get-number-of-arguments.util.spec.ts} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename packages/graphql/tests/utils/{function.utils.spec.ts => get-number-of-arguments.util.spec.ts} (100%) diff --git a/packages/graphql/tests/utils/function.utils.spec.ts b/packages/graphql/tests/utils/get-number-of-arguments.util.spec.ts similarity index 100% rename from packages/graphql/tests/utils/function.utils.spec.ts rename to packages/graphql/tests/utils/get-number-of-arguments.util.spec.ts From 93cbbd77a1608e21a01f95cfad91a17821a1cd25 Mon Sep 17 00:00:00 2001 From: Thiago Martins Date: Tue, 25 Oct 2022 10:17:06 -0300 Subject: [PATCH 2/4] fix(graphql): get number of arguments util ensure getNumberOfArguments returns the correct number of arguments for functions with body that use parenthesis. --- .../lib/utils/get-number-of-arguments.util.ts | 19 +++++++++++++++---- .../get-number-of-arguments.util.spec.ts | 11 +++++++++++ 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/packages/graphql/lib/utils/get-number-of-arguments.util.ts b/packages/graphql/lib/utils/get-number-of-arguments.util.ts index e2026bdf8..de0c4dd86 100644 --- a/packages/graphql/lib/utils/get-number-of-arguments.util.ts +++ b/packages/graphql/lib/utils/get-number-of-arguments.util.ts @@ -7,15 +7,20 @@ */ export function getNumberOfArguments(fn: Function): number | undefined { // Removing newlines is necessary to use easier regex and handle multi-line functions - const functionAsStringWithouNewLines = fn.toString().replace(/\n/g, ''); + const functionAsStringWithoutNewLines = fn.toString().replace(/\n/g, ''); - const anythingEnclosedInParenthesesRegex = /\(.+\)/; + /* The RegExp below uses a non-greedy match (the question mark), meaning that it tries to find + * the smallest possible match. This ensures that we don't accidentally + * consider the function's body as part of the regex match, since we aim + * to get only the parameters section. + */ + const anythingEnclosedInParenthesesRegex = /\(.*?\)/; - const regexMatchedArray = functionAsStringWithouNewLines.match( + const regexMatchedArray = functionAsStringWithoutNewLines.match( new RegExp(anythingEnclosedInParenthesesRegex), ); - if (regexMatchedArray) { + if (functionHasOneOrMoreArguments(regexMatchedArray)) { const functionParametersAsString = regexMatchedArray[0]; // Removing arrays and objects is also necessary because we count the number of commas in the string, @@ -30,3 +35,9 @@ export function getNumberOfArguments(fn: Function): number | undefined { return 0; } + +function functionHasOneOrMoreArguments( + functionRegexMatchArray: RegExpMatchArray, +) { + return functionRegexMatchArray && functionRegexMatchArray[0] !== '()'; +} diff --git a/packages/graphql/tests/utils/get-number-of-arguments.util.spec.ts b/packages/graphql/tests/utils/get-number-of-arguments.util.spec.ts index f882d4c7a..9cefe1f65 100644 --- a/packages/graphql/tests/utils/get-number-of-arguments.util.spec.ts +++ b/packages/graphql/tests/utils/get-number-of-arguments.util.spec.ts @@ -7,6 +7,17 @@ describe('getNumberOfArguments', () => { expect(getNumberOfArguments(zeroArgFunction)).toBe(0); }); + it('should return 0 for a 0-argument function with body', () => { + function zeroArgFunction() { + if (1 == +'2') { + return false; + } + + return true; + } + expect(getNumberOfArguments(zeroArgFunction)).toBe(0); + }); + it('should return 1 for a 1-argument function', () => { function oneArgFunction(_arg1: any) {} expect(getNumberOfArguments(oneArgFunction)).toBe(1); From 2a06e5e5027ed0e82af56bbc6f481d24acd969ba Mon Sep 17 00:00:00 2001 From: Thiago Martins Date: Tue, 25 Oct 2022 10:23:16 -0300 Subject: [PATCH 3/4] test(graphql): get number of arguments util create additional tests to ensure the arguments count is correct for all types of functions with and without body --- .../get-number-of-arguments.util.spec.ts | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/packages/graphql/tests/utils/get-number-of-arguments.util.spec.ts b/packages/graphql/tests/utils/get-number-of-arguments.util.spec.ts index 9cefe1f65..e2d883d83 100644 --- a/packages/graphql/tests/utils/get-number-of-arguments.util.spec.ts +++ b/packages/graphql/tests/utils/get-number-of-arguments.util.spec.ts @@ -23,6 +23,17 @@ describe('getNumberOfArguments', () => { expect(getNumberOfArguments(oneArgFunction)).toBe(1); }); + it('should return 1 for a 1-argument function with body', () => { + function oneArgFunction(_arg1: any) { + if (1 == +'2') { + return false; + } + + return true; + } + expect(getNumberOfArguments(oneArgFunction)).toBe(1); + }); + it('should return 2 for a 2-argument function', () => { function twoArgFunction(_arg1: any, _arg2: any) {} expect(getNumberOfArguments(twoArgFunction)).toBe(2); @@ -92,8 +103,24 @@ describe('getNumberOfArguments', () => { class TestStaticClass { static methodZeroArguments() {} + static methodZeroArgumentsWithBody() { + if (1 == +'2') { + return false; + } + + return true; + } + static methodOneArgument(_arg: any) {} + static methodOneArgumentWithBody(_arg: any) { + if (1 == +'2') { + return false; + } + + return true; + } + static methodTwoArguments(_arg1: any, _arg2: any) {} static methodTwoArgumentsOneOptional(_arg1: any, _arg2 = ['raw']) {} @@ -103,10 +130,22 @@ describe('getNumberOfArguments', () => { expect(getNumberOfArguments(TestStaticClass.methodZeroArguments)).toBe(0); }); + it('should return 0 for a 0-argument function with body', () => { + expect( + getNumberOfArguments(TestStaticClass.methodZeroArgumentsWithBody), + ).toBe(0); + }); + it('should return 1 for a 1-argument function', () => { expect(getNumberOfArguments(TestStaticClass.methodOneArgument)).toBe(1); }); + it('should return 1 for a 1-argument function with body', () => { + expect( + getNumberOfArguments(TestStaticClass.methodOneArgumentWithBody), + ).toBe(1); + }); + it('should return 2 for a 2-argument function', () => { expect(getNumberOfArguments(TestStaticClass.methodTwoArguments)).toBe(2); }); From 2ae15c9a434aeffadff5bd90667385908868992f Mon Sep 17 00:00:00 2001 From: Thiago Martins Date: Tue, 25 Oct 2022 10:41:57 -0300 Subject: [PATCH 4/4] test(graphql): add unsupported case test add test for unsupported case for documentation's sake. --- .../tests/utils/get-number-of-arguments.util.spec.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/graphql/tests/utils/get-number-of-arguments.util.spec.ts b/packages/graphql/tests/utils/get-number-of-arguments.util.spec.ts index e2d883d83..7ae5e4414 100644 --- a/packages/graphql/tests/utils/get-number-of-arguments.util.spec.ts +++ b/packages/graphql/tests/utils/get-number-of-arguments.util.spec.ts @@ -193,5 +193,13 @@ describe('getNumberOfArguments', () => { expect(getNumberOfArguments(functionWithArray)).toBe(2); }); + + // This test is skipped because we don't support it yet. + it.skip('should count correctly for arrow functions without parenthesis', () => { + // prettier-ignore + const leanArrowFunction = x => x + 2; + + expect(getNumberOfArguments(leanArrowFunction)).toBe(1); + }); }); });