make call to paths::join_paths and warn if path contains invalid chars

Author: RyanAD

crates/cargo-util/src/paths.rs

@@ -19,15 +19,17 @@ use tempfile::Builder as TempFileBuilder;
 /// error message.
 pub fn join_paths<T: AsRef<OsStr>>(paths: &[T], env: &str) -> Result<OsString> {
     env::join_paths(paths.iter()).with_context(|| {
-        let paths = paths.iter().map(Path::new).collect::<Vec<_>>();
+        let paths = paths.iter()
+            .map(|s| s.as_ref().to_string_lossy())
+            .map(|s| format!("  \"{s}\""))
+            .collect::<Vec<String>>()
+            .join(",\n");
+
         format!(
-            "failed to join paths from `${}` together\
-                 \n    \
-                 If you set `${}` manually, check if it contains an unterminated quote character \
-                 or path separators (usually `:` or `:`). Please avoid using them.\
-                 \n\n    \
-                 Otherwise, check if any of paths listed contain one of those characters: {:?}",
-            env, env, paths
+            "failed to join paths from `${env}` together\n\
+                 If you set `${env}` manually, check if it contains an unterminated quote character \
+                 or path separators (usually `:` or `;`). Please avoid using them. \
+                 Otherwise, check if any of paths listed below contain one of those characters:\n{paths}"
         )
     })
 }

src/cargo/ops/cargo_new.rs

@@ -7,10 +7,11 @@ use cargo_util::paths;
 use serde::de;
 use serde::Deserialize;
 use std::collections::BTreeMap;
-use std::fmt;
+use std::ffi::OsStr;
 use std::io::{BufRead, BufReader, ErrorKind};
 use std::path::{Path, PathBuf};
 use std::str::FromStr;
+use std::{fmt, slice};
 use toml_edit::easy as toml;
 
 #[derive(Clone, Copy, Debug, PartialEq)]
@@ -263,11 +264,12 @@ fn check_name(
 
 /// Checks if the path contains any invalid PATH env characters.
 fn check_path(path: &Path, shell: &mut Shell) -> CargoResult<()> {
-    if path.to_string_lossy().contains(&[';', ':', '"'][..]) {
+    // warn if the path contains characters that will break `env::join_paths`
+    if let Err(_) = paths::join_paths(slice::from_ref(&OsStr::new(path)), "") {
+        let path = path.to_string_lossy();
         shell.warn(format!(
-            "the path `{}` contains invalid PATH characters (usually `:`, `;`, or `\"`)\n\
-            It is recommended to use a different name to avoid problems.",
-            path.to_string_lossy()
+            "the path `{path}` contains invalid PATH characters (usually `:`, `;`, or `\"`)\n\
+            It is recommended to use a different name to avoid problems."
         ))?;
     }
     Ok(())

tests/testsuite/init/mod.rs

@@ -28,6 +28,7 @@ mod mercurial_autodetect;
 mod multibin_project_name_clash;
 #[cfg(not(windows))]
 mod no_filename;
+#[cfg(unix)]
 mod path_contains_separator;
 mod pijul_autodetect;
 mod reserved_name;

tests/testsuite/init/path_contains_separator/in/test:ing/mod.rs

@@ -1,6 +1,6 @@
 use cargo_test_support::compare::assert_ui;
 use cargo_test_support::prelude::*;
-use cargo_test_support::Project;
+use cargo_test_support::{t, Project};
 
 use cargo_test_support::curr_dir;
 
@@ -9,6 +9,10 @@ fn path_contains_separator() {
     let project = Project::from_template(curr_dir!().join("in"));
     let project_root = &project.root().join("test:ing");
 
+    if !project_root.exists() {
+        t!(std::fs::create_dir(&project_root));
+    }
+
     snapbox::cmd::Command::cargo_ui()
         .arg_line("init --bin --vcs none --edition 2015 --name testing")
         .current_dir(project_root)

tests/testsuite/init/path_contains_separator/mod.rs

@@ -1,3 +1,3 @@
-warning: the path `[ROOT]/case/test:ing/.` contains PATH separators (usually `:` or `:`)
+warning: the path `[ROOT]/case/test:ing/.` contains invalid PATH characters (usually `:`, `;`, or `"`)
 It is recommended to use a different name to avoid problems.
      Created binary (application) package