Skip to content

Commit 48f3bed

Browse files
SimonSapinSimonSapin
committed
Use Option instead of a custom ViolationFn enum
1 parent 90e65b2 commit 48f3bed

File tree

3 files changed

+41
-76
lines changed

3 files changed

+41
-76
lines changed

‎.travis.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ script: cargo test --all-features --all
33

44
jobs:
55
include:
6-
- rust: 1.24.0
6+
- rust: 1.27.0
77
- rust: stable
88
- rust: beta
99
- rust: nightly

‎src/lib.rs

+5-8
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ pub extern crate percent_encoding;
118118
use encoding::EncodingOverride;
119119
#[cfg(feature = "heapsize")] use heapsize::HeapSizeOf;
120120
use host::HostInternal;
121-
use parser::{Parser, Context, SchemeType, to_u32, ViolationFn};
121+
use parser::{Parser, Context, SchemeType, to_u32};
122122
use percent_encoding::{PATH_SEGMENT_ENCODE_SET, USERINFO_ENCODE_SET,
123123
percent_encode, percent_decode, utf8_percent_encode};
124124
use std::borrow::Borrow;
@@ -187,7 +187,7 @@ impl HeapSizeOf for Url {
187187
pub struct ParseOptions<'a> {
188188
base_url: Option<&'a Url>,
189189
encoding_override: encoding::EncodingOverride,
190-
violation_fn: ViolationFn<'a>,
190+
violation_fn: Option<&'a dyn Fn(SyntaxViolation)>,
191191
}
192192

193193
impl<'a> ParseOptions<'a> {
@@ -233,10 +233,7 @@ impl<'a> ParseOptions<'a> {
233233
/// # run().unwrap();
234234
/// ```
235235
pub fn syntax_violation_callback(mut self, new: Option<&'a dyn Fn(SyntaxViolation)>) -> Self {
236-
self.violation_fn = match new {
237-
Some(f) => ViolationFn::NewFn(f),
238-
None => ViolationFn::NoOp
239-
};
236+
self.violation_fn = new;
240237
self
241238
}
242239

@@ -259,7 +256,7 @@ impl<'a> Debug for ParseOptions<'a> {
259256
violation_fn: {:?} }}",
260257
self.base_url,
261258
self.encoding_override,
262-
self.violation_fn)
259+
self.violation_fn.map(|_| "…"))
263260
}
264261
}
265262

@@ -389,7 +386,7 @@ impl Url {
389386
ParseOptions {
390387
base_url: None,
391388
encoding_override: EncodingOverride::utf8(),
392-
violation_fn: ViolationFn::NoOp,
389+
violation_fn: None,
393390
}
394391
}
395392

‎src/parser.rs

+35-67
Original file line numberDiff line numberDiff line change
@@ -165,17 +165,17 @@ pub struct Input<'i> {
165165

