Merge pull request #4 from cuviper/sync

Sync changes from indexmap and release 0.5.3

Author: cuviper

Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 name = "ordermap"
 edition = "2021"
-version = "0.5.2"
+version = "0.5.3"
 documentation = "https://docs.rs/ordermap/"
 repository = "https://github.com/indexmap-rs/ordermap"
 license = "Apache-2.0 OR MIT"
@@ -14,7 +14,7 @@ rust-version = "1.63"
 bench = false
 
 [dependencies]
-indexmap = { version = "2.4.0", default-features = false }
+indexmap = { version = "2.5.0", default-features = false }
 
 arbitrary = { version = "1.0", optional = true, default-features = false }
 quickcheck = { version = "1.0", optional = true, default-features = false }

RELEASES.md

@@ -1,5 +1,12 @@
 # Releases
 
+## 0.5.3
+
+- Added an `insert_before` method to `OrderMap` and `OrderSet`, as an
+  alternative to `shift_insert` with different behavior on existing entries.
+- Added `first_entry` and `last_entry` methods to `OrderMap`.
+- Added `From` implementations between `IndexedEntry` and `OccupiedEntry`.
+
 ## 0.5.2
 
 - Added methods `OrderMap::append` and `OrderSet::append`, moving all items from

src/map.rs

@@ -393,7 +393,7 @@ where
     ///
     /// This is equivalent to finding the position with
     /// [`binary_search_keys`][Self::binary_search_keys], then either updating
-    /// it or calling [`shift_insert`][Self::shift_insert] for a new key.
+    /// it or calling [`insert_before`][Self::insert_before] for a new key.
     ///
     /// If the sorted key is found in the map, its corresponding value is
     /// updated with `value`, and the older value is returned inside
@@ -415,21 +415,115 @@ where
         self.inner.insert_sorted(key, value)
     }
 
-    /// Insert a key-value pair in the map at the given index.
+    /// Insert a key-value pair in the map before the entry at the given index, or at the end.
     ///
     /// If an equivalent key already exists in the map: the key remains and
     /// is moved to the new position in the map, its corresponding value is updated
+    /// with `value`, and the older value is returned inside `Some(_)`. The returned index
+    /// will either be the given index or one less, depending on how the entry moved.
+    /// (See [`shift_insert`](Self::shift_insert) for different behavior here.)
+    ///
+    /// If no equivalent key existed in the map: the new key-value pair is
+    /// inserted exactly at the given index, and `None` is returned.
+    ///
+    /// ***Panics*** if `index` is out of bounds.
+    /// Valid indices are `0..=map.len()` (inclusive).
+    ///
+    /// Computes in **O(n)** time (average).
+    ///
+    /// See also [`entry`][Self::entry] if you want to insert *or* modify,
+    /// perhaps only using the index for new entries with [`VacantEntry::shift_insert`].
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use ordermap::OrderMap;
+    /// let mut map: OrderMap<char, ()> = ('a'..='z').map(|c| (c, ())).collect();
+    ///
+    /// // The new key '*' goes exactly at the given index.
+    /// assert_eq!(map.get_index_of(&'*'), None);
+    /// assert_eq!(map.insert_before(10, '*', ()), (10, None));
+    /// assert_eq!(map.get_index_of(&'*'), Some(10));
+    ///
+    /// // Moving the key 'a' up will shift others down, so this moves *before* 10 to index 9.
+    /// assert_eq!(map.insert_before(10, 'a', ()), (9, Some(())));
+    /// assert_eq!(map.get_index_of(&'a'), Some(9));
+    /// assert_eq!(map.get_index_of(&'*'), Some(10));
+    ///
+    /// // Moving the key 'z' down will shift others up, so this moves to exactly 10.
+    /// assert_eq!(map.insert_before(10, 'z', ()), (10, Some(())));
+    /// assert_eq!(map.get_index_of(&'z'), Some(10));
+    /// assert_eq!(map.get_index_of(&'*'), Some(11));
+    ///
+    /// // Moving or inserting before the endpoint is also valid.
+    /// assert_eq!(map.len(), 27);
+    /// assert_eq!(map.insert_before(map.len(), '*', ()), (26, Some(())));
+    /// assert_eq!(map.get_index_of(&'*'), Some(26));
+    /// assert_eq!(map.insert_before(map.len(), '+', ()), (27, None));
+    /// assert_eq!(map.get_index_of(&'+'), Some(27));
+    /// assert_eq!(map.len(), 28);
+    /// ```
+    pub fn insert_before(&mut self, index: usize, key: K, value: V) -> (usize, Option<V>) {
+        self.inner.insert_before(index, key, value)
+    }
+
+    /// Insert a key-value pair in the map at the given index.
+    ///
+    /// If an equivalent key already exists in the map: the key remains and
+    /// is moved to the given index in the map, its corresponding value is updated
     /// with `value`, and the older value is returned inside `Some(_)`.
