Address minor TODOs

packages/cli/package.json

@@ -38,12 +38,14 @@
     "consola": "^3.2.3",
     "find-up": "^6.3.0",
     "open": "^9.1.0",
+    "semver": "^7.6.0",
     "yargs": "^17.7.2"
   },
   "devDependencies": {
     "@types/archiver": "^6.0.0",
     "@types/ngeohash": "^0.6.4",
     "@types/node": "^18.0.0",
+    "@types/semver": "^7.5.7",
     "@types/yargs": "^17.0.29",
     "ts-expect": "^1.3.0",
     "typescript": "^5.2.2"
@@ -54,7 +56,7 @@
   "imports": {
     "#net": "./src/net/index.mts"
   },
-  "keywords": [ ],
+  "keywords": [],
   "bin": {
     "osdk": "./bin/osdk.mjs"
   },

packages/cli/src/commands/site/deploy/siteDeployCommand.mts

@@ -59,7 +59,7 @@ export default async function siteDeployCommand(
   if (version != null) {
     siteVersion = version;
   } else {
-    siteVersion = await findAutoVersion(gitTagPrefix);
+    siteVersion = findAutoVersion(gitTagPrefix);
     consola.info(
       `Auto version inferred next version to be: ${siteVersion}`,
     );

packages/cli/src/commands/site/version/list/versionListCommand.mts

@@ -55,7 +55,10 @@ export default async function versionListCommand(
   }
 
   consola.success("Found versions:");
-  for (const version of versions) {
+
+  const semver = await import("semver");
+  const sortedVersions = semver.rsort(versions.filter(v => semver.valid(v)));
+  for (const version of sortedVersions) {
     consola.log(
       `    - ${version}${
         deployedVersion

packages/cli/src/util/autoVersion.test.ts

@@ -23,11 +23,11 @@ vi.mock("node:child_process");
 describe("autoVersion", () => {
   const execSyncMock = vi.mocked(execSync);
 
-  it("should return a valid SemVer version from git describe", async () => {
+  it("should return a valid SemVer version from git describe", () => {
     const validGitVersion = "1.2.3";
     execSyncMock.mockReturnValue(validGitVersion);
 
-    const version = await autoVersion();
+    const version = autoVersion();
     expect(version).toBe("1.2.3");
 
     expect(execSyncMock).toHaveBeenCalledWith(
@@ -36,10 +36,10 @@ describe("autoVersion", () => {
     );
   });
 
-  it("should replace default prefix v from git describe output", async () => {
+  it("should replace default prefix v from git describe output", () => {
     const validGitVersion = "v1.2.3";
     execSyncMock.mockReturnValue(validGitVersion);
-    const version = await autoVersion();
+    const version = autoVersion();
 
     expect(version).toBe("1.2.3");
     expect(execSyncMock).toHaveBeenCalledWith(
@@ -48,11 +48,11 @@ describe("autoVersion", () => {
     );
   });
 
-  it("should replace the prefix from the found git tag", async () => {
+  it("should replace the prefix from the found git tag", () => {
     const validGitVersion = "@[email protected]";
     execSyncMock.mockReturnValue(validGitVersion);
 
-    const version = await autoVersion("@package@");
+    const version = autoVersion("@package@");
 
     expect(version).toBe("1.2.3");
     expect(execSyncMock).toHaveBeenCalledWith(
@@ -61,11 +61,11 @@ describe("autoVersion", () => {
     );
   });
 
-  it("should only replace the prefix if found at the start of the tag only", async () => {
+  it("should only replace the prefix if found at the start of the tag only", () => {
     const validGitVersion = "1.2.3-package";
     execSyncMock.mockReturnValue(validGitVersion);
 
-    const version = await autoVersion("-package");
+    const version = autoVersion("-package");
 
     expect(version).toBe("1.2.3-package");
     expect(execSyncMock).toHaveBeenCalledWith(
@@ -74,10 +74,39 @@ describe("autoVersion", () => {
     );
   });
 
-  it("should throw an error if git describe returns a non-SemVer string", async () => {
+  it("should throw an error if git describe returns a non-SemVer string", () => {
     const nonSemVerGitVersion = "not-semver";
     execSyncMock.mockReturnValue(nonSemVerGitVersion);
+    expect(() => autoVersion()).toThrowError();
+  });
+
+  it("should throw an error if git isn't found", () => {
+    execSyncMock.mockImplementation(() => {
+      throw new Error("Command not found");
+    });
+
+    expect(() => autoVersion()).toThrowError(
+      "Unable to determine the version using git-describe as git is not installed",
+    );
+  });
+
+  it("should throw an error if git isn't found", () => {
+    execSyncMock.mockImplementation(() => {
+      throw new Error("fatal: not a git repository");
+    });
 
-    await expect(autoVersion()).rejects.toThrow();
+    expect(() => autoVersion()).toThrowError(
+      "Unable to determine the version using git-describe as the current directory is not a git repository",
+    );
+  });
+
+  it("should throw an error if git isn't found", () => {
+    execSyncMock.mockImplementation(() => {
+      throw new Error("fatal: no names found, cannot describe anything.");
+    });
+
+    expect(() => autoVersion()).toThrowError(
+      "Unable to determine the version using git-describe as no tags were found.",
+    );
   });
 });

packages/cli/src/util/autoVersion.ts

@@ -25,30 +25,72 @@ import { isValidSemver } from "./isValidSemver.js";
  * @returns A promise that resolves to the version string.
  * @throws An error if the version string is not SemVer compliant or if the version cannot be determined.
  */
-export async function autoVersion(tagPrefix: string = ""): Promise<string> {
+export function autoVersion(tagPrefix: string = ""): string {
   const [matchPrefix, prefixRegex] = tagPrefix !== ""
     ? [tagPrefix, new RegExp(`^${tagPrefix}`)]
     : [undefined, new RegExp(`^v?`)];
+
+  const gitVersion = gitDescribe(matchPrefix);
+  const version = gitVersion.trim().replace(prefixRegex, "");
+  if (!isValidSemver(version)) {
+    throw new ExitProcessError(
+      2,
+      `The version string ${version} is not SemVer compliant.`,
+    );
+  }
+
+  return version;
+}
+
+function gitDescribe(matchPrefix: string | undefined): string {
+  let gitVersion;
   try {
-    const gitVersion = execSync(
+    gitVersion = execSync(
       `git describe --tags --first-parent --dirty${
         matchPrefix != null ? ` --match="${matchPrefix}*"` : ""
       }`,
       { encoding: "utf8" },
     );
-    const version = gitVersion.trim().replace(prefixRegex, "");
-    if (!isValidSemver(version)) {
-      throw new ExitProcessError(
-        2,
-        `The version string ${version} is not SemVer compliant.`,
-      );
+  } catch (error: any) {
+    if (error instanceof Error) {
+      const errorMessage: string = error.message.toLowerCase();
+
+      if (
+        errorMessage.includes("not recognized")
+        || errorMessage.includes("command not found")
+        || errorMessage.includes("no such file or directory")
+      ) {
+        throw new ExitProcessError(
+          2,
+          `Unable to determine the version using git-describe as git is not installed or found in the PATH.\nPlease correctly configure git or supply a --version option.`,
+        );
+      }
+
+      if (
+        errorMessage.includes("fatal: not a git repository")
+      ) {
+        throw new ExitProcessError(
+          2,
+          `Unable to determine the version using git-describe as the current directory is not a git repository.\nPlease run the command in a git respository or supply a --version option.`,
+        );
+      }
+
+      if (
+        errorMessage.includes(
+          "fatal: no names found, cannot describe anything.",
+        )
+      ) {
+        throw new ExitProcessError(
+          2,
+          `Unable to determine the version using git-describe as no tags were found.\nPlease tag a version or supply a --version option.`,
+        );
+      }
     }
 
-    return version;
-  } catch (error) {
     throw new ExitProcessError(
       2,
-      `Unable to determine the version automatically. Please supply a --version argument. ${error}`,
+      `Unable to determine the version automatically: ${error}.\nPlease supply a --version argument.`,
     );
   }
+  return gitVersion;
 }

packages/cli/src/util/config.test.ts

@@ -16,14 +16,19 @@
 
 import { findUp } from "find-up";
 import { promises as fsPromises } from "node:fs";
-import { describe, expect, it, vi } from "vitest";
+import { extname } from "node:path";
+import { beforeEach, describe, expect, it, vi } from "vitest";
 import { loadFoundryConfig } from "./config.js";
 
 vi.mock("find-up");
 vi.mock("node:fs");
+vi.mock("node:path");
 
 describe("loadFoundryConfig", () => {
-  vi.mocked(findUp).mockResolvedValue("/path/foundry.config.json");
+  beforeEach(() => {
+    vi.mocked(findUp).mockResolvedValue("/path/foundry.config.json");
+    vi.mocked(extname).mockReturnValue(".json");
+  });
 
   it("should load and parse the configuration file correctly", async () => {
     const correctConfig = {
@@ -149,4 +154,11 @@ describe("loadFoundryConfig", () => {
     vi.mocked(findUp).mockResolvedValue(undefined);
     await expect(loadFoundryConfig()).resolves.toBeUndefined();
   });
+
+  it("should throw if config file extension isn't supported", async () => {
+    vi.mocked(extname).mockResolvedValue(".yaml");
+    await expect(loadFoundryConfig()).rejects.toThrow(
+      "Unsupported file extension:",
+    );
+  });
 });

packages/cli/src/util/config.ts

@@ -16,6 +16,7 @@
 
 import type { JSONSchemaType } from "ajv";
 import { promises as fsPromises } from "node:fs";
+import { extname } from "node:path";
 import { ExitProcessError } from "../ExitProcessError.js";
 
 export interface GitDescribeAutoVersionConfig {
@@ -44,6 +45,10 @@ const CONFIG_FILE_NAMES: string[] = [
   "foundry.config.json",
 ];
 
+const SUPPORTED_EXTENSIONS: string[] = [
+  ".json",
+];
+
 const CONFIG_FILE_SCHEMA: JSONSchemaType<FoundryConfig> = {
   type: "object",
   properties: {
@@ -91,10 +96,17 @@ export async function loadFoundryConfig(): Promise<
   const configFilePath = await findUp(CONFIG_FILE_NAMES);
 
   if (configFilePath) {
+    const extension = extname(configFilePath);
+    if (!SUPPORTED_EXTENSIONS.includes(extension)) {
+      throw new ExitProcessError(
+        2,
+        `Unsupported file extension: ${extension} for config file. Only the following extensions are allowed ${SUPPORTED_EXTENSIONS}`,
+      );
+    }
     let foundryConfig: FoundryConfig;
     try {
       const fileContent = await fsPromises.readFile(configFilePath, "utf-8");
-      foundryConfig = JSON.parse(fileContent);
+      foundryConfig = parseConfigFile(fileContent, extension);
     } catch {
       throw new ExitProcessError(
         2,
@@ -116,3 +128,13 @@ export async function loadFoundryConfig(): Promise<
 
   return undefined;
 }
+
+function parseConfigFile(
+  fileContent: string,
+  extension: string,
+): FoundryConfig {
+  if (extension === ".json") {
+    return JSON.parse(fileContent);
+  }
+  throw Error();
+}

packages/cli/src/util/isValidSemver.ts

@@ -14,10 +14,8 @@
  * limitations under the License.
  */
 
-// https://semver.org/#is-there-a-suggested-regular-expression-regex-to-check-a-semver-string
-const semver =
-  /^(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)(?:-((?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\+([0-9a-zA-Z-]+(?:\.[0-9a-zA-Z-]+)*))?$/;
+import { valid } from "semver";
 
 export function isValidSemver(semverString: string): boolean {
-  return semver.test(semverString);
+  return valid(semverString) != null;
 }