Report CSS parse errors via enum instead of strings.

components/script_layout_interface/reporter.rs

@@ -9,7 +9,7 @@ use msg::constellation_msg::PipelineId;
 use script_traits::ConstellationControlMsg;
 use servo_url::ServoUrl;
 use std::sync::{Mutex, Arc};
-use style::error_reporting::ParseErrorReporter;
+use style::error_reporting::{ParseErrorReporter, ParseError};
 
 #[derive(HeapSizeOf, Clone)]
 pub struct CSSErrorReporter {
@@ -22,10 +22,10 @@ pub struct CSSErrorReporter {
 }
 
 impl ParseErrorReporter for CSSErrorReporter {
-    fn report_error(&self,
+    fn report_error<'a>(&self,
                         input: &mut Parser,
                         position: SourcePosition,
-                    message: &str,
+                        error: ParseError<'a>,
                         url: &ServoUrl,
                         line_number_offset: u64) {
         let location = input.source_location(position);
@@ -35,7 +35,7 @@ impl ParseErrorReporter for CSSErrorReporter {
                   url.as_str(),
                   line_offset,
                   location.column,
-                  message)
+                  error.to_string())
         }
 
         //TODO: report a real filename
@@ -44,6 +44,6 @@ impl ParseErrorReporter for CSSErrorReporter {
                                                     "".to_owned(),
                                                     location.line,
                                                     location.column,
-                                                    message.to_owned()));
+                                                    error.to_string()));
     }
 }

components/style/counter_style/mod.rs

@@ -9,6 +9,7 @@
 use Atom;
 use cssparser::{AtRuleParser, DeclarationListParser, DeclarationParser};
 use cssparser::{Parser, Token, serialize_identifier};
+use error_reporting::ParseError;
 #[cfg(feature = "gecko")] use gecko::rules::CounterStyleDescriptors;
 #[cfg(feature = "gecko")] use gecko_bindings::structs::nsCSSCounterDesc;
 use parser::{ParserContext, log_css_error, Parse};
@@ -61,9 +62,8 @@ pub fn parse_counter_style_body(name: CustomIdent, context: &ParserContext, inpu
         while let Some(declaration) = iter.next() {
             if let Err(range) = declaration {
                 let pos = range.start;
-                let message = format!("Unsupported @counter-style descriptor declaration: '{}'",
+                let error = ParseError::UnsupportedViewportDescriptorDeclaration(iter.input.slice(range));
-                                      iter.input.slice(range));
+                log_css_error(iter.input, pos, error, context);
-                log_css_error(iter.input, pos, &*message, context);
             }
         }
     }
@@ -75,31 +75,27 @@ pub fn parse_counter_style_body(name: CustomIdent, context: &ParserContext, inpu
         ref system @ System::Numeric
         if rule.symbols.is_none() => {
             let system = system.to_css_string();
-            Some(format!("Invalid @counter-style rule: 'system: {}' without 'symbols'", system))
+            Some(ParseError::InvalidCounterStyleWithoutSymbols(system))
         }
         ref system @ System::Alphabetic |
         ref system @ System::Numeric
         if rule.symbols().unwrap().0.len() < 2 => {
             let system = system.to_css_string();
-            Some(format!("Invalid @counter-style rule: 'system: {}' less than two 'symbols'",
+            Some(ParseError::InvalidCounterStyleNotEnoughSymbols(system))
-                         system))
         }
         System::Additive if rule.additive_symbols.is_none() => {
-            let s = "Invalid @counter-style rule: 'system: additive' without 'additive-symbols'";
+            Some(ParseError::InvalidCounterStyleWithoutAdditiveSymbols)
-            Some(s.to_owned())
         }
         System::Extends(_) if rule.symbols.is_some() => {
-            let s = "Invalid @counter-style rule: 'system: extends …' with 'symbols'";
+            Some(ParseError::InvalidCounterStyleExtendsWithSymbols)
-            Some(s.to_owned())
         }
         System::Extends(_) if rule.additive_symbols.is_some() => {
-            let s = "Invalid @counter-style rule: 'system: extends …' with 'additive-symbols'";
+            Some(ParseError::InvalidCounterStyleExtendsWithAdditiveSymbols)
-            Some(s.to_owned())
         }
         _ => None
     };
