Skip to content
Permalink
Browse files

style: Centralize a bit invalid value error reporting.

Also, buffer the errors, since we're going to want to look at the whole
declaration block to skip reporting them.

This shouldn't change behavior, just moves some work to the caller, and defers a
bit the work so that it happens only when error reporting is enabled.

Differential Revision: https://phabricator.services.mozilla.com/D30200
  • Loading branch information...
emilio committed May 8, 2019
1 parent 29cecf6 commit 02210264e7f0dae8221826c60df2706d5e8f9194
@@ -136,6 +136,12 @@ impl<'a> ParserContext<'a> {
.expect("Rule type expected, but none was found.")
}

/// Returns whether CSS error reporting is enabled.
#[inline]
pub fn error_reporting_enabled(&self) -> bool {
self.error_reporter.is_some()
}

/// Record a CSS parse error with this context’s error reporting.
pub fn log_css_error(&self, location: SourceLocation, error: ContextualParseError) {
let error_reporter = match self.error_reporter {
@@ -1240,26 +1240,37 @@ pub fn parse_one_declaration_into(
None,
);

let property_id_for_error_reporting = if context.error_reporting_enabled() {
Some(id.clone())
} else {
None
};

let mut input = ParserInput::new(input);
let mut parser = Parser::new(&mut input);
let start_position = parser.position();
parser.parse_entirely(|parser| {
PropertyDeclaration::parse_into(declarations, id, &context, parser)
}).map_err(|err| {
let location = err.location;
let error = ContextualParseError::UnsupportedPropertyDeclaration(
parser.slice_from(start_position),
err,
None
);
context.log_css_error(location, error);
if context.error_reporting_enabled() {
report_one_css_error(
&context,
None,
None,
err,
parser.slice_from(start_position),
property_id_for_error_reporting,
)
}
})
}

/// A struct to parse property declarations.
struct PropertyDeclarationParser<'a, 'b: 'a> {
context: &'a ParserContext<'b>,
declarations: &'a mut SourcePropertyDeclaration,
/// The last parsed property id if non-custom, and if any.
last_parsed_property_id: Option<PropertyId>,
}


@@ -1296,6 +1307,9 @@ impl<'a, 'b, 'i> DeclarationParser<'i> for PropertyDeclarationParser<'a, 'b> {
}));
}
};
if self.context.error_reporting_enabled() {
self.last_parsed_property_id = Some(id.clone());
}
input.parse_until_before(Delimiter::Bang, |input| {
PropertyDeclaration::parse_into(self.declarations, id, self.context, input)
})?;
@@ -1309,6 +1323,48 @@ impl<'a, 'b, 'i> DeclarationParser<'i> for PropertyDeclarationParser<'a, 'b> {
}
}

type SmallParseErrorVec<'i> = SmallVec<[(ParseError<'i>, &'i str, Option<PropertyId>); 2]>;

#[cold]
fn report_one_css_error<'i>(
context: &ParserContext,
_block: Option<&PropertyDeclarationBlock>,
selectors: Option<&SelectorList<SelectorImpl>>,
mut error: ParseError<'i>,
slice: &str,
property: Option<PropertyId>,
) {
debug_assert!(context.error_reporting_enabled());

// If the unrecognized property looks like a vendor-specific property,
// silently ignore it instead of polluting the error output.
if let ParseErrorKind::Custom(StyleParseErrorKind::UnknownVendorProperty) = error.kind {
return;
}

if let Some(ref property) = property {
error = match *property {
PropertyId::Custom(ref c) => StyleParseErrorKind::new_invalid(format!("--{}", c), error),
_ => StyleParseErrorKind::new_invalid(property.non_custom_id().unwrap().name(), error),
};
}

let location = error.location;
let error = ContextualParseError::UnsupportedPropertyDeclaration(slice, error, selectors);
context.log_css_error(location, error);
}

