Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stricter type checking for Python integers and floats #1554

Merged
merged 3 commits into from
Jun 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
- `External` types must be available in the Rust crate root.
- External bindings: The `ExternalBindingsConfig` trait was replaced with `BindingsConfig`. External bindings implementations will need to make minor changes to implement the new trait instead.
- Removed support for the `--config` flag when running the `scaffolding` command. This flag has never an effect, because there was no scaffolding configuration options.
- Python bindings are now more strict with their types. You can no longer pass strings to methods taking integers or floats, or floats to methods taking integers.

### What's changed

Expand Down
7 changes: 7 additions & 0 deletions fixtures/type-limits/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,11 @@ fn take_u64(v: u64) -> u64 {
v
}

fn take_f32(v: f32) -> f32 {
v
}
fn take_f64(v: f64) -> f64 {
v
}

uniffi::include_scaffolding!("type-limits");
3 changes: 3 additions & 0 deletions fixtures/type-limits/src/type-limits.udl
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,7 @@ namespace uniffi_type_limits {
u16 take_u16(u16 v);
u32 take_u32(u32 v);
u64 take_u64(u64 v);

f32 take_f32(f32 v);
f64 take_f64(f64 v);
};
235 changes: 172 additions & 63 deletions fixtures/type-limits/tests/bindings/test_type_limits.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,66 +4,175 @@

from uniffi_type_limits import *

def assert_raises(func, exception_cls):
try:
func()
except exception_cls:
return
raise AssertionError("didn't raise the required exception")

# strict lower bounds
assert_raises(lambda: take_i8(-2**7 - 1), ValueError)
assert_raises(lambda: take_i16(-2**15 - 1), ValueError)
assert_raises(lambda: take_i32(-2**31 - 1), ValueError)
assert_raises(lambda: take_i64(-2**63 - 1), ValueError)
assert_raises(lambda: take_u8(-1), ValueError)
assert_raises(lambda: take_u16(-1), ValueError)
assert_raises(lambda: take_u32(-1), ValueError)
assert_raises(lambda: take_u64(-1), ValueError)

assert take_i8(-2**7) == -2**7
assert take_i16(-2**15) == -2**15
assert take_i32(-2**31) == -2**31
assert take_i64(-2**63) == -2**63
assert take_u8(0) == 0
assert take_u16(0) == 0
assert take_u32(0) == 0
assert take_u64(0) == 0

# strict upper bounds
assert_raises(lambda: take_i8(2**7), ValueError)
assert_raises(lambda: take_i16(2**15), ValueError)
assert_raises(lambda: take_i32(2**31), ValueError)
assert_raises(lambda: take_i64(2**63), ValueError)
assert_raises(lambda: take_u8(2**8), ValueError)
assert_raises(lambda: take_u16(2**16), ValueError)
assert_raises(lambda: take_u32(2**32), ValueError)
assert_raises(lambda: take_u64(2**64), ValueError)

assert take_i8(2**7 - 1) == 2**7 - 1
assert take_i16(2**15 - 1) == 2**15 - 1
assert take_i32(2**31 - 1) == 2**31 - 1
assert take_i64(2**63 - 1) == 2**63 - 1
assert take_u8(2**8 - 1) == 2**8 - 1
assert take_u16(2**16 - 1) == 2**16 - 1
assert take_u32(2**32 - 1) == 2**32 - 1
assert take_u64(2**64 - 1) == 2**64 - 1

# larger numbers
assert_raises(lambda: take_i8(10**3), ValueError)
assert_raises(lambda: take_i16(10**5), ValueError)
assert_raises(lambda: take_i32(10**10), ValueError)
assert_raises(lambda: take_i64(10**19), ValueError)
assert_raises(lambda: take_u8(10**3), ValueError)
assert_raises(lambda: take_u16(10**5), ValueError)
assert_raises(lambda: take_u32(10**10), ValueError)
assert_raises(lambda: take_u64(10**20), ValueError)

assert take_i8(10**2) == 10**2
assert take_i16(10**4) == 10**4
assert take_i32(10**9) == 10**9
assert take_i64(10**18) == 10**18
assert take_u8(10**2) == 10**2
assert take_u16(10**4) == 10**4
assert take_u32(10**9) == 10**9
assert take_u64(10**19) == 10**19
import math
import unittest