+    /// Note that existing entries **cannot** be moved to `index == map.len()`!
+    /// (See [`insert_before`](Self::insert_before) for different behavior here.)
     ///
     /// If no equivalent key existed in the map: the new key-value pair is
     /// inserted at the given index, and `None` is returned.
     ///
     /// ***Panics*** if `index` is out of bounds.
+    /// Valid indices are `0..map.len()` (exclusive) when moving an existing entry, or
+    /// `0..=map.len()` (inclusive) when inserting a new key.
     ///
     /// Computes in **O(n)** time (average).
     ///
     /// See also [`entry`][Self::entry] if you want to insert *or* modify,
     /// perhaps only using the index for new entries with [`VacantEntry::shift_insert`].
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use ordermap::OrderMap;
+    /// let mut map: OrderMap<char, ()> = ('a'..='z').map(|c| (c, ())).collect();
+    ///
+    /// // The new key '*' goes exactly at the given index.
+    /// assert_eq!(map.get_index_of(&'*'), None);
+    /// assert_eq!(map.shift_insert(10, '*', ()), None);
+    /// assert_eq!(map.get_index_of(&'*'), Some(10));
+    ///
+    /// // Moving the key 'a' up to 10 will shift others down, including the '*' that was at 10.
+    /// assert_eq!(map.shift_insert(10, 'a', ()), Some(()));
+    /// assert_eq!(map.get_index_of(&'a'), Some(10));
+    /// assert_eq!(map.get_index_of(&'*'), Some(9));
+    ///
+    /// // Moving the key 'z' down to 9 will shift others up, including the '*' that was at 9.
+    /// assert_eq!(map.shift_insert(9, 'z', ()), Some(()));
+    /// assert_eq!(map.get_index_of(&'z'), Some(9));
+    /// assert_eq!(map.get_index_of(&'*'), Some(10));
+    ///
+    /// // Existing keys can move to len-1 at most, but new keys can insert at the endpoint.
+    /// assert_eq!(map.len(), 27);
+    /// assert_eq!(map.shift_insert(map.len() - 1, '*', ()), Some(()));
+    /// assert_eq!(map.get_index_of(&'*'), Some(26));
+    /// assert_eq!(map.shift_insert(map.len(), '+', ()), None);
+    /// assert_eq!(map.get_index_of(&'+'), Some(27));
+    /// assert_eq!(map.len(), 28);
+    /// ```
+    ///
+    /// ```should_panic
+    /// use ordermap::OrderMap;
+    /// let mut map: OrderMap<char, ()> = ('a'..='z').map(|c| (c, ())).collect();
+    ///
+    /// // This is an invalid index for moving an existing key!
+    /// map.shift_insert(map.len(), 'a', ());
+    /// ```
     pub fn shift_insert(&mut self, index: usize, key: K, value: V) -> Option<V> {
         self.inner.shift_insert(index, key, value)
     }
@@ -687,6 +781,7 @@ impl<K, V, S> OrderMap<K, V, S> {
     /// This preserves the order of the remaining elements.
     ///
     /// Computes in **O(1)** time (average).
+    #[doc(alias = "pop_last")] // like `BTreeMap`
     pub fn pop(&mut self) -> Option<(K, V)> {
         self.inner.pop()
     }
@@ -932,6 +1027,7 @@ impl<K, V, S> OrderMap<K, V, S> {
     /// Get the first key-value pair
     ///
     /// Computes in **O(1)** time.
+    #[doc(alias = "first_key_value")] // like `BTreeMap`
     pub fn first(&self) -> Option<(&K, &V)> {
         self.inner.first()
     }
@@ -943,9 +1039,17 @@ impl<K, V, S> OrderMap<K, V, S> {
         self.inner.first_mut()
     }
 
+    /// Get the first entry in the map for in-place manipulation.
+    ///
+    /// Computes in **O(1)** time.
+    pub fn first_entry(&mut self) -> Option<IndexedEntry<'_, K, V>> {
+        self.inner.first_entry().map(IndexedEntry::new)
+    }
+
     /// Get the last key-value pair
     ///
     /// Computes in **O(1)** time.
+    #[doc(alias = "last_key_value")] // like `BTreeMap`
     pub fn last(&self) -> Option<(&K, &V)> {
         self.inner.last()
     }
@@ -957,6 +1061,13 @@ impl<K, V, S> OrderMap<K, V, S> {
         self.inner.last_mut()
     }
 
+    /// Get the last entry in the map for in-place manipulation.
+    ///
+    /// Computes in **O(1)** time.
+    pub fn last_entry(&mut self) -> Option<IndexedEntry<'_, K, V>> {
+        self.inner.last_entry().map(IndexedEntry::new)
+    }
+
     /// Remove the key-value pair by index
     ///
     /// Valid indices are *0 <= index < self.len()*

src/map/entry.rs