166166
impl<'i> Input<'i> {
167167
pub fn new(input: &'i str) -> Self {
168-
Input::with_log(input, ViolationFn::NoOp)
168+
Input::with_log(input, None)
169169
}
170170

171-
pub fn with_log(original_input: &'i str, vfn: ViolationFn) -> Self {
171+
pub fn with_log(original_input: &'i str, vfn: Option<&dyn Fn(SyntaxViolation)>) -> Self {
172172
let input = original_input.trim_matches(c0_control_or_space);
173-
if vfn.is_set() {
173+
if let Some(vfn) = vfn {
174174
if input.len() < original_input.len() {
175-
vfn.call(SyntaxViolation::C0SpaceIgnored)
175+
vfn(SyntaxViolation::C0SpaceIgnored)
176176
}
177177
if input.chars().any(|c| matches!(c, '\t' | '\n' | '\r')) {
178-
vfn.call(SyntaxViolation::TabOrNewlineIgnored)
178+
vfn(SyntaxViolation::TabOrNewlineIgnored)
179179
}
180180
}
181181
Input { chars: input.chars() }
@@ -268,56 +268,11 @@ impl<'i> Iterator for Input<'i> {
268268
}
269269
}
270270

271-
/// Wrapper for syntax violation callback functions.
272-
#[derive(Copy, Clone)]
273-
pub enum ViolationFn<'a> {
274-
NewFn(&'a (dyn Fn(SyntaxViolation) + 'a)),
275-
NoOp
276-
}
277-
278-
impl<'a> ViolationFn<'a> {
279-
/// Call with a violation.
280-
pub fn call(self, v: SyntaxViolation) {
281-
match self {
282-
ViolationFn::NewFn(f) => f(v),
283-
ViolationFn::NoOp => {}
284-
}
285-
}
286-
287-
/// Call with a violation, if provided test returns true. Avoids
288-
/// the test entirely if `NoOp`.
289-
pub fn call_if<F>(self, v: SyntaxViolation, test: F)
290-
where F: Fn() -> bool
291-
{
292-
match self {
293-
ViolationFn::NewFn(f) => if test() { f(v) },
294-
ViolationFn::NoOp => {} // avoid test
295-
}
296-
}
297-
298-
/// True if not `NoOp`
299-
pub fn is_set(self) -> bool {
300-
match self {
301-
ViolationFn::NoOp => false,
302-
_ => true
303-
}
304-
}
305-
}
306-
307-
impl<'a> fmt::Debug for ViolationFn<'a> {
308-
fn fmt(&self, f: &mut Formatter) -> fmt::Result {
309-
match *self {
310-
ViolationFn::NewFn(_) => write!(f, "NewFn(Fn(SyntaxViolation))"),
311-
ViolationFn::NoOp => write!(f, "NoOp")
312-
}
313-
}
314-
}
315-
316271
pub struct Parser<'a> {
317272
pub serialization: String,
318273
pub base_url: Option<&'a Url>,
319274
pub query_encoding_override: EncodingOverride,
320-
pub violation_fn: ViolationFn<'a>,
275+
pub violation_fn: Option<&'a dyn Fn(SyntaxViolation)>,
321276
pub context: Context,
322277
}
323278

@@ -329,12 +284,26 @@ pub enum Context {
329284
}
330285

