Auto merge of #9803 - ehuss:fix-edition-resolve-differences-dev-deps, r=alexcrichton

Show feature resolver differences for dev-dependencies.

During the crater run for 2021, there was a package that failed to update in a confusing way. The issue is that a feature was removed in the new resolver, but only for a dev-dependency. The report displayed with `cargo fix --edition` did not say anything about that, so it took me a bit to figure it out.  This changes it so that the report also includes changes to features of dev-dependencies.  I honestly don't remember my thought process behind the original code.

For example, the offending package now says:

```
When building the following dependencies, the given features will no longer be used:

  log v0.4.8 removed features: std
  syn v0.15.44 (as host dependency) removed features: extra-traits, visit

The following differences only apply when building with dev-dependencies:

  phf_shared v0.7.24 (as host dependency) removed features: unicase
```

And the error that happens after updating to 2021 is:

```
error[E0277]: the trait bound `UniCase<&str>: phf_shared::PhfHash` is not satisfied
  --> /Users/eric/.cargo/registry/src/github.com-1ecc6299db9ec823/mime_guess-1.8.7/build.rs:37:21
   |
37 |         forward_map.entry(UniCase(key), &format!("{:?}", val));
   |                     ^^^^^ the trait `phf_shared::PhfHash` is not implemented for `UniCase<&str>`
```

Hopefully developers will be able to see the note about the feature `unicase` being removed from `phf_shared`, and the error message about `UniCase` not implementing `PhfHash`, and connect the two together.  Previously, the upgrade report didn't mention anything about `phf_shared`, and thus no clues on what went wrong.

Author: bors

src/cargo/ops/fix.rs

@@ -52,9 +52,10 @@ use rustfix::diagnostics::Diagnostic;
 use rustfix::{self, CodeFix};
 
 use crate::core::compiler::RustcTargetData;
-use crate::core::resolver::features::{FeatureOpts, FeatureResolver};
+use crate::core::resolver::features::{DiffMap, FeatureOpts, FeatureResolver};
 use crate::core::resolver::{HasDevUnits, Resolve, ResolveBehavior};
 use crate::core::{Edition, MaybePackage, Workspace};
+use crate::ops::resolve::WorkspaceResolve;
 use crate::ops::{self, CompileOptions};
 use crate::util::diagnostic_server::{Message, RustfixDiagnosticServer};
 use crate::util::errors::CargoResult;
