src: use for loop in Dotenv::GetPathFromArgs

Author: avivkeller

node.gyp

@@ -421,6 +421,7 @@
       'test/cctest/test_traced_value.cc',
       'test/cctest/test_util.cc',
       'test/cctest/test_dataqueue.cc',
+      'test/cctest/test_dotenv.cc',
     ],
     'node_cctest_openssl_sources': [
       'test/cctest/test_crypto_clienthello.cc',

src/node_dotenv.cc

@@ -13,33 +13,22 @@ using v8::String;
 
 std::vector<std::string> Dotenv::GetPathFromArgs(
     const std::vector<std::string>& args) {
-  const auto find_match = [](const std::string& arg) {
-    return arg == "--" || arg == "--env-file" || arg.starts_with("--env-file=");
-  };
   std::vector<std::string> paths;
-  auto path = std::find_if(args.begin(), args.end(), find_match);
+  for (size_t i = 1; i < args.size(); ++i) {
+    const auto& arg = args[i];
 
-  while (path != args.end()) {
-    if (*path == "--") {
-      return paths;
+    if (arg == "--" || arg[0] != '-') {
+      break;
     }
-    auto equal_char = path->find('=');
-
-    if (equal_char != std::string::npos) {
-      paths.push_back(path->substr(equal_char + 1));
-    } else {
-      auto next_path = std::next(path);
 
-      if (next_path == args.end()) {
-        return paths;
-      }
-
-      paths.push_back(*next_path);
+    if (arg.starts_with("--env-file=")) {
+      paths.push_back(arg.substr(11));  // Directly extract the path
+    } else if (arg == "--env-file" && i + 1 < args.size()) {
+      paths.push_back(args[++i]);  // Advance to the next argument
+    } else if (arg[1] != '-') {
+      ++i;  // Skip short argument values (like `-e <...>`)
     }
-
-    path = std::find_if(++path, args.end(), find_match);
   }
-
   return paths;
 }
 

test/cctest/test_dotenv.cc

@@ -0,0 +1,34 @@
+#include <iostream>
+#include <vector>
+#include "gtest/gtest.h"
+#include "node_dotenv.h"
+
+using node::Dotenv;
+
+#define CHECK_DOTENV(args, expected)                                           \
+  {                                                                            \
+    auto check_result =                                                        \
+        Dotenv::GetPathFromArgs(std::vector<std::string> args);                \
+    ASSERT_EQ(check_result.size(), expected.size());                           \
+    for (size_t i = 0; i < check_result.size(); ++i) {                         \
+      EXPECT_EQ(check_result[i], expected[i]);                                 \
+    }
+
+TEST(Dotenv, GetPathFromArgs) {
+  CHECK_DOTENV(({"node", "--env-file=.env"}),
+               (std::vector<std::string>{".env"}));
+  CHECK_DOTENV(({"node", "--env-file", ".env"}),
+               (std::vector<std::string>{".env"}));
+  CHECK_DOTENV(({"node", "--env-file=.env", "--env-file", "other.env"}),
+               (std::vector<std::string>{".env", "other.env"}));
+
+  CHECK_DOTENV(
+      ({"node", "--env-file=before.env", "script.js", "--env-file=after.env"}),
+      (std::vector<std::string>{"before.env"}));
+  CHECK_DOTENV(
+      ({"node", "--env-file=before.env", "--", "--env-file=after.env"}),
+      (std::vector<std::string>{"before.env"}));
+
+  CHECK_DOTENV(({"node", "-e", "...", "--env-file=after.env"}),
+               (std::vector<std::string>{"after.env"}));
+}

test/parallel/test-dotenv-edge-cases.js

@@ -98,18 +98,4 @@ describe('.env supports edge cases', () => {
     assert.strictEqual(child.stderr, '');
     assert.strictEqual(child.code, 0);
   });
-
-  it('should handle when --env-file is passed along with --', async () => {
-    const child = await common.spawnPromisified(
-      process.execPath,
-      [
-        '--eval', `require('assert').strictEqual(process.env.BASIC, undefined);`,
-        '--', '--env-file', validEnvFilePath,
-      ],
-      { cwd: fixtures.path('dotenv') },
-    );
-    assert.strictEqual(child.stdout, '');
-    assert.strictEqual(child.stderr, '');
-    assert.strictEqual(child.code, 0);
-  });
 });