improve docstrings for config::environment

Author: kylematsuda

src/cargo/util/config/environment.rs

@@ -17,25 +17,49 @@ fn make_case_insensitive_and_normalized_env(
         .collect();
     let normalized_env = env
         .iter()
-        // Only keep entries where both the key and value are valid UTF-8
+        // Only keep entries where both the key and value are valid UTF-8,
+        // since the config env vars only support UTF-8 keys and values.
+        // Otherwise, the normalized map warning could incorrectly warn about entries that can't be
+        // read by the config system.
+        // Please see the docs for `Env` for more context.
         .filter_map(|(k, v)| Some((k.to_str()?, v.to_str()?)))
         .map(|(k, _)| (k.to_uppercase().replace("-", "_"), k.to_owned()))
         .collect();
     (case_insensitive_env, normalized_env)
 }
 
+/// A snapshot of the environment variables available to [`super::Config`].
+///
+/// Currently, the [`Config`](super::Config) supports lookup of environment variables
+/// through two different means:
+///
+/// - [`Config::get_env`](super::Config::get_env)
+///   and [`Config::get_env_os`](super::Config::get_env_os)
+///   for process environment variables (similar to [`std::env::var`] and [`std::env::var_os`]),
+/// - Typed Config Value API via [`Config::get`](super::Config::get).
+///   This is only available for `CARGO_` prefixed environment keys.
+///
+/// This type contains the env var snapshot and helper methods for both APIs.
 #[derive(Debug)]
 pub struct Env {
     /// A snapshot of the process's environment variables.
     env: HashMap<OsString, OsString>,
-    /// A map from normalized (upper case and with "-" replaced by "_") env keys to the actual key
-    /// in the environment.
-    /// The "normalized" key is the format expected by Cargo.
-    /// This is used to warn users when env keys are not provided in this format.
+    /// Used in the typed Config value API for warning messages when config keys are
+    /// given in the wrong format.
+    ///
+    /// Maps from "normalized" (upper case and with "-" replaced by "_") env keys
+    /// to the actual keys in the environment.
+    /// The normalized format is the one expected by Cargo.
+    ///
+    /// This only holds env keys that are valid UTF-8, since [`super::ConfigKey`] only supports UTF-8 keys.
+    /// In addition, this only holds env keys whose value in the environment is also valid UTF-8,
+    /// since the typed Config value API only supports UTF-8 values.
     normalized_env: HashMap<String, String>,
-    /// A map from uppercased env keys to the actual key in the environment.
-    /// This is relevant on Windows, where env keys are case-insensitive.
+    /// Used to implement `get_env` and `get_env_os` on Windows, where env keys are case-insensitive.
+    ///
+    /// Maps from uppercased env keys to the actual key in the environment.
     /// For example, this might hold a pair `("PATH", "Path")`.
+    /// Currently only supports UTF-8 keys and values.
     case_insensitive_env: HashMap<String, String>,
 }
 
@@ -125,6 +149,18 @@ impl Env {
     ///
     /// This is intended for use in private methods of `Config`,
     /// and does not check for env key case mismatch.
+    ///
+    /// This is case-sensitive on Windows (even though environment keys on Windows are usually
+    /// case-insensitive) due to an unintended regression in 1.28 (via #5552).
+    /// This should only affect keys used for cargo's config-system env variables (`CARGO_`
+    /// prefixed ones), which are currently all uppercase.
+    /// We may want to consider rectifying it if users report issues.
+    /// One thing that adds a wrinkle here is the unstable advanced-env option that *requires*
+    /// case-sensitive keys.
+    ///
+    /// Do not use this for any other purposes.
+    /// Use [`Env::get_env_os`] or [`Env::get_env`] instead, which properly handle case
+    /// insensitivity on Windows.
     pub(super) fn get_str(&self, key: impl AsRef<OsStr>) -> Option<&str> {
         self.env.get(key.as_ref()).and_then(|s| s.to_str())
     }
@@ -133,14 +169,15 @@ impl Env {
     ///
     /// This is intended for use in private methods of `Config`,
     /// and does not check for env key case mismatch.
+    /// See the docstring of [`Env::get_str`] for more context.
     pub(super) fn contains_key(&self, key: impl AsRef<OsStr>) -> bool {
         self.env.contains_key(key.as_ref())
     }
 
     /// Looks up a normalized `key` in the `normalized_env`.
     /// Returns the corresponding (non-normalized) env key if it exists, else `None`.
     ///
-    /// This is used by `Config::check_environment_key_mismatch`.
+    /// This is used by [`super::Config::check_environment_key_case_mismatch`].
     pub(super) fn get_normalized(&self, key: &str) -> Option<&str> {
         self.normalized_env.get(key).map(|s| s.as_ref())
     }