fix: allow for workplace descriptors without a protocol (#4755)

### Description

Fixes the issue found by @quinnturner in
[#2891](#2791 (comment)).

Crux of the issue was we were being too restrictive when tracking
dependency protocols to properly infer them for later construction. We
now allow for at most two descriptors in the protocol resolver: one
without a protocol and one with a protocol. Another change was no longer
adding all descriptors for a workspace and instead only including
descriptors that come from other workspaces.

### Testing Instructions

New testing fixture contains a lockfile where one workspace depends on
another via `*` and the other depends on it via `workspace:*`.

crates/turborepo-lockfiles/fixtures/berry-protocol-collision.lock

@@ -0,0 +1,33 @@
+# This file is generated by running "yarn install" inside your project.
+# Manual changes might be lost - proceed with caution!
+
+__metadata:
+  version: 6
+
+"a@workspace:packages/a":
+  version: 0.0.0-use.local
+  resolution: "a@workspace:packages/a"
+  dependencies:
+    c: "*"
+  languageName: unknown
+  linkType: soft
+
+"b@workspace:packages/b":
+  version: 0.0.0-use.local
+  resolution: "b@workspace:packages/b"
+  dependencies:
+    c: "workspace:*"
+  languageName: unknown
+  linkType: soft
+
+"c@*, c@workspace:*, c@workspace:packages/c":
+  version: 0.0.0-use.local
+  resolution: "c@workspace:packages/c"
+  languageName: unknown
+  linkType: soft
+
+"prune-edge@workspace:.":
+  version: 0.0.0-use.local
+  resolution: "prune-edge@workspace:."
+  languageName: unknown
+  linkType: soft

crates/turborepo-lockfiles/src/berry/mod.rs

@@ -266,11 +266,10 @@ impl<'a> BerryLockfile<'a> {
                     resolutions.insert(dependency, dep_locator.clone());
                 }
 
-                if let Some(descriptors) = reverse_lookup.get(locator) {
-                    for descriptor in descriptors {
-                        resolutions.insert((*descriptor).clone(), locator.clone());
-                    }
-                }
+                // Included workspaces will always have their locator listed as a descriptor.
+                // All other descriptors should show up in the other workspace package
+                // dependencies.
+                resolutions.insert(Descriptor::from(locator.clone()), locator.clone());
             }
         }
 
@@ -779,4 +778,45 @@ mod test {
             version: "4.4.1".into()
         }));
     }
+
+    #[test]
+    fn test_workspace_collission() {
+        let data = LockfileData::from_bytes(include_bytes!(
+            "../../fixtures/berry-protocol-collision.lock"
+        ))
+        .unwrap();
+        let lockfile = BerryLockfile::new(&data, None).unwrap();
+        let no_proto = Descriptor::try_from("c@*").unwrap();
+        let workspace_proto = Descriptor::try_from("c@workspace:*").unwrap();
+        let full_path = Descriptor::try_from("c@workspace:packages/c").unwrap();
+        let a_lockfile = lockfile
+            .subgraph(&["packages/a".into(), "packages/c".into()], &[])
+            .unwrap();
+        let a_reverse_lookup = a_lockfile.locator_to_descriptors();
+        let a_c_descriptors = a_reverse_lookup
+            .get(&Locator::try_from("c@workspace:packages/c").unwrap())
+            .unwrap();
+
+        assert_eq!(
+            a_c_descriptors,
+            &(vec![&no_proto, &full_path]
+                .into_iter()
+                .collect::<HashSet<_>>())
+        );
+
+        let b_lockfile = lockfile
+            .subgraph(&["packages/b".into(), "packages/c".into()], &[])
+            .unwrap();
+        let b_reverse_lookup = b_lockfile.locator_to_descriptors();
+        let b_c_descriptors = b_reverse_lookup
+            .get(&Locator::try_from("c@workspace:packages/c").unwrap())
+            .unwrap();
+
+        assert_eq!(
+            b_c_descriptors,
+            &(vec![&workspace_proto, &full_path]
+                .into_iter()
+                .collect::<HashSet<_>>())
+        );
+    }
 }

crates/turborepo-lockfiles/src/berry/protocol_resolver.rs

@@ -5,7 +5,7 @@ use super::identifiers::{Descriptor, Ident};
 /// A data structure for resolving descriptors when the protocol isn't known
 #[derive(Debug, Clone, PartialEq, Eq, Default)]
 pub struct DescriptorResolver<'a> {
-    mapping: HashMap<Key<'a>, &'a str>,
+    mapping: HashMap<Key<'a>, Entry<'a>>,
 }
 
 #[derive(Debug, Clone, Hash, PartialEq, Eq, PartialOrd, Ord)]
@@ -14,18 +14,25 @@ struct Key<'a> {
     range: &'a str,
 }
 
+#[derive(Debug, Clone, Hash, PartialEq, Eq, PartialOrd, Ord, Default)]
+struct Entry<'a> {
+    without: Option<&'a str>,
+    with: Option<&'a str>,
+}
+
 impl<'a> DescriptorResolver<'a> {
     /// Add a descriptor to the resolver
     pub fn insert(&mut self, descriptor: &Descriptor<'a>) -> Option<&'a str> {
         let key = Key::new(descriptor)?;
-        self.mapping.insert(key, descriptor.range()?)
+        let entry = self.mapping.entry(key).or_default();
+        entry.insert_descriptor(descriptor)
     }
 
     /// If given a descriptor without a protocol it will return all matching
     /// descriptors with a protocol
     pub fn get(&self, descriptor: &Descriptor) -> Option<&'a str> {
         let key = Key::new(descriptor)?;
-        self.mapping.get(&key).copied()
+        self.mapping.get(&key).and_then(|e| e.get(descriptor))
     }
 }
 
@@ -37,6 +44,28 @@ impl<'a> Key<'a> {
     }
 }
 
+impl<'a> Entry<'a> {
+    // Insert the given descriptor's range into the correct slot depending if it is
+    // with or without a protocol
+    fn insert_descriptor(&mut self, descriptor: &Descriptor<'a>) -> Option<&'a str> {
+        let range = descriptor.range()?;
+        match descriptor.protocol().is_some() {
+            true => self.with.replace(range),
+            false => self.without.replace(range),
+        }
+    }
+
+    fn get(&self, descriptor: &Descriptor) -> Option<&'a str> {
+        // We only return the without protocol range if `without` is present
+        // and the given descriptor is also without a protocol
+        if self.without.is_some() && descriptor.protocol().is_none() {
+            self.without
+        } else {
+            self.with
+        }
+    }
+}
+
 #[cfg(test)]
 mod test {
     use super::*;
@@ -58,7 +87,10 @@ mod test {
     fn test_descriptors_without_protocols() {
         let mut resolver = DescriptorResolver::default();
         let workspace = Descriptor::new("internal-workspace", "*").unwrap();
+        let workspace_with_protocol = Descriptor::new("internal-workspace", "workspace:*").unwrap();
         assert!(resolver.insert(&workspace).is_none());
+        assert!(resolver.insert(&workspace_with_protocol).is_none());
         assert_eq!(resolver.get(&workspace), Some("*"));
+        assert_eq!(resolver.get(&workspace_with_protocol), Some("workspace:*"));
     }
 }