Merge remote-tracking branch 'upstream/main' into conduwuit-changes

.github/workflows/ci.yml

@@ -76,9 +76,6 @@ jobs:
           - name: Check All Features
             cmd: msrv-all
 
-          - name: Check Client
-            cmd: msrv-client
-
           - name: Check Ruma
             cmd: msrv-ruma
 

crates/ruma-events/src/room/message/reply.rs

@@ -100,7 +100,7 @@ impl fmt::Display for FormattedOrPlainBody<'_> {
         if let Some(formatted_body) = self.formatted {
             #[cfg(feature = "html")]
             if self.is_reply {
-                let mut html = Html::parse(&formatted_body.body);
+                let html = Html::parse(&formatted_body.body);
                 html.sanitize();
 
                 write!(f, "{html}")

crates/ruma-html/CHANGELOG.md

@@ -4,6 +4,12 @@ Breaking Changes:
 
 - `MatrixElement::Div` is now a newtype variant.
 - `AnchorData`'s `name` field was removed, according to MSC4159.
+- html5ever was bumped to a new major version. A breaking change in the parsing
+  API required us to rewrite the `Html` type.
+  - `Html::sanitize()` and `Html::sanitize_with()` take a non-mutable reference.
+  - `NodeRef` and `Children` are now owned types and no longer implement `Copy`.
+  - `NodeData::Text`'s inner string and the `attrs` field of `ElementData` are
+    now wrapped in `RefCell`s. 
 
 Improvements:
 

crates/ruma-html/Cargo.toml

@@ -18,7 +18,7 @@ matrix = ["dep:ruma-common"]
 
 [dependencies]
 as_variant = { workspace = true }
-html5ever = "0.27.0"
+html5ever = "0.28.0"
 phf = { version = "0.11.1", features = ["macros"] }
 ruma-common = { workspace = true, optional = true }
 tracing = { workspace = true, features = ["attributes"] }

crates/ruma-html/src/helpers.rs

@@ -52,7 +52,7 @@ pub fn remove_html_reply_fallback(s: &str) -> String {
 }
 
 fn sanitize_inner(s: &str, config: &SanitizerConfig) -> String {
-    let mut html = Html::parse(s);
+    let html = Html::parse(s);
     html.sanitize_with(config);
     html.to_string()
 }

crates/ruma-html/src/html.rs

@@ -1,4 +1,10 @@
-use std::{collections::BTreeSet, fmt, io, iter::FusedIterator};
+use std::{
+    cell::RefCell,
+    collections::BTreeSet,
+    fmt, io,
+    iter::FusedIterator,
+    rc::{Rc, Weak},
+};
 
 use as_variant::as_variant;
 use html5ever::{
@@ -6,7 +12,7 @@ use html5ever::{
     serialize::{serialize, Serialize, SerializeOpts, Serializer, TraversalScope},
     tendril::{StrTendril, TendrilSink},
     tree_builder::{NodeOrText, TreeSink},
-    Attribute, ParseOpts, QualName,
+    Attribute, LocalName, ParseOpts, QualName,
 };
 use tracing::debug;
 
@@ -21,7 +27,7 @@ use crate::SanitizerConfig;
 /// parsed, note that malformed HTML and comments will be stripped from the output.
 #[derive(Debug)]
 pub struct Html {
-    pub(crate) nodes: Vec<Node>,
+    document: NodeRef,
 }
 
 impl Html {
@@ -45,179 +51,116 @@ impl Html {
     ///
     /// This is equivalent to calling [`Self::sanitize_with()`] with a `config` value of
     /// `SanitizerConfig::compat().remove_reply_fallback()`.
-    pub fn sanitize(&mut self) {
+    pub fn sanitize(&self) {
         let config = SanitizerConfig::compat().remove_reply_fallback();
         self.sanitize_with(&config);
     }
 
     /// Sanitize this HTML according to the given configuration.
-    pub fn sanitize_with(&mut self, config: &SanitizerConfig) {
+    pub fn sanitize_with(&self, config: &SanitizerConfig) {
         config.clean(self);
     }
 
-    /// Construct a new `Node` with the given data and add it to this `Html`.
-    ///
-    /// Returns the index of the new node.
-    pub(crate) fn new_node(&mut self, data: NodeData) -> usize {
-        self.nodes.push(Node::new(data));
-        self.nodes.len() - 1
-    }
-
-    /// Append the given node to the given parent in this `Html`.
-    ///
-    /// The node is detached from its previous position.
-    pub(crate) fn append_node(&mut self, parent_id: usize, node_id: usize) {
-        self.detach(node_id);
-
-        self.nodes[node_id].parent = Some(parent_id);
-        if let Some(last_child) = self.nodes[parent_id].last_child.take() {
-            self.nodes[node_id].prev_sibling = Some(last_child);
-            self.nodes[last_child].next_sibling = Some(node_id);
-        } else {
-            self.nodes[parent_id].first_child = Some(node_id);
-        }
-        self.nodes[parent_id].last_child = Some(node_id);
-    }
-
-    /// Insert the given node before the given sibling in this `Html`.
-    ///
-    /// The node is detached from its previous position.
-    pub(crate) fn insert_before(&mut self, sibling_id: usize, node_id: usize) {
-        self.detach(node_id);
-
-        self.nodes[node_id].parent = self.nodes[sibling_id].parent;
-        self.nodes[node_id].next_sibling = Some(sibling_id);
-        if let Some(prev_sibling) = self.nodes[sibling_id].prev_sibling.take() {
-            self.nodes[node_id].prev_sibling = Some(prev_sibling);
-            self.nodes[prev_sibling].next_sibling = Some(node_id);
-        } else if let Some(parent) = self.nodes[sibling_id].parent {
-            self.nodes[parent].first_child = Some(node_id);
-        }
-        self.nodes[sibling_id].prev_sibling = Some(node_id);
-    }
-
-    /// Detach the given node from this `Html`.
-    pub(crate) fn detach(&mut self, node_id: usize) {
-        let (parent, prev_sibling, next_sibling) = {
-            let node = &mut self.nodes[node_id];
-            (node.parent.take(), node.prev_sibling.take(), node.next_sibling.take())
-        };
-
-        if let Some(next_sibling) = next_sibling {
-            self.nodes[next_sibling].prev_sibling = prev_sibling;
-        } else if let Some(parent) = parent {
-            self.nodes[parent].last_child = prev_sibling;
-        }
-
-        if let Some(prev_sibling) = prev_sibling {
-            self.nodes[prev_sibling].next_sibling = next_sibling;
-        } else if let Some(parent) = parent {
-            self.nodes[parent].first_child = next_sibling;
-        }
-    }
-
-    /// Get the ID of the root node of the HTML.
-    pub(crate) fn root_id(&self) -> usize {
-        self.nodes[0].first_child.expect("html should always have a root node")
-    }
-
     /// Get the root node of the HTML.
-    pub(crate) fn root(&self) -> &Node {
+    fn root(&self) -> NodeRef {
-        &self.nodes[self.root_id()]
+        self.document.first_child().expect("html should always have a root node")
     }
 
     /// Whether the root node of the HTML has children.
     pub fn has_children(&self) -> bool {
-        self.root().first_child.is_some()
+        self.root().has_children()
     }
 
     /// The first child node of the root node of the HTML.
     ///
     /// Returns `None` if the root node has no children.
-    pub fn first_child(&self) -> Option<NodeRef<'_>> {
+    pub fn first_child(&self) -> Option<NodeRef> {
-        self.root().first_child.map(|id| NodeRef::new(self, id))
+        self.root().first_child()
     }
 
     /// The last child node of the root node of the HTML .
     ///
     /// Returns `None` if the root node has no children.
-    pub fn last_child(&self) -> Option<NodeRef<'_>> {
+    pub fn last_child(&self) -> Option<NodeRef> {
-        self.root().last_child.map(|id| NodeRef::new(self, id))
+        self.root().last_child()
     }
 
     /// Iterate through the children of the root node of the HTML.
-    pub fn children(&self) -> Children<'_> {
+    pub fn children(&self) -> Children {
         Children::new(self.first_child())
     }
 }
 
 impl Default for Html {
     fn default() -> Self {
-        Self { nodes: vec![Node::new(NodeData::Document)] }
+        Self { document: NodeRef::new(NodeData::Document) }
     }
 }
 
 impl TreeSink for Html {
-    type Handle = usize;
+    type Handle = NodeRef;
     type Output = Self;
 
     fn finish(self) -> Self::Output {
         self
     }
 
-    fn parse_error(&mut self, msg: std::borrow::Cow<'static, str>) {
+    fn parse_error(&self, msg: std::borrow::Cow<'static, str>) {
         debug!("HTML parse error: {msg}");
     }
 
-    fn get_document(&mut self) -> Self::Handle {
+    fn get_document(&self) -> Self::Handle {
-        0
+        self.document.clone()
     }
 
     fn elem_name<'a>(&'a self, target: &'a Self::Handle) -> html5ever::ExpandedName<'a> {
-        self.nodes[*target].as_element().expect("not an element").name.expanded()
+        target.as_element().expect("not an element").name.expanded()
     }
 
     fn create_element(
-        &mut self,
+        &self,
         name: QualName,
         attrs: Vec<Attribute>,
         _flags: html5ever::tree_builder::ElementFlags,
     ) -> Self::Handle {
-        self.new_node(NodeData::Element(ElementData { name, attrs: attrs.into_iter().collect() }))
+        NodeRef::new(NodeData::Element(ElementData {
+            name,
+            attrs: RefCell::new(attrs.into_iter().collect()),
+        }))
     }
 
-    fn create_comment(&mut self, _text: StrTendril) -> Self::Handle {
+    fn create_comment(&self, _text: StrTendril) -> Self::Handle {
-        self.new_node(NodeData::Other)
+        NodeRef::new(NodeData::Other)
     }
 
-    fn create_pi(&mut self, _target: StrTendril, _data: StrTendril) -> Self::Handle {
+    fn create_pi(&self, _target: StrTendril, _data: StrTendril) -> Self::Handle {
-        self.new_node(NodeData::Other)
+        NodeRef::new(NodeData::Other)
     }
 
-    fn append(&mut self, parent: &Self::Handle, child: NodeOrText<Self::Handle>) {
+    fn append(&self, parent: &Self::Handle, child: NodeOrText<Self::Handle>) {
         match child {
-            NodeOrText::AppendNode(index) => self.append_node(*parent, index),
+            NodeOrText::AppendNode(node) => parent.append_child(node),
             NodeOrText::AppendText(text) => {
                 // If the previous sibling is also text, add this text to it.
-                if let Some(sibling) =
+                if let Some(prev_text) =
-                    self.nodes[*parent].last_child.and_then(|child| self.nodes[child].as_text_mut())
+                    parent.last_child().as_ref().and_then(|sibling| sibling.as_text())
                 {
-                    sibling.push_tendril(&text);
+                    prev_text.borrow_mut().push_tendril(&text);
                 } else {
-                    let index = self.new_node(NodeData::Text(text));
+                    let node = NodeRef::new(NodeData::Text(text.into()));
-                    self.append_node(*parent, index);
+                    parent.append_child(node);
                 }
             }
         }
     }
 
     fn append_based_on_parent_node(
-        &mut self,
+        &self,
         element: &Self::Handle,
         prev_element: &Self::Handle,
         child: NodeOrText<Self::Handle>,
     ) {
-        if self.nodes[*element].parent.is_some() {
+        if element.0.parent.borrow().is_some() {
             self.append_before_sibling(element, child);
         } else {
             self.append(prev_element, child);
@@ -225,59 +168,53 @@ impl TreeSink for Html {
     }
 
     fn append_doctype_to_document(
-        &mut self,
+        &self,
         _name: StrTendril,
         _public_id: StrTendril,
         _system_id: StrTendril,
     ) {
     }
 
-    fn get_template_contents(&mut self, target: &Self::Handle) -> Self::Handle {
+    fn get_template_contents(&self, target: &Self::Handle) -> Self::Handle {
-        *target
+        target.clone()
     }
 
     fn same_node(&self, x: &Self::Handle, y: &Self::Handle) -> bool {
-        x == y
+        Rc::ptr_eq(&x.0, &y.0)
     }
 
-    fn set_quirks_mode(&mut self, _mode: html5ever::tree_builder::QuirksMode) {}
+    fn set_quirks_mode(&self, _mode: html5ever::tree_builder::QuirksMode) {}
 
-    fn append_before_sibling(
+    fn append_before_sibling(&self, sibling: &Self::Handle, new_node: NodeOrText<Self::Handle>) {
-        &mut self,
-        sibling: &Self::Handle,
-        new_node: NodeOrText<Self::Handle>,
-    ) {
         match new_node {
-            NodeOrText::AppendNode(index) => self.insert_before(*sibling, index),
+            NodeOrText::AppendNode(node) => node.insert_before_sibling(sibling),
             NodeOrText::AppendText(text) => {
                 // If the previous sibling is also text, add this text to it.
-                if let Some(prev_text) = self.nodes[*sibling]
+                if let Some(prev_text) =
-                    .prev_sibling
+                    sibling.prev_sibling().as_ref().and_then(|prev_sibling| prev_sibling.as_text())
-                    .and_then(|prev| self.nodes[prev].as_text_mut())
                 {
-                    prev_text.push_tendril(&text);
+                    prev_text.borrow_mut().push_tendril(&text);
                 } else {
-                    let index = self.new_node(NodeData::Text(text));
+                    let node = NodeRef::new(NodeData::Text(text.into()));
-                    self.insert_before(*sibling, index);
+                    node.insert_before_sibling(sibling);
                 }
             }
         }
     }
 
-    fn add_attrs_if_missing(&mut self, target: &Self::Handle, attrs: Vec<Attribute>) {
+    fn add_attrs_if_missing(&self, target: &Self::Handle, attrs: Vec<Attribute>) {
-        let target = self.nodes[*target].as_element_mut().unwrap();
+        let element = target.as_element().unwrap();
-        target.attrs.extend(attrs);
+        element.attrs.borrow_mut().extend(attrs);
     }
 
-    fn remove_from_parent(&mut self, target: &Self::Handle) {
+    fn remove_from_parent(&self, target: &Self::Handle) {
-        self.detach(*target);
+        target.detach();
     }
 
-    fn reparent_children(&mut self, node: &Self::Handle, new_parent: &Self::Handle) {
+    fn reparent_children(&self, node: &Self::Handle, new_parent: &Self::Handle) {
-        let mut next_child = self.nodes[*node].first_child;
+        for child in node.0.children.take() {
-        while let Some(child) = next_child {
+            child.0.parent.take();
-            next_child = self.nodes[child].next_sibling;
+            new_parent.append_child(child);
-            self.append_node(*new_parent, child);
         }
     }
 }
@@ -289,13 +226,8 @@ impl Serialize for Html {
     {
         match traversal_scope {
             TraversalScope::IncludeNode => {
-                let root = self.root();
+                for child in self.children() {
-
+                    child.serialize(serializer)?;
-                let mut next_child = root.first_child;
-                while let Some(child) = next_child {
-                    let child = &self.nodes[child];
-                    child.serialize(self, serializer)?;
-                    next_child = child.next_sibling;
                 }
 
                 Ok(())
@@ -324,85 +256,37 @@ impl fmt::Display for Html {
 /// An HTML node.
 #[derive(Debug)]
 #[non_exhaustive]
-pub(crate) struct Node {
+struct Node {
-    pub(crate) parent: Option<usize>,
+    parent: RefCell<Option<Weak<Node>>>,
-    pub(crate) prev_sibling: Option<usize>,
+    children: RefCell<Vec<NodeRef>>,
-    pub(crate) next_sibling: Option<usize>,
+    data: NodeData,
-    pub(crate) first_child: Option<usize>,
-    pub(crate) last_child: Option<usize>,
-    pub(crate) data: NodeData,
 }
 
 impl Node {
-    /// Constructs a new `Node` with the given data.
+    /// Constructs a new `NodeRef` with the given data.
     fn new(data: NodeData) -> Self {
-        Self {
+        Self { parent: Default::default(), children: Default::default(), data }
-            parent: None,
-            prev_sibling: None,
-            next_sibling: None,
-            first_child: None,
-            last_child: None,
-            data,
-        }
     }
 
     /// Returns the data of this `Node` if it is an Element (aka an HTML tag).
-    pub(crate) fn as_element(&self) -> Option<&ElementData> {
+    fn as_element(&self) -> Option<&ElementData> {
         as_variant!(&self.data, NodeData::Element)
     }
 
-    /// Returns the mutable `ElementData` of this `Node` if it is a `NodeData::Element`.
-    pub(crate) fn as_element_mut(&mut self) -> Option<&mut ElementData> {
-        as_variant!(&mut self.data, NodeData::Element)
-    }
-
     /// Returns the text content of this `Node`, if it is a `NodeData::Text`.
-    fn as_text(&self) -> Option<&StrTendril> {
+    fn as_text(&self) -> Option<&RefCell<StrTendril>> {
         as_variant!(&self.data, NodeData::Text)
     }
 
-    /// Returns the mutable text content of this `Node`, if it is a `NodeData::Text`.
+    /// Whether this is the root node of the HTML document.
-    fn as_text_mut(&mut self) -> Option<&mut StrTendril> {
+    fn is_root(&self) -> bool {
-        as_variant!(&mut self.data, NodeData::Text)
+        // The root node is the `html` element.
+        matches!(&self.data, NodeData::Element(element_data) if element_data.name.local.as_bytes() == b"html")
     }
-}
 
-impl Node {
+    /// The parent of this node, if any.
-    pub(crate) fn serialize<S>(&self, fragment: &Html, serializer: &mut S) -> io::Result<()>
+    fn parent(&self) -> Option<NodeRef> {
-    where
+        self.parent.borrow().as_ref()?.upgrade().map(NodeRef)
-        S: Serializer,
-    {
-        match &self.data {
-            NodeData::Element(data) => {
-                serializer.start_elem(
-                    data.name.clone(),
-                    data.attrs.iter().map(|attr| (&attr.name, &*attr.value)),
-                )?;
-
-                let mut next_child = self.first_child;
-                while let Some(child) = next_child {
-                    let child = &fragment.nodes[child];
-                    child.serialize(fragment, serializer)?;
-                    next_child = child.next_sibling;
-                }
-
-                serializer.end_elem(data.name.clone())?;
-
-                Ok(())
-            }
-            NodeData::Document => {
-                let mut next_child = self.first_child;
-                while let Some(child) = next_child {
-                    let child = &fragment.nodes[child];
-                    child.serialize(fragment, serializer)?;
-                    next_child = child.next_sibling;
-                }
-
-                Ok(())
-            }
-            NodeData::Text(text) => serializer.write_text(text),
-            _ => Ok(()),
-        }
     }
 }
 
@@ -414,7 +298,7 @@ pub enum NodeData {
     Document,
 
     /// A text node.
-    Text(StrTendril),
+    Text(RefCell<StrTendril>),
 
     /// An HTML element (aka a tag).
     Element(ElementData),
@@ -431,7 +315,7 @@ pub struct ElementData {
     pub name: QualName,
 
     /// The attributes of the element.
-    pub attrs: BTreeSet<Attribute>,
+    pub attrs: RefCell<BTreeSet<Attribute>>,
 }
 
 impl ElementData {
@@ -440,126 +324,215 @@ impl ElementData {
     /// [spec]: https://spec.matrix.org/latest/client-server-api/#mroommessage-msgtypes
     #[cfg(feature = "matrix")]
     pub fn to_matrix(&self) -> matrix::MatrixElementData {
-        matrix::MatrixElementData::parse(&self.name, &self.attrs)
+        matrix::MatrixElementData::parse(&self.name, &self.attrs.borrow())
     }
 }
 
 /// A reference to an HTML node.
-#[derive(Debug, Clone, Copy)]
+#[derive(Debug, Clone)]
 #[non_exhaustive]
-pub struct NodeRef<'a> {
+pub struct NodeRef(Rc<Node>);
-    /// The `Html` struct containing the nodes.
-    pub(crate) html: &'a Html,
-    /// The referenced node.
-    pub(crate) node: &'a Node,
-}
 
-impl<'a> NodeRef<'a> {
+impl NodeRef {
-    /// Construct a new `NodeRef` for the given HTML and node ID.
+    /// Constructs a new `NodeRef` with the given data.
-    fn new(html: &'a Html, id: usize) -> Self {
+    fn new(data: NodeData) -> Self {
-        Self { html, node: &html.nodes[id] }
+        Self(Node::new(data).into())
     }
 
-    /// Construct a new `NodeRef` from the same HTML as this node with the given node ID.
+    /// Detach this node from the tree, if it has a parent.
-    fn with_id(&self, id: usize) -> Self {
+    pub(crate) fn detach(&self) {
-        let html = self.html;
+        if let Some((parent, index)) = self.parent_and_index() {
-        Self::new(html, id)
+            parent.0.children.borrow_mut().remove(index);
+            self.0.parent.take();
+        }
+    }
+
+    /// Append the given child node to this node.
+    ///
+    /// The child node is detached from its previous position.
+    fn append_child(&self, child: NodeRef) {
+        child.detach();
+
+        child.0.parent.replace(Some(Rc::downgrade(&self.0)));
+        self.0.children.borrow_mut().push(child);
+    }
+
+    /// If this node has a parent, get it and the node's position in the parent's children.
+    fn parent_and_index(&self) -> Option<(NodeRef, usize)> {
+        let parent = self.0.parent()?;
+        let i = parent
+            .0
+            .children
+            .borrow()
+            .iter()
+            .position(|child| Rc::ptr_eq(&child.0, &self.0))
+            .expect("child should be in parent's children");
+        Some((parent, i))
+    }
+
+    /// Insert this node before the given sibling.
+    ///
+    /// This node is detached from its previous position.
+    pub(crate) fn insert_before_sibling(&self, sibling: &NodeRef) {
+        self.detach();
+
+        let (parent, index) = sibling.parent_and_index().expect("sibling should have parent");
+
+        self.0.parent.replace(Some(Rc::downgrade(&parent.0)));
+        parent.0.children.borrow_mut().insert(index, self.clone());
+    }
+
+    /// Constructs a new element `NodeRef` with the same data as this one, but with a different
+    /// element name and use it to replace this one in the parent.
+    ///
+    /// Panics if this node is not in the tree and is not an element node.
+    pub(crate) fn replace_with_element_name(self, name: LocalName) -> NodeRef {
+        let mut element_data = self.as_element().unwrap().clone();
+        element_data.name.local = name;
+
+        let new_node = NodeRef::new(NodeData::Element(element_data));
+
+        for child in self.children() {
+            new_node.append_child(child);
+        }
+
+        new_node.insert_before_sibling(&self);
+        self.detach();
+
+        new_node
     }
 
     /// The data of the node.
-    pub fn data(&self) -> &'a NodeData {
+    pub fn data(&self) -> &NodeData {
-        &self.node.data
+        &self.0.data
     }
 
-    /// Returns the data of this node if it is a `NodeData::Element`.
+    /// Returns the data of this `Node` if it is an Element (aka an HTML tag).
-    pub fn as_element(&self) -> Option<&'a ElementData> {
+    pub fn as_element(&self) -> Option<&ElementData> {
-        self.node.as_element()
+        self.0.as_element()
     }
 
-    /// Returns the text content of this node, if it is a `NodeData::Text`.
+    /// Returns the text content of this `Node`, if it is a `NodeData::Text`.
-    pub fn as_text(&self) -> Option<&'a StrTendril> {
+    pub fn as_text(&self) -> Option<&RefCell<StrTendril>> {
-        self.node.as_text()
+        self.0.as_text()
     }
 
     /// The parent node of this node.
     ///
     /// Returns `None` if the parent is the root node.
-    pub fn parent(&self) -> Option<NodeRef<'a>> {
+    pub fn parent(&self) -> Option<NodeRef> {
-        let parent_id = self.node.parent?;
+        let parent = self.0.parent()?;
 
         // We don't want users to be able to navigate to the root.
-        if parent_id == self.html.root_id() {
+        if parent.0.is_root() {
             return None;
         }
 
-        Some(self.with_id(parent_id))
+        Some(parent)
     }
 
     /// The next sibling node of this node.
     ///
     /// Returns `None` if this is the last of its siblings.
-    pub fn next_sibling(&self) -> Option<NodeRef<'a>> {
+    pub fn next_sibling(&self) -> Option<NodeRef> {
-        Some(self.with_id(self.node.next_sibling?))
+        let (parent, index) = self.parent_and_index()?;
+        let index = index.checked_add(1)?;
+        let sibling = parent.0.children.borrow().get(index).cloned();
+        sibling
     }
 
     /// The previous sibling node of this node.
     ///
     /// Returns `None` if this is the first of its siblings.
-    pub fn prev_sibling(&self) -> Option<NodeRef<'a>> {
+    pub fn prev_sibling(&self) -> Option<NodeRef> {
-        Some(self.with_id(self.node.prev_sibling?))
+        let (parent, index) = self.parent_and_index()?;
+        let index = index.checked_sub(1)?;
+        let sibling = parent.0.children.borrow().get(index).cloned();
+        sibling
     }
 
     /// Whether this node has children.
     pub fn has_children(&self) -> bool {
-        self.node.first_child.is_some()
+        !self.0.children.borrow().is_empty()
     }
 
     /// The first child node of this node.
     ///
     /// Returns `None` if this node has no children.
-    pub fn first_child(&self) -> Option<NodeRef<'a>> {
+    pub fn first_child(&self) -> Option<NodeRef> {
-        Some(self.with_id(self.node.first_child?))
+        self.0.children.borrow().first().cloned()
     }
 
     /// The last child node of this node.
     ///
     /// Returns `None` if this node has no children.
-    pub fn last_child(&self) -> Option<NodeRef<'a>> {
+    pub fn last_child(&self) -> Option<NodeRef> {
-        Some(self.with_id(self.node.last_child?))
+        self.0.children.borrow().last().cloned()
     }
 
     /// Get an iterator through the children of this node.
-    pub fn children(&self) -> Children<'a> {
+    pub fn children(&self) -> Children {
         Children::new(self.first_child())
     }
+
+    pub(crate) fn serialize<S>(&self, serializer: &mut S) -> io::Result<()>
+    where
+        S: Serializer,
+    {
+        match self.data() {
+            NodeData::Element(data) => {
+                serializer.start_elem(
+                    data.name.clone(),
+                    data.attrs.borrow().iter().map(|attr| (&attr.name, &*attr.value)),
+                )?;
+
+                for child in self.children() {
+                    child.serialize(serializer)?;
+                }
+
+                serializer.end_elem(data.name.clone())?;
+
+                Ok(())
+            }
+            NodeData::Document => {
+                for child in self.children() {
+                    child.serialize(serializer)?;
+                }
+
+                Ok(())
+            }
+            NodeData::Text(text) => serializer.write_text(&text.borrow()),
+            _ => Ok(()),
+        }
+    }
 }
 
 /// An iterator through the children of a node.
 ///
 /// Can be constructed with [`Html::children()`] or [`NodeRef::children()`].
-#[derive(Debug, Clone, Copy)]
+#[derive(Debug, Clone)]
-pub struct Children<'a> {
+pub struct Children {
-    next: Option<NodeRef<'a>>,
+    next: Option<NodeRef>,
 }
 
-impl<'a> Children<'a> {
+impl Children {
     /// Construct a `Children` starting from the given node.
-    fn new(start_node: Option<NodeRef<'a>>) -> Self {
+    fn new(start_node: Option<NodeRef>) -> Self {
         Self { next: start_node }
     }
 }
 
-impl<'a> Iterator for Children<'a> {
+impl Iterator for Children {
-    type Item = NodeRef<'a>;
+    type Item = NodeRef;
 
     fn next(&mut self) -> Option<Self::Item> {
-        let next = self.next?;
+        let next = self.next.take()?;
         self.next = next.next_sibling();
         Some(next)
     }
 }
 
-impl<'a> FusedIterator for Children<'a> {}
+impl FusedIterator for Children {}
 
 #[cfg(test)]
 mod tests {

crates/ruma-html/src/sanitizer_config/clean.rs

@@ -2,7 +2,7 @@ use html5ever::{tendril::StrTendril, Attribute, LocalName};
 use phf::{phf_map, phf_set, Map, Set};
 use wildmatch::WildMatch;
 
-use crate::{ElementData, Html, HtmlSanitizerMode, NodeData, SanitizerConfig};
+use crate::{ElementData, Html, HtmlSanitizerMode, NodeData, NodeRef, SanitizerConfig};
 
 /// HTML elements allowed in the Matrix specification.
 static ALLOWED_ELEMENTS_STRICT: Set<&str> = phf_set! {
@@ -104,43 +104,41 @@ impl SanitizerConfig {
     }
 
     /// Clean the given HTML with this sanitizer.
-    pub(crate) fn clean(&self, html: &mut Html) {
+    pub(crate) fn clean(&self, html: &Html) {
-        let root = html.root();
+        for child in html.children() {
-        let mut next_child = root.first_child;
+            self.clean_node(child, 0);
-
-        while let Some(child) = next_child {
-            next_child = html.nodes[child].next_sibling;
-            self.clean_node(html, child, 0);
         }
     }
 
-    fn clean_node(&self, html: &mut Html, node_id: usize, depth: u32) {
+    fn clean_node(&self, node: NodeRef, depth: u32) {
-        self.apply_replacements(html, node_id);
+        let node = self.apply_replacements(node);
 
-        let action = self.node_action(html, node_id, depth);
+        let action = self.node_action(&node, depth);
 
         if action != NodeAction::Remove {
-            let mut next_child = html.nodes[node_id].first_child;
+            for child in node.children() {
-            while let Some(child) = next_child {
-                next_child = html.nodes[child].next_sibling;
-
                 if action == NodeAction::Ignore {
-                    html.insert_before(node_id, child);
+                    child.insert_before_sibling(&node);
                 }
 
-                self.clean_node(html, child, depth + 1);
+                self.clean_node(child, depth + 1);
             }
         }
 
         if matches!(action, NodeAction::Ignore | NodeAction::Remove) {
-            html.detach(node_id);
+            node.detach();
-        } else if let Some(data) = html.nodes[node_id].as_element_mut() {
+        } else if let Some(data) = node.as_element() {
             self.clean_element_attributes(data);
         }
     }
 
-    fn apply_replacements(&self, html: &mut Html, node_id: usize) {
+    /// Apply the attributes and element name replacements to the given node.
-        if let NodeData::Element(ElementData { name, attrs, .. }) = &mut html.nodes[node_id].data {
+    ///
+    /// This might return a different node than the one provided.
+    fn apply_replacements(&self, node: NodeRef) -> NodeRef {
+        let mut element_replacement = None;
+
+        if let NodeData::Element(ElementData { name, attrs, .. }) = node.data() {
             let element_name = name.local.as_ref();
 
             // Replace attributes.
@@ -153,6 +151,7 @@ impl SanitizerConfig {
                 .flatten();
 
             if list_replacements.is_some() || mode_replacements.is_some() {
+                let mut attrs = attrs.borrow_mut();
                 *attrs = attrs
                     .clone()
                     .into_iter()
@@ -174,7 +173,7 @@ impl SanitizerConfig {
             }
 
             // Replace element.
-            let mut element_replacement = self
+            element_replacement = self
                 .replace_elements
                 .as_ref()
                 .and_then(|list| list.content.get(element_name))
@@ -191,17 +190,20 @@ impl SanitizerConfig {
                     .flatten()
                     .copied();
             }
+        }
 
-            if let Some(element_replacement) = element_replacement {
+        if let Some(element_replacement) = element_replacement {
-                name.local = LocalName::from(element_replacement);
+            node.replace_with_element_name(LocalName::from(element_replacement))
-            }
+        } else {
+            node
         }
     }
 
-    fn node_action(&self, html: &Html, node_id: usize, depth: u32) -> NodeAction {
+    fn node_action(&self, node: &NodeRef, depth: u32) -> NodeAction {
-        match &html.nodes[node_id].data {
+        match node.data() {
             NodeData::Element(ElementData { name, attrs, .. }) => {
                 let element_name = name.local.as_ref();
+                let attrs = attrs.borrow();
 
                 // Check if element should be removed.
                 if self.remove_elements.as_ref().is_some_and(|set| set.contains(element_name)) {
@@ -321,9 +323,10 @@ impl SanitizerConfig {
         }
     }
 
-    fn clean_element_attributes(&self, data: &mut ElementData) {
+    fn clean_element_attributes(&self, data: &ElementData) {
         let ElementData { name, attrs } = data;
         let element_name = name.local.as_ref();
+        let mut attrs = attrs.borrow_mut();
 
         let list_remove_attrs = self.remove_attrs.as_ref().and_then(|map| map.get(element_name));
 

crates/ruma-html/tests/it/html/navigate.rs

@@ -21,7 +21,7 @@ fn navigate_tree() {
 
     let h1_element = h1_node.as_element().unwrap();
     assert_eq!(&h1_element.name.local, "h1");
-    assert!(h1_element.attrs.is_empty());
+    assert!(h1_element.attrs.borrow().is_empty());
 
     assert!(h1_node.parent().is_none());
     assert!(h1_node.next_sibling().is_some());
@@ -35,7 +35,7 @@ fn navigate_tree() {
     // Text of `<h1>` element.
     let h1_text_node = h1_children.next().unwrap();
     let h1_text = h1_text_node.as_text().unwrap();
-    assert_eq!(h1_text.as_ref(), "Title");
+    assert_eq!(h1_text.borrow().as_ref(), "Title");
 
     assert!(h1_text_node.parent().is_some());
     assert!(h1_text_node.next_sibling().is_none());
@@ -54,8 +54,9 @@ fn navigate_tree() {
 
     let div_element = div_node.as_element().unwrap();
     assert_eq!(&div_element.name.local, "div");
-    assert_eq!(div_element.attrs.len(), 1);
+    let attrs = div_element.attrs.borrow();
-    let class_attr = div_element.attrs.first().unwrap();
+    assert_eq!(attrs.len(), 1);
+    let class_attr = attrs.first().unwrap();
     assert_eq!(&class_attr.name.local, "class");
     assert_eq!(class_attr.value.as_ref(), "text");
 
@@ -73,7 +74,7 @@ fn navigate_tree() {
 
     let p_element = p_node.as_element().unwrap();
     assert_eq!(&p_element.name.local, "p");
-    assert!(p_element.attrs.is_empty());
+    assert!(p_element.attrs.borrow().is_empty());
 
     assert!(p_node.parent().is_some());
     assert!(p_node.next_sibling().is_none());
@@ -87,7 +88,7 @@ fn navigate_tree() {
     // Text of `<p>` element.
     let p_text_node = p_children.next().unwrap();
     let p_text = p_text_node.as_text().unwrap();
-    assert_eq!(p_text.as_ref(), "This is some ");
+    assert_eq!(p_text.borrow().as_ref(), "This is some ");
 
     assert!(p_text_node.parent().is_some());
     assert!(p_text_node.next_sibling().is_some());
@@ -104,7 +105,7 @@ fn navigate_tree() {
 
     let em_element = em_node.as_element().unwrap();
     assert_eq!(&em_element.name.local, "em");
-    assert!(em_element.attrs.is_empty());
+    assert!(em_element.attrs.borrow().is_empty());
 
     assert!(em_node.parent().is_some());
     assert!(em_node.next_sibling().is_none());
@@ -118,7 +119,7 @@ fn navigate_tree() {
     // Text of `<em>` element.
     let em_text_node = em_children.next().unwrap();
     let em_text = em_text_node.as_text().unwrap();
-    assert_eq!(em_text.as_ref(), "text");
+    assert_eq!(em_text.borrow().as_ref(), "text");
 
     assert!(em_text_node.parent().is_some());
     assert!(em_text_node.next_sibling().is_none());

crates/ruma-html/tests/it/html/sanitize.rs

@@ -6,7 +6,7 @@ use ruma_html::{
 #[test]
 fn strict_mode_valid_input() {
     let config = SanitizerConfig::strict().remove_reply_fallback();
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         <ul><li>This</li><li>has</li><li>no</li><li>tag</li></ul>\
         <p>This is a paragraph <span data-mx-color=\"green\">with some color</span></p>\
@@ -30,7 +30,7 @@ fn strict_mode_valid_input() {
 #[test]
 fn strict_mode_elements_remove() {
     let config = SanitizerConfig::strict();
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         <mx-reply>\
             <blockquote>\
@@ -66,7 +66,7 @@ fn strict_mode_elements_remove() {
 #[test]
 fn strict_mode_elements_reply_remove() {
     let config = SanitizerConfig::strict().remove_reply_fallback();
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         <mx-reply>\
             <blockquote>\
@@ -94,7 +94,7 @@ fn strict_mode_elements_reply_remove() {
 #[test]
 fn remove_only_reply_fallback() {
     let config = SanitizerConfig::new().remove_reply_fallback();
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         <mx-reply>\
             <blockquote>\
@@ -122,7 +122,7 @@ fn remove_only_reply_fallback() {
 #[test]
 fn strict_mode_attrs_remove() {
     let config = SanitizerConfig::strict();
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         <h1 id=\"anchor1\">Title for important stuff</h1>\
         <p class=\"important\">Look at <span data-mx-color=\"#0000ff\" size=20>me!</span></p>\
@@ -142,7 +142,7 @@ fn strict_mode_attrs_remove() {
 #[test]
 fn strict_mode_img_remove_scheme() {
     let config = SanitizerConfig::strict();
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         <p>Look at that picture:</p>\
         <img src=\"https://notareal.hs/abcdef\">\
@@ -156,7 +156,7 @@ fn strict_mode_img_remove_scheme() {
 #[test]
 fn strict_mode_link_remove_scheme() {
     let config = SanitizerConfig::strict();
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         <p>Go see <a href=\"file://local/file.html\">my local website</a></p>\
         ",
@@ -174,7 +174,7 @@ fn strict_mode_link_remove_scheme() {
 #[test]
 fn compat_mode_link_remove_scheme() {
     let config = SanitizerConfig::strict();
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         <p>Join <a href=\"matrix:r/myroom:notareal.hs\">my room</a></p>\
         <p>To talk about <a href=\"https://mycat.org\">my cat</a></p>\
@@ -190,7 +190,7 @@ fn compat_mode_link_remove_scheme() {
     );
 
     let config = SanitizerConfig::compat();
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         <p>Join <a href=\"matrix:r/myroom:notareal.hs\">my room</a></p>\
         <p>To talk about <a href=\"https://mycat.org\">my cat</a></p>\
@@ -209,7 +209,7 @@ fn compat_mode_link_remove_scheme() {
 #[test]
 fn strict_mode_class_remove() {
     let config = SanitizerConfig::strict();
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         <pre><code class=\"language-rust custom-class\">
             type StringList = Vec&lt;String&gt;;
@@ -242,7 +242,7 @@ fn strict_mode_depth_remove() {
         .chain(std::iter::repeat("</div>").take(100))
         .collect();
 
-    let mut html = Html::parse(&deeply_nested_html);
+    let html = Html::parse(&deeply_nested_html);
     html.sanitize_with(&config);
 
     let res = html.to_string();
@@ -253,7 +253,7 @@ fn strict_mode_depth_remove() {
 #[test]
 fn strict_mode_replace_deprecated() {
     let config = SanitizerConfig::strict();
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         <p>Look at <strike>you </strike><font data-mx-bg-color=\"#ff0000\" color=\"#0000ff\">me!</span></p>\
         ",
@@ -271,7 +271,7 @@ fn strict_mode_replace_deprecated() {
 #[test]
 fn allow_elements() {
     let config = SanitizerConfig::new().allow_elements(["ul", "li", "p", "img"], ListBehavior::Add);
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         <ul><li>This</li><li>has</li><li>no</li><li>tag</li></ul>\
         <p>This is a paragraph <span data-mx-color=\"green\">with some color</span></p>\
@@ -296,7 +296,7 @@ fn allow_elements() {
 fn override_elements() {
     let config =
         SanitizerConfig::strict().allow_elements(["ul", "li", "p", "img"], ListBehavior::Override);
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         <ul><li>This</li><li>has</li><li>no</li><li>tag</li></ul>\
         <p>This is a paragraph <span data-mx-color=\"green\">with some color</span></p>\
@@ -320,7 +320,7 @@ fn override_elements() {
 #[test]
 fn add_elements() {
     let config = SanitizerConfig::strict().allow_elements(["keep-me"], ListBehavior::Add);
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         <ul><li>This</li><li>has</li><li>no</li><li>tag</li></ul>\
         <p>This is a paragraph <span data-mx-color=\"green\">with some color</span></p>\
@@ -346,7 +346,7 @@ fn add_elements() {
 #[test]
 fn remove_elements() {
     let config = SanitizerConfig::strict().remove_elements(["span", "code"]);
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         <ul><li>This</li><li>has</li><li>no</li><li>tag</li></ul>\
         <p>This is a paragraph <span data-mx-color=\"green\">with some color</span></p>\
@@ -369,7 +369,7 @@ fn remove_elements() {
 #[test]
 fn ignore_elements() {
     let config = SanitizerConfig::new().ignore_elements(["span", "code"]);
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         <ul><li>This</li><li>has</li><li>no</li><li>tag</li></ul>\
         <p>This is a paragraph <span data-mx-color=\"green\">with some color</span></p>\
@@ -394,7 +394,7 @@ fn ignore_elements() {
 fn replace_elements() {
     let config = SanitizerConfig::new()
         .replace_elements([NameReplacement { old: "ul", new: "ol" }], ListBehavior::Add);
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         <ul><li>This</li><li>has</li><li>no</li><li>tag</li></ul>\
         <p>This is a paragraph <span data-mx-color=\"green\">with some color</span></p>\
@@ -419,7 +419,7 @@ fn replace_elements() {
 fn replace_elements_override() {
     let config = SanitizerConfig::strict()
         .replace_elements([NameReplacement { old: "ul", new: "ol" }], ListBehavior::Override);
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         <ul><li>This</li><li>has</li><li>no</li><li>tag</li></ul>\
         <p>This is a paragraph <span data-mx-color=\"green\">with some color</span></p>\
@@ -446,7 +446,7 @@ fn replace_elements_override() {
 fn replace_elements_add() {
     let config = SanitizerConfig::strict()
         .replace_elements([NameReplacement { old: "ul", new: "ol" }], ListBehavior::Add);
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         <ul><li>This</li><li>has</li><li>no</li><li>tag</li></ul>\
         <p>This is a paragraph <span data-mx-color=\"green\">with some color</span></p>\
@@ -475,7 +475,7 @@ fn allow_attributes() {
         [PropertiesNames { parent: "img", properties: &["src"] }],
         ListBehavior::Add,
     );
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         <ul><li>This</li><li>has</li><li>no</li><li>tag</li></ul>\
         <p>This is a paragraph <span data-mx-color=\"green\">with some color</span></p>\
@@ -502,7 +502,7 @@ fn override_attributes() {
         [PropertiesNames { parent: "img", properties: &["src"] }],
         ListBehavior::Override,
     );
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         <ul><li>This</li><li>has</li><li>no</li><li>tag</li></ul>\
         <p>This is a paragraph <span data-mx-color=\"green\">with some color</span></p>\
@@ -529,7 +529,7 @@ fn add_attributes() {
         [PropertiesNames { parent: "img", properties: &["id"] }],
         ListBehavior::Add,
     );
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         <ul><li>This</li><li>has</li><li>no</li><li>tag</li></ul>\
         <p>This is a paragraph <span data-mx-color=\"green\">with some color</span></p>\
@@ -554,7 +554,7 @@ fn add_attributes() {
 fn remove_attributes() {
     let config = SanitizerConfig::strict()
         .remove_attributes([PropertiesNames { parent: "span", properties: &["data-mx-color"] }]);
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         <ul><li>This</li><li>has</li><li>no</li><li>tag</li></ul>\
         <p>This is a paragraph <span data-mx-color=\"green\">with some color</span></p>\
@@ -584,7 +584,7 @@ fn replace_attributes() {
         }],
         ListBehavior::Add,
     );
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         <ul><li>This</li><li>has</li><li>no</li><li>tag</li></ul>\
         <p>This is a paragraph <span data-mx-color=\"green\">with some color</span></p>\
@@ -614,7 +614,7 @@ fn replace_attributes_override() {
         }],
         ListBehavior::Override,
     );
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         <ul><li>This</li><li>has</li><li>no</li><li>tag</li></ul>\
         <p>This is a paragraph <font color=\"green\">with some color</font></p>\
@@ -644,7 +644,7 @@ fn replace_attributes_add() {
         }],
         ListBehavior::Add,
     );
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         <ul><li>This</li><li>has</li><li>no</li><li>tag</li></ul>\
         <p>This is a paragraph <font color=\"green\">with some color</font></p>\
@@ -674,7 +674,7 @@ fn allow_schemes() {
         }],
         ListBehavior::Add,
     );
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         <img src=\"mxc://notareal.hs/abcdef\">\
         <img src=\"https://notareal.hs/abcdef.png\">\
@@ -699,7 +699,7 @@ fn override_schemes() {
         }],
         ListBehavior::Override,
     );
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         <img src=\"mxc://notareal.hs/abcdef\">\
         <img src=\"https://notareal.hs/abcdef.png\">\
@@ -724,7 +724,7 @@ fn add_schemes() {
         }],
         ListBehavior::Add,
     );
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         <img src=\"mxc://notareal.hs/abcdef\">\
         <img src=\"https://notareal.hs/abcdef.png\">\
@@ -747,7 +747,7 @@ fn deny_schemes() {
         element: "a",
         attr_schemes: &[PropertiesNames { parent: "href", properties: &["http"] }],
     }]);
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         <a href=\"https://notareal.hs/abcdef.png\">Secure link to an image</a>\
         <a href=\"http://notareal.hs/abcdef.png\">Insecure link to an image</a>\
@@ -770,7 +770,7 @@ fn allow_classes() {
         [PropertiesNames { parent: "img", properties: &["custom-class", "custom-class-*"] }],
         ListBehavior::Add,
     );
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         <code class=\"language-html\">&lt;mx-reply&gt;This is a fake reply&lt;/mx-reply&gt;</code>\
         <img class=\"custom-class custom-class-img img\" src=\"mxc://notareal.hs/abcdef\">\
@@ -793,7 +793,7 @@ fn override_classes() {
         [PropertiesNames { parent: "code", properties: &["custom-class", "custom-class-*"] }],
         ListBehavior::Override,
     );
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         <code class=\"language-html custom-class custom-class-code code\">&lt;mx-reply&gt;This is a fake reply&lt;/mx-reply&gt;</code>\
         ",
@@ -814,7 +814,7 @@ fn add_classes() {
         [PropertiesNames { parent: "code", properties: &["custom-class", "custom-class-*"] }],
         ListBehavior::Add,
     );
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         <code class=\"language-html custom-class custom-class-code code\">&lt;mx-reply&gt;This is a fake reply&lt;/mx-reply&gt;</code>\
         ",
@@ -833,7 +833,7 @@ fn add_classes() {
 fn remove_classes() {
     let config = SanitizerConfig::strict()
         .remove_classes([PropertiesNames { parent: "code", properties: &["language-rust"] }]);
-    let mut html = Html::parse(
+    let html = Html::parse(
         "\
         <code class=\"language-html language-rust\">&lt;mx-reply&gt;This is a fake reply&lt;/mx-reply&gt;</code>\
         ",

crates/ruma-macros/Cargo.toml

@@ -14,7 +14,14 @@ rust-version = { workspace = true }
 [lib]
 proc-macro = true
 
+[features]
+# Make the request and response macros expand internal derives they would
+# usually emit in the `#[derive()]` list directly, such that Rust Analyzer's
+# expand macro helper can render their output. Requires a nightly toolchain.
+__internal_macro_expand = ["syn/visit-mut"]
+
 [dependencies]
+cfg-if = "1.0.0"
 once_cell = "1.13.0"
 proc-macro-crate = "3.1.0"
 proc-macro2 = "1.0.24"

crates/ruma-macros/src/api/request.rs

@@ -1,9 +1,10 @@
+use cfg_if::cfg_if;
 use proc_macro2::TokenStream;
 use quote::{quote, ToTokens};
 use syn::{
     parse::{Parse, ParseStream},
     punctuated::Punctuated,
-    DeriveInput, Field, Generics, Ident, ItemStruct, Token, Type,
+    Field, Generics, Ident, ItemStruct, Token, Type,
 };
 
 use super::{
@@ -26,13 +27,34 @@ pub fn expand_request(attr: RequestAttr, item: ItemStruct) -> TokenStream {
         |DeriveRequestMeta::Error(ty)| quote! { #ty },
     );
 
+    cfg_if! {
+        if #[cfg(feature = "__internal_macro_expand")] {
+            use syn::parse_quote;
+
+            let mut derive_input = item.clone();
+            derive_input.attrs.push(parse_quote! { #[ruma_api(error = #error_ty)] });
+            crate::util::cfg_expand_struct(&mut derive_input);
+
+            let extra_derive = quote! { #ruma_macros::_FakeDeriveRumaApi };
+            let ruma_api_attribute = quote! {};
+            let request_impls =
+                expand_derive_request(derive_input).unwrap_or_else(syn::Error::into_compile_error);
+        } else {
+            let extra_derive = quote! { #ruma_macros::Request };
+            let ruma_api_attribute = quote! { #[ruma_api(error = #error_ty)] };
+            let request_impls = quote! {};
+        }
+    }
+
     quote! {
         #maybe_feature_error
 
-        #[derive(Clone, Debug, #ruma_macros::Request, #ruma_common::serde::_FakeDeriveSerde)]
+        #[derive(Clone, Debug, #ruma_common::serde::_FakeDeriveSerde, #extra_derive)]
         #[cfg_attr(not(feature = "unstable-exhaustive-types"), non_exhaustive)]
-        #[ruma_api(error = #error_ty)]
+        #ruma_api_attribute
         #item
+
+        #request_impls
     }
 }
 
@@ -44,13 +66,9 @@ impl Parse for RequestAttr {
     }
 }
 
-pub fn expand_derive_request(input: DeriveInput) -> syn::Result<TokenStream> {
+pub fn expand_derive_request(input: ItemStruct) -> syn::Result<TokenStream> {
-    let fields = match input.data {
+    let fields =
-        syn::Data::Struct(s) => s.fields,
+        input.fields.into_iter().map(RequestField::try_from).collect::<syn::Result<_>>()?;
-        _ => panic!("This derive macro only works on structs"),
-    };
-
-    let fields = fields.into_iter().map(RequestField::try_from).collect::<syn::Result<_>>()?;
 
     let mut error_ty = None;
 

crates/ruma-macros/src/api/response.rs

@@ -1,12 +1,13 @@
 use std::ops::Not;
 
+use cfg_if::cfg_if;
 use proc_macro2::TokenStream;
 use quote::{quote, ToTokens};
 use syn::{
     parse::{Parse, ParseStream},
     punctuated::Punctuated,
     visit::Visit,
-    DeriveInput, Field, Generics, Ident, ItemStruct, Lifetime, Token, Type,
+    Field, Generics, Ident, ItemStruct, Lifetime, Token, Type,
 };
 
 use super::{
@@ -41,13 +42,38 @@ pub fn expand_response(attr: ResponseAttr, item: ItemStruct) -> TokenStream {
         })
         .unwrap_or_else(|| quote! { OK });
 
+    cfg_if! {
+        if #[cfg(feature = "__internal_macro_expand")] {
+            use syn::parse_quote;
+
+            let mut derive_input = item.clone();
+            derive_input.attrs.push(parse_quote! {
+                #[ruma_api(error = #error_ty, status = #status_ident)]
+            });
+            crate::util::cfg_expand_struct(&mut derive_input);
+
+            let extra_derive = quote! { #ruma_macros::_FakeDeriveRumaApi };
+            let ruma_api_attribute = quote! {};
+            let response_impls =
+                expand_derive_response(derive_input).unwrap_or_else(syn::Error::into_compile_error);
+        } else {
+            let extra_derive = quote! { #ruma_macros::Response };
+            let ruma_api_attribute = quote! {
+                #[ruma_api(error = #error_ty, status = #status_ident)]
+            };
+            let response_impls = quote! {};
+        }
+    }
+
     quote! {
         #maybe_feature_error
 
-        #[derive(Clone, Debug, #ruma_macros::Response, #ruma_common::serde::_FakeDeriveSerde)]
+        #[derive(Clone, Debug, #ruma_common::serde::_FakeDeriveSerde, #extra_derive)]
         #[cfg_attr(not(feature = "unstable-exhaustive-types"), non_exhaustive)]
-        #[ruma_api(error = #error_ty, status = #status_ident)]
+        #ruma_api_attribute
         #item
+
+        #response_impls
     }
 }
 
@@ -59,13 +85,9 @@ impl Parse for ResponseAttr {
     }
 }
 
-pub fn expand_derive_response(input: DeriveInput) -> syn::Result<TokenStream> {
+pub fn expand_derive_response(input: ItemStruct) -> syn::Result<TokenStream> {
-    let fields = match input.data {
+    let fields =
-        syn::Data::Struct(s) => s.fields,
+        input.fields.into_iter().map(ResponseField::try_from).collect::<syn::Result<_>>()?;
-        _ => panic!("This derive macro only works on structs"),
-    };
-
-    let fields = fields.into_iter().map(ResponseField::try_from).collect::<syn::Result<_>>()?;
     let mut manual_body_serde = false;
     let mut error_ty = None;
     let mut status_ident = None;
@@ -90,8 +112,8 @@ pub fn expand_derive_response(input: DeriveInput) -> syn::Result<TokenStream> {
         generics: input.generics,
         fields,
         manual_body_serde,
-        error_ty: error_ty.unwrap(),
+        error_ty: error_ty.expect("missing error_ty attribute"),
-        status_ident: status_ident.unwrap(),
+        status_ident: status_ident.expect("missing status_ident attribute"),
     };
 
     response.check()?;

crates/ruma-macros/src/lib.rs

@@ -4,6 +4,7 @@
 //!
 //! See the documentation for the individual macros for usage details.
 
+#![cfg_attr(feature = "__internal_macro_expand", feature(proc_macro_expand))]
 #![warn(missing_docs)]
 #![allow(unreachable_pub)]
 // https://github.com/rust-lang/rust-clippy/issues/9029
@@ -397,14 +398,14 @@ pub fn response(attr: TokenStream, item: TokenStream) -> TokenStream {
 /// Internal helper that the request macro delegates most of its work to.
 #[proc_macro_derive(Request, attributes(ruma_api))]
 pub fn derive_request(input: TokenStream) -> TokenStream {
-    let input = parse_macro_input!(input as DeriveInput);
+    let input = parse_macro_input!(input);
     expand_derive_request(input).unwrap_or_else(syn::Error::into_compile_error).into()
 }
 
 /// Internal helper that the response macro delegates most of its work to.
 #[proc_macro_derive(Response, attributes(ruma_api))]
 pub fn derive_response(input: TokenStream) -> TokenStream {
-    let input = parse_macro_input!(input as DeriveInput);
+    let input = parse_macro_input!(input);
     expand_derive_response(input).unwrap_or_else(syn::Error::into_compile_error).into()
 }
 

crates/ruma-macros/src/util.rs

@@ -91,3 +91,114 @@ impl ToTokens for PrivateField<'_> {
         ty.to_tokens(tokens);
     }
 }
+
+#[cfg(feature = "__internal_macro_expand")]
+pub fn cfg_expand_struct(item: &mut syn::ItemStruct) {
+    use std::mem;
+
+    use proc_macro2::TokenTree;
+    use syn::{visit_mut::VisitMut, Fields, LitBool, Meta};
+
+    fn eval_cfg(cfg_expr: TokenStream) -> Option<bool> {
+        let cfg_macro_call = quote! { ::core::cfg!(#cfg_expr) };
+        let expanded = match proc_macro::TokenStream::from(cfg_macro_call).expand_expr() {
+            Ok(t) => t,
+            Err(e) => {
+                eprintln!("Failed to expand cfg! {e}");
+                return None;
+            }
+        };
+
+        let lit: LitBool = syn::parse(expanded).expect("cfg! must expand to a boolean literal");
+        Some(lit.value())
+    }
+
+    fn tokentree_not_comma(tree: &TokenTree) -> bool {
+        match tree {
+            TokenTree::Punct(p) => p.as_char() != ',',
+            _ => true,
+        }
+    }
+
+    struct CfgAttrExpand;
+
+    impl VisitMut for CfgAttrExpand {
+        fn visit_attribute_mut(&mut self, attr: &mut syn::Attribute) {
+            if attr.meta.path().is_ident("cfg_attr") {
+                // Ignore invalid cfg attributes
+                let Meta::List(list) = &attr.meta else { return };
+                let mut token_iter = list.tokens.clone().into_iter();
+
+                // Take all the tokens until the first toplevel comma.
+                // That's the cfg-expression part of cfg_attr.
+                let cfg_expr: TokenStream =
+                    token_iter.by_ref().take_while(tokentree_not_comma).collect();
+
+                let Some(cfg_value) = eval_cfg(cfg_expr) else { return };
+                if cfg_value {
+                    // If we had the whole attribute list and could emit more
+                    // than one attribute, we'd split the remaining arguments to
+                    // cfg_attr by commas and turn them into regular attributes
+                    //
+                    // Because we can emit only one, do the first and error if
+                    // there's any more after it.
+                    let attr_tokens: TokenStream =
+                        token_iter.by_ref().take_while(tokentree_not_comma).collect();
+
+                    if attr_tokens.is_empty() {
+                        // no-op cfg_attr??
+                        return;
+                    }
+
+                    attr.meta = syn::parse2(attr_tokens)
+                        .expect("syn must be able to parse cfg-attr arguments as syn::Meta");
+
+                    let rest: TokenStream = token_iter.collect();
+                    assert!(
+                        rest.is_empty(),
+                        "cfg_attr's with multiple arguments after the cfg expression are not \
+                         currently supported by __internal_macro_expand."
+                    );
+                }
+            }
+        }
+    }
+
+    CfgAttrExpand.visit_item_struct_mut(item);
+
+    let Fields::Named(fields) = &mut item.fields else {
+        panic!("only named fields are currently supported by __internal_macro_expand");
+    };
+
+    // Take out all the fields
+    'fields: for mut field in mem::take(&mut fields.named) {
+        // Take out all the attributes
+        for attr in mem::take(&mut field.attrs) {
+            // For non-cfg attrs, put them back
+            if !attr.meta.path().is_ident("cfg") {
+                field.attrs.push(attr);
+                continue;
+            }
+
+            // Also put back / ignore invalid cfg attributes
+            let Meta::List(list) = &attr.meta else {
+                field.attrs.push(attr);
+                continue;
+            };
+            // Also put back / ignore cfg attributes we can't eval
+            let Some(cfg_value) = eval_cfg(list.tokens.clone()) else {
+                field.attrs.push(attr);
+                continue;
+            };
+
+            // Finally, if the cfg is `false`, skip the part where it's put back
+            if !cfg_value {
+                continue 'fields;
+            }
+        }
+
+        // If `continue 'fields` above wasn't hit, we didn't find a cfg that
+        // evals to false, so put the field back
+        fields.named.push(field);
+    }
+}

xtask/src/ci.rs

@@ -28,8 +28,6 @@ pub enum CiCmd {
     Msrv,
     /// Check all crates with all features (msrv)
     MsrvAll,
-    /// Check ruma-client with default features (msrv)
-    MsrvClient,
     /// Check ruma crate with default features (msrv)
     MsrvRuma,
     /// Check ruma-identifiers with `ruma_identifiers_storage="Box"`
@@ -101,7 +99,6 @@ impl CiTask {
         match self.cmd {
             Some(CiCmd::Msrv) => self.msrv()?,
             Some(CiCmd::MsrvAll) => self.msrv_all()?,
-            Some(CiCmd::MsrvClient) => self.msrv_client()?,
             Some(CiCmd::MsrvRuma) => self.msrv_ruma()?,
             Some(CiCmd::MsrvOwnedIdBox) => self.msrv_owned_id_box()?,
             Some(CiCmd::MsrvOwnedIdArc) => self.msrv_owned_id_arc()?,
@@ -139,21 +136,20 @@ impl CiTask {
     /// Check that the crates compile with the MSRV.
     fn msrv(&self) -> Result<()> {
         self.msrv_all()?;
-        self.msrv_client()?;
         self.msrv_ruma()
     }
 
     /// Check all crates with all features with the MSRV, except:
     /// * ruma (would pull in ruma-signatures)
-    /// * ruma-client (tested only with client-api feature due to most / all optional HTTP client
+    /// * ruma-macros (it's still pulled as a dependency but don't want to enable its nightly-only
-    ///   deps having less strict MSRV)
+    ///   internal feature here)
     /// * ruma-signatures (MSRV exception)
     /// * xtask (no real reason to enforce an MSRV for it)
     fn msrv_all(&self) -> Result<()> {
         cmd!(
             "rustup run {MSRV} cargo check --workspace --all-features
                 --exclude ruma
-                --exclude ruma-client
+                --exclude ruma-macros
                 --exclude ruma-signatures
                 --exclude xtask"
         )
@@ -161,13 +157,6 @@ impl CiTask {
         .map_err(Into::into)
     }
 
-    /// Check ruma-client with default features with the MSRV.
-    fn msrv_client(&self) -> Result<()> {
-        cmd!("rustup run {MSRV} cargo check -p ruma-client --features client-api")
-            .run()
-            .map_err(Into::into)
-    }
-
     /// Check ruma crate with default features with the MSRV.
     fn msrv_ruma(&self) -> Result<()> {
         cmd!("rustup run {MSRV} cargo check -p ruma").run().map_err(Into::into)
@@ -185,7 +174,14 @@ impl CiTask {
 
     /// Check all crates with all features with the stable version.
     fn stable_all(&self) -> Result<()> {
-        cmd!("rustup run stable cargo check --workspace --all-features").run().map_err(Into::into)
+        // ruma-macros is pulled in as a dependency, but excluding it on the command line means its
+        // features don't get activated. It has only a single feature, which is nightly-only.
+        cmd!(
+            "rustup run stable cargo check
+                --workspace --all-features --exclude ruma-macros"
+        )
+        .run()
+        .map_err(Into::into)
     }
 
     /// Check ruma-client without default features with the stable version.