Skip to content

Commit

Permalink
Make return type of TreeSink::elem_name an associated type. (#553)
Browse files Browse the repository at this point in the history 
* 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]>
  • Loading branch information
@nicoburns
nicoburns authored Sep 10, 2024
1 parent bb62eac commit 396f68a
Show file tree
Hide file tree
Showing 12 changed files with 133 additions and 46 deletions.
7 changes: 4 additions & 3 deletions html5ever/examples/arena.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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!"),
}
}
Expand Down
1 change: 1 addition & 0 deletions html5ever/examples/noop-tree-builder.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
16 changes: 7 additions & 9 deletions html5ever/examples/print-tree-actions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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>,
Expand All @@ -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
}
Expand Down Expand Up @@ -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 {
Expand Down
47 changes: 27 additions & 20 deletions html5ever/src/tree_builder/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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!(),
}
}
Expand All @@ -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!(),
}
},
Expand Down Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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".
Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
}
}
Expand All @@ -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 {
Expand Down Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -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;
}
},
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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),
Expand Down
18 changes: 11 additions & 7 deletions html5ever/src/tree_builder/rules.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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();
Expand Down
1 change: 1 addition & 0 deletions html5ever/src/tree_builder/types.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
51 changes: 50 additions & 1 deletion markup5ever/interface/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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() {
Expand Down Expand Up @@ -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.
///
Expand Down
Loading

0 comments on commit 396f68a

Please sign in to comment.