#8636: handle optional chaining corner cases

src/components/fields/schemaFields/widgets/varPopup/likelyVariableUtils.test.ts

@@ -325,8 +325,23 @@ describe("getFullVariableName", () => {
     );
   });
 
-  it("handles array", () => {
-    // TODO: handle optional chaining with arrays
-    // expect(getFullVariableName("@foo.bar[42]?", ["@foo", "bar", "42", "qux"])).toBe("@foo.bar[42]?.qux");
+  it("handles optional chaining with bracket notation", () => {
+    expect(
+      getFullVariableName("@foo.bar?.[42]", ["@foo", "bar", "42", "qux"]),
+    ).toBe("@foo.bar?.[42].qux");
+    expect(
+      getFullVariableName("@foo.bar[42]?", ["@foo", "bar", "42", "qux"]),
+    ).toBe("@foo.bar[42]?.qux");
+    expect(
+      getFullVariableName('@foo.bar[""]?', ["@foo", "bar", "", "qux"]),
+    ).toBe('@foo.bar[""]?.qux');
+    expect(
+      getFullVariableName('@foo.bar["hello world?"]?', [
+        "@foo",
+        "bar",
+        "hello world?",
+        "qux",
+      ]),
+    ).toBe('@foo.bar["hello world?"]?.qux');
   });
 });

src/components/fields/schemaFields/widgets/varPopup/likelyVariableUtils.ts

