Make return type of TreeSink::elem_name an associated type. (servo#553)

* Allow clippy::enum_variant_names to avoid a breaking change

Signed-off-by: Nico Burns <[email protected]>

* Fix clippy::too_long_first_doc_paragraph lint

Signed-off-by: Nico Burns <[email protected]>

* Make return type of `TreeSink::elem_name` an associated type.

This allows this type to be owned or contain data protected by a RefCell
or Mutex. It can also simply be `ExpandedName` - the type which was
previously the return type of `TreeSink::elem_name`

Signed-off-by: Nico Burns <[email protected]>

---------

Signed-off-by: Nico Burns <[email protected]>

Author: nicoburns

html5ever/examples/arena.rs

@@ -12,7 +12,7 @@ extern crate typed_arena;
 
 use html5ever::interface::tree_builder::{ElementFlags, NodeOrText, QuirksMode, TreeSink};
 use html5ever::tendril::{StrTendril, TendrilSink};
-use html5ever::{parse_document, Attribute, ExpandedName, QualName};
+use html5ever::{parse_document, Attribute, QualName};
 use std::borrow::Cow;
 use std::cell::{Cell, RefCell};
 use std::collections::HashSet;
@@ -183,6 +183,7 @@ impl<'arena> Sink<'arena> {
 impl<'arena> TreeSink for Sink<'arena> {
     type Handle = Ref<'arena>;
     type Output = Ref<'arena>;
+    type ElemName<'a> = &'a QualName where Self : 'a;
 
     fn finish(self) -> Ref<'arena> {
         self.document
@@ -202,9 +203,9 @@ impl<'arena> TreeSink for Sink<'arena> {
         ptr::eq::<Node>(*x, *y)
     }
 
-    fn elem_name<'a>(&self, target: &'a Ref<'arena>) -> ExpandedName<'a> {
+    fn elem_name(&self, target: &Ref<'arena>) -> Self::ElemName<'_> {
         match target.data {
-            NodeData::Element { ref name, .. } => name.expanded(),
+            NodeData::Element { ref name, .. } => name,
             _ => panic!("not an element!"),
         }
     }

html5ever/examples/noop-tree-builder.rs

@@ -40,6 +40,7 @@ impl Sink {
 impl TreeSink for Sink {
     type Handle = usize;
     type Output = Self;
+    type ElemName<'a> = ExpandedName<'a>;
     fn finish(self) -> Self {
         self
     }

html5ever/examples/print-tree-actions.rs

@@ -11,7 +11,7 @@
 extern crate html5ever;
 
 use std::borrow::Cow;
-use std::cell::{Cell, RefCell};
+use std::cell::{Cell, Ref, RefCell};
 use std::collections::HashMap;
 use std::io;
 
@@ -20,7 +20,7 @@ use html5ever::tendril::*;
 use html5ever::tree_builder::{
     AppendNode, AppendText, ElementFlags, NodeOrText, QuirksMode, TreeSink,
 };
-use html5ever::{Attribute, ExpandedName, QualName};
+use html5ever::{Attribute, QualName};
 
 struct Sink {
     next_id: Cell<usize>,
@@ -38,6 +38,7 @@ impl Sink {
 impl TreeSink for Sink {
     type Handle = usize;
     type Output = Self;
+    type ElemName<'a> = Ref<'a, QualName>;
     fn finish(self) -> Self {
         self
     }
@@ -68,13 +69,10 @@ impl TreeSink for Sink {
         x == y
     }
 
-    fn elem_name(&self, target: &usize) -> ExpandedName {
-        self.names
-            .borrow()
-            .get(target)
-            .cloned()
-            .expect("not an element")
-            .expanded()
+    fn elem_name(&self, target: &usize) -> Self::ElemName<'_> {
+        Ref::map(self.names.borrow(), |map| {
+            *map.get(target).expect("not an element")
+        })
     }
 
     fn create_element(&self, name: QualName, _: Vec<Attribute>, _: ElementFlags) -> usize {

html5ever/src/tree_builder/mod.rs

@@ -9,7 +9,9 @@
 
 //! The HTML5 tree builder.
 
-pub use crate::interface::{create_element, ElementFlags, NextParserState, Tracer, TreeSink};
+pub use crate::interface::{
+    create_element, ElemName, ElementFlags, NextParserState, Tracer, TreeSink,
+};
 pub use crate::interface::{AppendNode, AppendText, Attribute, NodeOrText};
 pub use crate::interface::{LimitedQuirks, NoQuirks, Quirks, QuirksMode};
 
@@ -187,7 +189,8 @@ where
         opts: TreeBuilderOpts,
     ) -> TreeBuilder<Handle, Sink> {
         let doc_handle = sink.get_document();
-        let context_is_template = sink.elem_name(&context_elem) == expanded_name!(html "template");
+        let context_is_template =
+            sink.elem_name(&context_elem).expanded() == expanded_name!(html "template");
         let template_modes = if context_is_template {
             RefCell::new(vec![InTemplate])
         } else {
@@ -231,7 +234,8 @@ where
     pub fn tokenizer_state_for_context_elem(&self) -> tok_state::State {
         let context_elem = self.context_elem.borrow();
         let elem = context_elem.as_ref().expect("no context element");
-        let name = match self.sink.elem_name(elem) {
+        let elem_name = self.sink.elem_name(elem);
+        let name = match elem_name.expanded() {
             ExpandedName {
                 ns: &ns!(html),
                 local,
@@ -296,8 +300,8 @@ where
         print!("    open_elems:");
         for node in self.open_elems.borrow().iter() {
             let name = self.sink.elem_name(node);
-            match *name.ns {
-                ns!(html) => print!(" {}", name.local),
+            match *name.ns() {
+                ns!(html) => print!(" {}", name.local_name()),
                 _ => panic!(),
             }
         }
@@ -308,8 +312,8 @@ where
                 &Marker => print!(" Marker"),
                 Element(h, _) => {
                     let name = self.sink.elem_name(h);
-                    match *name.ns {
-                        ns!(html) => print!(" {}", name.local),
+                    match *name.ns() {
+                        ns!(html) => print!(" {}", name.local_name()),
                         _ => panic!(),
                     }
                 },
@@ -541,7 +545,7 @@ where
 
     fn adjusted_current_node_present_but_not_in_html_namespace(&self) -> bool {
         !self.open_elems.borrow().is_empty()
-            && self.sink.elem_name(&self.adjusted_current_node()).ns != &ns!(html)
+            && *self.sink.elem_name(&self.adjusted_current_node()).ns() != ns!(html)
     }
 }
 
@@ -689,7 +693,7 @@ where
     where
         TagSet: Fn(ExpandedName) -> bool,
     {
-        set(self.sink.elem_name(&self.current_node()))
+        set(self.sink.elem_name(&self.current_node()).expanded())
     }
 
     // Insert at the "appropriate place for inserting a node".
@@ -1014,7 +1018,8 @@ where
         for elem in self.open_elems.borrow().iter() {
             let error;
             {
-                let name = self.sink.elem_name(elem);
+                let elem_name = self.sink.elem_name(elem);
+                let name = elem_name.expanded();
                 if body_end_ok(name) {
                     continue;
                 }
@@ -1041,7 +1046,7 @@ where
             if pred(node.clone()) {
                 return true;
             }
-            if scope(self.sink.elem_name(node)) {
+            if scope(self.sink.elem_name(node).expanded()) {
                 return false;
             }
         }
@@ -1055,12 +1060,12 @@ where
     where
         TagSet: Fn(ExpandedName) -> bool,
     {
-        set(self.sink.elem_name(elem))
+        set(self.sink.elem_name(elem).expanded())
     }
 
     fn html_elem_named(&self, elem: &Handle, name: LocalName) -> bool {
-        let expanded = self.sink.elem_name(elem);
-        *expanded.ns == ns!(html) && *expanded.local == name
+        let elem_name = self.sink.elem_name(elem);
+        *elem_name.ns() == ns!(html) && *elem_name.local_name() == name
     }
 
     fn in_html_elem_named(&self, name: LocalName) -> bool {
@@ -1090,8 +1095,8 @@ where
             {
                 let open_elems = self.open_elems.borrow();
                 let elem = unwrap_or_return!(open_elems.last());
-                let nsname = self.sink.elem_name(elem);
-                if !set(nsname) {
+                let elem_name = self.sink.elem_name(elem);
+                if !set(elem_name.expanded()) {
                     return;
                 }
             }
@@ -1132,7 +1137,7 @@ where
             match self.open_elems.borrow_mut().pop() {
                 None => break,
                 Some(elem) => {
-                    if pred(self.sink.elem_name(&elem)) {
+                    if pred(self.sink.elem_name(&elem).expanded()) {
                         break;
                     }
                 },
@@ -1217,7 +1222,8 @@ where
             if let (true, Some(ctx)) = (last, context_elem.as_ref()) {
                 node = ctx;
             }
-            let name = match self.sink.elem_name(node) {
+            let elem_name = self.sink.elem_name(node);
+            let name = match elem_name.expanded() {
                 ExpandedName {
                     ns: &ns!(html),
                     local,
@@ -1470,7 +1476,8 @@ where
         }
 
         let current = self.adjusted_current_node();
-        let name = self.sink.elem_name(&current);
+        let elem_name = self.sink.elem_name(&current);
+        let name = elem_name.expanded();
         if let ns!(html) = *name.ns {
             return false;
         }
@@ -1681,7 +1688,7 @@ where
         let current_ns = self
             .sink
             .elem_name(&self.adjusted_current_node())
-            .ns
+            .ns()
             .clone();
         match current_ns {
             ns!(mathml) => self.adjust_mathml_attributes(&mut tag),

html5ever/src/tree_builder/rules.rs

@@ -9,15 +9,18 @@
 
 // The tree builder rules, as a single, enormous nested match expression.
 
-use markup5ever::{expanded_name, local_name, namespace_prefix, namespace_url, ns};
+use crate::interface::Quirks;
 use crate::tokenizer::states::{Plaintext, Rawtext, Rcdata, ScriptData};
+use crate::tokenizer::TagKind::{EndTag, StartTag};
 use crate::tree_builder::tag_sets::*;
 use crate::tree_builder::types::*;
+use crate::tree_builder::{
+    create_element, html_elem, ElemName, NodeOrText::AppendNode, StrTendril, Tag, TreeBuilder,
+    TreeSink,
+};
 use crate::QualName;
-use crate::tree_builder::{create_element, html_elem, TreeSink, Tag, NodeOrText::AppendNode, StrTendril, TreeBuilder};
-use crate::tokenizer::TagKind::{StartTag, EndTag};
+use markup5ever::{expanded_name, local_name, namespace_prefix, namespace_url, ns};
 use std::borrow::Cow::Borrowed;
-use crate::interface::Quirks;
 
 use std::borrow::ToOwned;
 
@@ -402,7 +405,8 @@ where
 
                     let mut to_close = None;
                     for node in self.open_elems.borrow().iter().rev() {
-                        let name = self.sink.elem_name(node);
+                        let elem_name = self.sink.elem_name(node);
+                        let name = elem_name.expanded();
                         let can_close = if list {
                             close_list(name)
                         } else {
@@ -1441,8 +1445,8 @@ where
                     {
                         let open_elems = self.open_elems.borrow();
                         let node_name = self.sink.elem_name(&open_elems[stack_idx]);
-                        html = *node_name.ns == ns!(html);
-                        eq = node_name.local.eq_ignore_ascii_case(&tag.name);
+                        html = *node_name.ns() == ns!(html);
+                        eq = node_name.local_name().eq_ignore_ascii_case(&tag.name);
                     }
                     if !first && html {
                         let mode = self.mode.get();

html5ever/src/tree_builder/types.rs

@@ -58,6 +58,7 @@ pub(crate) enum SplitStatus {
 /// A subset/refinement of `tokenizer::Token`.  Everything else is handled
 /// specially at the beginning of `process_token`.
 #[derive(PartialEq, Eq, Clone, Debug)]
+#[allow(clippy::enum_variant_names)]
 pub(crate) enum Token {
     TagToken(Tag),
     CommentToken(StrTendril),

markup5ever/interface/mod.rs

@@ -8,12 +8,13 @@
 // except according to those terms.
 //! Types for tag and attribute names, and tree-builder functionality.
 
+use std::cell::Ref;
 use std::fmt;
 use tendril::StrTendril;
 
 pub use self::tree_builder::{create_element, AppendNode, AppendText, ElementFlags, NodeOrText};
+pub use self::tree_builder::{ElemName, NextParserState, Tracer, TreeSink};
 pub use self::tree_builder::{LimitedQuirks, NoQuirks, Quirks, QuirksMode};
-pub use self::tree_builder::{NextParserState, Tracer, TreeSink};
 use super::{LocalName, Namespace, Prefix};
 
 /// An [expanded name], containing the tag and the namespace.
@@ -25,6 +26,30 @@ pub struct ExpandedName<'a> {
     pub local: &'a LocalName,
 }
 
+impl<'a> ElemName for ExpandedName<'a> {
+    #[inline(always)]
+    fn ns(&self) -> &Namespace {
+        self.ns
+    }
+
+    #[inline(always)]
+    fn local_name(&self) -> &LocalName {
+        self.local
+    }
+}
+
+impl<'a> ElemName for Ref<'a, ExpandedName<'a>> {
+    #[inline(always)]
+    fn ns(&self) -> &Namespace {
+        self.ns
+    }
+
+    #[inline(always)]
+    fn local_name(&self) -> &LocalName {
+        self.local
+    }
+}
+
 impl<'a> fmt::Debug for ExpandedName<'a> {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         if self.ns.is_empty() {
@@ -241,6 +266,30 @@ pub struct QualName {
     pub local: LocalName,
 }
 
+impl<'a> ElemName for Ref<'a, QualName> {
+    #[inline(always)]
+    fn ns(&self) -> &Namespace {
+        &self.ns
+    }
+
+    #[inline(always)]
+    fn local_name(&self) -> &LocalName {
+        &self.local
+    }
+}
+
+impl<'a> ElemName for &'a QualName {
+    #[inline(always)]
+    fn ns(&self) -> &Namespace {
+        &self.ns
+    }
+
+    #[inline(always)]
+    fn local_name(&self) -> &LocalName {
+        &self.local
+    }
+}
+
 impl QualName {
     /// Basic constructor function.
     ///