From dd8e4f231c04054260796ac179bac6e3574e8afc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20W=C3=BClker?= Date: Tue, 23 Sep 2025 23:58:17 +0200 Subject: [PATCH] script: Remove dead code in xpath implementation (#39454) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Testing: Verified by the fact that the code still compiles, and existing web platform tests of course Part of https://github.com/servo/servo/issues/34527 Signed-off-by: Simon Wülker --- components/script/xpath/eval.rs | 82 --------------- components/script/xpath/eval_function.rs | 57 ----------- components/script/xpath/eval_value.rs | 15 --- components/script/xpath/mod.rs | 3 - components/script/xpath/parser.rs | 124 ----------------------- 5 files changed, 281 deletions(-) diff --git a/components/script/xpath/eval.rs b/components/script/xpath/eval.rs index 57a35e96cff..9040cbc763a 100644 --- a/components/script/xpath/eval.rs +++ b/components/script/xpath/eval.rs @@ -29,25 +29,15 @@ use crate::xpath::context::PredicateCtx; #[derive(Clone, Debug)] pub(crate) enum Error { NotANodeset, - InvalidPath, - UnknownFunction { - name: QualName, - }, /// It is not clear where variables used in XPath expression should come from. /// Firefox throws "NS_ERROR_ILLEGAL_VALUE" when using them, chrome seems to return /// an empty result. We also error out. /// /// See CannotUseVariables, - UnknownNamespace { - prefix: String, - }, InvalidQName { qname: ParserQualName, }, - FunctionEvaluation { - fname: String, - }, Internal { msg: String, }, @@ -59,18 +49,10 @@ impl std::fmt::Display for Error { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { Error::NotANodeset => write!(f, "expression did not evaluate to a nodeset"), - Error::InvalidPath => write!(f, "invalid path expression"), - Error::UnknownFunction { name } => write!(f, "unknown function {:?}", name), Error::CannotUseVariables => write!(f, "cannot use variables"), - Error::UnknownNamespace { prefix } => { - write!(f, "unknown namespace prefix {:?}", prefix) - }, Error::InvalidQName { qname } => { write!(f, "invalid QName {:?}", qname) }, - Error::FunctionEvaluation { fname } => { - write!(f, "error while evaluating function: {}", fname) - }, Error::Internal { msg } => { write!(f, "internal error: {}", msg) }, @@ -92,8 +74,6 @@ pub(crate) fn try_extract_nodeset(v: Value) -> Result>, Error> pub(crate) trait Evaluatable: fmt::Debug { fn evaluate(&self, context: &EvaluationCtx) -> Result; - /// Returns true if this expression evaluates to a primitive value, without needing to touch the DOM - fn is_primitive(&self) -> bool; } impl Evaluatable for Box @@ -103,10 +83,6 @@ where fn evaluate(&self, context: &EvaluationCtx) -> Result { (**self).evaluate(context) } - - fn is_primitive(&self) -> bool { - (**self).is_primitive() - } } impl Evaluatable for Option @@ -119,10 +95,6 @@ where None => Ok(Value::Nodeset(vec![])), } } - - fn is_primitive(&self) -> bool { - self.as_ref().is_some_and(|t| T::is_primitive(t)) - } } impl Evaluatable for Expr { @@ -201,20 +173,6 @@ impl Evaluatable for Expr { Expr::Path(path_expr) => path_expr.evaluate(context), } } - - fn is_primitive(&self) -> bool { - match self { - Expr::Or(left, right) => left.is_primitive() && right.is_primitive(), - Expr::And(left, right) => left.is_primitive() && right.is_primitive(), - Expr::Equality(left, _, right) => left.is_primitive() && right.is_primitive(), - Expr::Relational(left, _, right) => left.is_primitive() && right.is_primitive(), - Expr::Additive(left, _, right) => left.is_primitive() && right.is_primitive(), - Expr::Multiplicative(left, _, right) => left.is_primitive() && right.is_primitive(), - Expr::Unary(_, expr) => expr.is_primitive(), - Expr::Union(_, _) => false, - Expr::Path(path_expr) => path_expr.is_primitive(), - } - } } impl Evaluatable for PathExpr { @@ -268,13 +226,6 @@ impl Evaluatable for PathExpr { Ok(Value::Nodeset(current_nodes)) } - - fn is_primitive(&self) -> bool { - !self.is_absolute && - !self.is_descendant && - self.steps.len() == 1 && - self.steps[0].is_primitive() - } } /// Error types for validate and extract a qualified name following @@ -612,13 +563,6 @@ impl Evaluatable for StepExpr { }, } } - - fn is_primitive(&self) -> bool { - match self { - StepExpr::Filter(filter_expr) => filter_expr.is_primitive(), - StepExpr::Axis(_) => false, - } - } } impl Evaluatable for PredicateListExpr { @@ -668,10 +612,6 @@ impl Evaluatable for PredicateListExpr { }) } } - - fn is_primitive(&self) -> bool { - self.predicates.len() == 1 && self.predicates[0].is_primitive() - } } impl Evaluatable for PredicateExpr { @@ -704,10 +644,6 @@ impl Evaluatable for PredicateExpr { Ok(Value::Nodeset(narrowed_nodes?)) } - - fn is_primitive(&self) -> bool { - self.expr.is_primitive() - } } impl Evaluatable for FilterExpr { @@ -738,10 +674,6 @@ impl Evaluatable for FilterExpr { (true, _) => Err(Error::NotANodeset), } } - - fn is_primitive(&self) -> bool { - self.predicates.predicates.is_empty() && self.primary.is_primitive() - } } impl Evaluatable for PrimaryExpr { @@ -754,16 +686,6 @@ impl Evaluatable for PrimaryExpr { PrimaryExpr::Function(core_function) => core_function.evaluate(context), } } - - fn is_primitive(&self) -> bool { - match self { - PrimaryExpr::Literal(_) => true, - PrimaryExpr::Variable(_qname) => false, - PrimaryExpr::Parenthesized(expr) => expr.is_primitive(), - PrimaryExpr::ContextItem => false, - PrimaryExpr::Function(_) => false, - } - } } impl Evaluatable for Literal { @@ -777,8 +699,4 @@ impl Evaluatable for Literal { Literal::String(s) => Ok(Value::String(s.into())), } } - - fn is_primitive(&self) -> bool { - true - } } diff --git a/components/script/xpath/eval_function.rs b/components/script/xpath/eval_function.rs index 53c14944474..72a64ebe226 100644 --- a/components/script/xpath/eval_function.rs +++ b/components/script/xpath/eval_function.rs @@ -304,63 +304,6 @@ impl Evaluatable for CoreFunction { }, } } - - fn is_primitive(&self) -> bool { - match self { - CoreFunction::Last => false, - CoreFunction::Position => false, - CoreFunction::Count(_) => false, - CoreFunction::Id(_) => false, - CoreFunction::LocalName(_) => false, - CoreFunction::NamespaceUri(_) => false, - CoreFunction::Name(_) => false, - CoreFunction::String(expr_opt) => expr_opt - .as_ref() - .map(|expr| expr.is_primitive()) - .unwrap_or(false), - CoreFunction::Concat(vec) => vec.iter().all(|expr| expr.is_primitive()), - CoreFunction::StartsWith(expr, substr) => expr.is_primitive() && substr.is_primitive(), - CoreFunction::Contains(expr, substr) => expr.is_primitive() && substr.is_primitive(), - CoreFunction::SubstringBefore(expr, substr) => { - expr.is_primitive() && substr.is_primitive() - }, - CoreFunction::SubstringAfter(expr, substr) => { - expr.is_primitive() && substr.is_primitive() - }, - CoreFunction::Substring(expr, start_pos, length_opt) => { - expr.is_primitive() && - start_pos.is_primitive() && - length_opt - .as_ref() - .map(|length| length.is_primitive()) - .unwrap_or(false) - }, - CoreFunction::StringLength(expr_opt) => expr_opt - .as_ref() - .map(|expr| expr.is_primitive()) - .unwrap_or(false), - CoreFunction::NormalizeSpace(expr_opt) => expr_opt - .as_ref() - .map(|expr| expr.is_primitive()) - .unwrap_or(false), - CoreFunction::Translate(expr, from_chars, to_chars) => { - expr.is_primitive() && from_chars.is_primitive() && to_chars.is_primitive() - }, - CoreFunction::Number(expr_opt) => expr_opt - .as_ref() - .map(|expr| expr.is_primitive()) - .unwrap_or(false), - CoreFunction::Sum(expr) => expr.is_primitive(), - CoreFunction::Floor(expr) => expr.is_primitive(), - CoreFunction::Ceiling(expr) => expr.is_primitive(), - CoreFunction::Round(expr) => expr.is_primitive(), - CoreFunction::Boolean(expr) => expr.is_primitive(), - CoreFunction::Not(expr) => expr.is_primitive(), - CoreFunction::True => true, - CoreFunction::False => true, - CoreFunction::Lang(_) => false, - } - } } #[cfg(test)] mod tests { diff --git a/components/script/xpath/eval_value.rs b/components/script/xpath/eval_value.rs index 925a8cb7b13..f8cd495c009 100644 --- a/components/script/xpath/eval_value.rs +++ b/components/script/xpath/eval_value.rs @@ -86,10 +86,6 @@ impl Value { } } - pub(crate) fn into_boolean(self) -> bool { - self.boolean() - } - pub(crate) fn number(&self) -> f64 { match *self { Value::Boolean(val) => { @@ -105,10 +101,6 @@ impl Value { } } - pub(crate) fn into_number(self) -> f64 { - self.number() - } - pub(crate) fn string(&self) -> string::String { match *self { Value::Boolean(v) => v.to_string(), @@ -133,13 +125,6 @@ impl Value { }, } } - - pub(crate) fn into_string(self) -> string::String { - match self { - Value::String(val) => val, - other => other.string(), - } - } } macro_rules! from_impl { diff --git a/components/script/xpath/mod.rs b/components/script/xpath/mod.rs index 15424019253..2bba2c01b9f 100644 --- a/components/script/xpath/mod.rs +++ b/components/script/xpath/mod.rs @@ -14,12 +14,9 @@ use crate::dom::bindings::codegen::Bindings::XPathNSResolverBinding::XPathNSReso use crate::dom::bindings::error::{Error as JsError, Error, Fallible}; mod context; -#[allow(dead_code)] mod eval; mod eval_function; -#[allow(dead_code)] mod eval_value; -#[allow(dead_code)] mod parser; /// Parse an XPath expression from a string diff --git a/components/script/xpath/parser.rs b/components/script/xpath/parser.rs index 699c9bf235a..1c015b0f975 100644 --- a/components/script/xpath/parser.rs +++ b/components/script/xpath/parser.rs @@ -234,130 +234,6 @@ pub(crate) enum CoreFunction { Lang(Box), } -impl CoreFunction { - pub(crate) fn name(&self) -> &'static str { - match self { - CoreFunction::Last => "last", - CoreFunction::Position => "position", - CoreFunction::Count(_) => "count", - CoreFunction::Id(_) => "id", - CoreFunction::LocalName(_) => "local-name", - CoreFunction::NamespaceUri(_) => "namespace-uri", - CoreFunction::Name(_) => "name", - CoreFunction::String(_) => "string", - CoreFunction::Concat(_) => "concat", - CoreFunction::StartsWith(_, _) => "starts-with", - CoreFunction::Contains(_, _) => "contains", - CoreFunction::SubstringBefore(_, _) => "substring-before", - CoreFunction::SubstringAfter(_, _) => "substring-after", - CoreFunction::Substring(_, _, _) => "substring", - CoreFunction::StringLength(_) => "string-length", - CoreFunction::NormalizeSpace(_) => "normalize-space", - CoreFunction::Translate(_, _, _) => "translate", - CoreFunction::Number(_) => "number", - CoreFunction::Sum(_) => "sum", - CoreFunction::Floor(_) => "floor", - CoreFunction::Ceiling(_) => "ceiling", - CoreFunction::Round(_) => "round", - CoreFunction::Boolean(_) => "boolean", - CoreFunction::Not(_) => "not", - CoreFunction::True => "true", - CoreFunction::False => "false", - CoreFunction::Lang(_) => "lang", - } - } - - pub(crate) fn min_args(&self) -> usize { - match self { - // No args - CoreFunction::Last | - CoreFunction::Position | - CoreFunction::True | - CoreFunction::False => 0, - - // Optional single arg - CoreFunction::LocalName(_) | - CoreFunction::NamespaceUri(_) | - CoreFunction::Name(_) | - CoreFunction::String(_) | - CoreFunction::StringLength(_) | - CoreFunction::NormalizeSpace(_) | - CoreFunction::Number(_) => 0, - - // Required single arg - CoreFunction::Count(_) | - CoreFunction::Id(_) | - CoreFunction::Sum(_) | - CoreFunction::Floor(_) | - CoreFunction::Ceiling(_) | - CoreFunction::Round(_) | - CoreFunction::Boolean(_) | - CoreFunction::Not(_) | - CoreFunction::Lang(_) => 1, - - // Required two args - CoreFunction::StartsWith(_, _) | - CoreFunction::Contains(_, _) | - CoreFunction::SubstringBefore(_, _) | - CoreFunction::SubstringAfter(_, _) => 2, - - // Special cases - CoreFunction::Concat(_) => 2, // Minimum 2 args - CoreFunction::Substring(_, _, _) => 2, // 2 or 3 args - CoreFunction::Translate(_, _, _) => 3, // Exactly 3 args - } - } - - pub(crate) fn max_args(&self) -> Option { - match self { - // No args - CoreFunction::Last | - CoreFunction::Position | - CoreFunction::True | - CoreFunction::False => Some(0), - - // Optional single arg (0 or 1) - CoreFunction::LocalName(_) | - CoreFunction::NamespaceUri(_) | - CoreFunction::Name(_) | - CoreFunction::String(_) | - CoreFunction::StringLength(_) | - CoreFunction::NormalizeSpace(_) | - CoreFunction::Number(_) => Some(1), - - // Exactly one arg - CoreFunction::Count(_) | - CoreFunction::Id(_) | - CoreFunction::Sum(_) | - CoreFunction::Floor(_) | - CoreFunction::Ceiling(_) | - CoreFunction::Round(_) | - CoreFunction::Boolean(_) | - CoreFunction::Not(_) | - CoreFunction::Lang(_) => Some(1), - - // Exactly two args - CoreFunction::StartsWith(_, _) | - CoreFunction::Contains(_, _) | - CoreFunction::SubstringBefore(_, _) | - CoreFunction::SubstringAfter(_, _) => Some(2), - - // Special cases - CoreFunction::Concat(_) => None, // Unlimited args - CoreFunction::Substring(_, _, _) => Some(3), // 2 or 3 args - CoreFunction::Translate(_, _, _) => Some(3), // Exactly 3 args - } - } - - /// Returns true if the number of arguments is valid for this function - pub(crate) fn is_valid_arity(&self, num_args: usize) -> bool { - let min = self.min_args(); - let max = self.max_args(); - - num_args >= min && max.is_none_or(|max| num_args <= max) - } -} - #[derive(Clone, Debug, PartialEq)] pub(crate) struct OwnedParserError { input: String,