Auto merge of #12631 - Eh2406:strip_prefix, r=epage

Ues strip_prefix for cleaner code ### What does this PR try to resolve? In #12629 (review) Ed pointed out how much cleaner the code can be using `strip_prefix`, so I found a bunch more places where we should be using it. ### How should we test and review this PR? Internal refactor and test still pass.
rust-lang · Sep 7, 2023 · f399c21 · f399c21
2 parents 5c77b5d + 44666f7
commit f399c21
Show file tree

Hide file tree

Showing 10 changed files with 28 additions and 30 deletions.
diff --git a/crates/cargo-platform/examples/matches.rs b/crates/cargo-platform/examples/matches.rs
@@ -35,8 +35,8 @@ fn get_target() -> String {
         .expect("rustc failed to run");
     let stdout = String::from_utf8(output.stdout).unwrap();
     for line in stdout.lines() {
-        if line.starts_with("host: ") {
-            return String::from(&line[6..]);
+        if let Some(line) = line.strip_prefix("host: ") {
+            return String::from(line);
         }
     }
     panic!("Failed to find host: {}", stdout);

diff --git a/crates/cargo-platform/src/lib.rs b/crates/cargo-platform/src/lib.rs
@@ -126,8 +126,7 @@ impl FromStr for Platform {
     type Err = ParseError;
 
     fn from_str(s: &str) -> Result<Platform, ParseError> {
-        if s.starts_with("cfg(") && s.ends_with(')') {
-            let s = &s[4..s.len() - 1];
+        if let Some(s) = s.strip_prefix("cfg(").and_then(|s| s.strip_suffix(')')) {
             s.parse().map(Platform::Cfg)
         } else {
             Platform::validate_named_platform(s)?;

diff --git a/crates/cargo-test-support/src/publish.rs b/crates/cargo-test-support/src/publish.rs
@@ -131,8 +131,11 @@ pub fn validate_crate_contents(
             (name, contents)
         })
         .collect();
-    assert!(expected_crate_name.ends_with(".crate"));
-    let base_crate_name = Path::new(&expected_crate_name[..expected_crate_name.len() - 6]);
+    let base_crate_name = Path::new(
+        expected_crate_name
+            .strip_suffix(".crate")
+            .expect("must end with .crate"),
+    );
     let actual_files: HashSet<PathBuf> = files.keys().cloned().collect();
     let expected_files: HashSet<PathBuf> = expected_files
         .iter()

diff --git a/src/bin/cargo/main.rs b/src/bin/cargo/main.rs
@@ -106,17 +106,18 @@ fn list_commands(config: &Config) -> BTreeMap<String, CommandInfo> {
         };
         for entry in entries.filter_map(|e| e.ok()) {
             let path = entry.path();
-            let filename = match path.file_name().and_then(|s| s.to_str()) {
-                Some(filename) => filename,
-                _ => continue,
+            let Some(filename) = path.file_name().and_then(|s| s.to_str()) else {
+                continue;
             };
-            if !filename.starts_with(prefix) || !filename.ends_with(suffix) {
+            let Some(name) = filename
+                .strip_prefix(prefix)
+                .and_then(|s| s.strip_suffix(suffix))
+            else {
                 continue;
-            }
+            };
             if is_executable(entry.path()) {
-                let end = filename.len() - suffix.len();
                 commands.insert(
-                    filename[prefix.len()..end].to_string(),
+                    name.to_string(),
                     CommandInfo::External { path: path.clone() },
                 );
             }

diff --git a/src/cargo/core/resolver/encode.rs b/src/cargo/core/resolver/encode.rs
@@ -338,7 +338,7 @@ impl EncodableResolve {
         let mut to_remove = Vec::new();
         for (k, v) in metadata.iter().filter(|p| p.0.starts_with(prefix)) {
             to_remove.push(k.to_string());
-            let k = &k[prefix.len()..];
+            let k = k.strip_prefix(prefix).unwrap();
             let enc_id: EncodablePackageId = k
                 .parse()
                 .with_context(|| internal("invalid encoding of checksum in lockfile"))?;
@@ -601,8 +601,8 @@ impl FromStr for EncodablePackageId {
         let version = s.next();
         let source_id = match s.next() {
             Some(s) => {
-                if s.starts_with('(') && s.ends_with(')') {
-                    Some(SourceId::from_url(&s[1..s.len() - 1])?)
+                if let Some(s) = s.strip_prefix('(').and_then(|s| s.strip_suffix(')')) {
+                    Some(SourceId::from_url(s)?)
                 } else {
                     anyhow::bail!("invalid serialized PackageId")
                 }

diff --git a/src/cargo/util/config/de.rs b/src/cargo/util/config/de.rs
@@ -216,9 +216,8 @@ impl<'config> ConfigMapAccess<'config> {
             // `CARGO_PROFILE_DEV_PACKAGE_`
             let env_prefix = format!("{}_", de.key.as_env_key());
             for env_key in de.config.env_keys() {
-                if env_key.starts_with(&env_prefix) {
-                    // `CARGO_PROFILE_DEV_PACKAGE_bar_OPT_LEVEL = 3`
-                    let rest = &env_key[env_prefix.len()..];
+                // `CARGO_PROFILE_DEV_PACKAGE_bar_OPT_LEVEL = 3`
+                if let Some(rest) = env_key.strip_prefix(&env_prefix) {
                     // `rest = bar_OPT_LEVEL`
                     let part = rest.splitn(2, '_').next().unwrap();
                     // `part = "bar"`

diff --git a/src/cargo/util/rustc.rs b/src/cargo/util/rustc.rs
@@ -63,8 +63,7 @@ impl Rustc {
         let extract = |field: &str| -> CargoResult<&str> {
             verbose_version
                 .lines()
-                .find(|l| l.starts_with(field))
-                .map(|l| &l[field.len()..])
+                .find_map(|l| l.strip_prefix(field))
                 .ok_or_else(|| {
                     anyhow::format_err!(
                         "`rustc -vV` didn't have a line for `{}`, got:\n{}",

diff --git a/tests/testsuite/install.rs b/tests/testsuite/install.rs
@@ -2292,9 +2292,9 @@ fn failed_install_retains_temp_directory() {
     .unwrap();
 
     // Find the path in the output.
-    let start = stderr.find("found at `").unwrap() + 10;
-    let end = stderr[start..].find('.').unwrap() - 1;
-    let path = Path::new(&stderr[start..(end + start)]);
+    let stderr = stderr.split_once("found at `").unwrap().1;
+    let end = stderr.find('.').unwrap() - 1;
+    let path = Path::new(&stderr[..end]);
     assert!(path.exists());
     assert!(path.join("release/deps").exists());
 }

diff --git a/tests/testsuite/lto.rs b/tests/testsuite/lto.rs
@@ -453,10 +453,8 @@ fn verify_lto(output: &Output, krate: &str, krate_info: &str, expected_lto: Lto)
             krate, krate_info, line, line2, stderr
         );
     }
-    let actual_lto = if let Some(index) = line.find("-C lto=") {
-        let s = &line[index..];
-        let end = s.find(' ').unwrap();
-        let mode = &line[index..index + end];
+    let actual_lto = if let Some((_, line)) = line.split_once("-C lto=") {
+        let mode = line.splitn(2, ' ').next().unwrap();
         if mode == "off" {
             Lto::Off
         } else {

diff --git a/tests/testsuite/ssh.rs b/tests/testsuite/ssh.rs
@@ -184,10 +184,9 @@ fn known_host_works() {
     // Validate the fingerprint while we're here.
     let fingerprint = stderr
         .lines()
-        .find(|line| line.starts_with("  The ECDSA key fingerprint"))
+        .find_map(|line| line.strip_prefix("  The ECDSA key fingerprint"))
         .unwrap()
         .trim();
-    let fingerprint = &fingerprint[30..];
     let finger_out = sshd.exec(&["ssh-keygen", "-l", "-f", "/etc/ssh/ssh_host_ecdsa_key.pub"]);
     let gen_finger = std::str::from_utf8(&finger_out.stdout).unwrap();
     // <key-size> <fingerprint> <comments…>