Merge pull request #2473 from thiagomini/test/function-utils

Fix getNumberOfParameters utility function
nestjs · Oct 26, 2022 · 8dc5123 · 8dc5123
2 parents 359c16a + acc261e
commit 8dc5123
Show file tree

Hide file tree

Showing 2 changed files with 73 additions and 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
@@ -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/...raphql/tests/utils/function.utils.spec.ts → ...tils/get-number-of-arguments.util.spec.ts b/...raphql/tests/utils/function.utils.spec.ts → ...tils/get-number-of-arguments.util.spec.ts
@@ -7,11 +7,33 @@ 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);
     });
 
+    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);
@@ -81,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']) {}
@@ -92,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);
     });
@@ -143,6 +193,14 @@ 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);
+    });
   });
 });