Skip to content

Commit

Permalink
Removes dependency on ion-c-sys
Browse files Browse the repository at this point in the history 
This PR removes all code that was previously behind the `ion_c` feature
gate. Many of the native implementation APIs were developed with an eye
towards maintaining compatability with the `ion-c-sys` implementation.
However, this limited our ability to offer idiomatic APIs.

`ion-hash` continues to have a dependency on `ion-c-sys` for the time being,
but has been modified to depend on to the most recently published version of
`ion-rust` that supported it. A follow-on PR will update `ion-hash` to
depend on the native implementations instead.
  • Loading branch information
Zack Slayton committed Sep 16, 2022
1 parent dcf22f5 commit bafce2e
Show file tree
Hide file tree
Showing 12 changed files with 3 additions and 1,227 deletions.
8 changes: 0 additions & 8 deletions Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,6 @@ members = [
"ion-hash"
]

[features]
ion_c = ["dep:ion-c-sys"]

[dependencies]
base64 = "0.12"
bigdecimal = "0.2"
Expand All @@ -42,11 +39,6 @@ num-integer = "0.1.44"
num-traits = "0.2"
arrayvec = "0.7"

# NB: We use the tree dependency here for development and CI.
# Note that when publishing you should update the version
# so that users can get the correct underlying ion-c-sys version.
ion-c-sys = { path = "./ion-c-sys", version = "0.4", optional = true }

[dev-dependencies]
rstest = "0.9"
# Used by ion-tests integration
Expand Down
4 changes: 2 additions & 2 deletions ion-hash/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ version = "0.0.9"
edition = "2021"

[dependencies]
ion-rs = { path = "../", version = "0.13", features = ["ion_c"]}
ion-c-sys = { path = "../ion-c-sys", version = "0.4" }
ion-rs = {version = "0.13", features = ["ion_c"]}
ion-c-sys = "0.4"
num-bigint = "0.3"
digest = "0.9"
sha2 = "0.9"
14 changes: 0 additions & 14 deletions src/lib.rs
View file
Original file line number Diff line number Diff line change
@@ -1,20 +1,6 @@
#![allow(dead_code)]
#![deny(rustdoc::broken_intra_doc_links)]

#[cfg(feature = "ion_c")]
/// A [`try`]-like macro to workaround the [`Option`]/[`Result`] nested APIs.
/// These API require checking the type and then calling the appropriate getter function
/// (which returns a None if you got it wrong). This macro turns the `None` into
/// an `IonError` which cannot be currently done with `?`.
macro_rules! try_to {
($getter:expr) => {
match $getter {
Some(value) => value,
None => illegal_operation(format!("Missing a value: {}", stringify!($getter)))?,
}
};
}

pub mod result;

pub mod binary;
Expand Down
49 changes: 1 addition & 48 deletions src/result.rs
View file
Original file line number Diff line number Diff line change
@@ -1,31 +1,12 @@
use std::convert::From;
use std::fmt::{Debug, Display, Error, Formatter};
use std::fmt::{Debug, Error};
use std::{fmt, io};

use thiserror::Error;

/// A unified Result type representing the outcome of method calls that may fail.
pub type IonResult<T> = Result<T, IonError>;

// If the ion-c-sys feature is enabled, use the actual IonCError type as our IonCErrorSource...
#[cfg(feature = "ion_c")]
pub type IonCErrorSource = ion_c_sys::result::IonCError;
// ...otherwise, use a placeholder error type.
#[cfg(not(feature = "ion_c"))]
pub type IonCErrorSource = ErrorStub;

/// Placeholder Error type for error variants that require conditional compilation.
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
pub struct ErrorStub;

impl Display for ErrorStub {
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
write!(f, "ion-c error support was not enabled at build time")
}
}

impl std::error::Error for ErrorStub {}

/// Represents the different types of high-level failures that might occur when reading Ion data.
#[derive(Debug, Error)]
pub enum IonError {
Expand Down Expand Up @@ -56,13 +37,6 @@ pub enum IonError {
"The user has performed an operation that is not legal in the current state: {operation}"
)]
IllegalOperation { operation: String },

/// Indicates that the underlying failure is due to a problem in [`ion_c_sys`].
#[error("{source:?}")]
IonCError {
#[from]
source: IonCErrorSource,
},
}

impl From<fmt::Error> for IonError {
Expand Down Expand Up @@ -98,7 +72,6 @@ impl Clone for IonError {
IllegalOperation { operation } => IllegalOperation {
operation: operation.clone(),
},
IonCError { source } => IonCError { source: *source },
}
}
}
Expand All @@ -125,7 +98,6 @@ impl PartialEq for IonError {
(EncodingError { description: s1 }, EncodingError { description: s2 }) => s1 == s2,
(DecodingError { description: s1 }, DecodingError { description: s2 }) => s1 == s2,
(IllegalOperation { operation: s1 }, IllegalOperation { operation: s2 }) => s1 == s2,
(IonCError { source: s1 }, IonCError { source: s2 }) => s1 == s2,
_ => false,
}
}
Expand Down Expand Up @@ -168,22 +140,3 @@ pub fn illegal_operation_raw<S: AsRef<str>>(operation: S) -> IonError {
operation: operation.as_ref().to_string(),
}
}

