style: Make PropertyId::parse less of a footgun.

Bug: 1466095
Reviewed-by: xidorn
MozReview-Commit-ID: 2BmtSDPmHj9
This commit is contained in:
Emilio Cobos Álvarez 2018-06-01 14:00:57 +02:00
parent 5db1387f39
commit 600f19540e
No known key found for this signature in database
GPG key ID: 056B727BB9C1027C
5 changed files with 44 additions and 26 deletions

View file

@ -1158,7 +1158,7 @@ impl<'a, 'b, 'i> DeclarationParser<'i> for PropertyDeclarationParser<'a, 'b> {
name: CowRcStr<'i>,
input: &mut Parser<'i, 't>,
) -> Result<Importance, ParseError<'i>> {
let id = match PropertyId::parse(&name) {
let id = match PropertyId::parse(&name, self.context) {
Ok(id) => id,
Err(..) => {
return Err(input.new_custom_error(if is_non_mozilla_vendor_identifier(&name) {

View file

@ -1595,7 +1595,7 @@ impl PropertyId {
/// Returns a given property from the string `s`.
///
/// Returns Err(()) for unknown non-custom properties.
pub fn parse(property_name: &str) -> Result<Self, ()> {
fn parse_unchecked(property_name: &str) -> Result<Self, ()> {
// FIXME(https://github.com/rust-lang/rust/issues/33156): remove this
// enum and use PropertyId when stable Rust allows destructors in
// statics.
@ -1639,13 +1639,39 @@ impl PropertyId {
PropertyId::ShorthandAlias(id, alias)
},
None => {
return ::custom_properties::parse_name(property_name).map(|name| {
PropertyId::Custom(::custom_properties::Name::from(name))
})
let name = ::custom_properties::parse_name(property_name)?;
PropertyId::Custom(::custom_properties::Name::from(name))
},
})
}
/// Parses a property name, and returns an error if it's unknown or isn't
/// enabled for all content.
#[inline]
pub fn parse_enabled_for_all_content(name: &str) -> Result<Self, ()> {
let id = Self::parse_unchecked(name)?;
if !id.enabled_for_all_content() {
return Err(());
}
Ok(id)
}
/// Parses a property name, and returns an error if it's unknown or isn't
/// allowed in this context.
#[inline]
pub fn parse(name: &str, context: &ParserContext) -> Result<Self, ()> {
let id = Self::parse_unchecked(name)?;
if !id.allowed_in(context) {
return Err(());
}
Ok(id)
}
/// Returns a property id from Gecko's nsCSSPropertyID.
#[cfg(feature = "gecko")]
#[allow(non_upper_case_globals)]
@ -1715,8 +1741,7 @@ impl PropertyId {
}
}
/// Returns the non-custom property id for this property.
pub fn non_custom_id(&self) -> Option<NonCustomPropertyId> {
fn non_custom_id(&self) -> Option<NonCustomPropertyId> {
Some(match *self {
PropertyId::Custom(_) => return None,
PropertyId::Shorthand(shorthand_id) => shorthand_id.into(),
@ -1751,8 +1776,7 @@ impl PropertyId {
id.enabled_for_all_content()
}
/// Returns whether the property is allowed in a given context.
pub fn allowed_in(&self, context: &ParserContext) -> bool {
fn allowed_in(&self, context: &ParserContext) -> bool {
let id = match self.non_custom_id() {
// Custom properties are allowed everywhere
None => return true,
@ -1974,12 +1998,7 @@ impl PropertyDeclaration {
input: &mut Parser<'i, 't>,
) -> Result<(), ParseError<'i>> {
assert!(declarations.is_empty());
if !id.allowed_in(context) {
return Err(input.new_custom_error(
StyleParseErrorKind::UnknownProperty(name)
));
}
debug_assert!(id.allowed_in(context), "{:?}", id);
let start = input.state();
match id {

View file

@ -620,9 +620,12 @@ impl<'a, 'b, 'i> DeclarationParser<'i> for KeyframeDeclarationParser<'a, 'b> {
name: CowRcStr<'i>,
input: &mut Parser<'i, 't>,
) -> Result<(), ParseError<'i>> {
let id = PropertyId::parse(&name).map_err(|()| {
input.new_custom_error(StyleParseErrorKind::UnknownProperty(name.clone()))
})?;
let id = match PropertyId::parse(&name, self.context) {
Ok(id) => id,
Err(()) => return Err(input.new_custom_error(
StyleParseErrorKind::UnknownProperty(name.clone())
)),
};
// TODO(emilio): Shouldn't this use parse_entirely?
PropertyDeclaration::parse_into(self.declarations, id, name, self.context, input)?;

View file

@ -315,12 +315,12 @@ impl Declaration {
let mut input = ParserInput::new(&self.0);
let mut input = Parser::new(&mut input);
input
.parse_entirely(|input| -> Result<(), CssParseError<()>> {
input.parse_entirely(|input| -> Result<(), CssParseError<()>> {
let prop = input.expect_ident_cloned().unwrap();
input.expect_colon().unwrap();
let id = PropertyId::parse(&prop).map_err(|_| input.new_custom_error(()))?;
let id = PropertyId::parse(&prop, context)
.map_err(|_| input.new_custom_error(()))?;
let mut declarations = SourcePropertyDeclaration::new();
input.parse_until_before(Delimiter::Bang, |input| {

View file

@ -444,15 +444,11 @@ fn change_bits_for_longhand(longhand: LonghandId) -> WillChangeBits {
}
fn change_bits_for_maybe_property(ident: &str, context: &ParserContext) -> WillChangeBits {
let id = match PropertyId::parse(ident) {
let id = match PropertyId::parse(ident, context) {
Ok(id) => id,
Err(..) => return WillChangeBits::empty(),
};
if !id.allowed_in(context) {
return WillChangeBits::empty();
}
match id.as_shorthand() {
Ok(shorthand) => {
shorthand.longhands().fold(WillChangeBits::empty(), |flags, p| {