diff --git a/src/cargo/ops/fix.rs b/src/cargo/ops/fix.rs index 88c90e31af4..c952d153ed2 100644 --- a/src/cargo/ops/fix.rs +++ b/src/cargo/ops/fix.rs @@ -50,6 +50,7 @@ use rustfix::CodeFix; use semver::Version; use tracing::{debug, trace, warn}; +use crate::core::compiler::CompileKind; use crate::core::compiler::RustcTargetData; use crate::core::resolver::features::{DiffMap, FeatureOpts, FeatureResolver, FeaturesFor}; use crate::core::resolver::{HasDevUnits, Resolve, ResolveBehavior}; @@ -78,6 +79,14 @@ const EDITION_ENV_INTERNAL: &str = "__CARGO_FIX_EDITION"; /// **Internal only.** /// For passing [`FixOptions::idioms`] through to cargo running in proxy mode. const IDIOMS_ENV_INTERNAL: &str = "__CARGO_FIX_IDIOMS"; +/// **Internal only.** +/// The sysroot path. +/// +/// This is for preventing `cargo fix` from fixing rust std/core libs. See +/// +/// * +/// * +const SYSROOT_INTERNAL: &str = "__CARGO_FIX_RUST_SRC"; pub struct FixOptions { pub edition: bool, @@ -97,6 +106,8 @@ pub fn fix( ) -> CargoResult<()> { check_version_control(gctx, opts)?; + let mut target_data = + RustcTargetData::new(original_ws, &opts.compile_opts.build_config.requested_kinds)?; if opts.edition { let specs = opts.compile_opts.spec.to_package_id_specs(&original_ws)?; let members: Vec<&Package> = original_ws @@ -105,7 +116,7 @@ pub fn fix( .collect(); migrate_manifests(original_ws, &members)?; - check_resolver_change(&original_ws, opts)?; + check_resolver_change(&original_ws, &mut target_data, opts)?; } let mut ws = Workspace::new(&root_manifest, gctx)?; ws.set_honor_rust_version(original_ws.honor_rust_version()); @@ -129,6 +140,11 @@ pub fn fix( wrapper.env(IDIOMS_ENV_INTERNAL, "1"); } + let sysroot = &target_data.info(CompileKind::Host).sysroot; + if sysroot.is_dir() { + wrapper.env(SYSROOT_INTERNAL, sysroot); + } + *opts .compile_opts .build_config @@ -328,7 +344,11 @@ fn add_feature_for_unused_deps(pkg: &Package, parent: &mut dyn toml_edit::TableL fixes } -fn check_resolver_change(ws: &Workspace<'_>, opts: &FixOptions) -> CargoResult<()> { +fn check_resolver_change<'gctx>( + ws: &Workspace<'gctx>, + target_data: &mut RustcTargetData<'gctx>, + opts: &FixOptions, +) -> CargoResult<()> { let root = ws.root_maybe(); match root { MaybePackage::Package(root_pkg) => { @@ -355,12 +375,10 @@ fn check_resolver_change(ws: &Workspace<'_>, opts: &FixOptions) -> CargoResult<( // 2018 without `resolver` set must be V1 assert_eq!(ws.resolve_behavior(), ResolveBehavior::V1); let specs = opts.compile_opts.spec.to_package_id_specs(ws)?; - let mut target_data = - RustcTargetData::new(ws, &opts.compile_opts.build_config.requested_kinds)?; let mut resolve_differences = |has_dev_units| -> CargoResult<(WorkspaceResolve<'_>, DiffMap)> { let ws_resolve = ops::resolve_ws_with_opts( ws, - &mut target_data, + target_data, &opts.compile_opts.build_config.requested_kinds, &opts.compile_opts.cli_features, &specs, @@ -371,7 +389,7 @@ fn check_resolver_change(ws: &Workspace<'_>, opts: &FixOptions) -> CargoResult<( let feature_opts = FeatureOpts::new_behavior(ResolveBehavior::V2, has_dev_units); let v2_features = FeatureResolver::resolve( ws, - &mut target_data, + target_data, &ws_resolve.targeted_resolve, &ws_resolve.pkg_set, &opts.compile_opts.cli_features, @@ -677,7 +695,8 @@ fn rustfix_crate( // We'll generate new errors below. file.errors_applying_fixes.clear(); } - (last_output, last_made_changes) = rustfix_and_fix(&mut files, rustc, filename, gctx)?; + (last_output, last_made_changes) = + rustfix_and_fix(&mut files, rustc, filename, args, gctx)?; if current_iteration == 0 { first_output = Some(last_output.clone()); } @@ -734,6 +753,7 @@ fn rustfix_and_fix( files: &mut HashMap, rustc: &ProcessBuilder, filename: &Path, + args: &FixArgs, gctx: &GlobalContext, ) -> CargoResult<(Output, bool)> { // If not empty, filter by these lints. @@ -798,10 +818,17 @@ fn rustfix_and_fix( continue; }; + let file_path = Path::new(&file_name); // Do not write into registry cache. See rust-lang/cargo#9857. - if Path::new(&file_name).starts_with(home_path) { + if file_path.starts_with(home_path) { continue; } + // Do not write into standard library source. See rust-lang/cargo#9857. + if let Some(sysroot) = args.sysroot.as_deref() { + if file_path.starts_with(sysroot) { + continue; + } + } if !file_names.clone().all(|f| f == &file_name) { trace!("rejecting as it changes multiple files: {:?}", suggestion); @@ -958,6 +985,8 @@ struct FixArgs { other: Vec, /// Path to the `rustc` executable. rustc: PathBuf, + /// Path to host sysroot. + sysroot: Option, } impl FixArgs { @@ -1029,6 +1058,11 @@ impl FixArgs { .saturating_next() }); + // ALLOWED: For the internal mechanism of `cargo fix` only. + // Shouldn't be set directly by anyone. + #[allow(clippy::disallowed_methods)] + let sysroot = env::var_os(SYSROOT_INTERNAL).map(PathBuf::from); + Ok(FixArgs { file, prepare_for_edition, @@ -1036,6 +1070,7 @@ impl FixArgs { enabled_edition, other, rustc, + sysroot, }) } diff --git a/tests/testsuite/fix.rs b/tests/testsuite/fix.rs index 7997a68ae62..4175a35b2d7 100644 --- a/tests/testsuite/fix.rs +++ b/tests/testsuite/fix.rs @@ -1906,6 +1906,333 @@ warning: `foo` (lib) generated 1 warning (run `cargo fix --lib -p foo` to apply .run(); } +#[cargo_test] +fn fix_in_rust_src() { + // Tests what happens if rustc emits a suggestion to modify the standard + // library in rust source. This should never happen, and indicates a bug in + // rustc. However, there are several known bugs in rustc where it does this + // (often involving macros), so `cargo fix` has a guard that says if the + // suggestion points to rust source under sysroot to not apply it. + // + // See https://github.com/rust-lang/cargo/issues/9857 for some other + // examples. + // + // This test uses a simulated rustc which replays a suggestion via a JSON + // message that points into rust-src. This does not use the real rustc + // because as the bugs are fixed in the real rustc, that would cause this + // test to stop working. + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + edition = "2021" + "#, + ) + .file( + "src/lib.rs", + r#" + pub fn bug_report(w: &mut W) -> std::fmt::Result { + if true { + writeln!(w, "`;?` here ->")?; + } else { + writeln!(w, "but not here") + } + Ok(()) + } + "#, + ) + .build(); + p.cargo("fetch").run(); + + // Since this is a substitution into a Rust string (representing a JSON + // string), deal with backslashes like on Windows. + let sysroot = paths::sysroot().replace("\\", "/"); + + // This is a fake rustc that will emit a JSON message when the `foo` crate + // builds that tells cargo to modify a file it shouldn't. + let rustc = project() + .at("rustc-replay") + .file("Cargo.toml", &basic_manifest("rustc-replay", "1.0.0")) + .file("src/main.rs", + &r##" + fn main() { + let pkg_name = match std::env::var("CARGO_PKG_NAME") { + Ok(pkg_name) => pkg_name, + Err(_) => { + let r = std::process::Command::new("rustc") + .args(std::env::args_os().skip(1)) + .status(); + std::process::exit(r.unwrap().code().unwrap_or(2)); + } + }; + if pkg_name == "foo" { + eprintln!("{}", r#"{ + "$message_type": "diagnostic", + "message": "mismatched types", + "code": + { + "code": "E0308", + "explanation": "Expected type did not match the received type.\n\nErroneous code examples:\n\n```compile_fail,E0308\nfn plus_one(x: i32) -> i32 {\n x + 1\n}\n\nplus_one(\"Not a number\");\n// ^^^^^^^^^^^^^^ expected `i32`, found `&str`\n\nif \"Not a bool\" {\n// ^^^^^^^^^^^^ expected `bool`, found `&str`\n}\n\nlet x: f32 = \"Not a float\";\n// --- ^^^^^^^^^^^^^ expected `f32`, found `&str`\n// |\n// expected due to this\n```\n\nThis error occurs when an expression was used in a place where the compiler\nexpected an expression of a different type. It can occur in several cases, the\nmost common being when calling a function and passing an argument which has a\ndifferent type than the matching type in the function declaration.\n" + }, + "level": "error", + "spans": + [ + { + "file_name": "__SYSROOT__/lib/rustlib/src/rust/library/core/src/macros/mod.rs", + "byte_start": 23568, + "byte_end": 23617, + "line_start": 670, + "line_end": 670, + "column_start": 9, + "column_end": 58, + "is_primary": true, + "text": + [ + { + "text": " $dst.write_fmt($crate::format_args_nl!($($arg)*))", + "highlight_start": 9, + "highlight_end": 58 + } + ], + "label": "expected `()`, found `Result<(), Error>`", + "suggested_replacement": null, + "suggestion_applicability": null, + "expansion": + { + "span": + { + "file_name": "lib.rs", + "byte_start": 144, + "byte_end": 171, + "line_start": 5, + "line_end": 5, + "column_start": 9, + "column_end": 36, + "is_primary": false, + "text": + [ + { + "text": " writeln!(w, \"but not here\")", + "highlight_start": 9, + "highlight_end": 36 + } + ], + "label": null, + "suggested_replacement": null, + "suggestion_applicability": null, + "expansion": null + }, + "macro_decl_name": "writeln!", + "def_site_span": + { + "file_name": "__SYSROOT__/lib/rustlib/src/rust/library/core/src/macros/mod.rs", + "byte_start": 23434, + "byte_end": 23454, + "line_start": 665, + "line_end": 665, + "column_start": 1, + "column_end": 21, + "is_primary": false, + "text": + [ + { + "text": "macro_rules! writeln {", + "highlight_start": 1, + "highlight_end": 21 + } + ], + "label": null, + "suggested_replacement": null, + "suggestion_applicability": null, + "expansion": null + } + } + }, + { + "file_name": "lib.rs", + "byte_start": 75, + "byte_end": 177, + "line_start": 2, + "line_end": 6, + "column_start": 5, + "column_end": 6, + "is_primary": false, + "text": + [ + { + "text": " if true {", + "highlight_start": 5, + "highlight_end": 14 + }, + { + "text": " writeln!(w, \"`;?` here ->\")?;", + "highlight_start": 1, + "highlight_end": 38 + }, + { + "text": " } else {", + "highlight_start": 1, + "highlight_end": 13 + }, + { + "text": " writeln!(w, \"but not here\")", + "highlight_start": 1, + "highlight_end": 36 + }, + { + "text": " }", + "highlight_start": 1, + "highlight_end": 6 + } + ], + "label": "expected this to be `()`", + "suggested_replacement": null, + "suggestion_applicability": null, + "expansion": null + } + ], + "children": + [ + { + "message": "use the `?` operator to extract the `Result<(), std::fmt::Error>` value, propagating a `Result::Err` value to the caller", + "code": null, + "level": "help", + "spans": + [ + { + "file_name": "__SYSROOT__/lib/rustlib/src/rust/library/core/src/macros/mod.rs", + "byte_start": 23617, + "byte_end": 23617, + "line_start": 670, + "line_end": 670, + "column_start": 58, + "column_end": 58, + "is_primary": true, + "text": + [ + { + "text": " $dst.write_fmt($crate::format_args_nl!($($arg)*))", + "highlight_start": 58, + "highlight_end": 58 + } + ], + "label": null, + "suggested_replacement": "?", + "suggestion_applicability": "HasPlaceholders", + "expansion": + { + "span": + { + "file_name": "lib.rs", + "byte_start": 144, + "byte_end": 171, + "line_start": 5, + "line_end": 5, + "column_start": 9, + "column_end": 36, + "is_primary": false, + "text": + [ + { + "text": " writeln!(w, \"but not here\")", + "highlight_start": 9, + "highlight_end": 36 + } + ], + "label": null, + "suggested_replacement": null, + "suggestion_applicability": null, + "expansion": null + }, + "macro_decl_name": "writeln!", + "def_site_span": + { + "file_name": "__SYSROOT__/lib/rustlib/src/rust/library/core/src/macros/mod.rs", + "byte_start": 23434, + "byte_end": 23454, + "line_start": 665, + "line_end": 665, + "column_start": 1, + "column_end": 21, + "is_primary": false, + "text": + [ + { + "text": "macro_rules! writeln {", + "highlight_start": 1, + "highlight_end": 21 + } + ], + "label": null, + "suggested_replacement": null, + "suggestion_applicability": null, + "expansion": null + } + } + } + ], + "children": + [], + "rendered": null + } + ], + "rendered": "error[E0308]: mismatched types\n --> lib.rs:5:9\n |\n2 | / if true {\n3 | | writeln!(w, \"`;?` here ->\")?;\n4 | | } else {\n5 | | writeln!(w, \"but not here\")\n | | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `()`, found `Result<(), Error>`\n6 | | }\n | |_____- expected this to be `()`\n |\n = note: expected unit type `()`\n found enum `Result<(), std::fmt::Error>`\n = note: this error originates in the macro `writeln` (in Nightly builds, run with -Z macro-backtrace for more info)\nhelp: consider using a semicolon here\n |\n6 | };\n | +\nhelp: you might have meant to return this value\n |\n5 | return writeln!(w, \"but not here\");\n | ++++++ +\nhelp: use the `?` operator to extract the `Result<(), std::fmt::Error>` value, propagating a `Result::Err` value to the caller\n --> __SYSROOT__/lib/rustlib/src/rust/library/core/src/macros/mod.rs:670:58\n |\n67| $dst.write_fmt($crate::format_args_nl!($($arg)*))?\n | +\n\n" +}"#.replace("\n", "")); + + std::process::exit(2); + } + } + "##.replace("__SYSROOT__", &sysroot)) + .build(); + rustc.cargo("build").run(); + let rustc_bin = rustc.bin("rustc-replay"); + + // The output here should not say `Fixed`. + // + // It is OK to compare the full diagnostic output here because the text is + // hard-coded in rustc-replay. Normally tests should not be checking the + // compiler output. + p.cargo("fix --lib --allow-no-vcs --broken-code") + .env("__CARGO_FIX_YOLO", "1") + .env("RUSTC", &rustc_bin) + .with_status(101) + .with_stderr(r#"[CHECKING] foo v0.0.0 ([..]) +error[E0308]: mismatched types + --> lib.rs:5:9 + | +2 | / if true { +3 | | writeln!(w, "`;?` here ->")?; +4 | | } else { +5 | | writeln!(w, "but not here") + | | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `()`, found `Result<(), Error>` +6 | | } + | |_____- expected this to be `()` + | + = note: expected unit type `()` + found enum `Result<(), std::fmt::Error>` + = note: this error originates in the macro `writeln` (in Nightly builds, run with -Z macro-backtrace for more info) +help: consider using a semicolon here + | +6 | }; + | + +help: you might have meant to return this value + | +5 | return writeln!(w, "but not here"); + | ++++++ + +help: use the `?` operator to extract the `Result<(), std::fmt::Error>` value, propagating a `Result::Err` value to the caller + --> [..]/lib/rustlib/src/rust/library/core/src/macros/mod.rs:670:58 + | +67| $dst.write_fmt($crate::format_args_nl!($($arg)*))? + | + + +[ERROR] could not compile `foo` (lib) due to 1 previous error +"#) + .run(); +} + // This fixes rust-lang/rust#123304. // If that lint stops emitting duplicate suggestions, // we might need to find a substitution.