331286
impl<'a> Parser<'a> {
287+
fn log_violation(&self, v: SyntaxViolation) {
288+
if let Some(f) = self.violation_fn {
289+
f(v)
290+
}
291+
}
292+
293+
fn log_violation_if(&self, v: SyntaxViolation, test: impl FnOnce() -> bool) {
294+
if let Some(f) = self.violation_fn {
295+
if test() {
296+
f(v)
297+
}
298+
}
299+
}
300+
332301
pub fn for_setter(serialization: String) -> Parser<'a> {
333302
Parser {
334303
serialization: serialization,
335304
base_url: None,
336305
query_encoding_override: EncodingOverride::utf8(),
337-
violation_fn: ViolationFn::NoOp,
306+
violation_fn: None,
338307
context: Context::Setter,
339308
}
340309
}
@@ -398,7 +367,7 @@ impl<'a> Parser<'a> {
398367
self.serialization.push(':');
399368
match scheme_type {
400369
SchemeType::File => {
401-
self.violation_fn.call_if(ExpectedFileDoubleSlash, || !input.starts_with("//"));
370+
self.log_violation_if(ExpectedFileDoubleSlash, || !input.starts_with("//"));
402371
let base_file_url = self.base_url.and_then(|base| {
403372
if base.scheme() == "file" { Some(base) } else { None }
404373
});
@@ -418,7 +387,7 @@ impl<'a> Parser<'a> {
418387
}
419388
}
420389
// special authority slashes state
421-
self.violation_fn.call_if(ExpectedDoubleSlash, || {
390+
self.log_violation_if(ExpectedDoubleSlash, || {
422391
input.clone().take_while(|&c| matches!(c, '/' | '\\'))
423392
.collect::<String>() != "//"
424393
});
@@ -552,10 +521,10 @@ impl<'a> Parser<'a> {
552521
}
553522
}
554523
Some('/') | Some('\\') => {
555-
self.violation_fn.call_if(Backslash, || first_char == Some('\\'));
524+
self.log_violation_if(Backslash, || first_char == Some('\\'));
556525
// file slash state
557526
let (next_char, input_after_next_char) = input_after_first_char.split_first();
558-
self.violation_fn.call_if(Backslash, || next_char == Some('\\'));
527+
self.log_violation_if(Backslash, || next_char == Some('\\'));
559528
if matches!(next_char, Some('/') | Some('\\')) {
560529
// file host state
561530
self.serialization.push_str("file://");
@@ -707,7 +676,7 @@ impl<'a> Parser<'a> {
707676
Some('/') | Some('\\') => {
708677
let (slashes_count, remaining) = input.count_matching(|c| matches!(c, '/' | '\\'));
709678
if slashes_count >= 2 {
710-
self.violation_fn.call_if(SyntaxViolation::ExpectedDoubleSlash, || {
679+
self.log_violation_if(SyntaxViolation::ExpectedDoubleSlash, || {
711680
input.clone().take_while(|&c| matches!(c, '/' | '\\'))
712681
.collect::<String>() != "//"
713682
});
@@ -771,9 +740,9 @@ impl<'a> Parser<'a> {
771740
match c {
772741
'@' => {
773742
if last_at.is_some() {
774-
self.violation_fn.call(SyntaxViolation::UnencodedAtSign)
743+
self.log_violation(SyntaxViolation::UnencodedAtSign)
775744
} else {
776-
self.violation_fn.call(SyntaxViolation::EmbeddedCredentials)
745+
self.log_violation(SyntaxViolation::EmbeddedCredentials)
777746
}
778747
last_at = Some((char_count, remaining.clone()))
779748
},
@@ -971,7 +940,7 @@ impl<'a> Parser<'a> {
971940
match input.split_first() {
972941
(Some('/'), remaining) => input = remaining,
973942
(Some('\\'), remaining) => if scheme_type.is_special() {
974-
self.violation_fn.call(SyntaxViolation::Backslash);
943+
self.log_violation(SyntaxViolation::Backslash);
975944
input = remaining
976945
},
977946
_ => {}
@@ -999,7 +968,7 @@ impl<'a> Parser<'a> {
999968
},
1000969
'\\' if self.context != Context::PathSegmentSetter &&
1001970
scheme_type.is_special() => {
1002-
self.violation_fn.call(SyntaxViolation::Backslash);
971+
self.log_violation(SyntaxViolation::Backslash);
1003972
ends_with_slash = true;
1004973
break
1005974
},
@@ -1045,7 +1014,7 @@ impl<'a> Parser<'a> {
10451014
self.serialization.push(':');
10461015
}
10471016
if *has_host {
1048-
self.violation_fn.call(SyntaxViolation::FileWithHostAndWindowsDrive);
1017+
self.log_violation(SyntaxViolation::FileWithHostAndWindowsDrive);
10491018
*has_host = false; // FIXME account for this in callers
10501019
}
10511020
}
@@ -1187,7 +1156,7 @@ impl<'a> Parser<'a> {
11871156
pub fn parse_fragment(&mut self, mut input: Input) {
11881157
while let Some((c, utf8_c)) = input.next_utf8() {
11891158
if c == '\0' {
1190-
self.violation_fn.call(SyntaxViolation::NullInFragment)
1159+
self.log_violation(SyntaxViolation::NullInFragment)
11911160
} else {
11921161
self.check_url_code_point(c, &input);
11931162
self.serialization.extend(utf8_percent_encode(utf8_c,
@@ -1197,16 +1166,15 @@ impl<'a> Parser<'a> {
11971166
}
11981167

11991168
fn check_url_code_point(&self, c: char, input: &Input) {
1200-
let vfn = self.violation_fn;
1201-
if vfn.is_set() {
1169+
if let Some(vfn) = self.violation_fn {
12021170
if c == '%' {
12031171
let mut input = input.clone();
12041172
if !matches!((input.next(), input.next()), (Some(a), Some(b))
12051173
if is_ascii_hex_digit(a) && is_ascii_hex_digit(b)) {
1206-
vfn.call(SyntaxViolation::PercentDecode)
1174+
vfn(SyntaxViolation::PercentDecode)
12071175
}
12081176
} else if !is_url_code_point(c) {
1209-
vfn.call(SyntaxViolation::NonUrlCodePoint)
1177+
vfn(SyntaxViolation::NonUrlCodePoint)
12101178
}
12111179
}
12121180
}

0 commit comments

Comments
 (0)