class TestTypeLimits(unittest.TestCase):
def test_strict_lower_bounds(self):
self.assertRaises(ValueError, lambda: take_i8(-2**7 - 1))
self.assertRaises(ValueError, lambda: take_i16(-2**15 - 1))
self.assertRaises(ValueError, lambda: take_i32(-2**31 - 1))
self.assertRaises(ValueError, lambda: take_i64(-2**63 - 1))
self.assertRaises(ValueError, lambda: take_u8(-1))
self.assertRaises(ValueError, lambda: take_u16(-1))
self.assertRaises(ValueError, lambda: take_u32(-1))
self.assertRaises(ValueError, lambda: take_u64(-1))

self.assertEqual(take_i8(-2**7), -2**7)
self.assertEqual(take_i16(-2**15), -2**15)
self.assertEqual(take_i32(-2**31), -2**31)
self.assertEqual(take_i64(-2**63), -2**63)
self.assertEqual(take_u8(0), 0)
self.assertEqual(take_u16(0), 0)
self.assertEqual(take_u32(0), 0)
self.assertEqual(take_u64(0), 0)

def test_strict_upper_bounds(self):
self.assertRaises(ValueError, lambda: take_i8(2**7))
self.assertRaises(ValueError, lambda: take_i16(2**15))
self.assertRaises(ValueError, lambda: take_i32(2**31))
self.assertRaises(ValueError, lambda: take_i64(2**63))
self.assertRaises(ValueError, lambda: take_u8(2**8))
self.assertRaises(ValueError, lambda: take_u16(2**16))
self.assertRaises(ValueError, lambda: take_u32(2**32))
self.assertRaises(ValueError, lambda: take_u64(2**64))

self.assertEqual(take_i8(2**7 - 1), 2**7 - 1)
self.assertEqual(take_i16(2**15 - 1), 2**15 - 1)
self.assertEqual(take_i32(2**31 - 1), 2**31 - 1)
self.assertEqual(take_i64(2**63 - 1), 2**63 - 1)
self.assertEqual(take_u8(2**8 - 1), 2**8 - 1)
self.assertEqual(take_u16(2**16 - 1), 2**16 - 1)
self.assertEqual(take_u32(2**32 - 1), 2**32 - 1)
self.assertEqual(take_u64(2**64 - 1), 2**64 - 1)

def test_larger_numbers(self):
self.assertRaises(ValueError, lambda: take_i8(10**3))
self.assertRaises(ValueError, lambda: take_i16(10**5))
self.assertRaises(ValueError, lambda: take_i32(10**10))
self.assertRaises(ValueError, lambda: take_i64(10**19))
self.assertRaises(ValueError, lambda: take_u8(10**3))
self.assertRaises(ValueError, lambda: take_u16(10**5))
self.assertRaises(ValueError, lambda: take_u32(10**10))
self.assertRaises(ValueError, lambda: take_u64(10**20))

self.assertEqual(take_i8(10**2), 10**2)
self.assertEqual(take_i16(10**4), 10**4)
self.assertEqual(take_i32(10**9), 10**9)
self.assertEqual(take_i64(10**18), 10**18)
self.assertEqual(take_u8(10**2), 10**2)
self.assertEqual(take_u16(10**4), 10**4)
self.assertEqual(take_u32(10**9), 10**9)
self.assertEqual(take_u64(10**19), 10**19)

def test_non_integer(self):
self.assertRaises(TypeError, lambda: take_i8("0"))
self.assertRaises(TypeError, lambda: take_i16("0"))
self.assertRaises(TypeError, lambda: take_i32("0"))
self.assertRaises(TypeError, lambda: take_i64("0"))
self.assertRaises(TypeError, lambda: take_u8("0"))
self.assertRaises(TypeError, lambda: take_u16("0"))
self.assertRaises(TypeError, lambda: take_u32("0"))
self.assertRaises(TypeError, lambda: take_u64("0"))

self.assertRaises(TypeError, lambda: take_i8(0.0))
self.assertRaises(TypeError, lambda: take_i16(0.0))
self.assertRaises(TypeError, lambda: take_i32(0.0))
self.assertRaises(TypeError, lambda: take_i64(0.0))
self.assertRaises(TypeError, lambda: take_u8(0.0))
self.assertRaises(TypeError, lambda: take_u16(0.0))
self.assertRaises(TypeError, lambda: take_u32(0.0))
self.assertRaises(TypeError, lambda: take_u64(0.0))

