Description
openedon Aug 30, 2021
Problem
If rustc provides a suggestion to modify code in a dependency, cargo fix will happily oblige. However, cargo should never apply fixes to non-path dependencies. I might say it shouldn't apply fixes outside of the current package, but I'm not sure about that (or how to detect that reliably).
For example, it may attempt to write to ~/.cargo/registry/src/github.com-1ecc6299db9ec823 which really should be read-only.
rustc normally tries to avoid diagnostics that point into dependencies, but sometimes there are bugs where it points to the wrong span, and ends up giving a suggestion into a dependency. See rust-lang/rust#88514 for more detail.
Steps
It's hard to give a good example, since it either requires using a real crates.io package, or depending on diagnostics in rustc that are arguably not working correctly.
Method 1 — Use crates.io
With dependency proconio = "=0.3.8"
pub fn foo(
source: proconio::source::line::LineSource<std::io::BufReader<&[u8]>>,
) {
proconio::input! {
from source,
mut x:usize,
}
}This will suggest making a change to the remote package:
warning: unused variable: `x`
--> /Users/eric/.cargo/registry/src/github.com-1ecc6299db9ec823/proconio-0.3.8/src/lib.rs:579:22
|
579 | let $($mut)* $var = $crate::read_value!(@source [$source] @kind [$($kind)*]);
| ^^^^ help: if this is intentional, prefix it with an underscore: `_x`
|
= note: `#[warn(unused_variables)]` on by default
Method 2 — Sample macro
Put this macro in a dependency, and notice rustc will suggest modifying the macro.
#[macro_export]
macro_rules! m {
($i:tt) => {
let $i = 1;
};
}The key part is that the identifier uses a tt instead of an ident.
Method 3 — cargo_test
This is a cargo testsuite test. I don't think we can use it since it is dependent on rustc's diagnostic output, but maybe we can simulate it with a rustc wrapper?
#[cargo_test]
fn fix_in_dependency() {
Package::new("bar", "1.0.0")
.file(
"src/lib.rs",
r#"
#[macro_export]
macro_rules! m {
($i:tt) => {
let $i = 1;
};
}
"#,
)
.publish();
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"
[dependencies]
bar = "1.0"
"#,
)
.file(
"src/lib.rs",
r#"
pub fn foo() {
bar::m!(abc);
}
"#,
)
.build();
p.cargo("fix --allow-no-vcs")
// This most definitely should not say "Fixed" in registry/src
.with_stderr(
"\
[UPDATING] `dummy-registry` index
[DOWNLOADING] crates ...
[DOWNLOADED] bar v1.0.0 [..]
[CHECKING] bar v1.0.0
[CHECKING] foo v0.1.0 [..]
[FIXED] [ROOT]/home/.cargo/registry/src/-88e05c124b5ecfe9/bar-1.0.0/src/lib.rs (1 fix)
warning: unused variable: `abc`
|
= note: `#[warn(unused_variables)]` on by default
warning: `foo` (lib) generated 1 warning
warning: `foo` (lib test) generated 1 warning (1 duplicate)
[FINISHED] [..]
",
)
.run();
}Possible Solution(s)
Cargo should detect diagnostic paths outside of the package and skip them.
Notes
Output of cargo version: cargo 1.56.0-nightly (f559c109c 2021-08-26)
