Wrote some unit tests to make splitToParamList more maintainable

Author: lannonbr

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## 0.8.4 - Sep 15, 2018
+- [Bug] Continued working on cleaning up parameter splitting algorithm. Now doesn't break with functions
+- [Feature] Wrote unit tests for splitToParamList function
+
 ## 0.8.3 - Sep 14, 2018
 - [Bug] Importing functions with require don't cause issues with annotations anymore.
 - [Bug] TS functions with Generic types now properly annotate

package-lock.json

@@ -2,7 +2,7 @@
   "name": "vscode-js-annotations",
   "displayName": "JS Parameter Annotations",
   "description": "Annotations for parameters in your JS / TS Files to mimic named parameters",
-  "version": "0.8.3",
+  "version": "0.8.4",
   "publisher": "lannonbr",
   "engines": {
     "vscode": "^1.25.0"

package.json

@@ -1,15 +1,18 @@
 import * as vscode from "vscode";
 import { Annotations } from "./annotationProvider";
 import { IFunctionCallObject } from "./functionCallObject";
+import { grabPossibleParameters } from "./paramExtractor";
 
 export async function decorateFunctionCall(currentEditor: vscode.TextEditor, documentCache: any, decArray, errDecArray: vscode.DecorationOptions[], fc: IFunctionCallObject, diagnostics: vscode.Diagnostic[]): Promise<void> {
   // Check for existence of functionCallObject and a defintion location
   if (fc === undefined || fc.definitionLocation === undefined) {
     return;
   }
 
-  // config option to hide annotations if param and arg names match
+  // configs for decorations
   const hideIfEqual = vscode.workspace.getConfiguration("jsannotations").get("hideIfEqual");
+  const willShowDiagnostics = vscode.workspace.getConfiguration("jsannotations").get("hideDiagnostics") === false;
+  const willShowErrorAnnotation = vscode.workspace.getConfiguration("jsannotations").get("hideInvalidAnnotation") === false;
 
   const document = await loadDefinitionDocument(fc.definitionLocation.uri, documentCache);
   const definitionLine = document.lineAt(fc.definitionLocation.range.start.line).text;
@@ -49,12 +52,12 @@ export async function decorateFunctionCall(currentEditor: vscode.TextEditor, doc
             decoration = Annotations.paramAnnotation(paramList[restParamIdx] + `[${idx - restParamIdx}]: `, currentArgRange);
           } else {
             if (idx >= paramList.length) {
-              if (currentEditor.document.languageId === "javascript" && vscode.workspace.getConfiguration("jsannotations").get("hideDiagnostics") === false) {
+              if (currentEditor.document.languageId === "javascript" && willShowDiagnostics) {
                 const diag = new vscode.Diagnostic(currentArgRange, "[JS Param Annotations] Invalid parameter", vscode.DiagnosticSeverity.Error);
                 diagnostics.push(diag);
               }
 
-              if (vscode.workspace.getConfiguration("jsannotations").get("hideInvalidAnnotation") === false) {
+              if (willShowErrorAnnotation) {
                 const errorDecoration = Annotations.errorParamAnnotation(currentArgRange);
                 errDecArray.push(errorDecoration);
               }
@@ -89,113 +92,6 @@ async function loadDefinitionDocument(uri: vscode.Uri, documentCache: any) {
   return document;
 }
 
-function grabPossibleParameters(fc: IFunctionCallObject, definitionLine: string): string[] {
-  let paramList: string[] = [];
-
-  // Grab any params inside the definition line
-  const defintionParam = grabDefLineParams(definitionLine);
-
-  if (defintionParam !== "") {
-    if (fc.functionRange === undefined) {
-      return;
-    }
-
-    paramList = defintionParam.split(/\s*,\s*/);
-
-    paramList = squishGenerics(paramList);
-
-    paramList = paramList.map((param) => {
-      // Extract identifiers
-
-      const words = param.trim().split(" ");
-
-      // If there are multiple words and the first word doesn't end with a colon, use the 2nd word as the param
-      // this will make sure the param name is used and not the access modifier in TS functions
-      if (words.length > 1 && !words[0].endsWith(":")) {
-        param = words[1];
-      }
-
-      const identifiers = param.match(/([\.a-zA-Z0-9]+):?/);
-
-      if (identifiers && identifiers.length > 1) {
-        return identifiers[1];
-      }
-      return "";
-    }).filter((param) => param !== "");
-  }
-
-  return paramList;
-}
-
-// This function will cycle through all of the current strings split by a comma, and combine strings that were of generic types and split incorrectly
-function squishGenerics(paramList: string[]): string[] {
-  const newParamList = [];
-
-  let currentStr = "";
-  let numToGet = 1; // Always grab 1 string at the beginning
-
-  for (const item of paramList) {
-     currentStr += item;
-     numToGet--;
-
-     // If numToGet is zero, check the difference between '<' and '>' characters
-     if (numToGet === 0) {
-       const numOfLeftBrackets = currentStr.split("<").length - 1;
-       const numOfRightBrackets = currentStr.split(">").length - 1;
-       const numOfEqualSigns = currentStr.split("=").length - 1;
-
-       // Diff is |num of left brackets ('<') minus the num of solo right brackets (which is the number of '>' minus the num of '=' signs)|
-       const diff = Math.abs(numOfLeftBrackets - (numOfRightBrackets - numOfEqualSigns));
-
-       if ((numOfEqualSigns > 0) || diff === 0) {
-        // If the difference is zero, we have a full param, push it to the new params, and start over at the next string
-        // Also, do this if there is equal signs in the params which exist with arrow functions.
-         newParamList.push(currentStr);
-         currentStr = "";
-         numToGet = 1;
-       } else {
-         // Otherwise, set the number of strings to grab to the diff
-         numToGet = diff;
-       }
-     }
-  }
-
-  return newParamList;
-}
-
-function grabDefLineParams(definitionLine: string): string {
-  let startIdx;
-  let endIdx;
-
-  startIdx = definitionLine.indexOf("(");
-
-  if (startIdx === -1) {
-    return "";
-  } else {
-    startIdx++;
-  }
-
-  let depth = 1;
-
-  for (let i = startIdx; i < definitionLine.length; i++) {
-    if (definitionLine[i] === "(") {
-      depth++;
-    } else if (definitionLine[i] === ")") {
-      depth--;
-      if (depth === 0) {
-        endIdx = i;
-        break;
-      }
-    }
-  }
-
-  if (endIdx === undefined) {
-    return "";
-  }
-
-  return definitionLine.substring(startIdx, endIdx);
-}
-
 function shouldntBeDecorated(paramList: string[], functionCall: IFunctionCallObject, functionCallLine: string, definitionLine: string): boolean {
   // If the functionName is one of the parameters, don't decorate it
   if (paramList.some((param) => param === functionCall.functionName)) {

src/decorator.ts

@@ -0,0 +1,114 @@
+import { IFunctionCallObject } from "./functionCallObject";
+
+export function grabPossibleParameters(fc: IFunctionCallObject, definitionLine: string): string[] {
+  let paramList: string[] = [];
+
+  // Grab any params inside the definition line
+  const defintionParam = grabDefLineParams(definitionLine);
+
+  if (defintionParam !== "") {
+    if (fc.functionRange === undefined) {
+      return;
+    }
+
+    paramList = splitToParamList(defintionParam);
+
+    paramList = paramList.map((param) => {
+      // Extract identifiers
+      const words = param.trim().split(" ");
+
+      // If there are multiple words and the first word doesn't end with a colon, use the 2nd word as the param
+      // this will make sure the param name is used and not the access modifier in TS functions
+      if (words.length > 1 && !words[0].endsWith(":")) {
+        param = words[1];
+      }
+
+      const identifiers = param.match(/([\.a-zA-Z0-9]+):?/);
+
+      if (identifiers && identifiers.length > 1) {
+        return identifiers[1];
+      }
+      return "";
+    }).filter((param) => param !== "");
+  }
+
+  return paramList;
+}
+
+// This function will cycle through all of the current strings split by a comma, and combine strings that were split incorrectly
+export function splitToParamList(defintionParam: string): string[] {
+  const paramList = defintionParam.split(/\s*,\s*/);
+  const newParamList = [];
+
+  let currentStr = "";
+  let numToGet = 1; // Always grab 1 string at the beginning
+
+  for (const item of paramList) {
+    currentStr += item + ", ";
+    numToGet--;
+
+    // If numToGet is zero, check the difference between '<' and '>' characters
+    if (numToGet === 0) {
+      const numOfLessThanBrackets = currentStr.split("<").length - 1;
+      const numOfGreaterThanBrackets = currentStr.split(">").length - 1;
+
+      const numOfOpenParen = currentStr.split("(").length - 1;
+      const numOfCloseParen = currentStr.split(")").length - 1;
+
+      const numOfEqualSigns = currentStr.split("=").length - 1;
+
+      // Diff is |num of left brackets ('<') minus the num of solo right brackets (which is the number of '>' minus the num of '=' signs)|
+      const triBracketDiff = Math.abs(numOfLessThanBrackets - (numOfGreaterThanBrackets - numOfEqualSigns));
+
+      const parenDiff = Math.abs(numOfOpenParen - numOfCloseParen);
+
+      if (((numOfEqualSigns > 0) && (numOfGreaterThanBrackets - numOfEqualSigns) === numOfLessThanBrackets) || (triBracketDiff === 0 && parenDiff === 0)) {
+        // If the difference is zero, we have a full param, push it to the new params, and start over at the next string
+        // Also, do this if there is equal signs in the params which exist with arrow functions.
+        currentStr = currentStr.substr(0, currentStr.length - 2);
+
+        newParamList.push(currentStr);
+        currentStr = "";
+        numToGet = 1;
+      } else {
+        // Otherwise, set the number of strings to grab to the diff
+        numToGet = triBracketDiff + parenDiff;
+      }
+    }
+  }
+
+  return newParamList;
+}
+
+function grabDefLineParams(definitionLine: string): string {
+  let startIdx;
+  let endIdx;
+
+  startIdx = definitionLine.indexOf("(");
+
+  if (startIdx === -1) {
+    return "";
+  } else {
+    startIdx++;
+  }
+
+  let depth = 1;
+
+  for (let i = startIdx; i < definitionLine.length; i++) {
+    if (definitionLine[i] === "(") {
+      depth++;
+    } else if (definitionLine[i] === ")") {
+      depth--;
+      if (depth === 0) {
+        endIdx = i;
+        break;
+      }
+    }
+  }
+
+  if (endIdx === undefined) {
+    return "";
+  }
+
+  return definitionLine.substring(startIdx, endIdx);
+}

src/paramExtractor.ts

@@ -1,9 +1,9 @@
 import * as assert from "assert";
 import * as path from "path";
 import * as vscode from "vscode";
-import * as Extension from "../extension";
+import * as Extension from "../../extension";
 
-const testFolderLocation = "/../../src/test/examples/";
+const testFolderLocation = "/../../../src/test/examples/";
 
 suite("js annotations", () => {
   test("should annotate function with parameters", async () => {

src/test/extension.test.ts src/test/e2e/extension.test.tssrc/test/extension.test.ts renamed to src/test/e2e/extension.test.ts

@@ -0,0 +1,60 @@
+import * as assert from "assert";
+import { splitToParamList } from "../../paramExtractor";
+
+suite("splitToParamList tests", () => {
+  test("should properly split simple parameter def with generic type", () => {
+    const paramDef = "op1: Opt<T, A>";
+    const expectedList = ["op1: Opt<T, A>"];
+    const paramList = splitToParamList(paramDef);
+
+    assert.deepEqual(paramList, expectedList);
+  });
+
+  test("should properly split parameter def with nested generic type", () => {
+    const paramDef = "op1: Opt<T, Test<A, B, C>, D>";
+    const expectedList = ["op1: Opt<T, Test<A, B, C>, D>"];
+    const paramList = splitToParamList(paramDef);
+
+    assert.deepEqual(paramList, expectedList);
+  });
+
+  test("should work with parameter def with arrow functions", () => {
+    const paramDef = "fn: (num: number, str: string) => string, opts: any";
+    const expectedList = ["fn: (num: number, str: string) => string", "opts: any"];
+    const paramList = splitToParamList(paramDef);
+
+    assert.deepEqual(paramList, expectedList);
+  });
+
+  test("should work with parameter def with arrow functions and generic return type", () => {
+    const paramDef = "fn: (num: number, str: string) => Opt<A, B>, opts: any";
+    const expectedList = ["fn: (num: number, str: string) => Opt<A, B>", "opts: any"];
+    const paramList = splitToParamList(paramDef);
+
+    assert.deepEqual(paramList, expectedList);
+  });
+
+  test("should work with complex parameter def with arrow functions and generic return type", () => {
+    const paramDef = "fn: (num: number, strOpt: Opt<A, B, C>) => Opt<A, B>, opts: any";
+    const expectedList = ["fn: (num: number, strOpt: Opt<A, B, C>) => Opt<A, B>", "opts: any"];
+    const paramList = splitToParamList(paramDef);
+
+    assert.deepEqual(paramList, expectedList);
+  });
+
+  test("should be fine with primitive types", () => {
+    const paramDef = "str: string, opts: any";
+    const expectedList = ["str: string", "opts: any"];
+    const paramList = splitToParamList(paramDef);
+
+    assert.deepEqual(paramList, expectedList);
+  });
+
+  test("should be fine with rest params", () => {
+    const paramDef = "cats: number, ...rest: string[]";
+    const expectedList = ["cats: number", "...rest: string[]"];
+    const paramList = splitToParamList(paramDef);
+
+    assert.deepEqual(paramList, expectedList);
+  });
+});