-    if let Some(message) = error {
+    if let Some(error) = error {
-        log_css_error(input, start, &message, context);
+        log_css_error(input, start, error, context);
         Err(())
     } else {
         Ok(rule)

components/style/error_reporting.rs

@@ -10,16 +10,80 @@ use cssparser::{Parser, SourcePosition};
 use log;
 use stylesheets::UrlExtraData;
 
+/// Errors that can be encountered while parsing CSS.
+pub enum ParseError<'a> {
+    /// A property declaration was not recognized.
+    UnsupportedPropertyDeclaration(&'a str),
+    /// A font face descriptor was not recognized.
+    UnsupportedFontFaceDescriptor(&'a str),
+    /// A keyframe rule was not valid.
+    InvalidKeyframeRule(&'a str),
+    /// A keyframe property declaration was not recognized.
+    UnsupportedKeyframePropertyDeclaration(&'a str),
+    /// A rule was invalid for some reason.
+    InvalidRule(&'a str),
+    /// A rule was not recognized.
+    UnsupportedRule(&'a str),
+    /// A viewport descriptor declaration was not recognized.
+    UnsupportedViewportDescriptorDeclaration(&'a str),
+    /// A counter style descriptor declaration was not recognized.
+    UnsupportedCounterStyleDescriptorDeclaration(&'a str),
+    /// A counter style rule had no symbols.
+    InvalidCounterStyleWithoutSymbols(String),
+    /// A counter style rule had less than two symbols.
+    InvalidCounterStyleNotEnoughSymbols(String),
+    /// A counter style rule did not have additive-symbols.
+    InvalidCounterStyleWithoutAdditiveSymbols,
+    /// A counter style rule had extends with symbols.
+    InvalidCounterStyleExtendsWithSymbols,
+    /// A counter style rule had extends with additive-symbols.
+    InvalidCounterStyleExtendsWithAdditiveSymbols
+}
+
+impl<'a> ParseError<'a> {
+    /// Turn a parse error into a string representation.
+    pub fn to_string(&self) -> String {
+        match *self {
+            ParseError::UnsupportedPropertyDeclaration(decl) =>
+                format!("Unsupported property declaration: '{}'", decl),
+            ParseError::UnsupportedFontFaceDescriptor(decl) =>
+                format!("Unsupported @font-face descriptor declaration: '{}'", decl),
+            ParseError::InvalidKeyframeRule(rule) =>
+                format!("Invalid keyframe rule: '{}'", rule),
+            ParseError::UnsupportedKeyframePropertyDeclaration(decl) =>
+                format!("Unsupported keyframe property declaration: '{}'", decl),
+            ParseError::InvalidRule(rule) =>
+                format!("Invalid rule: '{}'", rule),
+            ParseError::UnsupportedRule(rule) =>
+                format!("Unsupported rule: '{}'", rule),
+            ParseError::UnsupportedViewportDescriptorDeclaration(decl) =>
+                format!("Unsupported @viewport descriptor declaration: '{}'", decl),
+            ParseError::UnsupportedCounterStyleDescriptorDeclaration(decl) =>
+                format!("Unsupported @counter-style descriptor declaration: '{}'", decl),
+            ParseError::InvalidCounterStyleWithoutSymbols(ref system) =>
+                format!("Invalid @counter-style rule: 'system: {}' without 'symbols'", system),
+            ParseError::InvalidCounterStyleNotEnoughSymbols(ref system) =>
+                format!("Invalid @counter-style rule: 'system: {}' less than two 'symbols'", system),
+            ParseError::InvalidCounterStyleWithoutAdditiveSymbols =>
+                "Invalid @counter-style rule: 'system: additive' without 'additive-symbols'".into(),
+            ParseError::InvalidCounterStyleExtendsWithSymbols =>
+                "Invalid @counter-style rule: 'system: extends …' with 'symbols'".into(),
+            ParseError::InvalidCounterStyleExtendsWithAdditiveSymbols =>
+                "Invalid @counter-style rule: 'system: extends …' with 'additive-symbols'".into(),
+        }
+    }
+}
+
 /// A generic trait for an error reporter.
 pub trait ParseErrorReporter : Sync + Send {
     /// Called when the style engine detects an error.
     ///
     /// Returns the current input being parsed, the source position it was
     /// reported from, and a message.
-    fn report_error(&self,
+    fn report_error<'a>(&self,
                         input: &mut Parser,
                         position: SourcePosition,
-                    message: &str,
+                        error: ParseError<'a>,
                         url: &UrlExtraData,
                         line_number_offset: u64);
 }
@@ -33,16 +97,16 @@ pub trait ParseErrorReporter : Sync + Send {
 pub struct RustLogReporter;
 
 impl ParseErrorReporter for RustLogReporter {
-    fn report_error(&self,
+    fn report_error<'a>(&self,
                         input: &mut Parser,
                         position: SourcePosition,
-                    message: &str,
+                        error: ParseError<'a>,
                         url: &UrlExtraData,
                         line_number_offset: u64) {
         if log_enabled!(log::LogLevel::Info) {
             let location = input.source_location(position);
             let line_offset = location.line + line_number_offset as usize;
-            info!("Url:\t{}\n{}:{} {}", url.as_str(), line_offset, location.column, message)
+            info!("Url:\t{}\n{}:{} {}", url.as_str(), line_offset, location.column, error.to_string())
         }
     }
 }
@@ -51,10 +115,10 @@ impl ParseErrorReporter for RustLogReporter {
 pub struct NullReporter;
 
 impl ParseErrorReporter for NullReporter {
-    fn report_error(&self,
+    fn report_error<'a>(&self,
             _: &mut Parser,
             _: SourcePosition,
-            _: &str,
+            _: ParseError<'a>,
             _: &UrlExtraData,
             _: u64) {
         // do nothing

components/style/font_face.rs

@@ -13,6 +13,7 @@ use computed_values::{font_feature_settings, font_stretch, font_style, font_weig
 use computed_values::font_family::FamilyName;
 use cssparser::{AtRuleParser, DeclarationListParser, DeclarationParser, Parser};
 use cssparser::SourceLocation;
+use error_reporting::ParseError;
 #[cfg(feature = "gecko")] use gecko_bindings::structs::CSSFontFaceDescriptors;
 #[cfg(feature = "gecko")] use cssparser::UnicodeRange;
 use parser::{ParserContext, log_css_error, Parse};
@@ -99,9 +100,8 @@ pub fn parse_font_face_block(context: &ParserContext, input: &mut Parser, locati
         while let Some(declaration) = iter.next() {
             if let Err(range) = declaration {
                 let pos = range.start;
-                let message = format!("Unsupported @font-face descriptor declaration: '{}'",
+                let error = ParseError::UnsupportedFontFaceDescriptor(iter.input.slice(range));
-                                      iter.input.slice(range));
+                log_css_error(iter.input, pos, error, context);
-                log_css_error(iter.input, pos, &*message, context);
             }
         }
     }

components/style/parser.rs

@@ -6,7 +6,7 @@
 
 use context::QuirksMode;
 use cssparser::{Parser, SourcePosition, UnicodeRange};
-use error_reporting::ParseErrorReporter;
+use error_reporting::{ParseErrorReporter, ParseError};
 use style_traits::OneOrMoreCommaSeparated;
 use stylesheets::{CssRuleType, Origin, UrlExtraData, Namespaces};
 
@@ -164,14 +164,14 @@ impl<'a> ParserContext<'a> {
 /// Defaults to a no-op.
 /// Set a `RUST_LOG=style::errors` environment variable
 /// to log CSS parse errors to stderr.
-pub fn log_css_error(input: &mut Parser,
+pub fn log_css_error<'a>(input: &mut Parser,
                          position: SourcePosition,
-                     message: &str,
+                         error: ParseError<'a>,
                          parsercontext: &ParserContext) {
     let url_data = parsercontext.url_data;
     let line_number_offset = parsercontext.line_number_offset;
     parsercontext.error_reporter.report_error(input, position,
-                                              message, url_data,
+                                              error, url_data,
                                               line_number_offset);
 }
 

components/style/properties/declaration_block.rs

@@ -9,7 +9,7 @@
 use context::QuirksMode;
 use cssparser::{DeclarationListParser, parse_important};
 use cssparser::{Parser, AtRuleParser, DeclarationParser, Delimiter};
-use error_reporting::ParseErrorReporter;
+use error_reporting::{ParseErrorReporter, ParseError};
 use parser::{PARSING_MODE_DEFAULT, ParsingMode, ParserContext, log_css_error};
 use properties::animated_properties::AnimationValue;
 use shared_lock::Locked;
@@ -955,9 +955,8 @@ pub fn parse_property_declaration_list(context: &ParserContext,
             Err(range) => {
                 iter.parser.declarations.clear();
                 let pos = range.start;
-                let message = format!("Unsupported property declaration: '{}'",
+                let error = ParseError::UnsupportedPropertyDeclaration(iter.input.slice(range));
-                                      iter.input.slice(range));
+                log_css_error(iter.input, pos, error, &context);
-                log_css_error(iter.input, pos, &*message, &context);
             }
         }
     }

components/style/stylesheets/keyframes_rule.rs

@@ -6,7 +6,7 @@
 
 use cssparser::{AtRuleParser, Parser, QualifiedRuleParser, RuleListParser};
 use cssparser::{DeclarationListParser, DeclarationParser, parse_one_rule, SourceLocation};
-use error_reporting::NullReporter;
+use error_reporting::{NullReporter, ParseError};
 use parser::{PARSING_MODE_DEFAULT, ParserContext, log_css_error};
 use properties::{Importance, PropertyDeclaration, PropertyDeclarationBlock, PropertyId};
 use properties::{PropertyDeclarationId, LonghandId, SourcePropertyDeclaration};
@@ -459,8 +459,8 @@ impl<'a> QualifiedRuleParser for KeyframeListParser<'a> {
         match KeyframeSelector::parse(input) {
             Ok(sel) => Ok(sel),
             Err(()) => {
-                let message = format!("Invalid keyframe rule: '{}'", input.slice_from(start));
+                let error = ParseError::InvalidKeyframeRule(input.slice_from(start));
-                log_css_error(input, start, &message, self.context);
+                log_css_error(input, start, error, self.context);
                 Err(())
             }
         }
@@ -483,9 +483,8 @@ impl<'a> QualifiedRuleParser for KeyframeListParser<'a> {
                 Err(range) => {
                     iter.parser.declarations.clear();
                     let pos = range.start;
-                    let message = format!("Unsupported keyframe property declaration: '{}'",
+                    let error = ParseError::UnsupportedKeyframePropertyDeclaration(iter.input.slice(range));
-                                          iter.input.slice(range));
+                    log_css_error(iter.input, pos, error, &context);
-                    log_css_error(iter.input, pos, &*message, &context);
                 }
             }
             // `parse_important` is not called here, `!important` is not allowed in keyframe blocks.

components/style/stylesheets/rule_parser.rs

@@ -7,6 +7,7 @@
 use {Namespace, Prefix};
 use counter_style::{parse_counter_style_body, parse_counter_style_name};
 use cssparser::{AtRuleParser, AtRuleType, Parser, QualifiedRuleParser, RuleListParser, SourceLocation};
+use error_reporting::ParseError;
 use font_face::parse_font_face_block;
 use media_queries::{parse_media_query_list, MediaList};
 use parking_lot::RwLock;
@@ -304,8 +305,8 @@ impl<'a, 'b> NestedRuleParser<'a, 'b> {
                 Ok(rule) => rules.push(rule),
                 Err(range) => {
                     let pos = range.start;
-                    let message = format!("Unsupported rule: '{}'", iter.input.slice(range));
+                    let error = ParseError::UnsupportedRule(iter.input.slice(range));
-                    log_css_error(iter.input, pos, &*message, self.context);
+                    log_css_error(iter.input, pos, error, self.context);
                 }
             }
         }

components/style/stylesheets/stylesheet.rs

@@ -5,7 +5,7 @@
 use {Prefix, Namespace};
 use context::QuirksMode;
 use cssparser::{Parser, RuleListParser};
-use error_reporting::ParseErrorReporter;
+use error_reporting::{ParseErrorReporter, ParseError};
 use fnv::FnvHashMap;
 use media_queries::{MediaList, Device};
 use parking_lot::RwLock;
@@ -142,8 +142,8 @@ impl Stylesheet {
                     Ok(rule) => rules.push(rule),
                     Err(range) => {
                         let pos = range.start;
-                        let message = format!("Invalid rule: '{}'", iter.input.slice(range));
+                        let error = ParseError::InvalidRule(iter.input.slice(range));
-                        log_css_error(iter.input, pos, &*message, iter.parser.context());
+                        log_css_error(iter.input, pos, error, iter.parser.context());
                     }
                 }
             }

components/style/stylesheets/viewport_rule.rs

@@ -11,6 +11,7 @@ use app_units::Au;
 use context::QuirksMode;
 use cssparser::{AtRuleParser, DeclarationListParser, DeclarationParser, Parser, parse_important};
 use cssparser::ToCss as ParserToCss;
+use error_reporting::ParseError;
 use euclid::size::TypedSize2D;
 use font_metrics::get_metrics_provider_for_product;
 use media_queries::Device;
@@ -342,9 +343,8 @@ impl Parse for ViewportRule {
                 }
                 Err(range) => {
                     let pos = range.start;
-                    let message = format!("Unsupported @viewport descriptor declaration: '{}'",
+                    let error = ParseError::UnsupportedViewportDescriptorDeclaration(parser.input.slice(range));
-                                          parser.input.slice(range));
+                    log_css_error(parser.input, pos, error, &context);
-                    log_css_error(parser.input, pos, &*message, &context);
                 }
             }
         }