Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change cargo fix --edition to only fix edition lints. #9846

Merged
merged 1 commit into from
Aug 30, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion src/cargo/ops/fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -824,7 +824,17 @@ impl FixArgs {

fn apply(&self, cmd: &mut Command, config: &Config) {
cmd.arg(&self.file);
cmd.args(&self.other).arg("--cap-lints=warn");
cmd.args(&self.other);
if self.prepare_for_edition.is_some() && config.nightly_features_allowed {
// When migrating an edition, we don't want to fix other lints as
// they can sometimes add suggestions that fail to apply, causing
// the entire migration to fail. But those lints aren't needed to
// migrate.
cmd.arg("--cap-lints=allow");
} else {
// This allows `cargo fix` to work even if the crate has #[deny(warnings)].
cmd.arg("--cap-lints=warn");
}
if let Some(edition) = self.enabled_edition {
cmd.arg("--edition").arg(edition.to_string());
if self.idioms && edition.supports_idiom_lint() {
Expand Down
47 changes: 47 additions & 0 deletions tests/testsuite/fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1745,3 +1745,50 @@ fn fix_with_run_cargo_in_proc_macros() {
.with_stderr_does_not_contain("error: could not find .rs file in rustc args")
.run();
}

#[cargo_test]
fn non_edition_lint_migration() {
// Migrating to a new edition where a non-edition lint causes problems.
if !is_nightly() {
// Remove once force-warn hits stable.
return;
}
let p = project()
.file("Cargo.toml", &basic_manifest("foo", "0.1.0"))
.file(
"src/lib.rs",
r#"
// This is only used in a test.
// To be correct, this should be gated on #[cfg(test)], but
// sometimes people don't do that. If the unused_imports
// lint removes this, then the unittest will fail to compile.
use std::str::from_utf8;

pub mod foo {
pub const FOO: &[u8] = &[102, 111, 111];
}

#[test]
fn example() {
assert_eq!(
from_utf8(::foo::FOO), Ok("foo")
);
}
"#,
)
.build();
// Check that it complains about an unused import.
p.cargo("check --lib")
.with_stderr_contains("[..]unused_imports[..]")
.with_stderr_contains("[..]std::str::from_utf8[..]")
.run();
p.cargo("fix --edition --allow-no-vcs")
// Remove once --force-warn is stabilized
.masquerade_as_nightly_cargo()
.run();
let contents = p.read_file("src/lib.rs");
// Check it does not remove the "unused" import.
assert!(contents.contains("use std::str::from_utf8;"));
// Check that it made the edition migration.
assert!(contents.contains("from_utf8(crate::foo::FOO)"));
}