Add diff applying error & improve CI (#32)

* Use stable rust

* Add From impls

* Revert to nightly

* Improve dev env & CI setup

* Update lock

* Add thiserror

* Add diff error

* Fix tests

* Lint

* Rename NumberOrString

* Format

* Fix lint script

.github/workflows/check.yml

@@ -37,20 +37,8 @@ jobs:
           restore-keys: |
             ${{ runner.os }}-cargo-
 
-      - name: Setup rust
-        run: |
-          which wasm-pack || cargo install wasm-pack
-          which cargo-machete || cargo install cargo-machete
-
-      - name: Build wasm
-        run: |
-          wasm-pack build --target web --features wasm
-
       - name: Lint
-        run: |
-          cargo clippy --all-targets --all-features
-          cargo fmt --all -- --check
-          cargo machete
+        run: scripts/lint.sh
 
       - name: Test
         run: scripts/test.sh
@@ -117,19 +105,8 @@ jobs:
           restore-keys: |
             ${{ runner.os }}-npm-
 
-      - name: Setup rust
-        run: |
-          which wasm-pack || cargo install wasm-pack
-
-      - name: Build wasm
-        run: |
-          wasm-pack build --target web --features wasm
-
-      - name: Build reconcile-js
-        run: |
-          cd reconcile-js
-          npm ci
-          npm run build
+      - name: Build website
+        run: scripts/build-website.sh
 
       - name: Publish reconcile-js to NPM
         run: |

Cargo.lock

@@ -245,6 +245,7 @@ dependencies = [
  "serde",
  "serde_yaml",
  "test-case",
+ "thiserror",
  "wasm-bindgen",
  "wasm-bindgen-test",
  "wee_alloc",
@@ -383,6 +384,26 @@ dependencies = [
  "test-case-core",
 ]
 
+[[package]]
+name = "thiserror"
+version = "2.0.17"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "f63587ca0f12b72a0600bcba1d40081f830876000bb46dd2337a3051618f4fc8"
+dependencies = [
+ "thiserror-impl",
+]
+
+[[package]]
+name = "thiserror-impl"
+version = "2.0.17"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "3ff15c8ecd7de3849db632e14d18d2571fa09dfc5ed93479bc4485c7a517c913"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "syn",
+]
+
 [[package]]
 name = "unicode-ident"
 version = "1.0.22"

Cargo.toml

@@ -22,6 +22,7 @@ path = "examples/merge-file.rs"
 
 [dependencies]
 serde = { version = "1.0.219", optional = true, features = ["derive"] }
+thiserror = "2.0.17"
 
 wasm-bindgen = { version = "0.2.99", optional = true }
 

README.md

@@ -125,14 +125,10 @@ Contributions are welcome!
 
 #### Rust toolchain
 
-1. Install [rustup](https://rustup.rs):
+Install [rustup](https://rustup.rs):
    ```bash
    curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh
    ```
-2. Install additional tools:
-   ```bash
-   cargo install wasm-pack cargo-insta cargo-edit
-   ```
 
 ### Scripts
 

scripts/build-website.sh

@@ -2,7 +2,9 @@
 
 set -e
 
+which wasm-pack || cargo install wasm-pack
 wasm-pack build --target web --features wasm
+
 cd reconcile-js
 npm ci
 npm run build

scripts/bump-version.sh

@@ -25,8 +25,12 @@ else
 fi
 
 echo "Bumping versions"
+
+which cargo-set-version || cargo install cargo-edit
 cargo set-version --bump $1
 
+which wasm-pack || cargo install wasm-pack
+
 wasm-pack build --target web --features wasm
 
 cd reconcile-js

scripts/lint.sh

@@ -2,13 +2,18 @@
 
 set -e
 
+which cargo-machete || cargo install cargo-machete
+cargo machete
+
 cargo clippy --all-targets --all-features --fix --allow-dirty --allow-staged
 cargo fmt --all
 
 cd reconcile-js
+npm ci
 npm run format
 
 cd ../examples/website
+npm ci
 npm run format
 
 echo "Success!"

scripts/test.sh

@@ -2,6 +2,15 @@
 
 set -e
 
+which cargo-insta || cargo install cargo-insta
+which wasm-pack || cargo install wasm-pack
+
+node_version=$(node --version | cut -d'.' -f1 | tr -d 'v')
+if [ "$node_version" != "22" ]; then
+    echo "Error: Node.js version 22 is required, but found version $node_version"
+    exit 1
+fi
+
 wasm-pack build --target web --features wasm,console_error_panic_hook
 cargo test --verbose --features serde -- --include-ignored 
 
@@ -13,7 +22,8 @@ cargo test --features all
 wasm-pack test --node --features wasm,console_error_panic_hook
 
 cd reconcile-js
-npm install
+npm ci
+npm run build
 npm run test
 cd -
 

src/lib.rs

@@ -187,7 +187,7 @@
 //!     original,
 //!     deserialized,
 //!     &*BuiltinTokenizer::Word
-//! );
+//! ).unwrap();
 //! assert_eq!(
 //!     reconstructed.apply().text(),
 //!     "Merging text is easy with reconcile!"
@@ -215,11 +215,11 @@ mod tokenizer;
 mod types;
 mod utils;
 
-pub use operation_transformation::{EditedText, reconcile};
+pub use operation_transformation::{DiffError, EditedText, reconcile};
 pub use tokenizer::{BuiltinTokenizer, Tokenizer, token::Token};
 pub use types::{
-    cursor_position::CursorPosition, history::History, number_or_string::NumberOrString,
-    side::Side, span_with_history::SpanWithHistory, text_with_cursors::TextWithCursors,
+    cursor_position::CursorPosition, history::History, number_or_text::NumberOrText, side::Side,
+    span_with_history::SpanWithHistory, text_with_cursors::TextWithCursors,
 };
 
 #[cfg(feature = "wasm")]

src/operation_transformation.rs

@@ -1,8 +1,10 @@
+mod diff_error;
 mod edited_text;
 mod operation;
 mod utils;
 use std::fmt::Debug;
 
+pub use diff_error::DiffError;
 pub use edited_text::EditedText;
 pub use operation::Operation;
 

src/operation_transformation/diff_error.rs

@@ -0,0 +1,19 @@
+use thiserror::Error;
+
+/// Error type for invalid diff operations
+#[derive(Error, Debug, Clone, PartialEq)]
+pub enum DiffError {
+    /// The diff references a range that exceeds the original text length
+    #[error(
+        "Invalid diff: attempting to access {requested} characters starting at position \
+         {position}, but original text only has {available} characters remaining"
+    )]
+    LengthExceedsOriginal {
+        /// The position where the operation starts
+        position: usize,
+        /// The number of characters requested
+        requested: usize,
+        /// The number of characters available from the position
+        available: usize,
+    },
+}

src/operation_transformation/edited_text.rs

@@ -6,13 +6,13 @@ use serde::{Deserialize, Serialize};
 use crate::{
     BuiltinTokenizer, CursorPosition, TextWithCursors,
     operation_transformation::{
-        Operation,
+        DiffError, Operation,
         utils::{cook_operations::cook_operations, elongate_operations::elongate_operations},
     },
     raw_operation::RawOperation,
     tokenizer::Tokenizer,
     types::{
-        history::History, number_or_string::NumberOrString, side::Side,
+        history::History, number_or_text::NumberOrText, side::Side,
         span_with_history::SpanWithHistory,
     },
     utils::string_builder::StringBuilder,
@@ -366,8 +366,8 @@ where
     ///
     /// Panics if there's an integer overflow in i64.
     #[must_use]
-    pub fn to_diff(&self) -> Vec<NumberOrString> {
-        let mut result: Vec<NumberOrString> = Vec::with_capacity(self.operations.len());
+    pub fn to_diff(&self) -> Vec<NumberOrText> {
+        let mut result: Vec<NumberOrText> = Vec::with_capacity(self.operations.len());
         let mut previous_equal: Option<usize> = None;
 
         for operation in &self.operations {
@@ -382,7 +382,7 @@ where
 
                 Operation::Insert { text, .. } => {
                     if let Some(prev_length) = previous_equal {
-                        result.push(NumberOrString::Number(
+                        result.push(NumberOrText::Number(
                             i64::try_from(prev_length).expect("prev_length must fit in i64"),
                         ));
                         previous_equal = None;
@@ -392,7 +392,7 @@ where
                         .iter()
                         .map(super::super::tokenizer::token::Token::original)
                         .collect();
-                    result.push(NumberOrString::Text(text));
+                    result.push(NumberOrText::Text(text));
                 }
 
                 Operation::Delete {
@@ -400,7 +400,7 @@ where
                     ..
                 } => {
                     if let Some(prev_length) = previous_equal {
-                        result.push(NumberOrString::Number(
+                        result.push(NumberOrText::Number(
                             i64::try_from(prev_length).expect("prev_length must fit in i64"),
                         ));
                         previous_equal = None;
@@ -408,13 +408,13 @@ where
 
                     let count = i64::try_from(*deleted_character_count)
                         .expect("deleted_character_count must fit in i64");
-                    result.push(NumberOrString::Number(-count));
+                    result.push(NumberOrText::Number(-count));
                 }
             }
         }
 
         if let Some(prev_length) = previous_equal {
-            result.push(NumberOrString::Number(
+            result.push(NumberOrText::Number(
                 i64::try_from(prev_length).expect("prev_length must fit in i64"),
             ));
         }
@@ -424,23 +424,38 @@ where
 
     /// Deserialize an `EditedText` from a change list and the original text.
     ///
+    /// # Errors
+    ///
+    /// Returns `DiffError::LengthExceedsOriginal` if the diff references a
+    /// range that exceeds the original text length.
+    ///
     /// # Panics
     ///
     /// Panics if there's an integer overflow in i64.
-    #[must_use]
     pub fn from_diff(
         original_text: &'a str,
-        diff: Vec<NumberOrString>,
+        diff: Vec<NumberOrText>,
         tokenizer: &Tokenizer<T>,
-    ) -> EditedText<'a, T> {
+    ) -> Result<EditedText<'a, T>, DiffError> {
         let mut operations: Vec<Operation<T>> = Vec::with_capacity(diff.len());
         let mut order = 0;
 
         for item in diff {
             match item {
-                NumberOrString::Number(length) => {
+                NumberOrText::Number(length) => {
                     if length >= 0 {
                         let length = usize::try_from(length).expect("length must fit in usize");
+
+                        // Validate that the range doesn't exceed the original text
+                        let text_length = original_text.chars().count();
+                        if order + length > text_length {
+                            return Err(DiffError::LengthExceedsOriginal {
+                                position: order,
+                                requested: length,
+                                available: text_length.saturating_sub(order),
+                            });
+                        }
+
                         let original_characters: String =
                             original_text.chars().skip(order).take(length).collect();
 
@@ -453,11 +468,22 @@ where
                     } else {
                         let length =
                             usize::try_from(-length).expect("negative length must fit in usize");
+
+                        // Validate that the delete range doesn't exceed the original text
+                        let text_length = original_text.chars().count();
+                        if order + length > text_length {
+                            return Err(DiffError::LengthExceedsOriginal {
+                                position: order,
+                                requested: length,
+                                available: text_length.saturating_sub(order),
+                            });
+                        }
+
                         operations.push(Operation::create_delete(order, length));
                         order += length;
                     }
                 }
-                NumberOrString::Text(text) => {
+                NumberOrText::Text(text) => {
                     let tokens = tokenizer(&text);
                     operations.push(Operation::create_insert(order, tokens));
                 }
@@ -465,12 +491,12 @@ where
         }
 
         let operation_count = operations.len();
-        EditedText::new(
+        Ok(EditedText::new(
             original_text,
             operations,
             vec![Side::Left; operation_count],
             vec![],
-        )
+        ))
     }
 }
 
@@ -520,6 +546,49 @@ mod tests {
         assert_eq!(operations.apply().text(), expected);
     }
 
+    #[test]
+    fn test_from_diff_length_exceeds_original() {
+        let result = EditedText::from_diff(
+            "hello",
+            vec![
+                10.into(), // too large equal span - should error
+                " world".into(),
+            ],
+            &*BuiltinTokenizer::Word,
+        );
+
+        assert!(result.is_err());
+        match result {
+            Err(DiffError::LengthExceedsOriginal {
+                position,
+                requested,
+                available,
+            }) => {
+                assert_eq!(position, 0);
+                assert_eq!(requested, 10);
+                assert_eq!(available, 5);
+            }
+            _ => panic!("Expected LengthExceedsOriginal error"),
+        }
+    }
+
+    #[test]
+    fn test_from_diff_valid() {
+        let edited_text = EditedText::from_diff(
+            "hello",
+            vec![
+                5.into(), // exact length
+                " world".into(),
+            ],
+            &*BuiltinTokenizer::Word,
+        )
+        .unwrap();
+
+        let content = edited_text.apply().text();
+
+        assert_eq!(content, "hello world");
+    }
+
     #[cfg(feature = "serde")]
     #[test]
     fn test_changes_deserialisation() {
@@ -542,7 +611,7 @@ mod tests {
 
         let changes = edited_text.to_diff();
         let deserialized_edited_text =
-            EditedText::from_diff(original, changes, &*BuiltinTokenizer::Word);
+            EditedText::from_diff(original, changes, &*BuiltinTokenizer::Word).unwrap();
 
         assert_eq!(deserialized_edited_text.apply().text(), updated);
     }

src/types.rs

@@ -1,6 +1,6 @@
 pub mod cursor_position;
 pub mod history;
-pub mod number_or_string;
+pub mod number_or_text;
 pub mod side;
 pub mod span_with_history;
 pub mod text_with_cursors;

src/types/number_or_text.rs

@@ -1,4 +1,4 @@
-use std::fmt::Debug;
+use std::{borrow::Cow, fmt::Debug};
 
 #[cfg(feature = "serde")]
 use serde::{Deserialize, Serialize};
@@ -12,18 +12,18 @@ const INTEGRAL_LIMIT: f64 = (1u64 << 53) as f64;
 #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
 #[cfg_attr(feature = "serde", serde(untagged))]
 #[derive(Debug, Clone, PartialEq)]
-pub enum NumberOrString {
+pub enum NumberOrText {
     Number(i64),
     Text(String),
 }
 
 #[cfg(feature = "wasm")]
-impl TryFrom<JsValue> for NumberOrString {
+impl TryFrom<JsValue> for NumberOrText {
     type Error = DeserialisationError;
 
     fn try_from(value: JsValue) -> Result<Self, Self::Error> {
         if let Ok(num) = value.clone().try_into() {
-            return Ok(NumberOrString::Number(num));
+            return Ok(NumberOrText::Number(num));
         }
 
         if let Some(num) = value.clone().as_f64() {
@@ -34,11 +34,11 @@ impl TryFrom<JsValue> for NumberOrString {
             }
 
             #[allow(clippy::cast_possible_truncation)]
-            return Ok(NumberOrString::Number(num.round() as i64));
+            return Ok(NumberOrText::Number(num.round() as i64));
         }
 
         if let Ok(text) = value.try_into() {
-            return Ok(NumberOrString::Text(text));
+            return Ok(NumberOrText::Text(text));
         }
 
         Err(DeserialisationError::new(
@@ -48,15 +48,31 @@ impl TryFrom<JsValue> for NumberOrString {
 }
 
 #[cfg(feature = "wasm")]
-impl From<NumberOrString> for JsValue {
-    fn from(value: NumberOrString) -> Self {
+impl From<NumberOrText> for JsValue {
+    fn from(value: NumberOrText) -> Self {
         match value {
-            NumberOrString::Number(num) => JsValue::from(num),
-            NumberOrString::Text(text) => JsValue::from(text),
+            NumberOrText::Number(num) => JsValue::from(num),
+            NumberOrText::Text(text) => JsValue::from(text),
         }
     }
 }
 
+impl From<i64> for NumberOrText {
+    fn from(value: i64) -> Self { NumberOrText::Number(value) }
+}
+
+impl From<String> for NumberOrText {
+    fn from(value: String) -> Self { NumberOrText::Text(value) }
+}
+
+impl From<&str> for NumberOrText {
+    fn from(value: &str) -> Self { NumberOrText::Text(value.to_owned()) }
+}
+
+impl<'a> From<Cow<'a, str>> for NumberOrText {
+    fn from(value: Cow<'a, str>) -> Self { NumberOrText::Text(value.into_owned()) }
+}
+
 /// Error type for deserialisation failures
 #[cfg(feature = "wasm")]
 #[derive(Debug, Clone)]

src/wasm.rs

@@ -105,16 +105,17 @@ pub fn diff(parent: &str, changed: &TextWithCursors, tokenizer: BuiltinTokenizer
 pub fn undiff(parent: &str, diff: Vec<JsValue>, tokenizer: BuiltinTokenizer) -> String {
     set_panic_hook();
 
-    EditedText::from_diff(
+    match EditedText::from_diff(
         parent,
         diff.into_iter()
             .map(std::convert::TryInto::try_into)
             .collect::<Result<_, _>>()
             .expect("Invalid diff format"),
         &*tokenizer,
-    )
-    .apply()
-    .text()
+    ) {
+        Ok(edited_text) => edited_text.apply().text(),
+        Err(e) => panic!("{}", e),
+    }
 }
 
 fn set_panic_hook() {

tests/test.rs

@@ -57,9 +57,9 @@ fn test_document_one_way_with_serialisation() {
                 .unwrap();
 
         let restored_left_operations =
-            EditedText::from_diff(&parent, serialised_left, &*BuiltinTokenizer::Word);
+            EditedText::from_diff(&parent, serialised_left, &*BuiltinTokenizer::Word).unwrap();
         let restored_right_operations =
-            EditedText::from_diff(&parent, serialised_right, &*BuiltinTokenizer::Word);
+            EditedText::from_diff(&parent, serialised_right, &*BuiltinTokenizer::Word).unwrap();
 
         doc.assert_eq_without_cursors(
             &restored_left_operations