Skip to content

Commit 32c2006

Browse files
author
bors-servo
authored
Auto merge of #433 - dekellum:improve-violations-interface-2, r=nox
Improve syntax violations interface and preserve Copy This is an alternative implementation of #430 which avoids introducing an `Rc<Fn>` and preserves `Copy` for `ParseOptions`. Thus, based on my current understanding with the prior review, it is completely backward compatible and with no breaking changes. Its a little less elegant in that, without the `Rc<Fn>`, I can not simple wrap the deprecated log_syntax_violation Fn with an adapting closure. But by moving the two possible function references into a new enum `ViolationFn` it is reasonably well contained. The interface for users is effectively the same, so I think the backward compatibility advantage makes this preferable to #430. Updated summary: ### Summary of changes This makes programmatic use of syntax violations more practical while further centralizing this concern in the parser, which should be a maintenance win. * New `SyntaxViolation` enum with descriptions the same as the prior violation static strings. This enables programmatic use of these violations by variant, e.g. ignoring some, warning on others, providing translated descriptions, etc. Enum implementation uses the same macro expansion pattern as was used for `ParseError`. * New `syntax_violation_callback` signature in `ParseOptions` and `Parser`. * Deprecated `log_syntax_violation` and re-implemented, passing the same violation descriptions for backward compatibility. * Unit tests for old `log_syntax_violation` compatibility, and new `syntax_violation_callback` * Includes rustdoc for old and new interfaces including tested example usage of new callback, as requested in #308. <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/rust-url/433) <!-- Reviewable:end -->
2 parents 49bea1e + 255df06 commit 32c2006

File tree

3 files changed

+239
-52
lines changed

3 files changed

+239
-52
lines changed

src/lib.rs

Lines changed: 51 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ pub extern crate percent_encoding;
117117
use encoding::EncodingOverride;
118118
#[cfg(feature = "heapsize")] use heapsize::HeapSizeOf;
119119
use host::HostInternal;
120-
use parser::{Parser, Context, SchemeType, to_u32};
120+
use parser::{Parser, Context, SchemeType, to_u32, ViolationFn};
121121
use percent_encoding::{PATH_SEGMENT_ENCODE_SET, USERINFO_ENCODE_SET,
122122
percent_encode, percent_decode, utf8_percent_encode};
123123
use std::borrow::Borrow;
@@ -135,7 +135,7 @@ use std::str;
135135
pub use origin::{Origin, OpaqueOrigin};
136136
pub use host::{Host, HostAndPort, SocketAddrs};
137137
pub use path_segments::PathSegmentsMut;
138-
pub use parser::ParseError;
138+
pub use parser::{ParseError, SyntaxViolation};
139139
pub use slicing::Position;
140140

141141
mod encoding;
@@ -186,7 +186,7 @@ impl HeapSizeOf for Url {
186186
pub struct ParseOptions<'a> {
187187
base_url: Option<&'a Url>,
188188
encoding_override: encoding::EncodingOverride,
189-
log_syntax_violation: Option<&'a Fn(&'static str)>,
189+
violation_fn: ViolationFn<'a>,
190190
}
191191

192192
impl<'a> ParseOptions<'a> {
@@ -209,9 +209,47 @@ impl<'a> ParseOptions<'a> {
209209
self
210210
}
211211

212-
/// Call the provided function or closure on non-fatal parse errors.
212+
/// Call the provided function or closure on non-fatal parse errors, passing
213+
/// a static string description. This method is deprecated in favor of
214+
/// `syntax_violation_callback` and is implemented as an adaptor for the
215+
/// latter, passing the `SyntaxViolation` description. Only the last value
216+
/// passed to either method will be used by a parser.
217+
#[deprecated]
213218
pub fn log_syntax_violation(mut self, new: Option<&'a Fn(&'static str)>) -> Self {
214-
self.log_syntax_violation = new;
219+
self.violation_fn = match new {
220+
Some(f) => ViolationFn::OldFn(f),
221+
None => ViolationFn::NoOp
222+
};
223+
self
224+
}
225+
226+
/// Call the provided function or closure for a non-fatal `SyntaxViolation`
227+
/// when it occurs during parsing. Note that since the provided function is
228+
/// `Fn`, the caller might need to utilize _interior mutability_, such as with
229+
/// a `RefCell`, to collect the violations.
230+
///
231+
/// ## Example
232+
/// ```
233+
/// use std::cell::RefCell;
234+
/// use url::{Url, SyntaxViolation};
235+
/// # use url::ParseError;
236+
/// # fn run() -> Result<(), url::ParseError> {
237+
/// let violations = RefCell::new(Vec::new());
238+
/// let url = Url::options()
239+
/// .syntax_violation_callback(Some(&|v| violations.borrow_mut().push(v)))
240+
/// .parse("https:////example.com")?;
241+
/// assert_eq!(url.as_str(), "https://example.com/");
242+
/// assert_eq!(violations.into_inner(),
243+
/// vec!(SyntaxViolation::ExpectedDoubleSlash));
244+
/// # Ok(())
245+
/// # }
246+
/// # run().unwrap();
247+
/// ```
248+
pub fn syntax_violation_callback(mut self, new: Option<&'a Fn(SyntaxViolation)>) -> Self {
249+
self.violation_fn = match new {
250+
Some(f) => ViolationFn::NewFn(f),
251+
None => ViolationFn::NoOp
252+
};
215253
self
216254
}
217255

@@ -221,19 +259,20 @@ impl<'a> ParseOptions<'a> {
221259
serialization: String::with_capacity(input.len()),
222260
base_url: self.base_url,
223261
query_encoding_override: self.encoding_override,
224-
log_syntax_violation: self.log_syntax_violation,
262+
violation_fn: self.violation_fn,
225263
context: Context::UrlParser,
226264
}.parse_url(input)
227265
}
228266
}
229267

230268
impl<'a> Debug for ParseOptions<'a> {
231269
fn fmt(&self, f: &mut Formatter) -> fmt::Result {
232-
write!(f, "ParseOptions {{ base_url: {:?}, encoding_override: {:?}, log_syntax_violation: ", self.base_url, self.encoding_override)?;
233-
match self.log_syntax_violation {
234-
Some(_) => write!(f, "Some(Fn(&'static str)) }}"),
235-
None => write!(f, "None }}")
236-
}
270+
write!(f,
271+
"ParseOptions {{ base_url: {:?}, encoding_override: {:?}, \
272+
violation_fn: {:?} }}",
273+
self.base_url,
274+
self.encoding_override,
275+
self.violation_fn)
237276
}
238277
}
239278

@@ -363,7 +402,7 @@ impl Url {
363402
ParseOptions {
364403
base_url: None,
365404
encoding_override: EncodingOverride::utf8(),
366-
log_syntax_violation: None,
405+
violation_fn: ViolationFn::NoOp,
367406
}
368407
}
369408

0 commit comments

Comments
 (0)