build: migrate the codebase to the 2024 Language Edition

download/tests/read-proxy-env.rs

@@ -15,24 +15,30 @@ use url::Url;
 
 static SERIALISE_TESTS: LazyLock<Mutex<()>> = LazyLock::new(|| Mutex::new(()));
 
-fn scrub_env() {
-    remove_var("http_proxy");
-    remove_var("https_proxy");
-    remove_var("HTTPS_PROXY");
-    remove_var("ftp_proxy");
-    remove_var("FTP_PROXY");
-    remove_var("all_proxy");
-    remove_var("ALL_PROXY");
-    remove_var("no_proxy");
-    remove_var("NO_PROXY");
+unsafe fn scrub_env() {
+    unsafe {
+        remove_var("http_proxy");
+        remove_var("https_proxy");
+        remove_var("HTTPS_PROXY");
+        remove_var("ftp_proxy");
+        remove_var("FTP_PROXY");
+        remove_var("all_proxy");
+        remove_var("ALL_PROXY");
+        remove_var("no_proxy");
+        remove_var("NO_PROXY");
+    }
 }
 
 // Tests for correctly retrieving the proxy (host, port) tuple from $https_proxy
 #[tokio::test]
 async fn read_basic_proxy_params() {
     let _guard = SERIALISE_TESTS.lock().await;
-    scrub_env();
-    set_var("https_proxy", "http://proxy.example.com:8080");
+    // SAFETY: We are setting environment variables when `SERIALISE_TESTS` is locked,
+    // and those environment variables in question are not relevant elsewhere in the test suite.
+    unsafe {
+        scrub_env();
+        set_var("https_proxy", "http://proxy.example.com:8080");
+    }
     let u = Url::parse("https://www.example.org").ok().unwrap();
     assert_eq!(
         for_url(&u).host_port(),
@@ -46,8 +52,12 @@ async fn socks_proxy_request() {
     static CALL_COUNT: AtomicUsize = AtomicUsize::new(0);
     let _guard = SERIALISE_TESTS.lock().await;
 
-    scrub_env();
-    set_var("all_proxy", "socks5://127.0.0.1:1080");
+    // SAFETY: We are setting environment variables when `SERIALISE_TESTS` is locked,
+    // and those environment variables in question are not relevant elsewhere in the test suite.
+    unsafe {
+        scrub_env();
+        set_var("all_proxy", "socks5://127.0.0.1:1080");
+    }
 
     thread::spawn(move || {
         let listener = TcpListener::bind("127.0.0.1:1080").unwrap();

src/cli/log.rs

@@ -20,7 +20,7 @@ use crate::{process::Process, utils::notify::NotificationLevel};
 pub fn tracing_subscriber(
     process: &Process,
 ) -> (
-    impl tracing::Subscriber,
+    impl tracing::Subscriber + use<>,
     reload::Handle<EnvFilter, Registry>,
 ) {
     #[cfg(feature = "otel")]
@@ -45,7 +45,7 @@ pub fn tracing_subscriber(
 /// When the `RUSTUP_LOG` environment variable is present, a standard [`tracing_subscriber`]
 /// formatter will be used according to the filtering directives set in its value.
 /// Otherwise, this logger will use [`EventFormatter`] to mimic "classic" Rustup `stderr` output.
-fn console_logger<S>(process: &Process) -> (impl Layer<S>, reload::Handle<EnvFilter, S>)
+fn console_logger<S>(process: &Process) -> (impl Layer<S> + use<S>, reload::Handle<EnvFilter, S>)
 where
     S: Subscriber + for<'span> LookupSpan<'span>,
 {
@@ -129,7 +129,7 @@ impl NotificationLevel {
 /// A [`tracing::Subscriber`] [`Layer`][`tracing_subscriber::Layer`] that corresponds to Rustup's
 /// optional `opentelemetry` (a.k.a. `otel`) feature.
 #[cfg(feature = "otel")]
-fn telemetry<S>(process: &Process) -> impl Layer<S>
+fn telemetry<S>(process: &Process) -> impl Layer<S> + use<S>
 where
     S: Subscriber + for<'span> LookupSpan<'span>,
 {

src/diskio/immediate.rs

@@ -69,8 +69,8 @@ impl Executor for ImmediateUnpacker {
     fn dispatch(&self, mut item: Item) -> Box<dyn Iterator<Item = CompletedIo> + '_> {
         item.result = match &mut item.kind {
             super::Kind::Directory => super::create_dir(&item.full_path),
-            super::Kind::File(ref contents) => {
-                if let super::FileBuffer::Immediate(ref contents) = &contents {
+            super::Kind::File(contents) => {
+                if let super::FileBuffer::Immediate(contents) = &contents {
                     super::write_file(&item.full_path, contents, item.mode)
                 } else {
                     unreachable!()
@@ -81,21 +81,20 @@ impl Executor for ImmediateUnpacker {
                     // If there is a pending error, return it, otherwise stash the
                     // Item for eventual return when the file is finished.
                     let mut guard = self.incremental_state.lock().unwrap();
-                    if let Some(ref mut state) = *guard {
-                        if state.err.is_some() {
-                            let err = state.err.take().unwrap();
-                            item.result = err;
-                            item.finish = item
-                                .start
-                                .map(|s| Instant::now().saturating_duration_since(s));
-                            *guard = None;
-                            Box::new(Some(CompletedIo::Item(item)).into_iter())
-                        } else {
-                            state.item = Some(item);
-                            Box::new(None.into_iter())
-                        }
+                    let Some(ref mut state) = *guard else {
+                        unreachable!()
+                    };
+                    if state.err.is_some() {
+                        let err = state.err.take().unwrap();
+                        item.result = err;
+                        item.finish = item
+                            .start
+                            .map(|s| Instant::now().saturating_duration_since(s));
+                        *guard = None;
+                        Box::new(Some(CompletedIo::Item(item)).into_iter())
                     } else {
-                        unreachable!();
+                        state.item = Some(item);
+                        Box::new(None.into_iter())
                     }
                 };
             }
@@ -181,9 +180,7 @@ impl IncrementalFileWriter {
         if (self.state.lock().unwrap()).is_none() {
             return false;
         }
-        let chunk = if let FileBuffer::Immediate(v) = chunk {
-            v
-        } else {
+        let FileBuffer::Immediate(chunk) = chunk else {
             unreachable!()
         };
         match self.write(chunk) {
@@ -203,25 +200,23 @@ impl IncrementalFileWriter {
 
     fn write(&mut self, chunk: Vec<u8>) -> std::result::Result<bool, io::Error> {
         let mut state = self.state.lock().unwrap();
-        if let Some(ref mut state) = *state {
-            if let Some(ref mut file) = self.file.as_mut() {
-                // Length 0 vector is used for clean EOF signalling.
-                if chunk.is_empty() {
-                    trace_scoped!("close", "name:": self.path_display);
-                    drop(std::mem::take(&mut self.file));
-                    state.finished = true;
-                } else {
-                    trace_scoped!("write_segment", "name": self.path_display, "len": chunk.len());
-                    file.write_all(&chunk)?;
-
-                    state.completed_chunks.push(chunk.len());
-                }
-                Ok(true)
-            } else {
-                Ok(false)
-            }
+        let Some(ref mut state) = *state else {
+            unreachable!()
+        };
+        let Some(ref mut file) = self.file.as_mut() else {
+            return Ok(false);
+        };
+        // Length 0 vector is used for clean EOF signalling.
+        if chunk.is_empty() {
+            trace_scoped!("close", "name:": self.path_display);
+            drop(std::mem::take(&mut self.file));
+            state.finished = true;
         } else {
-            unreachable!();
+            trace_scoped!("write_segment", "name": self.path_display, "len": chunk.len());
+            file.write_all(&chunk)?;
+
+            state.completed_chunks.push(chunk.len());
         }
+        Ok(true)
     }
 }

src/diskio/mod.rs

@@ -81,14 +81,14 @@ pub(crate) enum FileBuffer {
 impl FileBuffer {
     /// All the buffers space to be re-used when the last reference to it is dropped.
     pub(crate) fn clear(&mut self) {
-        if let FileBuffer::Threaded(ref mut contents) = self {
+        if let FileBuffer::Threaded(contents) = self {
             contents.clear()
         }
     }
 
     pub(crate) fn len(&self) -> usize {
         match self {
-            FileBuffer::Immediate(ref vec) => vec.len(),
+            FileBuffer::Immediate(vec) => vec.len(),
             FileBuffer::Threaded(PoolReference::Owned(owned, _)) => owned.len(),
             FileBuffer::Threaded(PoolReference::Mut(mutable, _)) => mutable.len(),
         }
@@ -109,7 +109,7 @@ impl Deref for FileBuffer {
 
     fn deref(&self) -> &Self::Target {
         match self {
-            FileBuffer::Immediate(ref vec) => vec,
+            FileBuffer::Immediate(vec) => vec,
             FileBuffer::Threaded(PoolReference::Owned(owned, _)) => owned,
             FileBuffer::Threaded(PoolReference::Mut(mutable, _)) => mutable,
         }
@@ -119,7 +119,7 @@ impl Deref for FileBuffer {
 impl DerefMut for FileBuffer {
     fn deref_mut(&mut self) -> &mut Self::Target {
         match self {
-            FileBuffer::Immediate(ref mut vec) => vec,
+            FileBuffer::Immediate(vec) => vec,
             FileBuffer::Threaded(PoolReference::Owned(_, _)) => {
                 unimplemented!()
             }
@@ -337,15 +337,11 @@ pub(crate) fn perform<F: Fn(usize)>(item: &mut Item, chunk_complete_callback: F)
     // Files, write them.
     item.result = match &mut item.kind {
         Kind::Directory => create_dir(&item.full_path),
-        Kind::File(ref mut contents) => {
+        Kind::File(contents) => {
             contents.clear();
             match contents {
-                FileBuffer::Immediate(ref contents) => {
-                    write_file(&item.full_path, contents, item.mode)
-                }
-                FileBuffer::Threaded(ref mut contents) => {
-                    write_file(&item.full_path, contents, item.mode)
-                }
+                FileBuffer::Immediate(contents) => write_file(&item.full_path, contents, item.mode),
+                FileBuffer::Threaded(contents) => write_file(&item.full_path, contents, item.mode),
             }
         }
         Kind::IncrementalFile(incremental_file) => write_file_incremental(

src/dist/component/package.rs

@@ -229,10 +229,9 @@ fn filter_result(op: &mut CompletedIo) -> io::Result<()> {
                     // mkdir of e.g. ~/.rustup already existing is just fine;
                     // for others it would be better to know whether it is
                     // expected to exist or not -so put a flag in the state.
-                    if let Kind::Directory = op.kind {
-                        Ok(())
-                    } else {
-                        Err(e)
+                    match op.kind {
+                        Kind::Directory => Ok(()),
+                        _ => Err(e),
                     }
                 }
                 _ => Err(e),
@@ -454,40 +453,40 @@ fn unpack_without_first_dir<R: Read>(
 
         let item = loop {
             // Create the full path to the entry if it does not exist already
-            if let Some(parent) = item.full_path.to_owned().parent() {
-                match directories.get_mut(parent) {
-                    None => {
-                        // Tar has item before containing directory
-                        // Complain about this so we can see if these exist.
-                        writeln!(
-                            process.stderr().lock(),
-                            "Unexpected: missing parent '{}' for '{}'",
-                            parent.display(),
-                            entry.path()?.display()
-                        )?;
-                        directories.insert(parent.to_owned(), DirStatus::Pending(vec![item]));
-                        item = Item::make_dir(parent.to_owned(), 0o755);
-                        // Check the parent's parent
-                        continue;
-                    }
-                    Some(DirStatus::Exists) => {
-                        break Some(item);
-                    }
-                    Some(DirStatus::Pending(pending)) => {
-                        // Parent dir is being made
-                        pending.push(item);
-                        if incremental_file_sender.is_none() {
-                            // take next item from tar
-                            continue 'entries;
-                        } else {
-                            // don't submit a new item for processing, but do be ready to feed data to the incremental file.
-                            break None;
-                        }
+            let full_path = item.full_path.to_owned();
+            let Some(parent) = full_path.parent() else {
+                // We should never see a path with no parent.
+                unreachable!()
+            };
+            match directories.get_mut(parent) {
+                None => {
+                    // Tar has item before containing directory
+                    // Complain about this so we can see if these exist.
+                    writeln!(
+                        process.stderr().lock(),
+                        "Unexpected: missing parent '{}' for '{}'",
+                        parent.display(),
+                        entry.path()?.display()
+                    )?;
+                    directories.insert(parent.to_owned(), DirStatus::Pending(vec![item]));
+                    item = Item::make_dir(parent.to_owned(), 0o755);
+                    // Check the parent's parent
+                    continue;
+                }
+                Some(DirStatus::Exists) => {
+                    break Some(item);
+                }
+                Some(DirStatus::Pending(pending)) => {
+                    // Parent dir is being made
+                    pending.push(item);
+                    if incremental_file_sender.is_none() {
+                        // take next item from tar
+                        continue 'entries;
+                    } else {
+                        // don't submit a new item for processing, but do be ready to feed data to the incremental file.
+                        break None;
                     }
                 }
-            } else {
-                // We should never see a path with no parent.
-                panic!();
             }
         };
 

src/dist/mod.rs

@@ -335,10 +335,9 @@ impl FromStr for ParsedToolchainDesc {
             }
         });
 
-        if let Some(d) = d {
-            Ok(d)
-        } else {
-            Err(RustupError::InvalidToolchainName(desc.to_string()).into())
+        match d {
+            Some(d) => Ok(d),
+            None => Err(RustupError::InvalidToolchainName(desc.to_string()).into()),
         }
     }
 }

src/test/mock/clitools.rs

@@ -192,19 +192,33 @@ impl ConstState {
 
 /// State a test can interact and mutate
 pub async fn setup_test_state(test_dist_dir: tempfile::TempDir) -> (tempfile::TempDir, Config) {
-    // Unset env variables that will break our testing
-    env::remove_var("RUSTUP_UPDATE_ROOT");
-    env::remove_var("RUSTUP_TOOLCHAIN");
-    env::remove_var("SHELL");
-    env::remove_var("ZDOTDIR");
-    // clap does its own terminal colour probing, and that isn't
-    // trait-controllable, but it does honour the terminal. To avoid testing
-    // claps code, lie about whatever terminal this process was started under.
-    env::set_var("TERM", "dumb");
-
-    match env::var("RUSTUP_BACKTRACE") {
-        Ok(val) => env::set_var("RUST_BACKTRACE", val),
-        _ => env::remove_var("RUST_BACKTRACE"),
+    // SAFETY: This is probably not the best way of doing such a thing, but it should be
+    // okay since we are setting the environment variables for the integration tests only.
+    // There are two types of integration test in rustup: in-process and subprocess.
+    // For the former, the environment variables are 100% injected via [`TestContext::vars`];
+    // for the latter, the environment variables in question are only relevant in the
+    // corresponding subprocesses. Thus, it should be safe to assume that the following won't
+    // cause inconsistencies as far as **this** particular process is concerned, as long as
+    // **each subprocess gets the same value for every environment variable listed below when
+    // it spins off**. To do so, we will have to ensure that:
+    // - The following `unsafe` block is idempotent, making its output absolutely stable.
+    // - The environment variables listed below are never modified to anything else
+    //   **in this process** when the tests are still running.
+    unsafe {
+        // Unset env variables that will break our testing
+        env::remove_var("RUSTUP_UPDATE_ROOT");
+        env::remove_var("RUSTUP_TOOLCHAIN");
+        env::remove_var("SHELL");
+        env::remove_var("ZDOTDIR");
+        // clap does its own terminal colour probing, and that isn't
+        // trait-controllable, but it does honour the terminal. To avoid testing
+        // claps code, lie about whatever terminal this process was started under.
+        env::set_var("TERM", "dumb");
+
+        match env::var("RUSTUP_BACKTRACE") {
+            Ok(val) => env::set_var("RUST_BACKTRACE", val),
+            _ => env::remove_var("RUST_BACKTRACE"),
+        }
     }
 
     let current_exe_path = env::current_exe().unwrap();

src/toolchain.rs

@@ -87,11 +87,11 @@ impl<'a> Toolchain<'a> {
             ActiveReason::CommandLine => {
                 "the +toolchain on the command line specifies an uninstalled toolchain".to_string()
             }
-            ActiveReason::OverrideDB(ref path) => format!(
+            ActiveReason::OverrideDB(path) => format!(
                 "the directory override for '{}' specifies an uninstalled toolchain",
                 utils::canonicalize_path(path, cfg.notify_handler.as_ref()).display(),
             ),
-            ActiveReason::ToolchainFile(ref path) => format!(
+            ActiveReason::ToolchainFile(path) => format!(
                 "the toolchain file at '{}' specifies an uninstalled toolchain",
                 utils::canonicalize_path(path, cfg.notify_handler.as_ref()).display(),
             ),