class A:
pass

self.assertRaises(TypeError, lambda: take_i8(A()))
self.assertRaises(TypeError, lambda: take_i16(A()))
self.assertRaises(TypeError, lambda: take_i32(A()))
self.assertRaises(TypeError, lambda: take_i64(A()))
self.assertRaises(TypeError, lambda: take_u8(A()))
self.assertRaises(TypeError, lambda: take_u16(A()))
self.assertRaises(TypeError, lambda: take_u32(A()))
self.assertRaises(TypeError, lambda: take_u64(A()))

def test_integer_like(self):
self.assertEqual(take_i8(False), 0)
self.assertEqual(take_i16(False), 0)
self.assertEqual(take_i32(False), 0)
self.assertEqual(take_i64(False), 0)
self.assertEqual(take_u8(False), 0)
self.assertEqual(take_u16(False), 0)
self.assertEqual(take_u32(False), 0)
self.assertEqual(take_u64(False), 0)

self.assertEqual(take_i8(True), 1)
self.assertEqual(take_i16(True), 1)
self.assertEqual(take_i32(True), 1)
self.assertEqual(take_i64(True), 1)
self.assertEqual(take_u8(True), 1)
self.assertEqual(take_u16(True), 1)
self.assertEqual(take_u32(True), 1)
self.assertEqual(take_u64(True), 1)

class A:
def __index__(self):
return 123

self.assertEqual(take_i8(A()), 123)
self.assertEqual(take_i16(A()), 123)
self.assertEqual(take_i32(A()), 123)
self.assertEqual(take_i64(A()), 123)
self.assertEqual(take_u8(A()), 123)
self.assertEqual(take_u16(A()), 123)
self.assertEqual(take_u32(A()), 123)
self.assertEqual(take_u64(A()), 123)

def test_non_float(self):
self.assertRaises(TypeError, lambda: take_f32("0"))
self.assertRaises(TypeError, lambda: take_f64("0"))

self.assertRaises(TypeError, lambda: take_f32(1j))
self.assertRaises(TypeError, lambda: take_f64(1j))

class A:
pass

self.assertRaises(TypeError, lambda: take_f32(A()))
self.assertRaises(TypeError, lambda: take_f64(A()))

def test_float_like(self):
self.assertEqual(take_f32(False), 0.0)
self.assertEqual(take_f64(False), 0.0)

self.assertEqual(take_f32(True), 1.0)
self.assertEqual(take_f64(True), 1.0)

self.assertEqual(take_f32(123), 123.0)
self.assertEqual(take_f64(123), 123.0)

class A:
def __float__(self):
return 456.0

self.assertEqual(take_f32(A()), 456.0)
self.assertEqual(take_f64(A()), 456.0)

def test_special_floats(self):
self.assertEqual(take_f32(math.inf), math.inf)
self.assertEqual(take_f64(math.inf), math.inf)

self.assertEqual(take_f32(-math.inf), -math.inf)
self.assertEqual(take_f64(-math.inf), -math.inf)

self.assertEqual(math.copysign(1.0, take_f32(0.0)), 1.0)
self.assertEqual(math.copysign(1.0, take_f64(0.0)), 1.0)

self.assertEqual(math.copysign(1.0, take_f32(-0.0)), -1.0)
self.assertEqual(math.copysign(1.0, take_f64(-0.0)), -1.0)

self.assertTrue(math.isnan(take_f32(math.nan)))
self.assertTrue(math.isnan(take_f64(math.nan)))

if __name__ == "__main__":
unittest.main()
9 changes: 0 additions & 9 deletions uniffi_bindgen/src/backend/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,6 @@ pub trait CodeType {
fn initialization_fn(&self, _oracle: &dyn CodeOracle) -> Option<String> {
None
}

/// An expression to coerce the given variable to the expected type.
fn coerce(&self, oracle: &dyn CodeOracle, _nm: &str) -> String {
panic!("Unimplemented for {}", self.type_label(oracle));
}
}

