Some improvements to findCommonParentDir and some more unit tests

Author: charisk

extensions/ql-vscode/src/common/files.ts

@@ -1,6 +1,6 @@
 import { pathExists, stat, readdir, opendir } from "fs-extra";
-import { isAbsolute, join, relative, resolve, sep } from "path";
-import { tmpdir as osTmpdir } from "os";
+import { isAbsolute, join, parse, relative, resolve, sep } from "path";
+import { homedir, tmpdir as osTmpdir } from "os";
 
 /**
  * Recursively finds all .ql files in this set of Uris.
@@ -134,24 +134,27 @@ export function tmpdir(): string {
 }
 
 /**
- * Finds the common parent directory of an arbitrary number of paths. If the paths are absolute,
- * the result is also absolute. If the paths are relative, the result is relative.
+ * Finds the common parent directory of an arbitrary number of paths. The result
+ * will be an absolute path.
  * @param paths The array of paths.
  * @returns The common parent directory of the paths.
  */
 export function findCommonParentDir(...paths: string[]): string {
+  const normalizedPaths = paths.map((path) => normalizePath(path));
+
   // Split each path into its components
-  const pathParts = paths.map((path) => path.split(sep));
+  const pathParts = normalizedPaths.map((path) => path.split(sep));
+
+  // Start with the root directory
+  let commonDir = parse(homedir()).root;
 
-  let commonDir = "";
-  // Iterate over the components of the first path and checks if the same
+  // Iterate over the components of the first path and check if the same
   // component exists at the same position in all the other paths. If it does,
   // add the component to the common directory. If it doesn't, stop the
   // iteration and returns the common directory found so far.
-  for (let i = 0; i < pathParts[0].length; i++) {
-    const part = pathParts[0][i];
+  for (const [i, part] of pathParts[0].entries()) {
     if (pathParts.every((parts) => parts[i] === part)) {
-      commonDir = `${commonDir}${part}${sep}`;
+      commonDir = join(commonDir, part);
     } else {
       break;
     }

extensions/ql-vscode/test/unit-tests/common/files.test.ts

@@ -1,4 +1,4 @@
-import { join } from "path";
+import { join, parse } from "path";
 
 import {
   containsPath,
@@ -13,6 +13,7 @@ import type { DirResult } from "tmp";
 import { dirSync } from "tmp";
 import { ensureDirSync, symlinkSync, writeFileSync } from "fs-extra";
 import "../../matchers/toEqualPath";
+import { homedir } from "os";
 
 describe("files", () => {
   const dataDir = join(__dirname, "../../data");
@@ -461,7 +462,11 @@ describe("walkDirectory", () => {
 });
 
 describe("findCommonParentDir", () => {
-  it("should find the common parent dir for multiple paths with common parent", () => {
+  const dataDir = join(__dirname, "../../data");
+  const rootDir = parse(homedir()).root;
+  const sourceDir = join(dataDir, "../..");
+
+  it("should find the common parent dir for multiple relative paths with common parent", () => {
     const paths = [
       join("foo", "bar", "baz"),
       join("foo", "bar", "qux"),
@@ -473,7 +478,7 @@ describe("findCommonParentDir", () => {
     expect(commonDir).toEqualPath(join("foo", "bar"));
   });
 
-  it("should return the root if paths have no common parent other than root", () => {
+  it("should return empty path if relative paths have no common parent", () => {
     const paths = [
       join("foo", "bar", "baz"),
       join("qux", "quux", "corge"),
@@ -485,15 +490,115 @@ describe("findCommonParentDir", () => {
     expect(commonDir).toEqualPath("");
   });
 
-  it("should return empty string for relative paths with no common parent", () => {
+  it("should find the common parent dir for multiple absolute paths with common parent", () => {
+    const paths = [
+      join(dataDir, "foo", "bar", "baz"),
+      join(dataDir, "foo", "bar", "qux"),
+      join(dataDir, "foo", "bar", "quux"),
+    ];
+
+    const commonDir = findCommonParentDir(...paths);
+
+    expect(commonDir).toEqualPath(join(dataDir, "foo", "bar"));
+  });
+
+  it("should return the root if absolute paths have no common parent other than root", () => {
+    const paths = [
+      join("/foo", "bar", "baz"),
+      join("/qux", "quux", "corge"),
+      join("/grault", "garply"),
+    ];
+
+    const commonDir = findCommonParentDir(...paths);
+
+    expect(commonDir).toEqualPath(rootDir);
+  });
+
+  it("should handle a mix of absolute and relative paths", async () => {
+    const paths = [
+      join(dataDir, "foo", "bar", "baz"),
+      join("foo", "bar", "qux"),
+      join(dataDir, "foo", "bar", "quux"),
+    ];
+
+    const commonDir = findCommonParentDir(...paths);
+
+    expect(commonDir).toEqualPath(sourceDir);
+  });
+
+  it("should handle a mix of dirs and files", async () => {
+    const paths = [
+      join(dataDir, "foo", "bar", "baz"),
+      join(dataDir, "foo", "bar", "qux.ql"),
+      join(dataDir, "foo", "bar", "quux"),
+    ];
+
+    const commonDir = findCommonParentDir(...paths);
+
+    expect(commonDir).toEqualPath(join(dataDir, "foo", "bar"));
+  });
+
+  it("should handle dirs that have the same name", async () => {
+    const paths = [
+      join("foo", "foo", "bar"),
+      join("foo", "foo", "baz"),
+      join("foo", "foo"),
+    ];
+
+    const commonDir = findCommonParentDir(...paths);
+
+    expect(commonDir).toEqualPath(join("foo", "foo"));
+  });
+
+  it("should handle dirs that have the same subdir structure but different base path", async () => {
+    const paths = [
+      join("foo", "bar"),
+      join("bar", "foo", "bar"),
+      join("foo", "foo", "bar"),
+    ];
+
+    const commonDir = findCommonParentDir(...paths);
+
+    expect(commonDir).toEqualPath("");
+  });
+
+  it("should handle a single path", async () => {
+    const paths = [join("foo", "bar", "baz")];
+
+    const commonDir = findCommonParentDir(...paths);
+
+    expect(commonDir).toEqualPath(join("foo", "bar", "baz"));
+  });
+
+  it("should return the same path if all paths are identical", () => {
     const paths = [
       join("foo", "bar", "baz"),
-      join("qux", "quux", "corge"),
-      join("grault", "garply"),
+      join("foo", "bar", "baz"),
+      join("foo", "bar", "baz"),
     ];
 
     const commonDir = findCommonParentDir(...paths);
 
+    expect(commonDir).toEqualPath(join("foo", "bar", "baz"));
+  });
+
+  it("should return the directory path if paths only differ by the file extension", () => {
+    const paths = [
+      join("foo", "bar", "baz.txt"),
+      join("foo", "bar", "baz.jpg"),
+      join("foo", "bar", "baz.pdf"),
+    ];
+
+    const commonDir = findCommonParentDir(...paths);
+
+    expect(commonDir).toEqualPath(join("foo", "bar"));
+  });
+
+  it("should handle empty paths", () => {
+    const paths = ["", "", ""];
+
+    const commonDir = findCommonParentDir(...paths);
+
     expect(commonDir).toEqualPath("");
   });
 });