`cargo add`: Handle paths automatically without requiring `--path`

[joshtriplett]

Problem

$ cargo add ../testlib
error: invalid character `.` in package name: `../testlib`, the first character must be a Unicode XID start character (most letters or `_`)

Ideally, when given an argument containing a path separator (which is never valid in a crate name), cargo add could automatically assume --path if the specified path exists as a directory.

If there's a compatibility reason why we can't do that, then at a minimum cargo add could suggest --path. But I think we could reasonably infer that and proceed.

Proposed Solution

When cargo add receives an argument containing a path separator, rather than erroring, cargo add should infer --path and proceed.

Notes

No response

[weihanglo]

If we are going to do this, should we consider other commands accepting --path all together for consistency? Like cargo remove, cargo run and cargo install?

(-Zscript supports and does that path separator check btw)

[epage]

Didn't cargo install used to do this and we transitioned away?

What happens when a subdirectory and a package name look the same? Its common to reference a directory as foo and I think it'd be confusing to say that ./foo is required. If it isn't, then it seems like we could run into dependency confusion attacks.

[joshtriplett]

@epage I don't remember if cargo install some/path used to work. cargo install with no arguments used to default to the current directory, and that now requires cargo install --path ., but that doesn't seem related.

I'm not proposing to eliminate the --path option. I'm proposing to handle this automatically if and only if the argument contains a path separator, which seems likely to be a common case; for instance, cargo add ../somedep.

I appreciate the possibility that once people are used to the idea that they can run cargo add some/path, they might also think they can run cargo add subdir (which I'm not suggesting we support). However, I don't think that possibility means we need to have cargo add ../path continue to error rather than doing what the user is clearly asking for.

At a minimum, I think there's value in augmenting the error message to tell people they need the --path option, but in general, this seems to fall in the category of "if you knew what I wanted to do..."

[epage]

I'm all for improving the error message.

This again gets into my concern with "if you knew what I wanted to do..." as it creates a sloppiness that makes behavior difficult to predict. This would allow me to do cargo add crates/foo but if I switch to a repo not using crates/, then if I do cargo add foo then it no longer works.

I think it's a useful to keep in mind but its not a principle to make top priority.

[joshtriplett]

@epage I would argue that the case of doing cargo add inside a workspace is less important, as we've made it so cargo new automatically adds something to the workspace. And arguably, if you cargo add a crate with the same name as a crate in your workspace, we should at the very least warn, if not just automatically use the crate in your workspace.

[epage]

Hmm, looks like we do auto-check the name against workspace members

cargo/src/cargo/ops/cargo_add/mod.rs

Lines 388 to 398 in a2d45dc

	} else if let Some(package) = ws.members().find(|p| p.name().as_str() == dependency.name) {
	// Only special-case workspaces when the user doesn't provide any extra
	// information, otherwise, trust the user.
	let mut src = PathSource::new(package.root());
	// dev-dependencies do not need the version populated
	if section.kind() != DepKind::Development {
	let op = "";
	let v = format!("{op}{version}", version = package.version());
	src = src.set_version(v);
	}
	dependency = dependency.set_source(src);