#[cold]
fn report_css_errors(
context: &ParserContext,
block: &PropertyDeclarationBlock,
selectors: Option<&SelectorList<SelectorImpl>>,
errors: &mut SmallParseErrorVec,
) {
for (mut error, slice, property) in errors.drain() {
report_one_css_error(context, Some(block), selectors, error, slice, property)
}
}

/// Parse a list of property declarations and return a property declaration
/// block.
@@ -1320,10 +1376,12 @@ pub fn parse_property_declaration_list(
let mut declarations = SourcePropertyDeclaration::new();
let mut block = PropertyDeclarationBlock::new();
let parser = PropertyDeclarationParser {
context: context,
context,
last_parsed_property_id: None,
declarations: &mut declarations,
};
let mut iter = DeclarationListParser::new(input, parser);
let mut errors = SmallParseErrorVec::new();
while let Some(declaration) = iter.next() {
match declaration {
Ok(importance) => {
@@ -1335,17 +1393,17 @@ pub fn parse_property_declaration_list(
Err((error, slice)) => {
iter.parser.declarations.clear();

// If the unrecognized property looks like a vendor-specific property,
// silently ignore it instead of polluting the error output.
if let ParseErrorKind::Custom(StyleParseErrorKind::UnknownVendorProperty) = error.kind {
continue;
if context.error_reporting_enabled() {
let property = iter.parser.last_parsed_property_id.take();
errors.push((error, slice, property));
}

let location = error.location;
let error = ContextualParseError::UnsupportedPropertyDeclaration(slice, error, selectors);
context.log_css_error(location, error);
}
}
}

if !errors.is_empty() {
report_css_errors(context, &block, selectors, &mut errors)
}

block
}
@@ -2209,13 +2209,9 @@ impl PropertyDeclaration {
// This probably affects some test results.
let value = match input.try(CSSWideKeyword::parse) {
Ok(keyword) => CustomDeclarationValue::CSSWideKeyword(keyword),
Err(()) => match crate::custom_properties::SpecifiedValue::parse(input) {
Ok(value) => CustomDeclarationValue::Value(value),
Err(e) => return Err(StyleParseErrorKind::new_invalid(
format!("--{}", property_name),
e,
)),
}
Err(()) => CustomDeclarationValue::Value(
crate::custom_properties::SpecifiedValue::parse(input)?
),
};
declarations.push(PropertyDeclaration::Custom(CustomDeclaration {
name: property_name,
@@ -2236,19 +2232,11 @@ impl PropertyDeclaration {
.or_else(|err| {
while let Ok(_) = input.next() {} // Look for var() after the error.
if !input.seen_var_or_env_functions() {
return Err(StyleParseErrorKind::new_invalid(
non_custom_id.unwrap().name(),
err,
));
return Err(err);
}
input.reset(&start);
let (first_token_type, css) =
crate::custom_properties::parse_non_custom_with_var(input).map_err(|e| {
StyleParseErrorKind::new_invalid(
non_custom_id.unwrap().name(),
e,
)
})?;
crate::custom_properties::parse_non_custom_with_var(input)?;
Ok(PropertyDeclaration::WithVariables(VariableDeclaration {
id,
value: Arc::new(UnparsedValue {
@@ -2287,20 +2275,12 @@ impl PropertyDeclaration {
id.parse_into(declarations, context, input).or_else(|err| {
while let Ok(_) = input.next() {} // Look for var() after the error.
if !input.seen_var_or_env_functions() {
return Err(StyleParseErrorKind::new_invalid(
non_custom_id.unwrap().name(),
err,
));
return Err(err);
}

input.reset(&start);
let (first_token_type, css) =
crate::custom_properties::parse_non_custom_with_var(input).map_err(|e| {
StyleParseErrorKind::new_invalid(
non_custom_id.unwrap().name(),
e,
)
})?;
crate::custom_properties::parse_non_custom_with_var(input)?;
let unparsed = Arc::new(UnparsedValue {
css: css.into_owned(),
first_token_type: first_token_type,

0 comments on commit 0221026

Please sign in to comment.
You can’t perform that action at this time.