employer
/
tame
1
0
Fork 0
Code Packages Releases Activity

tamer: xir::parse::attr: Error and recovery on duplicate attr 

This was a TODO for the attribute parser generator.  The first attribute
will be kept and later ones will be ignored, producing an error.  Recovery
permits further attribute parsing having ignored the duplicate.

DEV-7145
main
Mike Gerwitz 2022-07-20 11:01:18 -04:00
parent 21dfff0110
commit ce765d3b56
2 changed files with 155 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

84
tamer/src/xir/parse/attr.rs
View File

@ -41,11 +41,13 @@ use crate::{
diagnose::{Annotate, AnnotatedSpan, Diagnostic},
fmt::ListDisplayWrapper,
parse::ParseState,
span::Span,
xir::{attr::Attr, fmt::XmlAttrList, EleSpan, OpenSpan, QName},
};
use std::{error::Error, fmt::Display};
pub type ElementQName = QName;
pub type FirstSpan = Span;
/// Error while parsing element attributes.
#[derive(Debug, PartialEq)]
@ -73,6 +75,19 @@ pub enum AttrParseError<S: AttrParseState> {
///
/// Parsing may recover by simply ignoring this attribute.
UnexpectedAttr(Attr, ElementQName),
/// An attribute with the same name as a previous attribute has been
/// encountered within the context of this element.
///
/// The duplicate attribute is provided in its entirety.
/// The key span of the first-encountered attribute of the same name is
/// included to provide more robust diagnostic information.
/// The value of the previous attribute is not included because it is
/// expected that the diagnostic system will render the code
/// associated with the span;
/// displaying an attribute value in an error message is asking for
/// too much trouble given that it is arbitrary text.
DuplicateAttr(Attr, FirstSpan, ElementQName),
}
impl<S: AttrParseState> Display for AttrParseError<S> {
@ -88,7 +103,16 @@ impl<S: AttrParseState> Display for AttrParseError<S> {
Self::UnexpectedAttr(attr, ele_name) => {
write!(
f,
"element `{ele_name}` contains unexpected attribute `{attr}`"
"unexpected attribute `{attr}` for \
element element `{ele_name}`"
)
}
Self::DuplicateAttr(attr, _, ele_name) => {
write!(
f,
"duplicate attribute `{attr}` for \
element element `{ele_name}`"
)
}
}
@ -103,6 +127,8 @@ impl<S: AttrParseState> Error for AttrParseError<S> {
impl<S: AttrParseState> Diagnostic for AttrParseError<S> {
fn describe(&self) -> Vec<AnnotatedSpan> {
use crate::fmt::{DisplayWrapper, TtQuote};
match self {
Self::MissingRequired(st) => st
.element_span()
@ -118,6 +144,19 @@ impl<S: AttrParseState> Diagnostic for AttrParseError<S> {
.key_span()
.error(format!("element `{ele_name}` cannot contain `{attr}`"))
.into(),
Self::DuplicateAttr(Attr(name, _, aspan), first_span, _) => {
vec![
first_span.note(format!(
"{} previously encountered here",
TtQuote::wrap(name)
)),
aspan.key_span().error(format!(
"{} here is a duplicate",
TtQuote::wrap(name)
)),
]
}
}
}
}
@ -200,7 +239,8 @@ macro_rules! attr_parse {
#[doc(hidden)]
___done: bool,
$(
pub $field: Option<$ty>,
// Value + key span
pub $field: Option<($ty, Span)>,
)*
}
@ -342,10 +382,33 @@ macro_rules! attr_parse {
// forget to import a const for `$qname`.
// We don't use `$qname:pat` because we reuse
// `$qname` for error messages.
flat::XirfToken::Attr(attr @ Attr(qn, ..)) if qn == $qname => {
// TODO: Error on prev value
self.$field.replace(attr.into());
Transition(self).incomplete()
flat::XirfToken::Attr(
attr @ Attr(qn, _, AttrSpan(kspan, _))
) if qn == $qname => {
match self.$field {
// Duplicate attribute name
Some((_, first_kspan)) => {
let ele_name = self.element_name();
Transition(self).err(
AttrParseError::DuplicateAttr(
attr,
first_kspan,
ele_name,
)
)
}
// First time seeing attribute name
None => {
self.$field.replace((
attr.into(),
kspan,
));
Transition(self).incomplete()
}
}
}
)*
@ -402,7 +465,10 @@ macro_rules! attr_parse {
// This does not produce a great error if the user forgets to use an
// `Option` type for optional attributes,
// but the comment is better than nothing.
$from.$field.unwrap_or(None) // field type must be Option<T>
match $from.$field { // field type must be Option<T>
Some((value, _kspan)) => value,
None => None,
}
};
// Otherwise,
@ -412,7 +478,9 @@ macro_rules! attr_parse {
// This assumes that we've already validated via
// `@validate_req` above,
// and so should never actually panic.
$from.$field.unwrap()
match $from.$field.unwrap() {
(value, _kspan) => value
}
};
}

86
tamer/src/xir/parse/attr/test.rs
View File

@ -29,7 +29,7 @@ use crate::{
};
use std::assert_matches::assert_matches;
const S1: Span = DUMMY_SPAN;
const S1: Span = DUMMY_SPAN.offset_add(1).unwrap();
const S2: Span = S1.offset_add(1).unwrap();
const S3: Span = S2.offset_add(1).unwrap();
const SE: OpenSpan = OpenSpan(S1.offset_add(100).unwrap(), 0);
@ -294,10 +294,10 @@ mod required {
err,
ParseError::StateError(AttrParseError::MissingRequired(
ReqMissingState {
name: Some(ref given_name),
name: Some((ref given_name, _)),
src: None, // cause of the error
ty: None, // another cause of the error
yields: Some(ref given_yields),
yields: Some((ref given_yields, _)),
..
},
)) if given_name == &ATTR_NAME
@ -313,8 +313,8 @@ mod required {
// `required_missing_values` above verifies that this state is what
// is in fact constructed from a failed parsing attempt.
let mut partial = ReqMissingState::with_element(QN_ELE, SE);
partial.name.replace(ATTR_NAME);
partial.yields.replace(ATTR_YIELDS);
partial.name.replace((ATTR_NAME, S1));
partial.yields.replace((ATTR_YIELDS, S2));
let err = AttrParseError::MissingRequired(partial);
@ -346,8 +346,8 @@ mod required {
#[test]
fn diagnostic_message_contains_all_required_missing_attr_name() {
let mut partial = ReqMissingState::with_element(QN_ELE, SE);
partial.name.replace(ATTR_NAME);
partial.yields.replace(ATTR_YIELDS);
partial.name.replace((ATTR_NAME, S1));
partial.yields.replace((ATTR_YIELDS, S2));
let err = AttrParseError::MissingRequired(partial);
let desc = err.describe();
@ -430,3 +430,75 @@ fn unexpected_attr_with_recovery() {
parse_aggregate_with(&mut sut),
);
}
// A duplicate attribute will result in an error,
// and recovery will cause the duplicate to be ignored.
#[test]
fn duplicate_attr_with_recovery() {
attr_parse! {
struct DupState -> Dup {
name: (QN_NAME) => Attr,
src: (QN_SRC) => Attr,
}
}
let attr_keep = Attr(QN_NAME, "keep me".into(), AttrSpan(S1, S2));
let attr_dup = Attr(QN_NAME, "duplicate".into(), AttrSpan(S2, S3));
let attr_src = Attr(QN_SRC, "val_src".into(), AttrSpan(S3, S1));
let tok_dead = close_empty(S3, Depth(0));
let toks = vec![
// Both of these have the same name (@name).
XirfToken::Attr(attr_keep.clone()),
XirfToken::Attr(attr_dup.clone()),
// Another attribute just to show that error recovery permits
// further attribute parsing.
XirfToken::Attr(attr_src.clone()),
// Will cause dead state:
tok_dead.clone(),
]
.into_iter();
let mut sut = Parser::with_state(DupState::with_element(QN_ELE, SE), toks);
// The first token is good,
// since we haven't encountered the attribute yet.
assert_eq!(sut.next(), Some(Ok(Parsed::Incomplete)));
// The second one results in an error,
// since the name is the same.
let err = sut
.next()
.unwrap()
.expect_err("DuplicateAttr error expected");
assert_eq!(
ParseError::StateError(AttrParseError::DuplicateAttr(
attr_dup,
attr_keep.attr_span().key_span(),
QN_ELE,
)),
err,
);
// The diagnostic description of this error should contain first a
// reference to the original attribute,
// and then a reference to the duplicate.
let desc = err.describe();
assert_eq!(desc[0].span(), S1);
assert_eq!(desc[1].span(), S2);
// Once parsing is completed,
// we must have kept the first occurrence of the attribute and
// discarded the second.
assert_eq!(
Ok((
Dup {
name: attr_keep,
src: attr_src,
},
tok_dead
)),
parse_aggregate_with(&mut sut),
);
}