chore: address copilot suggestions

Author: himanshusinghs

src/common/config/configUtils.ts

@@ -42,10 +42,10 @@ export function isConnectionSpecifier(arg: string | undefined): boolean {
         arg !== undefined &&
         (arg.startsWith("mongodb://") ||
             arg.startsWith("mongodb+srv://") ||
-            // Strings starting with a hyphen `-` are generally a sign of CLI
-            // flag so we exclude them from the possibility of being a
+            // Strings starting with double hyphens `--` are generally a sign of
+            // CLI flag so we exclude them from the possibility of being a
             // connection specifier.
-            !(arg.endsWith(".js") || arg.endsWith(".mongodb") || arg.startsWith("-")))
+            !(arg.endsWith(".js") || arg.endsWith(".mongodb") || arg.startsWith("--")))
     );
 }
 

src/common/config/createUserConfig.ts

@@ -108,15 +108,23 @@ function parseUserConfigSources(cliArguments: string[]): {
     // no-op statement so ESLint does not complain.
     void endOfFlagArguments;
 
+    // A connectionSpecifier can be one of:
+    // - database name
+    // - host name
+    // - ip address
+    // - replica set specifier
+    // - complete connection string
+    let connectionSpecifier: string | undefined = undefined;
     const [maybeConnectionSpecifier, ...unknownArguments] = positionalAndUnknownArguments;
-    // If the extracted connection specifier is not a connection specifier
-    // indeed, then we push it back to the unknown arguments list. This might
-    // happen for example when an unknown argument is provided without ever
-    // specifying a positional argument.
-    if (typeof maybeConnectionSpecifier !== "string" || !isConnectionSpecifier(maybeConnectionSpecifier)) {
-        if (maybeConnectionSpecifier) {
-            unknownArguments.unshift(maybeConnectionSpecifier);
-        }
+
+    if (typeof maybeConnectionSpecifier === "string" && isConnectionSpecifier(maybeConnectionSpecifier)) {
+        connectionSpecifier = maybeConnectionSpecifier;
+    } else if (maybeConnectionSpecifier !== undefined) {
+        // If the extracted connection specifier is not a connection specifier
+        // indeed, then we push it back to the unknown arguments list. This might
+        // happen for example when an unknown argument is provided without ever
+        // specifying a positional argument.
+        unknownArguments.unshift(maybeConnectionSpecifier);
     }
 
     return {
@@ -142,16 +150,7 @@ function parseUserConfigSources(cliArguments: string[]): {
             })
             .filter((argument) => typeof argument === "string"),
         userAndArgsParserConfig: parsedUserAndArgsParserConfig,
-        // A connectionSpecifier can be one of:
-        // - database name
-        // - host name
-        // - ip address
-        // - replica set specifier
-        // - complete connection string
-        connectionSpecifier:
-            typeof maybeConnectionSpecifier === "string" && isConnectionSpecifier(maybeConnectionSpecifier)
-                ? String(maybeConnectionSpecifier)
-                : undefined,
+        connectionSpecifier,
     };
 }
 

tests/unit/common/config.test.ts

@@ -916,16 +916,17 @@ describe("keychain management", () => {
     const testCases = [
         { cliArg: "apiClientId", secretKind: "user" },
         { cliArg: "apiClientSecret", secretKind: "password" },
-        // Note: These arguments were part of original test cases before
-        // refactor of Config but because now we use yargs-parser to strictly
-        // parse the config and do not allow unknown arguments to creep into the
-        // final results, these arguments never end up in the config. It is
-        // because we have the mongosh OPTIONS copied over from the repo and the
-        // copied object does not contain these are parse targets.
-
-        //  TODO: Whenever we finish importing OPTIONS from mongosh these test
-        // cases should be good to be enabled again.
-
+        /*
+         * Note: These arguments were part of original test cases before
+         * refactor of Config but because now we use yargs-parser to strictly
+         * parse the config and do not allow unknown arguments to creep into the
+         * final results, these arguments never end up in the config. It is
+         * because we have the mongosh OPTIONS copied over from the repo and the
+         * copied object does not contain these as parse targets.
+         *
+         * TODO: Whenever we finish importing OPTIONS from mongosh these test
+         * cases should be good to be enabled again.
+         */
         // { cliArg: "awsAccessKeyId", secretKind: "password" },
         // { cliArg: "awsIamSessionToken", secretKind: "password" },
         // { cliArg: "awsSecretAccessKey", secretKind: "password" },