#[cfg(all(test, feature = "ion_c"))]
mod test {
use ion_c_sys::result::*;
use ion_c_sys::{ion_error_code_IERR_EOF, ion_error_code_IERR_INVALID_ARG};

use super::*;

#[test]
fn ion_c_error_eq() {
// make sure we can actually convert an Ion C error
let e1: IonError = IonCError::from(ion_error_code_IERR_EOF).into();
let e2: IonError = IonCError::from(ion_error_code_IERR_INVALID_ARG).into();

// make sure eq/clone works
assert_eq!(e1, e1.clone());
assert_ne!(e1, e2);
}
}
98 changes: 0 additions & 98 deletions src/types/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,6 @@ pub mod integer;
pub mod magnitude;
pub mod timestamp;

#[cfg(feature = "ion_c")]
use {
crate::result::{illegal_operation, IonError},
ion_c_sys::ION_TYPE,
std::convert::TryFrom,
};

use std::fmt;

/// Represents the Ion data type of a given value. To learn more about each data type,
Expand Down Expand Up @@ -105,94 +98,3 @@ fn num_decimal_digits_in_u64(value: u64) -> u64 {
count as u64
}
}

#[cfg(feature = "ion_c")]
impl TryFrom<ION_TYPE> for IonType {
type Error = IonError;

fn try_from(value: ION_TYPE) -> Result<Self, Self::Error> {
use IonType::*;
match value {
ion_c_sys::ION_TYPE_NULL => Ok(Null),
ion_c_sys::ION_TYPE_BOOL => Ok(Boolean),
ion_c_sys::ION_TYPE_INT => Ok(Integer),
ion_c_sys::ION_TYPE_FLOAT => Ok(Float),
ion_c_sys::ION_TYPE_DECIMAL => Ok(Decimal),
ion_c_sys::ION_TYPE_TIMESTAMP => Ok(Timestamp),
ion_c_sys::ION_TYPE_SYMBOL => Ok(Symbol),
ion_c_sys::ION_TYPE_STRING => Ok(String),
ion_c_sys::ION_TYPE_CLOB => Ok(Clob),
ion_c_sys::ION_TYPE_BLOB => Ok(Blob),
ion_c_sys::ION_TYPE_LIST => Ok(List),
ion_c_sys::ION_TYPE_SEXP => Ok(SExpression),
ion_c_sys::ION_TYPE_STRUCT => Ok(Struct),
_ => illegal_operation(format!("Could not convert Ion C ION_TYPE: ${:?}", value)),
}
}
}

#[cfg(feature = "ion_c")]
impl From<IonType> for ION_TYPE {
fn from(ion_type: IonType) -> ION_TYPE {
use IonType::*;
match ion_type {
Null => ion_c_sys::ION_TYPE_NULL,
Boolean => ion_c_sys::ION_TYPE_BOOL,
Integer => ion_c_sys::ION_TYPE_INT,
Float => ion_c_sys::ION_TYPE_FLOAT,
Decimal => ion_c_sys::ION_TYPE_DECIMAL,
Timestamp => ion_c_sys::ION_TYPE_TIMESTAMP,
Symbol => ion_c_sys::ION_TYPE_SYMBOL,
String => ion_c_sys::ION_TYPE_STRING,
Clob => ion_c_sys::ION_TYPE_CLOB,
Blob => ion_c_sys::ION_TYPE_BLOB,
List => ion_c_sys::ION_TYPE_LIST,
SExpression => ion_c_sys::ION_TYPE_SEXP,
Struct => ion_c_sys::ION_TYPE_STRUCT,
}
}
}

#[cfg(all(test, feature = "ion_c"))]
mod type_test {
use super::*;
use crate::result::IonResult;
use ion_c_sys::reader::*;
use rstest::*;
use std::convert::TryInto;
use IonType::*;

// XXX these tests are a little more complex than we'd normally like, but it at least
// tests the real cases from the underlying parser for the mapping, rather that just
// testing the opaque pointers.
#[rstest]
#[case::null("null", Null)]
#[case::bool("true", Boolean)]
#[case::int("-5", Integer)]
#[case::float("100e0", Float)]
#[case::decimal("1.1", Decimal)]
#[case::timestamp("2014-10-16T", Timestamp)]
#[case::symbol("foobar", Symbol)]
#[case::string("'''foosball'''", String)]
#[case::clob("{{'''moon'''}}", Clob)]
#[case::blob("{{Z29vZGJ5ZQ==}}", Blob)]
#[case::list("[1, 2, 3]", List)]
#[case::sexp("(a b c)", SExpression)]
#[case::structure("{a:1, b:2}", Struct)]
fn ionc_type_conversion(
#[case] data: &str,
#[case] expected_ion_type: IonType,
) -> IonResult<()> {
let mut reader: IonCReaderHandle = data.try_into()?;
let ion_type: IonType = reader.next()?.try_into()?;
assert_eq!(expected_ion_type, ion_type);

let eof_result: IonResult<IonType> = reader.next()?.try_into();
match eof_result {
Err(IonError::IllegalOperation { operation: _ }) => {}
otherwise => panic!("Unexpected conversion of EOF: {:?}", otherwise),
};

Ok(())
}
}
Loading

0 comments on commit bafce2e

Please sign in to comment.