use failure::{Error, ResultExt};
use git2;
use rustfix::diagnostics::Diagnostic;
-use rustfix;
+use rustfix::{self, CodeFix};
use serde_json;
use core::Workspace;
// If we didn't actually make any changes then we can immediately exec the
// new rustc, and otherwise we capture the output to hide it in the scenario
// that we have to back it all out.
- if !fixes.original_files.is_empty() {
+ if !fixes.files.is_empty() {
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() {
+ Message::Fixing {
+ file: path.clone(),
+ fixes: file.fixes_applied,
+ }.post()?;
}
}
// user's code with our changes. Back out everything and fall through
// below to recompile again.
if !output.status.success() {
- for (k, v) in fixes.original_files {
- fs::write(&k, v)
- .with_context(|_| format!("failed to write file `{}`", k))?;
+ for (path, file) in fixes.files.iter() {
+ fs::write(path, &file.original_code)
+ .with_context(|_| format!("failed to write file `{}`", path))?;
}
log_failed_fix(&output.stderr)?;
}
#[derive(Default)]
struct FixedCrate {
- messages: Vec<Message>,
- original_files: HashMap<String, String>,
+ files: HashMap<String, FixedFile>,
+}
+
+struct FixedFile {
+ errors_applying_fixes: Vec<String>,
+ fixes_applied: u32,
+ original_code: String,
}
fn rustfix_crate(lock_addr: &str, rustc: &Path, filename: &Path, args: &FixArgs)
args.verify_not_preparing_for_enabled_edition()?;
args.warn_if_preparing_probably_inert()?;
- // If not empty, filter by these lints
- //
- // TODO: Implement a way to specify this
- let only = HashSet::new();
-
// First up we want to make sure that each crate is only checked by one
// process at a time. If two invocations concurrently check a crate then
// it's likely to corrupt it.
// argument that looks like a Rust file.
let _lock = LockServerClient::lock(&lock_addr.parse()?, filename)?;
- let mut cmd = Command::new(&rustc);
+ // Next up this is a bit suspicious, but we *iteratively* execute rustc and
+ // collect suggestions to feed to rustfix. Once we hit our limit of times to
+ // execute rustc or we appear to be reaching a fixed point we stop running
+ // rustc.
+ //
+ // This is currently done to handle code like:
+ //
+ // ::foo::<::Bar>();
+ //
+ // where there are two fixes to happen here: `crate::foo::<crate::Bar>()`.
+ // The spans for these two suggestions are overlapping and its difficult in
+ // the compiler to *not* have overlapping spans here. As a result, a naive
+ // implementation would feed the two compiler suggestions for the above fix
+ // into `rustfix`, but one would be rejected because it overlaps with the
+ // other.
+ //
+ // In this case though, both suggestions are valid and can be automatically
+ // applied! To handle this case we execute rustc multiple times, collecting
+ // fixes each time we do so. Along the way we discard any suggestions that
+ // failed to apply, assuming that they can be fixed the next time we run
+ // rustc.
+ //
+ // Naturally we want a few protections in place here though to avoid looping
+ // forever or otherwise losing data. To that end we have a few termination
+ // conditions:
+ //
+ // * Do this whole process a fixed number of times. In theory we probably
+ // need an infinite number of times to apply fixes, but we're not gonna
+ // sit around waiting for that.
+ // * If it looks like a fix genuinely can't be applied we need to bail out.
+ // Detect this when a fix fails to get applied *and* no suggestions
+ // successfully applied to the same file. In that case looks like we
+ // definitely can't make progress, so bail out.
+ let mut fixes = FixedCrate::default();
+ let mut last_fix_counts = HashMap::new();
+ let iterations = env::var("CARGO_FIX_MAX_RETRIES")
+ .ok()
+ .and_then(|n| n.parse().ok())
+ .unwrap_or(4);
+ for _ in 0..iterations {
+ last_fix_counts.clear();
+ for (path, file) in fixes.files.iter_mut() {
+ last_fix_counts.insert(path.clone(), file.fixes_applied);
+ file.errors_applying_fixes.clear(); // we'll generate new errors below
+ }
+ rustfix_and_fix(&mut fixes, rustc, filename, args)?;
+ let mut progress_yet_to_be_made = false;
+ for (path, file) in fixes.files.iter_mut() {
+ if file.errors_applying_fixes.len() == 0 {
+ continue
+ }
+ // If anything was successfully fixed *and* there's at least one
+ // error, then assume the error was spurious and we'll try again on
+ // the next iteration.
+ if file.fixes_applied != *last_fix_counts.get(path).unwrap_or(&0) {
+ progress_yet_to_be_made = true;
+ }
+ }
+ if !progress_yet_to_be_made {
+ break
+ }
+ }
+
+ // Any errors still remaining at this point need to be reported as probably
+ // bugs in Cargo and/or rustfix.
+ for (path, file) in fixes.files.iter_mut() {
+ for error in file.errors_applying_fixes.drain(..) {
+ Message::ReplaceFailed {
+ file: path.clone(),
+ message: error,
+ }.post()?;
+ }
+ }
+
+ Ok(fixes)
+}
+
+/// Execute `rustc` to apply one round of suggestions to the crate in question.
+///
+/// This will fill in the `fixes` map with original code, suggestions applied,
+/// and any errors encountered while fixing files.
+fn rustfix_and_fix(fixes: &mut FixedCrate, rustc: &Path, filename: &Path, args: &FixArgs)
+ -> Result<(), Error>
+{
+ // If not empty, filter by these lints
+ //
+ // TODO: Implement a way to specify this
+ let only = HashSet::new();
+
+ let mut cmd = Command::new(rustc);
cmd.arg("--error-format=json");
args.apply(&mut cmd);
let output = cmd.output()
filename.display(),
);
- let mut original_files = HashMap::with_capacity(file_map.len());
- let mut messages = Vec::new();
for (file, suggestions) in file_map {
// Attempt to read the source code for this file. If this fails then
// that'd be pretty surprising, so log a message and otherwise keep
let num_suggestions = suggestions.len();
debug!("applying {} fixes to {}", num_suggestions, file);
- messages.push(Message::fixing(&file, num_suggestions));
-
- match rustfix::apply_suggestions(&code, &suggestions) {
- Err(e) => {
- Message::ReplaceFailed {
- file,
- message: e.to_string(),
- }.post()?;
- // TODO: Add flag to decide if we want to continue or bail out
- continue;
- }
- Ok(new_code) => {
- fs::write(&file, new_code)
- .with_context(|_| format!("failed to write file `{}`", file))?;
- original_files.insert(file, code);
+ // If this file doesn't already exist then we just read the original
+ // code, so save it. If the file already exists then the original code
+ // doesn't need to be updated as we've just read an interim state with
+ // some fixes but perhaps not all.
+ let fixed_file = fixes.files.entry(file.clone())
+ .or_insert_with(|| {
+ FixedFile {
+ errors_applying_fixes: Vec::new(),
+ fixes_applied: 0,
+ original_code: code.clone(),
+ }
+ });
+ let mut fixed = CodeFix::new(&code);
+
+ // As mentioned above in `rustfix_crate`, we don't immediately warn
+ // about suggestions that fail to apply here, and instead we save them
+ // off for later processing.
+ for suggestion in suggestions.iter().rev() {
+ match fixed.apply(suggestion) {
+ Ok(()) => fixed_file.fixes_applied += 1,
+ Err(e) => fixed_file.errors_applying_fixes.push(e.to_string()),
}
}
+ let new_code = fixed.finish()?;
+ fs::write(&file, new_code)
+ .with_context(|_| format!("failed to write file `{}`", file))?;
}
- Ok(FixedCrate {
- messages,
- original_files,
- })
+ Ok(())
}
fn exit_with(status: ExitStatus) -> ! {