Fixed bug where crate_universe could match aliases to bench/example deps (#2600)

A dependency which aliases a transitive dependency with a name that
happens to match another Cargo target in that (Cargo) package will
result in an incorrect dependency being collected. For the example of
[surrealdb-core](https://github.com/surrealdb/surrealdb/blob/v1.3.1/core/Cargo.toml#L83),
the `executor = { package "async-executor" }` dependency was incorrectly
matching [a benchmark
target](https://github.com/smol-rs/async-executor/blob/v1.9.1/Cargo.toml#L38-L40),
thus causing a broken dep to be rendered. This PR fixes this issue.

Author: UebelAndre

crate_universe/src/context.rs

@@ -285,6 +285,7 @@ mod test {
                 (&CrateId::new("log".to_owned(), Version::new(0, 4, 21)), false),
                 (&CrateId::new("names".to_owned(), Version::parse("0.12.1-dev").unwrap()), false),
                 (&CrateId::new("names".to_owned(), Version::new(0, 13, 0)), false),
+                (&CrateId::new("surrealdb".to_owned(), Version::new(1, 3, 1)), false),
                 (&CrateId::new("value-bag".to_owned(), Version::parse("1.0.0-alpha.7").unwrap()), false),
             ],
         }

crate_universe/src/metadata/dependency.rs

@@ -3,7 +3,7 @@ use std::collections::BTreeSet;
 
 use anyhow::{bail, Result};
 use cargo_metadata::{
-    DependencyKind, Metadata as CargoMetadata, Node, NodeDep, Package, PackageId,
+    DependencyKind, Metadata as CargoMetadata, Node, NodeDep, Package, PackageId, Target,
 };
 use cargo_platform::Platform;
 use serde::{Deserialize, Serialize};
@@ -187,19 +187,28 @@ fn collect_deps_selectable(
     select
 }
 
+/// Packages may have targets that match aliases of dependents. This function
+/// checks a target to see if it's an unexpected type for a dependency.
+fn is_ignored_package_target(target: &Target) -> bool {
+    target
+        .kind
+        .iter()
+        .any(|t| ["example", "bench", "test"].contains(&t.as_str()))
+}
+
 fn is_lib_package(package: &Package) -> bool {
     package.targets.iter().any(|target| {
         target
             .crate_types
             .iter()
             .any(|t| ["lib", "rlib"].contains(&t.as_str()))
+            && !is_ignored_package_target(target)
     })
 }
 
 fn is_proc_macro_package(package: &Package) -> bool {
     package.targets.iter().any(|target| {
-        target.crate_types.iter().any(|t| t == "proc-macro")
-            && !target.kind.iter().any(|t| t == "example")
+        target.crate_types.iter().any(|t| t == "proc-macro") && !is_ignored_package_target(target)
     })
 }
 
@@ -238,8 +247,13 @@ fn is_workspace_member(node_dep: &NodeDep, metadata: &CargoMetadata) -> bool {
 
 fn get_library_target_name(package: &Package, potential_name: &str) -> Result<String> {
     // If the potential name is not an alias in a dependent's package, a target's name
-    // should match which means we already know what the target library name is.
-    if package.targets.iter().any(|t| t.name == potential_name) {
+    // should match which means we already know what the target library name is. The
+    // only exception is for targets that are otherwise ignored (like benchmarks or examples).
+    if package
+        .targets
+        .iter()
+        .any(|t| t.name == potential_name && !is_ignored_package_target(t))
+    {
         return Ok(potential_name.to_string());
     }
 
@@ -271,11 +285,13 @@ fn get_library_target_name(package: &Package, potential_name: &str) -> Result<St
 /// for targets where packages (packages[#].targets[#].name) uses crate names. In order to
 /// determine whether or not a dependency is aliased, we compare it with all available targets
 /// on it's package. Note that target names are not guaranteed to be module names where Node
-/// dependencies are, so we need to do a conversion to check for this
+/// dependencies are, so we need to do a conversion to check for this. This function will
+/// return the name of a target's alias in the content of the current dependent if it is aliased.
 fn get_target_alias(target_name: &str, package: &Package) -> Option<String> {
     match package
         .targets
         .iter()
+        .filter(|t| !is_ignored_package_target(t))
         .all(|t| sanitize_module_name(&t.name) != target_name)
     {
         true => Some(target_name.to_string()),
@@ -449,6 +465,30 @@ mod test {
         assert_eq!(proc_macro_deps, Vec::<&str>::new());
     }
 
+    #[test]
+    fn bench_name_alias_dep() {
+        let metadata = metadata::alias();
+
+        let node = find_metadata_node("surrealdb-core", &metadata);
+        let dependencies = DependencySet::new_for_node(node, &metadata);
+
+        println!("{:#?}", dependencies);
+
+        let bindings = dependencies.normal_deps.items();
+
+        // It's critical that the dep be found with the correct name and not the
+        // alias that the `aliases` package is using that coincidentally matches the
+        // `bench` target `executor` in the `async-executor` package.
+        let async_executor = bindings
+            .iter()
+            .find(|(_, dep)| dep.target_name == "async-executor")
+            .map(|(_, dep)| dep)
+            .unwrap();
+
+        // Ensure alias data is still tracked.
+        assert_eq!(async_executor.alias, Some("executor".to_owned()));
+    }
+
     #[test]
     fn sys_dependencies() {
         let metadata = metadata::build_scripts();