fix!: split Change::Delete into Change::VersionDeleted and Change::CrateDeleted.

This resolves an ambiguity (facilitated by incorrect documentation) that would emit
`Change::Delete` when a single version was deleted, even though downstream that would
rightfully be interpreted as deleting the whole crate.

Adimittedly, previously there were no single-version deletions, just yanks, so we didn't
think of it either.

Author: Byron

Cargo.toml

@@ -35,7 +35,7 @@ http-reqwest = ["gix/blocking-http-transport-reqwest-rust-tls"]
 
 
 [dependencies]
-gix = { version = "0.47.0", default-features = false, features = ["max-performance-safe", "blocking-network-client" ] }
+gix = { version = "0.48.0", default-features = false, features = ["max-performance-safe", "blocking-network-client" ] }
 serde = { version = "1", features = ["std", "derive"] }
 hex = { version = "0.4.3", features = ["serde"] }
 smartstring = { version = "1.0.1", features = ["serde"] }

src/index/diff/delegate.rs

@@ -55,7 +55,7 @@ impl Delegate {
                     for line in obj.data.lines() {
                         deleted.push(version_from_json_line(line, change.location)?);
                     }
-                    self.changes.push(Change::Deleted {
+                    self.changes.push(Change::CrateDeleted {
                         name: change.location.to_string(),
                         versions: deleted,
                     });
@@ -90,7 +90,6 @@ impl Delegate {
                         );
                     }
 
-                    let mut deleted = Vec::new();
                     for line in old_lines.drain() {
                         let old_version = version_from_json_line(line, location)?;
                         let new_version = new_versions
@@ -106,15 +105,9 @@ impl Delegate {
                                 };
                                 self.changes.push(change)
                             }
-                            None => deleted.push(old_version),
+                            None => self.changes.push(Change::VersionDeleted(old_version)),
                         }
                     }
-                    if !deleted.is_empty() {
-                        self.changes.push(Change::Deleted {
-                            name: deleted[0].name.to_string(),
-                            versions: deleted,
-                        })
-                    }
                     for version in new_versions.drain() {
                         let change = if version.yanked {
                             Change::AddedAndYanked(version)

src/types.rs

@@ -33,12 +33,14 @@ pub enum Change {
     /// A crate version was yanked.
     Yanked(CrateVersion),
     /// The name of the crate whose file was deleted, which implies all versions were deleted as well.
-    Deleted {
+    CrateDeleted {
         /// The name of the deleted crate.
         name: String,
         /// All of its versions that were deleted along with the file.
         versions: Vec<CrateVersion>,
     },
+    /// A crate version was deleted.
+    VersionDeleted(CrateVersion),
 }
 
 impl Change {
@@ -67,9 +69,17 @@ impl Change {
     }
 
     /// Return the deleted crate, if this is this kind of change.
-    pub fn deleted(&self) -> Option<(&str, &[CrateVersion])> {
+    pub fn crate_deleted(&self) -> Option<(&str, &[CrateVersion])> {
         match self {
-            Change::Deleted { name, versions } => Some((name.as_str(), versions)),
+            Change::CrateDeleted { name, versions } => Some((name.as_str(), versions)),
+            _ => None,
+        }
+    }
+
+    /// Return the deleted version crate, if this is this kind of change.
+    pub fn version_deleted(&self) -> Option<&CrateVersion> {
+        match self {
+            Change::VersionDeleted(v) => Some(v),
             _ => None,
         }
     }
@@ -84,8 +94,9 @@ impl Change {
             Change::Added(v)
             | Change::Unyanked(v)
             | Change::AddedAndYanked(v)
-            | Change::Yanked(v) => slice::from_ref(v),
-            Change::Deleted { versions, .. } => versions,
+            | Change::Yanked(v)
+            | Change::VersionDeleted(v) => slice::from_ref(v),
+            Change::CrateDeleted { versions, .. } => versions,
         }
     }
 }
@@ -98,7 +109,8 @@ impl fmt::Display for Change {
             match *self {
                 Change::Added(_) => "added",
                 Change::Yanked(_) => "yanked",
-                Change::Deleted { .. } => "deleted",
+                Change::CrateDeleted { .. } => "crate deleted",
+                Change::VersionDeleted(_) => "version deleted",
                 Change::Unyanked(_) => "unyanked",
                 Change::AddedAndYanked(_) => "added and yanked",
             }

tests/index/changes_between_commits.rs

@@ -7,7 +7,7 @@ fn directory_deletions_are_not_picked_up() -> crate::Result {
     let changes = changes(index_ro()?, ":/reproduce issue #20")?;
     assert_eq!(changes.len(), 1);
     assert_eq!(
-        changes.first().and_then(|c| c.deleted().map(|t| t.0)),
+        changes.first().and_then(|c| c.crate_deleted().map(|t| t.0)),
         Some("allowed")
     );
     Ok(())
@@ -89,7 +89,7 @@ fn deletion() -> crate::Result {
     let changes = changes(index_ro()?, "@^{/Delete crates}")?;
     assert_eq!(changes.len(), 1);
     assert_eq!(
-        changes.first().and_then(|c| c.deleted().map(|t| t.0)),
+        changes.first().and_then(|c| c.crate_deleted().map(|t| t.0)),
         Some("girl")
     );
     Ok(())

tests/shared/mod.rs

@@ -118,14 +118,17 @@ pub fn baseline(mode: Step) -> Result<(), Box<dyn std::error::Error + Send + Syn
                                     .expect("these events mean `Added*` events have been emitted") =
                                     v.yanked
                             }
-                            Deleted {
+                            CrateDeleted {
                                 versions: deleted, ..
                             } => {
                                 // delete a yanked crate
                                 for deleted_version in deleted {
                                     versions.remove(&deleted_version.checksum);
                                 }
                             }
+                            VersionDeleted(v) => {
+                                versions.remove(&v.checksum);
+                            }
                         }
                     }
                     let elapsed = start.elapsed().as_secs_f32();