fix: Iteratively apply suggestions from the compiler #5842
Conversation
|
r? @matklad (rust_highfive has picked a reviewer for you, use r? to override) |
Cargo.toml
Outdated
| @@ -40,7 +40,7 @@ libc = "0.2" | |||
| log = "0.4" | |||
| libgit2-sys = "0.7.5" | |||
| num_cpus = "1.0" | |||
| rustfix = "0.4" | |||
| rustfix = { git = 'https://github.com/alexcrichton/rustfix', branch = 'rustfix-incremental' } | |||
There was a problem hiding this comment.
Note that this is waiting on rust-lang/rustfix#140 to be merged and published, so we'll want to hold off on merging this PR until that happens.
|
r? @ehuss |
|
cc @killercup, you may be interested in this as well! |
src/cargo/ops/fix.rs
Outdated
| // rustc. | ||
| // | ||
| // Naturally we want a few protections in place here though to avoid looping | ||
| // forever or othrewise losing data. To that end we have a few termination |
| let mut cmd = Command::new(&rustc); | ||
| args.apply(&mut cmd); | ||
| cmd.arg("--error-format=json"); | ||
| let output = cmd.output().context("failed to spawn rustc")?; | ||
|
|
||
| if output.status.success() { | ||
| for message in fixes.messages.drain(..) { | ||
| message.post()?; | ||
| for (path, file) in fixes.files.iter() { |
There was a problem hiding this comment.
any reason to explicitly call iter instead of just &fixes.files? (same below)
There was a problem hiding this comment.
I'm personally not a huge fan of for x in &y as opposed to for x in y or for x in y.iter(), the latter one feels more clear to me.
src/cargo/ops/fix.rs
Outdated
| // definitely can't make progress, so bail out. | ||
| let mut fixes = FixedCrate::default(); | ||
| let mut last_fix_counts = HashMap::new(); | ||
| for _ in 0..4 { |
There was a problem hiding this comment.
4 is a pretty magic number here, maybe put it in a binding whose name we can search for?
maybe even do env::var("CARGO_FIX_MAX_RETRIES").and_then(|n| n.parse()).unwrap_or(4)?
| ); | ||
|
|
||
| let contents = p.read_file("src/lib.rs"); | ||
| println!("{}", contents); |
There was a problem hiding this comment.
Ah this is a "poor man's way" of debugging the test if it fails (as the output is swallowed up if successful). That way it allows looking through just the test output to see why the output didn't exist!
There was a problem hiding this comment.
as the output is swallowed up if successful
TIL. Very neat!
This commit updates the `cargo fix` implementation to iteratively apply fixes
from the compiler instead of only once. Currently the compiler can sometimes
emit overlapping suggestions, such as in the case of transitioning
::foo::<::Bar>();
to ...
crate::foo::<crate::Bar>();
and `rustfix` rightfully can't handle overlapping suggestions as there's no
clear way of how to disambiguate the fixes. To fix this problem Cargo will now
run `rustc` and `rustfix` multiple times, attempting to reach a steady state
where no fixes failed to apply.
Naturally this is a pretty tricky thing to do and we want to be sure that Cargo
doesn't loop forever, for example. A number of safeguards are in place to
prevent Cargo from going off into the weeds when fixing files, notably avoiding
to reattempt fixes if no successful fixes ended up being applied.
Closes rust-lang#5813
Closes rust-lang/rust#52754
|
@bors r+ |
|
📌 Commit 876a503 has been approved by |
fix: Iteratively apply suggestions from the compiler
This commit updates the `cargo fix` implementation to iteratively apply fixes
from the compiler instead of only once. Currently the compiler can sometimes
emit overlapping suggestions, such as in the case of transitioning
::foo::<::Bar>();
to ...
crate::foo::<crate::Bar>();
and `rustfix` rightfully can't handle overlapping suggestions as there's no
clear way of how to disambiguate the fixes. To fix this problem Cargo will now
run `rustc` and `rustfix` multiple times, attempting to reach a steady state
where no fixes failed to apply.
Naturally this is a pretty tricky thing to do and we want to be sure that Cargo
doesn't loop forever, for example. A number of safeguards are in place to
prevent Cargo from going off into the weeds when fixing files, notably avoiding
to reattempt fixes if no successful fixes ended up being applied.
Closes #5813
Closes rust-lang/rust#52754
|
☀️ Test successful - status-appveyor, status-travis |
This commit updates the
cargo fiximplementation to iteratively apply fixesfrom the compiler instead of only once. Currently the compiler can sometimes
emit overlapping suggestions, such as in the case of transitioning
to ...
and
rustfixrightfully can't handle overlapping suggestions as there's noclear way of how to disambiguate the fixes. To fix this problem Cargo will now
run
rustcandrustfixmultiple times, attempting to reach a steady statewhere no fixes failed to apply.
Naturally this is a pretty tricky thing to do and we want to be sure that Cargo
doesn't loop forever, for example. A number of safeguards are in place to
prevent Cargo from going off into the weeds when fixing files, notably avoiding
to reattempt fixes if no successful fixes ended up being applied.
Closes #5813
Closes rust-lang/rust#52754