Auto merge of #15751 - avinash-vijayaraghavan:testing-csserror, r=cbrewster

first stab. added ServoUrl as a parameter to report_error(...) of Par…

@jdm @SimonSapin
<!-- Please describe your changes on the following line: -->

1. Added ServoUrl as a parameter to report_error(...) of ParseErrorReporter trait.
2. I am not sure how to handle the case of impl ParseErrorReporter for CSSErrorReporter and MemoryHoleReporter, so have not made any changes  (other than adding ServoUrl arg) to report_error implementations for these.
3. In StdoutErrorReporter i have added the ServoUrl arg to the info! function,
4. I would like to know if i am on the correct path and clarify what else needs to be done.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [ X] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [ ] These changes fix #15708 (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because wanted to clarify before writing tests

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/15751)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2017-03-06 07:11:51 -08:00 committed by GitHub
commit 0dbee36915
7 changed files with 36 additions and 19 deletions

View file

@ -7,6 +7,7 @@ use ipc_channel::ipc::IpcSender;
use log; use log;
use msg::constellation_msg::PipelineId; use msg::constellation_msg::PipelineId;
use script_traits::ConstellationControlMsg; use script_traits::ConstellationControlMsg;
use servo_url::ServoUrl;
use std::sync::{Mutex, Arc}; use std::sync::{Mutex, Arc};
use style::error_reporting::ParseErrorReporter; use style::error_reporting::ParseErrorReporter;
@ -21,11 +22,13 @@ pub struct CSSErrorReporter {
} }
impl ParseErrorReporter for CSSErrorReporter { impl ParseErrorReporter for CSSErrorReporter {
fn report_error(&self, input: &mut Parser, position: SourcePosition, message: &str) { fn report_error(&self, input: &mut Parser, position: SourcePosition, message: &str,
let location = input.source_location(position); url: &ServoUrl) {
if log_enabled!(log::LogLevel::Info) { let location = input.source_location(position);
info!("{}:{} {}", location.line, location.column, message) if log_enabled!(log::LogLevel::Info) {
} info!("Url:\t{}\n{}:{} {}", url.as_str(), location.line, location.column, message)
}
//TODO: report a real filename //TODO: report a real filename
let _ = self.script_chan.lock().unwrap().send( let _ = self.script_chan.lock().unwrap().send(
ConstellationControlMsg::ReportCSSError(self.pipelineid, ConstellationControlMsg::ReportCSSError(self.pipelineid,

View file

@ -8,6 +8,7 @@
use cssparser::{Parser, SourcePosition}; use cssparser::{Parser, SourcePosition};
use log; use log;
use servo_url::ServoUrl;
/// A generic trait for an error reporter. /// A generic trait for an error reporter.
pub trait ParseErrorReporter { pub trait ParseErrorReporter {
@ -15,7 +16,7 @@ pub trait ParseErrorReporter {
/// ///
/// Returns the current input being parsed, the source position it was /// Returns the current input being parsed, the source position it was
/// reported from, and a message. /// 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. /// Clone this error reporter.
/// ///
/// TODO(emilio): I'm pretty sure all the box shenanigans can go away. /// 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! /// TODO(emilio): The name of this reporter is a lie, and should be renamed!
pub struct StdoutErrorReporter; pub struct StdoutErrorReporter;
impl ParseErrorReporter for StdoutErrorReporter { impl ParseErrorReporter for StdoutErrorReporter {
fn report_error(&self, input: &mut Parser, position: SourcePosition, message: &str) { fn report_error(&self, input: &mut Parser, position: SourcePosition, message: &str,
if log_enabled!(log::LogLevel::Info) { url: &ServoUrl) {
let location = input.source_location(position); if log_enabled!(log::LogLevel::Info) {
info!("{}:{} {}", location.line, location.column, message) let location = input.source_location(position);
} info!("Url:\t{}\n{}:{} {}", url.as_str(), location.line, location.column, message)
}
} }
fn clone(&self) -> Box<ParseErrorReporter + Send + Sync> { fn clone(&self) -> Box<ParseErrorReporter + Send + Sync> {

View file

@ -90,7 +90,8 @@ impl<'a> ParserContext<'a> {
/// Set a `RUST_LOG=style::errors` environment variable /// Set a `RUST_LOG=style::errors` environment variable
/// to log CSS parse errors to stderr. /// to log CSS parse errors to stderr.
pub fn log_css_error(input: &mut Parser, position: SourcePosition, message: &str, parsercontext: &ParserContext) { 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 // XXXManishearth Replace all specified value parse impls with impls of this

View file

@ -252,7 +252,8 @@ impl ParseErrorReporter for MemoryHoleReporter {
fn report_error(&self, fn report_error(&self,
_: &mut Parser, _: &mut Parser,
_: SourcePosition, _: SourcePosition,
_: &str) { _: &str,
_: &ServoUrl) {
// do nothing // do nothing
} }
fn clone(&self) -> Box<ParseErrorReporter + Send + Sync> { fn clone(&self) -> Box<ParseErrorReporter + Send + Sync> {

View file

@ -18,8 +18,10 @@ use style_traits::ToCss;
pub struct CSSErrorReporterTest; pub struct CSSErrorReporterTest;
impl ParseErrorReporter for 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<ParseErrorReporter + Send + Sync> { fn clone(&self) -> Box<ParseErrorReporter + Send + Sync> {
Box::new(CSSErrorReporterTest) Box::new(CSSErrorReporterTest)
} }

View file

@ -17,8 +17,9 @@ use test::{self, Bencher};
struct ErrorringErrorReporter; struct ErrorringErrorReporter;
impl ParseErrorReporter for ErrorringErrorReporter { impl ParseErrorReporter for ErrorringErrorReporter {
fn report_error(&self, _: &mut Parser, position: SourcePosition, message: &str) { fn report_error(&self, input: &mut Parser, position: SourcePosition, message: &str,
panic!("CSS error: {:?} {}", position, message); url: &ServoUrl) {
panic!("CSS error: {}\t\n{:?} {}", url.as_str(), position, message);
} }
fn clone(&self) -> Box<ParseErrorReporter + Send + Sync> { fn clone(&self) -> Box<ParseErrorReporter + Send + Sync> {

View file

@ -276,6 +276,7 @@ fn test_parse_stylesheet() {
} }
struct CSSError { struct CSSError {
pub url : ServoUrl,
pub line: usize, pub line: usize,
pub column: usize, pub column: usize,
pub message: String pub message: String
@ -294,7 +295,9 @@ impl CSSInvalidErrorReporterTest {
} }
impl ParseErrorReporter for 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 location = input.source_location(position);
let errors = self.errors.clone(); let errors = self.errors.clone();
@ -302,6 +305,7 @@ impl ParseErrorReporter for CSSInvalidErrorReporterTest {
errors.push( errors.push(
CSSError{ CSSError{
url: url.clone(),
line: location.line, line: location.line,
column: location.column, column: location.column,
message: message.to_owned() message: message.to_owned()
@ -333,7 +337,7 @@ fn test_report_error_stylesheet() {
let errors = error_reporter.errors.clone(); 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, None,
error_reporter, error_reporter,
ParserContextExtraData::default()); ParserContextExtraData::default());
@ -349,4 +353,7 @@ fn test_report_error_stylesheet() {
assert_eq!("Unsupported property declaration: 'display: invalid;'", error.message); assert_eq!("Unsupported property declaration: 'display: invalid;'", error.message);
assert_eq!(4, error.line); assert_eq!(4, error.line);
assert_eq!(9, error.column); assert_eq!(9, error.column);
// testing for the url
assert_eq!(url, error.url);
} }