/// This trait is used to implement `CodeType` for `Type` and type-like structs (`Record`, `Enum`, `Field`,
Expand Down Expand Up @@ -211,8 +206,4 @@ impl<T: CodeTypeDispatch> CodeType for T {
fn initialization_fn(&self, oracle: &dyn CodeOracle) -> Option<String> {
self.code_type_impl(oracle).initialization_fn(oracle)
}

fn coerce(&self, oracle: &dyn CodeOracle, nm: &str) -> String {
self.code_type_impl(oracle).coerce(oracle, nm)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,4 @@ impl CodeType for CallbackInterfaceCodeType {
fn literal(&self, _oracle: &dyn CodeOracle, _literal: &Literal) -> String {
unreachable!();
}

fn coerce(&self, _oracle: &dyn CodeOracle, nm: &str) -> String {
nm.to_string()
}
}
25 changes: 0 additions & 25 deletions uniffi_bindgen/src/bindings/python/gen_python/compounds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,6 @@ impl CodeType for OptionalCodeType {
_ => oracle.find(&self.inner).literal(oracle, literal),
}
}

fn coerce(&self, oracle: &dyn CodeOracle, nm: &str) -> String {
format!(
"(None if {} is None else {})",
nm,
oracle.find(&self.inner).coerce(oracle, nm)
)
}
}

pub struct SequenceCodeType {
Expand Down Expand Up @@ -70,14 +62,6 @@ impl CodeType for SequenceCodeType {
_ => unimplemented!(),
}
}

fn coerce(&self, oracle: &dyn CodeOracle, nm: &str) -> String {
format!(
"list({} for x in {})",
oracle.find(&self.inner).coerce(oracle, "x"),
nm
)
}
}

pub struct MapCodeType {
Expand Down Expand Up @@ -110,13 +94,4 @@ impl CodeType for MapCodeType {
_ => unimplemented!(),
}
}

fn coerce(&self, oracle: &dyn CodeOracle, nm: &str) -> String {
format!(
"dict(({}, {}) for (k, v) in {}.items())",
self.key.coerce(oracle, "k"),
self.value.coerce(oracle, "v"),
nm
)
}
}
4 changes: 0 additions & 4 deletions uniffi_bindgen/src/bindings/python/gen_python/custom.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,4 @@ impl CodeType for CustomCodeType {
fn canonical_name(&self, _oracle: &dyn CodeOracle) -> String {
format!("Type{}", self.name)
}

fn coerce(&self, _oracle: &dyn CodeOracle, nm: &str) -> String {
nm.to_string()
}
}
4 changes: 0 additions & 4 deletions uniffi_bindgen/src/bindings/python/gen_python/enum_.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,4 @@ impl CodeType for EnumCodeType {
unreachable!();
}
}

fn coerce(&self, _oracle: &dyn CodeOracle, nm: &str) -> String {
nm.to_string()
}
}
4 changes: 0 additions & 4 deletions uniffi_bindgen/src/bindings/python/gen_python/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,4 @@ impl CodeType for ErrorCodeType {
fn literal(&self, _oracle: &dyn CodeOracle, _literal: &Literal) -> String {
unreachable!();
}

fn coerce(&self, _oracle: &dyn CodeOracle, nm: &str) -> String {
nm.to_string()
}
}
4 changes: 0 additions & 4 deletions uniffi_bindgen/src/bindings/python/gen_python/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,4 @@ impl CodeType for ForeignExecutorCodeType {
fn canonical_name(&self, _oracle: &dyn CodeOracle) -> String {
"ForeignExecutor".into()
}

fn coerce(&self, _oracle: &dyn CodeOracle, nm: &str) -> String {
nm.to_string()
}
}
4 changes: 0 additions & 4 deletions uniffi_bindgen/src/bindings/python/gen_python/external.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,4 @@ impl CodeType for ExternalCodeType {
fn canonical_name(&self, _oracle: &dyn CodeOracle) -> String {
format!("Type{}", self.name)
}

fn coerce(&self, _oracle: &dyn CodeOracle, nm: &str) -> String {
nm.into()
}
}
4 changes: 0 additions & 4 deletions uniffi_bindgen/src/bindings/python/gen_python/miscellany.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,6 @@ macro_rules! impl_code_type_for_miscellany {
fn literal(&self, _oracle: &dyn CodeOracle, _literal: &Literal) -> String {
unreachable!()
}

fn coerce(&self, _oracle: &dyn CodeOracle, nm: &str) -> String {
nm.to_string()
}
}
}
};
Expand Down
Loading