@@ -229,36 +230,40 @@ fn check_resolver_change(ws: &Workspace<'_>, opts: &FixOptions) -> CargoResult<(
     assert_eq!(ws.resolve_behavior(), ResolveBehavior::V1);
     let specs = opts.compile_opts.spec.to_package_id_specs(ws)?;
     let target_data = RustcTargetData::new(ws, &opts.compile_opts.build_config.requested_kinds)?;
-    // HasDevUnits::No because that may uncover more differences.
-    // This is not the same as what `cargo fix` is doing, since it is doing
-    // `--all-targets` which includes dev dependencies.
-    let ws_resolve = ops::resolve_ws_with_opts(
-        ws,
-        &target_data,
-        &opts.compile_opts.build_config.requested_kinds,
-        &opts.compile_opts.cli_features,
-        &specs,
-        HasDevUnits::No,
-        crate::core::resolver::features::ForceAllTargets::No,
-    )?;
+    let resolve_differences = |has_dev_units| -> CargoResult<(WorkspaceResolve<'_>, DiffMap)> {
+        let ws_resolve = ops::resolve_ws_with_opts(
+            ws,
+            &target_data,
+            &opts.compile_opts.build_config.requested_kinds,
+            &opts.compile_opts.cli_features,
+            &specs,
+            has_dev_units,
+            crate::core::resolver::features::ForceAllTargets::No,
+        )?;
 
-    let feature_opts = FeatureOpts::new_behavior(ResolveBehavior::V2, HasDevUnits::No);
-    let v2_features = FeatureResolver::resolve(
-        ws,
-        &target_data,
-        &ws_resolve.targeted_resolve,
-        &ws_resolve.pkg_set,
-        &opts.compile_opts.cli_features,
-        &specs,
-        &opts.compile_opts.build_config.requested_kinds,
-        feature_opts,
-    )?;
+        let feature_opts = FeatureOpts::new_behavior(ResolveBehavior::V2, has_dev_units);
+        let v2_features = FeatureResolver::resolve(
+            ws,
+            &target_data,
+            &ws_resolve.targeted_resolve,
+            &ws_resolve.pkg_set,
+            &opts.compile_opts.cli_features,
+            &specs,
+            &opts.compile_opts.build_config.requested_kinds,
+            feature_opts,
+        )?;
 
-    let differences = v2_features.compare_legacy(&ws_resolve.resolved_features);
-    if differences.is_empty() {
+        let diffs = v2_features.compare_legacy(&ws_resolve.resolved_features);
+        Ok((ws_resolve, diffs))
+    };
+    let (_, without_dev_diffs) = resolve_differences(HasDevUnits::No)?;
+    let (ws_resolve, mut with_dev_diffs) = resolve_differences(HasDevUnits::Yes)?;
+    if without_dev_diffs.is_empty() && with_dev_diffs.is_empty() {
         // Nothing is different, nothing to report.
         return Ok(());
     }
+    // Only display unique changes with dev-dependencies.
+    with_dev_diffs.retain(|k, vals| without_dev_diffs.get(k) != Some(vals));
     let config = ws.config();
     config.shell().note(
         "Switching to Edition 2021 will enable the use of the version 2 feature resolver in Cargo.",
@@ -277,16 +282,28 @@ fn check_resolver_change(ws: &Workspace<'_>, opts: &FixOptions) -> CargoResult<(
         "When building the following dependencies, \
          the given features will no longer be used:\n"
     );
-    for ((pkg_id, for_host), removed) in differences {
-        drop_eprint!(config, "  {}", pkg_id);
-        if for_host {
-            drop_eprint!(config, " (as host dependency)");
+    let show_diffs = |differences: DiffMap| {
+        for ((pkg_id, for_host), removed) in differences {
+            drop_eprint!(config, "  {}", pkg_id);
+            if for_host {
+                drop_eprint!(config, " (as host dependency)");
+            }
+            drop_eprint!(config, " removed features: ");
+            let joined: Vec<_> = removed.iter().map(|s| s.as_str()).collect();
+            drop_eprintln!(config, "{}", joined.join(", "));
         }
-        drop_eprint!(config, ": ");
-        let joined: Vec<_> = removed.iter().map(|s| s.as_str()).collect();
-        drop_eprintln!(config, "{}", joined.join(", "));
+        drop_eprint!(config, "\n");
+    };
+    if !without_dev_diffs.is_empty() {
+        show_diffs(without_dev_diffs);
+    }
+    if !with_dev_diffs.is_empty() {
+        drop_eprintln!(
+            config,
+            "The following differences only apply when building with dev-dependencies:\n"
+        );
+        show_diffs(with_dev_diffs);
     }
-    drop_eprint!(config, "\n");
     report_maybe_diesel(config, &ws_resolve.targeted_resolve)?;
     Ok(())
 }

tests/testsuite/fix.rs

@@ -1428,6 +1428,7 @@ fn edition_v2_resolver_report() {
     }
     Package::new("common", "1.0.0")
         .feature("f1", &[])
+        .feature("dev-feat", &[])
         .add_dep(Dependency::new("opt_dep", "1.0").optional(true))
         .publish();
     Package::new("opt_dep", "1.0.0").publish();
@@ -1455,6 +1456,9 @@ fn edition_v2_resolver_report() {
 
                 [build-dependencies]
                 common = { version = "1.0", features = ["opt_dep"] }
+
+                [dev-dependencies]
+                common = { version="1.0", features=["dev-feat"] }
             "#,
         )
         .file("src/lib.rs", "")
@@ -1473,8 +1477,12 @@ This may cause some dependencies to be built with fewer features enabled than pr
 More information about the resolver changes may be found at https://doc.rust-lang.org/nightly/edition-guide/rust-2021/default-cargo-resolver.html
 When building the following dependencies, the given features will no longer be used:
 
-  common v1.0.0: f1, opt_dep
-  common v1.0.0 (as host dependency): f1
+  common v1.0.0 removed features: dev-feat, f1, opt_dep
+  common v1.0.0 (as host dependency) removed features: dev-feat, f1
+
+The following differences only apply when building with dev-dependencies:
+
+  common v1.0.0 removed features: f1, opt_dep
 
 [CHECKING] opt_dep v1.0.0
 [CHECKING] common v1.0.0