@@ -250,6 +250,14 @@ impl<K: fmt::Debug, V: fmt::Debug> fmt::Debug for OccupiedEntry<'_, K, V> {
     }
 }
 
+impl<'a, K, V> From<IndexedEntry<'a, K, V>> for OccupiedEntry<'a, K, V> {
+    fn from(entry: IndexedEntry<'a, K, V>) -> Self {
+        Self {
+            inner: entry.inner.into(),
+        }
+    }
+}
+
 /// A view into a vacant entry in an [`OrderMap`][crate::OrderMap].
 /// It is part of the [`Entry`] enum.
 pub struct VacantEntry<'a, K, V> {
@@ -444,3 +452,11 @@ impl<K: fmt::Debug, V: fmt::Debug> fmt::Debug for IndexedEntry<'_, K, V> {
             .finish()
     }
 }
+
+impl<'a, K, V> From<OccupiedEntry<'a, K, V>> for IndexedEntry<'a, K, V> {
+    fn from(entry: OccupiedEntry<'a, K, V>) -> Self {
+        Self {
+            inner: entry.inner.into(),
+        }
+    }
+}

src/map/mutable.rs

@@ -99,6 +99,9 @@ where
 /// This trait is sealed and cannot be implemented for types outside this crate.
 pub trait MutableEntryKey: private::Sealed {
     type Key;
+
+    /// Gets a mutable reference to the entry's key, either within the map if occupied,
+    /// or else the new key that was used to find the entry.
     fn key_mut(&mut self) -> &mut Self::Key;
 }
 
@@ -107,9 +110,6 @@ pub trait MutableEntryKey: private::Sealed {
 /// See [`MutableEntryKey`] for more information.
 impl<K, V> MutableEntryKey for Entry<'_, K, V> {
     type Key = K;
-
-    /// Gets a mutable reference to the entry's key, either within the map if occupied,
-    /// or else the new key that was used to find the entry.
     fn key_mut(&mut self) -> &mut Self::Key {
         match self {
             Entry::Occupied(e) => e.key_mut(),

src/map/tests.rs

@@ -136,6 +136,34 @@ fn shift_insert() {
     }
 }
 
+#[test]
+fn insert_sorted_bad() {
+    let mut map = OrderMap::new();
+    map.insert(10, ());
+    for i in 0..10 {
+        map.insert(i, ());
+    }
+
+    // The binary search will want to insert this at the end (index == len()),
+    // but that's only possible for *new* inserts. It should still be handled
+    // without panicking though, and in this case it's simple enough that we
+    // know the exact result. (But don't read this as an API guarantee!)
+    assert_eq!(map.first(), Some((&10, &())));
+    map.insert_sorted(10, ());
+    assert_eq!(map.last(), Some((&10, &())));
+    assert!(map.keys().copied().eq(0..=10));
+
+    // Other out-of-order entries can also "insert" to a binary-searched
+    // position, moving in either direction.
+    map.move_index(5, 0);
+    map.move_index(6, 10);
+    assert_eq!(map.first(), Some((&5, &())));
+    assert_eq!(map.last(), Some((&6, &())));
+    map.insert_sorted(5, ()); // moves back up
+    map.insert_sorted(6, ()); // moves back down
+    assert!(map.keys().copied().eq(0..=10));
+}
+
 #[test]
 fn grow() {
     let insert = [0, 4, 2, 12, 8, 7, 11];
@@ -388,6 +416,8 @@ fn get_index_entry() {
     let mut map = OrderMap::new();
 
     assert!(map.get_index_entry(0).is_none());
+    assert!(map.first_entry().is_none());
+    assert!(map.last_entry().is_none());
 
     map.insert(0, "0");
     map.insert(1, "1");
@@ -411,6 +441,43 @@ fn get_index_entry() {
     }
 
     assert_eq!(*map.get(&3).unwrap(), "4");
+
+    {
+        let e = map.first_entry().unwrap();
+        assert_eq!(*e.key(), 0);
+        assert_eq!(*e.get(), "0");
+    }
+
+    {
+        let e = map.last_entry().unwrap();
+        assert_eq!(*e.key(), 2);
+        assert_eq!(*e.get(), "2");
+    }
+}
+
+#[test]
+fn from_entries() {
+    let mut map = OrderMap::from([(1, "1"), (2, "2"), (3, "3")]);
+
+    {
+        let e = match map.entry(1) {
+            Entry::Occupied(e) => IndexedEntry::from(e),
+            Entry::Vacant(_) => panic!(),
+        };
+        assert_eq!(e.index(), 0);
+        assert_eq!(*e.key(), 1);
+        assert_eq!(*e.get(), "1");
+    }
+
+    {
+        let e = match map.get_index_entry(1) {
+            Some(e) => OccupiedEntry::from(e),
+            None => panic!(),
+        };
+        assert_eq!(e.index(), 1);
+        assert_eq!(*e.key(), 2);
+        assert_eq!(*e.get(), "2");
+    }
 }
 
 #[test]