From f48f0567cf44ac87fb34756b4167b644ad6049db Mon Sep 17 00:00:00 2001 From: avinash Date: Mon, 27 Feb 2017 15:49:48 +0530 Subject: [PATCH] Add ServoUrl as a parameter to report_error(...) of ParseErrorReporter passes test-tidy removed match and used map added new test test_report_error_passing_correct_url(...) and modified old unit tests removed the option for servourl and tidied up removed the duplicate test. made a few more changes after @cbrewster suggestions. changed _url to url in few places fixed the indenting --- components/script_layout_interface/reporter.rs | 13 ++++++++----- components/style/error_reporting.rs | 14 ++++++++------ components/style/parser.rs | 3 ++- components/style/stylesheets.rs | 3 ++- tests/unit/style/media_queries.rs | 6 ++++-- tests/unit/style/rule_tree/bench.rs | 5 +++-- tests/unit/style/stylesheets.rs | 11 +++++++++-- 7 files changed, 36 insertions(+), 19 deletions(-) diff --git a/components/script_layout_interface/reporter.rs b/components/script_layout_interface/reporter.rs index 4561f2d63ca..171f96d9d06 100644 --- a/components/script_layout_interface/reporter.rs +++ b/components/script_layout_interface/reporter.rs @@ -7,6 +7,7 @@ use ipc_channel::ipc::IpcSender; use log; use msg::constellation_msg::PipelineId; use script_traits::ConstellationControlMsg; +use servo_url::ServoUrl; use std::sync::{Mutex, Arc}; use style::error_reporting::ParseErrorReporter; @@ -21,11 +22,13 @@ pub struct CSSErrorReporter { } impl ParseErrorReporter for CSSErrorReporter { - fn report_error(&self, input: &mut Parser, position: SourcePosition, message: &str) { - let location = input.source_location(position); - if log_enabled!(log::LogLevel::Info) { - info!("{}:{} {}", location.line, location.column, message) - } + fn report_error(&self, input: &mut Parser, position: SourcePosition, message: &str, + url: &ServoUrl) { + let location = input.source_location(position); + if log_enabled!(log::LogLevel::Info) { + info!("Url:\t{}\n{}:{} {}", url.as_str(), location.line, location.column, message) + } + //TODO: report a real filename let _ = self.script_chan.lock().unwrap().send( ConstellationControlMsg::ReportCSSError(self.pipelineid, diff --git a/components/style/error_reporting.rs b/components/style/error_reporting.rs index 3a802bdd6c6..e815d0bf29e 100644 --- a/components/style/error_reporting.rs +++ b/components/style/error_reporting.rs @@ -8,6 +8,7 @@ use cssparser::{Parser, SourcePosition}; use log; +use servo_url::ServoUrl; /// A generic trait for an error reporter. pub trait ParseErrorReporter { @@ -15,7 +16,7 @@ pub trait ParseErrorReporter { /// /// Returns the current input being parsed, the source position it was /// reported from, and a message. - fn report_error(&self, input: &mut Parser, position: SourcePosition, message: &str); + fn report_error(&self, input: &mut Parser, position: SourcePosition, message: &str, url: &ServoUrl); /// Clone this error reporter. /// /// TODO(emilio): I'm pretty sure all the box shenanigans can go away. @@ -27,11 +28,12 @@ pub trait ParseErrorReporter { /// TODO(emilio): The name of this reporter is a lie, and should be renamed! pub struct StdoutErrorReporter; impl ParseErrorReporter for StdoutErrorReporter { - fn report_error(&self, input: &mut Parser, position: SourcePosition, message: &str) { - if log_enabled!(log::LogLevel::Info) { - let location = input.source_location(position); - info!("{}:{} {}", location.line, location.column, message) - } + fn report_error(&self, input: &mut Parser, position: SourcePosition, message: &str, + url: &ServoUrl) { + if log_enabled!(log::LogLevel::Info) { + let location = input.source_location(position); + info!("Url:\t{}\n{}:{} {}", url.as_str(), location.line, location.column, message) + } } fn clone(&self) -> Box { diff --git a/components/style/parser.rs b/components/style/parser.rs index 4145650b2c8..b561fd6efea 100644 --- a/components/style/parser.rs +++ b/components/style/parser.rs @@ -90,7 +90,8 @@ impl<'a> ParserContext<'a> { /// Set a `RUST_LOG=style::errors` environment variable /// to log CSS parse errors to stderr. pub fn log_css_error(input: &mut Parser, position: SourcePosition, message: &str, parsercontext: &ParserContext) { - parsercontext.error_reporter.report_error(input, position, message); + let servo_url = parsercontext.base_url; + parsercontext.error_reporter.report_error(input, position, message, servo_url); } // XXXManishearth Replace all specified value parse impls with impls of this diff --git a/components/style/stylesheets.rs b/components/style/stylesheets.rs index be88e22ea7e..6ee7b6843a0 100644 --- a/components/style/stylesheets.rs +++ b/components/style/stylesheets.rs @@ -252,7 +252,8 @@ impl ParseErrorReporter for MemoryHoleReporter { fn report_error(&self, _: &mut Parser, _: SourcePosition, - _: &str) { + _: &str, + _: &ServoUrl) { // do nothing } fn clone(&self) -> Box { diff --git a/tests/unit/style/media_queries.rs b/tests/unit/style/media_queries.rs index 11e1987928a..b2fe60ea91e 100644 --- a/tests/unit/style/media_queries.rs +++ b/tests/unit/style/media_queries.rs @@ -18,8 +18,10 @@ use style_traits::ToCss; pub struct CSSErrorReporterTest; impl ParseErrorReporter for CSSErrorReporterTest { - fn report_error(&self, _input: &mut Parser, _position: SourcePosition, _message: &str) { - } + fn report_error(&self, _input: &mut Parser, _position: SourcePosition, _message: &str, + _url: &ServoUrl) { + } + fn clone(&self) -> Box { Box::new(CSSErrorReporterTest) } diff --git a/tests/unit/style/rule_tree/bench.rs b/tests/unit/style/rule_tree/bench.rs index de68d99b71d..fccab1252f9 100644 --- a/tests/unit/style/rule_tree/bench.rs +++ b/tests/unit/style/rule_tree/bench.rs @@ -17,8 +17,9 @@ use test::{self, Bencher}; struct ErrorringErrorReporter; impl ParseErrorReporter for ErrorringErrorReporter { - fn report_error(&self, _: &mut Parser, position: SourcePosition, message: &str) { - panic!("CSS error: {:?} {}", position, message); + fn report_error(&self, input: &mut Parser, position: SourcePosition, message: &str, + url: &ServoUrl) { + panic!("CSS error: {}\t\n{:?} {}", url.as_str(), position, message); } fn clone(&self) -> Box { diff --git a/tests/unit/style/stylesheets.rs b/tests/unit/style/stylesheets.rs index 83166e019e5..586f2616ecf 100644 --- a/tests/unit/style/stylesheets.rs +++ b/tests/unit/style/stylesheets.rs @@ -276,6 +276,7 @@ fn test_parse_stylesheet() { } struct CSSError { + pub url : ServoUrl, pub line: usize, pub column: usize, pub message: String @@ -294,7 +295,9 @@ impl CSSInvalidErrorReporterTest { } impl ParseErrorReporter for CSSInvalidErrorReporterTest { - fn report_error(&self, input: &mut CssParser, position: SourcePosition, message: &str) { + fn report_error(&self, input: &mut CssParser, position: SourcePosition, message: &str, + url: &ServoUrl) { + let location = input.source_location(position); let errors = self.errors.clone(); @@ -302,6 +305,7 @@ impl ParseErrorReporter for CSSInvalidErrorReporterTest { errors.push( CSSError{ + url: url.clone(), line: location.line, column: location.column, message: message.to_owned() @@ -333,7 +337,7 @@ fn test_report_error_stylesheet() { let errors = error_reporter.errors.clone(); - Stylesheet::from_str(css, url, Origin::UserAgent, Default::default(), + Stylesheet::from_str(css, url.clone(), Origin::UserAgent, Default::default(), None, error_reporter, ParserContextExtraData::default()); @@ -349,4 +353,7 @@ fn test_report_error_stylesheet() { assert_eq!("Unsupported property declaration: 'display: invalid;'", error.message); assert_eq!(4, error.line); assert_eq!(9, error.column); + + // testing for the url + assert_eq!(url, error.url); }