Skip to content

Commit

Permalink
disallow internal nodes without leaves
Browse files Browse the repository at this point in the history
  • Loading branch information
jordens committed Oct 24, 2024
1 parent 0c97a0f commit aa458e8
Show file tree
Hide file tree
Showing 12 changed files with 95 additions and 98 deletions.
11 changes: 10 additions & 1 deletion miniconf/src/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ macro_rules! impl_tuple {
}
}
}
impl_tuple!();
// Note: internal nodes must have at least one leaf
impl_tuple!(0 T0);
impl_tuple!(0 T0 1 T1);
impl_tuple!(0 T0 1 T1 2 T2);
Expand All @@ -109,8 +109,15 @@ impl_tuple!(0 T0 1 T1 2 T2 3 T3 4 T4 5 T5 6 T6 7 T7);

/////////////////////////////////////////////////////////////////////////////////////////

struct Assert<const L: usize, const R: usize>;
impl<const L: usize, const R: usize> Assert<L, R> {
const GREATER: () = assert!(L > R);
}

impl<T: TreeKey, const N: usize> TreeKey for [T; N] {
fn traverse_all<W: Walk>() -> Result<W, W::Error> {
let () = Assert::<N, 0>::GREATER;

W::internal().merge(&T::traverse_all()?, None, &KeyLookup::homogeneous(N))
}

Expand All @@ -119,6 +126,8 @@ impl<T: TreeKey, const N: usize> TreeKey for [T; N] {
K: Keys,
F: FnMut(usize, Option<&'static str>, usize) -> Result<(), E>,
{
let () = Assert::<N, 0>::GREATER;

let index = keys.next(&KeyLookup::homogeneous(N))?;
func(index, None, N).map_err(|err| Error::Inner(1, err))?;
Error::increment_result(T::traverse_by_key(keys, func))
Expand Down
11 changes: 7 additions & 4 deletions miniconf/src/iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,15 @@ impl<M: TreeKey + ?Sized, N, const D: usize> NodeIter<M, N, D> {
/// Note(panic): Panics, if the iterator had `next()` called or
/// if the iteration depth has been limited.
pub fn exact_size(self) -> ExactSize<Self> {
assert!(self.depth == D + 1);
assert!(self.root == 0);
assert_eq!(self.depth, D + 1, "NodeIter partially consumed");
assert_eq!(self.root, 0, "NodeIter on sub-tree");
debug_assert_eq!(&self.state, &[0; D]); // ensured by depth = D + 1 marker
let meta = M::traverse_all::<Metadata>().unwrap();
assert!(D >= meta.max_depth);
assert!(D > 0, "Depth-0 iterator will return a node");
assert!(
D >= meta.max_depth,
"depth D = {D} must be at least {}",
meta.max_depth
);
ExactSize::new(self, meta.count)
}
}
Expand Down
25 changes: 5 additions & 20 deletions miniconf/tests/arrays.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use miniconf::{
};

mod common;
use common::paths;

#[derive(Debug, Copy, Clone, Default, Tree, Deserialize, Serialize)]
struct Inner {
Expand Down Expand Up @@ -48,6 +47,11 @@ fn set_get(
Ok(node.depth())
}

#[test]
fn paths() {
common::paths::<Settings, 4>();
}

#[test]
fn atomic() {
let mut s = Settings::default();
Expand Down Expand Up @@ -130,22 +134,3 @@ fn metadata() {
assert_eq!(metadata.max_length("/"), "/aam/0/0/c".len());
assert_eq!(metadata.count, 11);
}

#[test]
fn empty() {
assert_eq!(paths::<[Leaf<u32>; 0], 1>(), [""; 0]);

#[derive(Tree, Serialize, Deserialize)]
struct S {}

assert_eq!(paths::<S, 1>(), [""; 0]);
assert_eq!(paths::<[[S; 0]; 0], 3>(), [""; 0]);

#[derive(Tree)]
struct Q {
a: [S; 0],
b: [Leaf<S>; 0],
}

assert_eq!(paths::<Q, 3>(), [""; 0]);
}
33 changes: 1 addition & 32 deletions miniconf/tests/basic.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use miniconf::{Indices, IntoKeys, Leaf, Metadata, Node, Path, Traversal, Tree, TreeKey};
use miniconf::{Indices, Leaf, Metadata, Node, Path, Traversal, Tree, TreeKey};

#[derive(Tree, Default)]
struct Inner {
Expand Down Expand Up @@ -58,34 +58,3 @@ fn indices() {
);
assert_eq!(it.count(), 2);
}

#[test]
fn traverse_empty() {
#[derive(Tree, Default)]
struct S {}
let f = |_, _, _| -> Result<(), ()> { unreachable!() };
assert_eq!(
S::traverse_by_key([0usize].into_keys(), f),
Err(Traversal::NotFound(1).into())
);
assert_eq!(
S::traverse_by_key([0usize; 0].into_keys(), f),
Err(Traversal::TooShort(0).into())
);
assert_eq!(
Option::<Leaf<i32>>::traverse_by_key([0usize].into_keys(), f),
Err(Traversal::TooLong(0).into())
);
assert_eq!(
Option::<Leaf<i32>>::traverse_by_key([0usize; 0].into_keys(), f),
Ok(0)
);
assert_eq!(
<Option::<S> as TreeKey>::traverse_by_key([0usize].into_keys(), f),
Err(Traversal::NotFound(1).into())
);
assert_eq!(
<Option::<S> as TreeKey>::traverse_by_key([0usize; 0].into_keys(), f),
Err(Traversal::TooShort(0).into())
);
}
4 changes: 2 additions & 2 deletions miniconf/tests/generics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,8 @@ fn generic_atomic() {
#[derive(Tree, Default)]
struct Settings<T> {
atomic: Leaf<Inner<T>>,
opt: [[Leaf<Option<T>>; 0]; 0],
opt1: [[Option<Leaf<T>>; 0]; 0],
opt: [[Leaf<Option<T>>; 1]; 1],
opt1: [[Option<Leaf<T>>; 1]; 1],
}

#[derive(Deserialize, Serialize, Default)]
Expand Down
13 changes: 11 additions & 2 deletions miniconf/tests/iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,17 +82,26 @@ fn short_iter() {
#[test]
#[should_panic]
fn panic_short_iter() {
NodeIter::<Settings, Path<String, '/'>, 1>::default().exact_size();
<[[Leaf<u32>; 1]; 1]>::nodes::<(), 1>().exact_size();
}

#[test]
#[should_panic]
fn panic_started_iter() {
let mut it = Settings::nodes::<Indices<[_; 3]>, 3>();
let mut it = <[[Leaf<u32>; 1]; 1]>::nodes::<(), 2>();
it.next();
it.exact_size();
}

#[test]
#[should_panic]
fn panic_rooted_iter() {
<[[Leaf<u32>; 1]; 1]>::nodes::<(), 2>()
.root([0usize])
.unwrap()
.exact_size();
}

#[test]
fn root() {
assert_eq!(
Expand Down
2 changes: 1 addition & 1 deletion miniconf/tests/option.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ fn option_test_defer_option() {
#[test]
fn option_absent() {
#[derive(Copy, Clone, Default, Tree)]
struct I {}
struct I(Leaf<()>);

#[derive(Copy, Clone, Default, Tree)]
struct S {
Expand Down
11 changes: 7 additions & 4 deletions miniconf/tests/packed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,28 +46,31 @@ fn packed() {
fn top() {
#[derive(Tree)]
struct S {
baz: [Leaf<i32>; 0],
baz: [Leaf<i32>; 1],
foo: Leaf<i32>,
}
assert_eq!(
S::nodes::<Path<String, '/'>, 2>()
.map(|p| p.unwrap().0.into_inner())
.collect::<Vec<_>>(),
["/foo"]
["/baz/0", "/foo"]
);
assert_eq!(
S::nodes::<Indices<_>, 2>()
.map(|p| p.unwrap())
.collect::<Vec<_>>(),
[(Indices([1, 0]), Node::leaf(1))]
[
(Indices([0, 0]), Node::leaf(2)),
(Indices([1, 0]), Node::leaf(1))
]
);
let (p, node) = S::transcode::<Packed, _>([1usize]).unwrap();
assert_eq!((p.into_lsb().get(), node), (0b11, Node::leaf(1)));
assert_eq!(
S::nodes::<Packed, 2>()
.map(|p| p.unwrap().0.into_lsb().get())
.collect::<Vec<_>>(),
[0b11]
[0b100, 0b11]
);
}

Expand Down
32 changes: 0 additions & 32 deletions miniconf/tests/structs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,38 +46,6 @@ fn structs() {
assert_eq!(paths::<Settings, 2>(), ["/a", "/b", "/c", "/d/a"]);
}

#[test]
fn empty_struct() {
#[derive(Tree, Default)]
struct Settings {}
assert_eq!(paths::<Settings, 1>(), [""; 0]);
}

#[test]
#[should_panic]
fn empty_iter_exact() {
<()>::nodes::<(), 0>().exact_size();
}

#[test]
fn empty_iter() {
assert!(<()>::nodes::<(), 0>().count() == 1);
}

#[test]
fn unit_struct() {
#[derive(Tree, Default)]
struct Settings;
assert_eq!(paths::<Settings, 1>(), [""; 0]);
}

#[test]
fn empty_tuple_struct() {
#[derive(Tree, Default)]
struct Settings();
assert_eq!(paths::<Settings, 1>(), [""; 0]);
}

#[test]
fn borrowed() {
// Can't derive TreeAny
Expand Down
18 changes: 18 additions & 0 deletions miniconf/tests/ui/internal_no_leaf.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
use miniconf::Tree;

#[derive(Tree)]
pub enum EnumUninhab {}

#[derive(Tree)]
pub enum EnumEmpty {#[tree(skip)] V}

#[derive(Tree)]
pub struct StructUnit;

#[derive(Tree)]
pub struct StructEmptyTuple (#[tree(skip)] ());

#[derive(Tree)]
pub struct StructEmpty {#[tree(skip)] _f: ()}

fn main() {}
29 changes: 29 additions & 0 deletions miniconf/tests/ui/internal_no_leaf.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
error: Internal nodes must have at least one leaf
--> tests/ui/internal_no_leaf.rs:4:10
|
4 | pub enum EnumUninhab {}
| ^^^^^^^^^^^

error: Internal nodes must have at least one leaf
--> tests/ui/internal_no_leaf.rs:7:10
|
7 | pub enum EnumEmpty {#[tree(skip)] V}
| ^^^^^^^^^

error: Internal nodes must have at least one leaf
--> tests/ui/internal_no_leaf.rs:10:12
|
10 | pub struct StructUnit;
| ^^^^^^^^^^

error: Internal nodes must have at least one leaf
--> tests/ui/internal_no_leaf.rs:13:12
|
13 | pub struct StructEmptyTuple (#[tree(skip)] ());
| ^^^^^^^^^^^^^^^^

error: Internal nodes must have at least one leaf
--> tests/ui/internal_no_leaf.rs:16:12
|
16 | pub struct StructEmpty {#[tree(skip)] _f: ()}
| ^^^^^^^^^^^
4 changes: 4 additions & 0 deletions miniconf_derive/src/tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,10 @@ impl Tree {
return Err(Error::custom("Can't flatten multiple fields/variants")
.with_span(&self.flatten.span()));
}
if self.fields().is_empty() {
return Err(Error::custom("Internal nodes must have at least one leaf")
.with_span(&self.ident.span()));
}
Ok(self)
}

Expand Down

0 comments on commit aa458e8

Please sign in to comment.