Merge pull request #3879 from epage/env

fix(parser): Ensure globals override env vars

src/parser/arg_matcher.rs

@@ -74,9 +74,7 @@ impl ArgMatcher {
                 // a default value of `other` myprog would have an existing MatchedArg for
                 // `--global-arg` where the value is `other`
                 let to_update = if let Some(parent_ma) = vals_map.get(global_arg) {
-                    if parent_ma.check_explicit(ArgPredicate::IsPresent)
-                        && !ma.check_explicit(ArgPredicate::IsPresent)
-                    {
+                    if parent_ma.source() > ma.source() {
                         parent_ma
                     } else {
                         ma

tests/builder/global_args.rs

@@ -126,3 +126,77 @@ fn deeply_nested_discovery() {
     let m = m.subcommand_matches("c").unwrap();
     assert!(*m.get_one::<bool>("long-c").expect("defaulted by clap"));
 }
+
+#[test]
+fn global_overrides_default() {
+    let cmd = Command::new("test")
+        .arg(
+            Arg::new("name")
+                .long("name")
+                .global(true)
+                .takes_value(true)
+                .default_value("from_default"),
+        )
+        .subcommand(Command::new("sub"));
+
+    let m = cmd.clone().try_get_matches_from(["test"]).unwrap();
+    assert_eq!(
+        m.get_one::<String>("name").unwrap().as_str(),
+        "from_default"
+    );
+
+    let m = cmd
+        .clone()
+        .try_get_matches_from(["test", "--name", "from_arg"])
+        .unwrap();
+    assert_eq!(m.get_one::<String>("name").unwrap().as_str(), "from_arg");
+
+    let m = cmd
+        .clone()
+        .try_get_matches_from(["test", "--name", "from_arg", "sub"])
+        .unwrap();
+    assert_eq!(m.get_one::<String>("name").unwrap().as_str(), "from_arg");
+
+    let m = cmd
+        .clone()
+        .try_get_matches_from(["test", "sub", "--name", "from_arg"])
+        .unwrap();
+    assert_eq!(m.get_one::<String>("name").unwrap().as_str(), "from_arg");
+}
+
+#[test]
+#[cfg(feature = "env")]
+fn global_overrides_env() {
+    std::env::set_var("GLOBAL_OVERRIDES_ENV", "from_env");
+
+    let cmd = Command::new("test")
+        .arg(
+            Arg::new("name")
+                .long("name")
+                .global(true)
+                .takes_value(true)
+                .env("GLOBAL_OVERRIDES_ENV"),
+        )
+        .subcommand(Command::new("sub"));
+
+    let m = cmd.clone().try_get_matches_from(["test"]).unwrap();
+    assert_eq!(m.get_one::<String>("name").unwrap().as_str(), "from_env");
+
+    let m = cmd
+        .clone()
+        .try_get_matches_from(["test", "--name", "from_arg"])
+        .unwrap();
+    assert_eq!(m.get_one::<String>("name").unwrap().as_str(), "from_arg");
+
+    let m = cmd
+        .clone()
+        .try_get_matches_from(["test", "--name", "from_arg", "sub"])
+        .unwrap();
+    assert_eq!(m.get_one::<String>("name").unwrap().as_str(), "from_arg");
+
+    let m = cmd
+        .clone()
+        .try_get_matches_from(["test", "sub", "--name", "from_arg"])
+        .unwrap();
+    assert_eq!(m.get_one::<String>("name").unwrap().as_str(), "from_arg");
+}