From 0a1633c52ae1d263aa74cfd5c679fcd2669fda64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20W=C3=BClker?= Date: Tue, 30 Sep 2025 15:20:31 +0200 Subject: [PATCH] script: Remove unreachable assertions in xpath parser (#39588) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `relative_path_expr` can only ever return `Expr::Path`. If we make it return a `PathExpr` then callers can rely on this invariant. Testing: There's only limited testing for this change due to https://github.com/servo/servo/issues/39551. But I think that's okay, since the change is fairly trivial. Part of https://github.com/servo/servo/issues/34527 Signed-off-by: Simon Wülker --- components/script/xpath/parser.rs | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/components/script/xpath/parser.rs b/components/script/xpath/parser.rs index 1c015b0f975..c0dadc699f8 100644 --- a/components/script/xpath/parser.rs +++ b/components/script/xpath/parser.rs @@ -394,40 +394,32 @@ fn path_expr(input: &str) -> IResult<&str, Expr> { // "//" RelativePathExpr map( pair(tag("//"), move |i| relative_path_expr(true, i)), - |(_, rel_path)| { + |(_, relative_path)| { Expr::Path(PathExpr { is_absolute: true, is_descendant: true, - steps: match rel_path { - Expr::Path(p) => p.steps, - _ => unreachable!(), - }, + steps: relative_path.steps, }) }, ), // "/" RelativePathExpr? map( pair(char('/'), opt(move |i| relative_path_expr(false, i))), - |(_, rel_path)| { + |(_, relative_path)| { Expr::Path(PathExpr { is_absolute: true, is_descendant: false, - steps: rel_path - .map(|p| match p { - Expr::Path(p) => p.steps, - _ => unreachable!(), - }) - .unwrap_or_default(), + steps: relative_path.map(|path| path.steps).unwrap_or_default(), }) }, ), // RelativePathExpr - move |i| relative_path_expr(false, i), + map(move |i| relative_path_expr(false, i), Expr::Path), ))) .parse(input) } -fn relative_path_expr(is_descendant: bool, input: &str) -> IResult<&str, Expr> { +fn relative_path_expr(is_descendant: bool, input: &str) -> IResult<&str, PathExpr> { let (input, first) = step_expr(is_descendant, input)?; let (input, steps) = many0(pair( ws(alt((value(true, tag("//")), value(false, char('/'))))), @@ -450,11 +442,11 @@ fn relative_path_expr(is_descendant: bool, input: &str) -> IResult<&str, Expr> { Ok(( input, - Expr::Path(PathExpr { + PathExpr { is_absolute: false, is_descendant: false, steps: all_steps, - }), + }, )) }