@@ -200,17 +200,33 @@ export function getFullVariableName(
   likelyVariable: string,
   selectedPath: string[],
 ): string {
-  const originalPath = toPath(likelyVariable);
+  // `toPath` will create a separate element for the ? symbol. So we need to merge them back. Eventually we need to
+  // switch from `toPath` to our own implementation/parser.
+  // @foo.bar[42]? -> [@foo, bar, 42, ?]
+  const pathWithChainElements = toPath(likelyVariable);
+
+  const likelyPath: string[] = [];
+  for (let i = 0; i < pathWithChainElements.length; i++) {
+    // eslint-disable-next-line security/detect-object-injection -- numeric index
+    const base: string = pathWithChainElements[i]!;
+
+    if (pathWithChainElements[i + 1] === "?") {
+      likelyPath.push(base + "?");
+      i++;
+    } else {
+      likelyPath.push(base);
+    }
+  }
 
   return getPathFromArray(
     selectedPath.map((part, index) => {
       // Preserve optional chaining from what the use has typed so far
-      // eslint-disable-next-line security/detect-object-injection -- numeric index
       if (
-        originalPath[index]?.endsWith("?") &&
+        // eslint-disable-next-line security/detect-object-injection -- numeric index
+        likelyPath[index]?.endsWith("?") &&
         index !== selectedPath.length - 1
       ) {
-        return part + "?";
+        return { part, isOptional: true };
       }
 
       return part;

src/runtime/pathHelpers.test.ts

@@ -145,35 +145,54 @@ describe("getFieldNamesFromPathString", () => {
   });
 });
 
-test("getPathFromArray", () => {
-  const expectMatch = (
-    pathArray: Array<number | string>,
-    expectedPathString: string,
-  ) => {
-    const pathString = getPathFromArray(pathArray);
-    const lodashArray = toPath(pathString);
-
-    // Compare the array to the expected string
-    expect(pathString).toBe(expectedPathString);
-
-    // Expect the same input, except that lodash only returns strings even for numbers
-    expect(lodashArray).toEqual(pathArray.map(String));
-  };
-
-  expectMatch(["user"], "user");
-  expectMatch(["users", 0], "users[0]");
-  expectMatch(["users", 0, "name"], "users[0].name");
-  expectMatch(["users", ""], 'users[""]');
-  expectMatch(["names", "Dante Alighieri"], 'names["Dante Alighieri"]');
-  expectMatch(
-    ["Ugo Foscolo", "User Location"],
-    '["Ugo Foscolo"]["User Location"]',
-  );
-  expectMatch(["User List", 100, "id"], '["User List"][100].id');
-  expectMatch(
-    ["User List", 100_000_000, "The name"],
-    '["User List"][100000000]["The name"]',
-  );
+describe("getPathFromArray", () => {
+  test("required path parts", () => {
+    const expectMatch = (
+      pathArray: Array<number | string>,
+      expectedPathString: string,
+    ) => {
+      const pathString = getPathFromArray(pathArray);
+      const lodashArray = toPath(pathString);
+
+      // Compare the array to the expected string
+      expect(pathString).toBe(expectedPathString);
+
+      // Expect the same input, except that lodash only returns strings even for numbers
+      expect(lodashArray).toEqual(pathArray.map(String));
+    };
+
+    expectMatch(["user"], "user");
+    expectMatch(["users", 0], "users[0]");
+    expectMatch(["users", 0, "name"], "users[0].name");
+    expectMatch(["users", ""], 'users[""]');
+    expectMatch(["names", "Dante Alighieri"], 'names["Dante Alighieri"]');
+    expectMatch(
+      ["Ugo Foscolo", "User Location"],
+      '["Ugo Foscolo"]["User Location"]',
+    );
+    expectMatch(["User List", 100, "id"], '["User List"][100].id');
+    expectMatch(
+      ["User List", 100_000_000, "The name"],
+      '["User List"][100000000]["The name"]',
+    );
+  });
+
+  test("optional chaining path parts", () => {
+    expect(
+      getPathFromArray(["users", { part: 0, isOptional: true }, "name"]),
+    ).toBe("users[0]?.name");
+    expect(
+      getPathFromArray(["users?", { part: 0, isOptional: true }, "name"]),
+    ).toBe("users?.[0]?.name");
+    expect(
+      getPathFromArray([
+        "users",
+        "foo bar?",
+        { part: 0, isOptional: true },
+        "name",
+      ]),
+    ).toBe('users["foo bar?"].[0]?.name');
+  });
 });
 
 test.each([

src/runtime/pathHelpers.ts

@@ -15,7 +15,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-import { identity, toPath } from "lodash";
+import { identity, toPath, trimEnd } from "lodash";
 import { getErrorMessage } from "@/errors/errorHelpers";
 import { cleanValue, isObject } from "@/utils/objectUtils";
 import { joinName } from "@/utils/formUtils";
@@ -151,6 +151,52 @@ export function getFieldNamesFromPathString(
   return [parentFieldName, fieldName];
 }
 
+/**
+ * Normalize an array of parts into an explicit format simpler for joining the path back together.
+ */
+function normalizePart(
+  partOrRecord:
+    | string
+    | number
+    | { part: string | number; isOptional: boolean },
+): { part: string; isOptional: boolean; isBrackets: boolean } {
+  if (typeof partOrRecord === "string") {
+    if (partOrRecord === "" || /[ .]/.test(partOrRecord)) {
+      return {
+        part: `["${partOrRecord}"]`,
+        isOptional: false,
+        isBrackets: true,
+      };
+    }
+
+    // Treat numeric strings as array access
+    if (/\d+/.test(partOrRecord)) {
+      return {
+        part: `[${partOrRecord}]`,
+        isOptional: false,
+        isBrackets: true,
+      };
+    }
+
+    return {
+      part: trimEnd(partOrRecord, "?"),
+      isOptional: partOrRecord.endsWith("?"),
+      isBrackets: false,
+    };
+  }
+
+  if (typeof partOrRecord === "number") {
+    return { part: `[${partOrRecord}]`, isOptional: false, isBrackets: true };
+  }
+
+  const normalized = normalizePart(partOrRecord.part);
+
+  return {
+    ...normalized,
+    isOptional: partOrRecord.isOptional || normalized.isOptional,
+  };
+}
+
 // Counterpart to _.toPath: https://lodash.com/docs/4.17.15#toPath
 // `fromPath` Missing from lodash: https://github.com/lodash/lodash/issues/2169
 /**
@@ -160,18 +206,25 @@ export function getFieldNamesFromPathString(
  * @example getPathFromArray(["user", "name"]) // => "user.name"
  * @example getPathFromArray(["title", "Divine Comedy"]) // => "title["Divine Comedy"]"
  */
-export function getPathFromArray(parts: Array<string | number>): string {
-  return parts
-    .map((part, index) => {
-      if (part === "" || (typeof part === "string" && /[ .]/.test(part))) {
-        return `["${part}"]`;
-      }
-
-      if (typeof part === "number" || /^\d+$/.test(part)) {
-        return `[${part}]`;
+export function getPathFromArray(
+  parts: Array<
+    string | number | { part: string | number; isOptional: boolean }
+  >,
+): string {
+  const normalizedParts = parts.map((x) => normalizePart(x));
+
+  return normalizedParts
+    .map(({ part, isOptional, isBrackets }, index) => {
+      let modified = part;
+
+      if (isOptional) {
+        modified = `${part}?`;
       }
 
-      return index === 0 ? part : `.${part}`;
+      return index === 0 ||
+        (isBrackets && !normalizedParts[index - 1]?.isOptional)
+        ? modified
+        : `.${modified}`;
     })
     .join("");
 }