Skip to content

Commit ccb9c70

Browse files
apoelstraapoelstra
committed
expression: drop Error::ParseTree variant
The correct way to build a Error::ParseTree is to make an Error::Parse containing a ParseError::Tree. Eventually when we get rid of the top-level Error enum this will be more sensible, and we will be able to use ? instead of this awful `.map_err(From::from).map_err(Error::parse)` construction. This commit rightfully belongs as part of the previous commit, but it's noisy and mechanical so I separated it out.
1 parent 0b6fcf4 commit ccb9c70

File tree

8 files changed

+40
-25
lines changed

8 files changed

+40
-25
lines changed

‎src/descriptor/bare.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -372,7 +372,8 @@ impl<Pk: FromStrKey> FromTree for Pkh<Pk> {
372372
fn from_tree(top: &expression::Tree) -> Result<Self, Error> {
373373
let top = top
374374
.verify_toplevel("pkh", 1..=1)
375-
.map_err(Error::ParseTree)?;
375+
.map_err(From::from)
376+
.map_err(Error::Parse)?;
376377
Ok(Pkh::new(expression::terminal(top, |pk| Pk::from_str(pk))?)?)
377378
}
378379
}

‎src/descriptor/mod.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1852,9 +1852,10 @@ mod tests {
18521852
// https://github.com/bitcoin/bitcoin/blob/7ae86b3c6845873ca96650fc69beb4ae5285c801/src/test/descriptor_tests.cpp#L355-L360
18531853
macro_rules! check_invalid_checksum {
18541854
($secp: ident,$($desc: expr),*) => {
1855+
use crate::{ParseError, ParseTreeError};
18551856
$(
18561857
match Descriptor::parse_descriptor($secp, $desc) {
1857-
Err(Error::ParseTree(crate::ParseTreeError::Checksum(_))) => {},
1858+
Err(Error::Parse(ParseError::Tree(ParseTreeError::Checksum(_)))) => {},
18581859
Err(e) => panic!("Expected bad checksum for {}, got '{}'", $desc, e),
18591860
_ => panic!("Invalid checksum treated as valid: {}", $desc),
18601861
};

‎src/descriptor/segwitv0.rs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,8 @@ impl<Pk: FromStrKey> crate::expression::FromTree for Wsh<Pk> {
250250
fn from_tree(top: &expression::Tree) -> Result<Self, Error> {
251251
let top = top
252252
.verify_toplevel("wsh", 1..=1)
253-
.map_err(Error::ParseTree)?;
253+
.map_err(From::from)
254+
.map_err(Error::Parse)?;
254255

255256
if top.name == "sortedmulti" {
256257
return Ok(Wsh { inner: WshInner::SortedMulti(SortedMultiVec::from_tree(top)?) });
@@ -485,7 +486,8 @@ impl<Pk: FromStrKey> crate::expression::FromTree for Wpkh<Pk> {
485486
fn from_tree(top: &expression::Tree) -> Result<Self, Error> {
486487
let top = top
487488
.verify_toplevel("wpkh", 1..=1)
488-
.map_err(Error::ParseTree)?;
489+
.map_err(From::from)
490+
.map_err(Error::Parse)?;
489491
Ok(Wpkh::new(expression::terminal(top, |pk| Pk::from_str(pk))?)?)
490492
}
491493
}

‎src/descriptor/sh.rs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,10 @@ impl<Pk: MiniscriptKey> fmt::Display for Sh<Pk> {
8282

8383
impl<Pk: FromStrKey> crate::expression::FromTree for Sh<Pk> {
8484
fn from_tree(top: &expression::Tree) -> Result<Self, Error> {
85-
let top = top.verify_toplevel("sh", 1..=1).map_err(Error::ParseTree)?;
85+
let top = top
86+
.verify_toplevel("sh", 1..=1)
87+
.map_err(From::from)
88+
.map_err(Error::Parse)?;
8689

8790
let inner = match top.name {
8891
"wsh" => ShInner::Wsh(Wsh::from_tree(top)?),

‎src/descriptor/sortedmulti.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,8 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> SortedMultiVec<Pk, Ctx> {
6565
<Pk as FromStr>::Err: fmt::Display,
6666
{
6767
tree.verify_toplevel("sortedmulti", 1..)
68-
.map_err(Error::ParseTree)?;
68+
.map_err(From::from)
69+
.map_err(Error::Parse)?;
6970

7071
let ret = Self {
7172
inner: tree

‎src/expression/mod.rs‎

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,9 @@ impl<'a> Tree<'a> {
239239
/// Parses a tree from a string
240240
#[allow(clippy::should_implement_trait)] // Cannot use std::str::FromStr because of lifetimes.
241241
pub fn from_str(s: &'a str) -> Result<Self, Error> {
242-
Self::from_str_inner(s).map_err(Error::ParseTree)
242+
Self::from_str_inner(s)
243+
.map_err(From::from)
244+
.map_err(Error::Parse)
243245
}
244246

245247
fn from_str_inner(s: &'a str) -> Result<Self, ParseTreeError> {
@@ -387,6 +389,7 @@ where
387389
#[cfg(test)]
388390
mod tests {
389391
use super::*;
392+
use crate::ParseError;
390393

391394
/// Test functions to manually build trees
392395
fn leaf(name: &str) -> Tree {
@@ -429,75 +432,81 @@ mod tests {
429432

430433
assert!(matches!(
431434
Tree::from_str("thresh,").unwrap_err(),
432-
Error::ParseTree(ParseTreeError::TrailingCharacter { ch: ',', pos: 6 }),
435+
Error::Parse(ParseError::Tree(ParseTreeError::TrailingCharacter { ch: ',', pos: 6 })),
433436
));
434437

435438
assert!(matches!(
436439
Tree::from_str("thresh,thresh").unwrap_err(),
437-
Error::ParseTree(ParseTreeError::TrailingCharacter { ch: ',', pos: 6 }),
440+
Error::Parse(ParseError::Tree(ParseTreeError::TrailingCharacter { ch: ',', pos: 6 })),
438441
));
439442

440443
assert!(matches!(
441444
Tree::from_str("thresh()thresh()").unwrap_err(),
442-
Error::ParseTree(ParseTreeError::TrailingCharacter { ch: 't', pos: 8 }),
445+
Error::Parse(ParseError::Tree(ParseTreeError::TrailingCharacter { ch: 't', pos: 8 })),
443446
));
444447

445448
assert_eq!(Tree::from_str("thresh()").unwrap(), paren_node("thresh", vec![leaf("")]));
446449

447450
assert!(matches!(
448451
Tree::from_str("thresh(a()b)"),
449-
Err(Error::ParseTree(ParseTreeError::ExpectedParenOrComma { ch: 'b', pos: 10 })),
452+
Err(Error::Parse(ParseError::Tree(ParseTreeError::ExpectedParenOrComma {
453+
ch: 'b',
454+
pos: 10
455+
}))),
450456
));
451457

452458
assert!(matches!(
453459
Tree::from_str("thresh()xyz"),
454-
Err(Error::ParseTree(ParseTreeError::TrailingCharacter { ch: 'x', pos: 8 })),
460+
Err(Error::Parse(ParseError::Tree(ParseTreeError::TrailingCharacter {
461+
ch: 'x',
462+
pos: 8
463+
}))),
455464
));
456465
}
457466

458467
#[test]
459468
fn parse_tree_parens() {
460469
assert!(matches!(
461470
Tree::from_str("a(").unwrap_err(),
462-
Error::ParseTree(ParseTreeError::UnmatchedOpenParen { ch: '(', pos: 1 }),
471+
Error::Parse(ParseError::Tree(ParseTreeError::UnmatchedOpenParen { ch: '(', pos: 1 })),
463472
));
464473

465474
assert!(matches!(
466475
Tree::from_str(")").unwrap_err(),
467-
Error::ParseTree(ParseTreeError::UnmatchedCloseParen { ch: ')', pos: 0 }),
476+
Error::Parse(ParseError::Tree(ParseTreeError::UnmatchedCloseParen { ch: ')', pos: 0 })),
468477
));
469478

470479
assert!(matches!(
471480
Tree::from_str("x(y))").unwrap_err(),
472-
Error::ParseTree(ParseTreeError::TrailingCharacter { ch: ')', pos: 4 }),
481+
Error::Parse(ParseError::Tree(ParseTreeError::TrailingCharacter { ch: ')', pos: 4 })),
473482
));
474483

475484
/* Will be enabled in a later PR which unifies TR and non-TR parsing.
476485
assert!(matches!(
477486
Tree::from_str("a{").unwrap_err(),
478-
Error::ParseTree(ParseTreeError::UnmatchedOpenParen { ch: '{', pos: 1 }),
487+
Error::Parse(ParseError::Tree(ParseTreeError::UnmatchedOpenParen { ch: '{', pos: 1 })),
479488
));
480489
481490
assert!(matches!(
482491
Tree::from_str("}").unwrap_err(),
483-
Error::ParseTree(ParseTreeError::UnmatchedCloseParen { ch: '}', pos: 0 }),
492+
Error::Parse(ParseError::Tree(ParseTreeError::UnmatchedCloseParen { ch: '}', pos: 0 })),
484493
));
485494
*/
486495

487496
assert!(matches!(
488497
Tree::from_str("x(y)}").unwrap_err(),
489-
Error::ParseTree(ParseTreeError::TrailingCharacter { ch: '}', pos: 4 }),
498+
Error::Parse(ParseError::Tree(ParseTreeError::TrailingCharacter { ch: '}', pos: 4 })),
490499
));
491500

492501
/* Will be enabled in a later PR which unifies TR and non-TR parsing.
493502
assert!(matches!(
494503
Tree::from_str("x{y)").unwrap_err(),
495-
Error::ParseTree(ParseTreeError::MismatchedParens {
504+
Error::Parse(ParseError::Tree(ParseTreeError::MismatchedParens {
496505
open_ch: '{',
497506
open_pos: 1,
498507
close_ch: ')',
499508
close_pos: 3,
500-
}),
509+
}),)
501510
));
502511
*/
503512
}

‎src/lib.rs‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -493,8 +493,6 @@ pub enum Error {
493493
/// Invalid threshold.
494494
ParseThreshold(ParseThresholdError),
495495
/// Invalid expression tree.
496-
ParseTree(ParseTreeError),
497-
/// Invalid expression tree.
498496
Parse(ParseError),
499497
}
500498

@@ -557,7 +555,6 @@ impl fmt::Display for Error {
557555
Error::RelativeLockTime(ref e) => e.fmt(f),
558556
Error::Threshold(ref e) => e.fmt(f),
559557
Error::ParseThreshold(ref e) => e.fmt(f),
560-
Error::ParseTree(ref e) => e.fmt(f),
561558
Error::Parse(ref e) => e.fmt(f),
562559
}
563560
}
@@ -609,7 +606,6 @@ impl std::error::Error for Error {
609606
RelativeLockTime(e) => Some(e),
610607
Threshold(e) => Some(e),
611608
ParseThreshold(e) => Some(e),
612-
ParseTree(e) => Some(e),
613609
Parse(e) => Some(e),
614610
}
615611
}

‎src/miniscript/mod.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -802,7 +802,9 @@ impl<Pk: FromStrKey, Ctx: ScriptContext> crate::expression::FromTree for Miniscr
802802
/// Parse an expression tree into a Miniscript. As a general rule, this
803803
/// should not be called directly; rather go through the descriptor API.
804804
fn from_tree(top: &expression::Tree) -> Result<Miniscript<Pk, Ctx>, Error> {
805-
top.verify_no_curly_braces().map_err(Error::ParseTree)?;
805+
top.verify_no_curly_braces()
806+
.map_err(From::from)
807+
.map_err(Error::Parse)?;
806808
let inner: Terminal<Pk, Ctx> = expression::FromTree::from_tree(top)?;
807809
Miniscript::from_ast(inner)
808810
}

0 commit comments

Comments
 (0)