From 57e74542ee0b33bf103064a194f4d7ba27ea5b49 Mon Sep 17 00:00:00 2001 From: coalman Date: Mon, 17 Apr 2017 03:45:30 -0400 Subject: [PATCH 1/2] Make tidy check that opening and closing braces that begin a line do so with proper alignment. --- python/tidy/servo_tidy/tidy.py | 15 ++++++++++++--- python/tidy/servo_tidy_tests/rust_tidy.rs | 9 +++++++++ python/tidy/servo_tidy_tests/test_tidy.py | 2 ++ 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/python/tidy/servo_tidy/tidy.py b/python/tidy/servo_tidy/tidy.py index 1468d00bf42..ccca233db9c 100644 --- a/python/tidy/servo_tidy/tidy.py +++ b/python/tidy/servo_tidy/tidy.py @@ -450,6 +450,8 @@ def check_rust(file_name, lines): prev_crate = {} prev_mod = {} prev_feature_name = "" + indent = 0 + prev_indent = 0 decl_message = "{} is not in alphabetical order" decl_expected = "\n\t\033[93mexpected: {}\033[0m" @@ -458,6 +460,9 @@ def check_rust(file_name, lines): for idx, original_line in enumerate(lines): # simplify the analysis line = original_line.strip() + prev_indent = indent + indent = len(original_line) - len(line) + is_attribute = re.search(r"#\[.*\]", line) is_comment = re.search(r"^//|^/\*|^\*", line) @@ -577,11 +582,17 @@ def check_rust(file_name, lines): yield (idx + 1, "found an empty line following a {") prev_open_brace = line.endswith("{") + # ensure a line starting with { or } has a number of leading spaces that is a multiple of 4 + if line.startswith(("{", "}")): + match = re.match(r"(?: {4})* {1,3}([{}])", original_line) + if match: + if indent != prev_indent - 4: + yield (idx + 1, "space before {} is not a multiple of 4".format(match.group(1))) + # check alphabetical order of extern crates if line.startswith("extern crate "): # strip "extern crate " from the begin and ";" from the end crate_name = line[13:-1] - indent = len(original_line) - len(line) if indent not in prev_crate: prev_crate[indent] = "" if prev_crate[indent] > crate_name: @@ -616,7 +627,6 @@ def check_rust(file_name, lines): # into a single import block if line.startswith("use "): import_block = True - indent = len(original_line) - len(line) if not line.endswith(";") and '{' in line: yield (idx + 1, "use statement spans multiple lines") if '{ ' in line: @@ -645,7 +655,6 @@ def check_rust(file_name, lines): # modules must be in the same line and alphabetically sorted if line.startswith("mod ") or line.startswith("pub mod "): - indent = len(original_line) - len(line) # strip /(pub )?mod/ from the left and ";" from the right mod = line[4:-1] if line.startswith("mod ") else line[8:-1] diff --git a/python/tidy/servo_tidy_tests/rust_tidy.rs b/python/tidy/servo_tidy_tests/rust_tidy.rs index 7a65de0ffcc..bb1b4e00572 100644 --- a/python/tidy/servo_tidy_tests/rust_tidy.rs +++ b/python/tidy/servo_tidy_tests/rust_tidy.rs @@ -63,4 +63,13 @@ impl test { let var = "val"; + + fn test_fun4() + { + } + let var = if true { + "true" + } else { // Should not trigger + "false" + } // Should not trigger } diff --git a/python/tidy/servo_tidy_tests/test_tidy.py b/python/tidy/servo_tidy_tests/test_tidy.py index a50d92b241e..07dbaa020dc 100644 --- a/python/tidy/servo_tidy_tests/test_tidy.py +++ b/python/tidy/servo_tidy_tests/test_tidy.py @@ -131,6 +131,8 @@ class CheckTidiness(unittest.TestCase): self.assertEqual('extra space after (', errors.next()[2]) self.assertEqual('extra space after test_fun', errors.next()[2]) self.assertEqual('no = in the beginning of line', errors.next()[2]) + self.assertEqual('space before { is not a multiple of 4', errors.next()[2]) + self.assertEqual('space before } is not a multiple of 4', errors.next()[2]) self.assertNoMoreErrors(errors) feature_errors = tidy.collect_errors_for_files(iterFile('lib.rs'), [], [tidy.check_rust], print_text=False) From fc34b9a14dfb357f15116a5e53a73146be14845a Mon Sep 17 00:00:00 2001 From: coalman Date: Tue, 18 Apr 2017 02:21:10 -0400 Subject: [PATCH 2/2] Fix indentation errors in servo rust code that tidy now finds. --- components/devtools/actors/network_event.rs | 4 +- components/net/hsts.rs | 2 +- components/net/mime_classifier.rs | 2 +- components/script/dom/bindings/trace.rs | 2 +- components/script/dom/blob.rs | 12 +- components/script/dom/dommatrixreadonly.rs | 2 +- components/script/dom/element.rs | 2 +- components/script/dom/node.rs | 2 +- components/script/dom/treewalker.rs | 2 +- .../script/dom/webglrenderingcontext.rs | 4 +- .../script_layout_interface/reporter.rs | 38 ++--- components/selectors/parser.rs | 2 +- components/style/animation.rs | 2 +- components/style/attr.rs | 4 +- components/style/values/specified/length.rs | 2 +- components/style/values/specified/mod.rs | 2 +- components/webdriver_server/lib.rs | 2 +- ports/geckolib/glue.rs | 156 +++++++++--------- tests/unit/style/media_queries.rs | 12 +- 19 files changed, 127 insertions(+), 127 deletions(-) diff --git a/components/devtools/actors/network_event.rs b/components/devtools/actors/network_event.rs index 3d63a5ea691..d05d21cc882 100644 --- a/components/devtools/actors/network_event.rs +++ b/components/devtools/actors/network_event.rs @@ -365,7 +365,7 @@ impl NetworkEventActor { RawStatus(s, Cow::from(status_text)) }); self.response.body = response.body.clone(); - } + } pub fn event_actor(&self) -> EventActor { // TODO: Send the correct values for startedDateTime, isXHR, private @@ -413,7 +413,7 @@ impl NetworkEventActor { transferredSize: 0, discardResponseBody: true, } - } + } pub fn response_cookies(&self) -> ResponseCookiesMsg { let mut cookies_size = 0; diff --git a/components/net/hsts.rs b/components/net/hsts.rs index 3abf1c34d64..85896287715 100644 --- a/components/net/hsts.rs +++ b/components/net/hsts.rs @@ -113,7 +113,7 @@ impl HstsList { fn has_subdomain(&self, host: &str, base_domain: &str) -> bool { self.entries_map.get(base_domain).map_or(false, |entries| { - entries.iter().any(|e| e.matches_subdomain(host)) + entries.iter().any(|e| e.matches_subdomain(host)) }) } diff --git a/components/net/mime_classifier.rs b/components/net/mime_classifier.rs index 02c6f3ca8ed..e4e1d7c0093 100644 --- a/components/net/mime_classifier.rs +++ b/components/net/mime_classifier.rs @@ -815,7 +815,7 @@ impl ByteMatcher { mask: b"\xFF\xFF\xDF\xDF\xDF\xDF\xDF\xDF\xDF\xFF\xDF\xDF\xDF\xDF", content_type: (TopLevel::Text, "html"), leading_ignore: b"\t\n\x0C\r " - } + } } } diff --git a/components/script/dom/bindings/trace.rs b/components/script/dom/bindings/trace.rs index 9c798c42500..1bc9c165fb1 100644 --- a/components/script/dom/bindings/trace.rs +++ b/components/script/dom/bindings/trace.rs @@ -728,7 +728,7 @@ impl RootableVec { RootableVec { v: vec![], } - } + } } /// A vector of items that are rooted for the lifetime 'a. diff --git a/components/script/dom/blob.rs b/components/script/dom/blob.rs index cb61ced9e3a..c076282dcc1 100644 --- a/components/script/dom/blob.rs +++ b/components/script/dom/blob.rs @@ -353,12 +353,12 @@ pub fn blob_parts_to_bytes(blobparts: Vec) -> Result, ()> impl BlobMethods for Blob { // https://w3c.github.io/FileAPI/#dfn-size fn Size(&self) -> u64 { - match *self.blob_impl.borrow() { - BlobImpl::File(ref f) => f.size, - BlobImpl::Memory(ref v) => v.len() as u64, - BlobImpl::Sliced(ref parent, ref rel_pos) => - rel_pos.to_abs_range(parent.Size() as usize).len() as u64, - } + match *self.blob_impl.borrow() { + BlobImpl::File(ref f) => f.size, + BlobImpl::Memory(ref v) => v.len() as u64, + BlobImpl::Sliced(ref parent, ref rel_pos) => + rel_pos.to_abs_range(parent.Size() as usize).len() as u64, + } } // https://w3c.github.io/FileAPI/#dfn-type diff --git a/components/script/dom/dommatrixreadonly.rs b/components/script/dom/dommatrixreadonly.rs index 4f6331e0dd7..875e8e6f42c 100644 --- a/components/script/dom/dommatrixreadonly.rs +++ b/components/script/dom/dommatrixreadonly.rs @@ -37,7 +37,7 @@ impl DOMMatrixReadOnly { reflector_: Reflector::new(), matrix: DOMRefCell::new(matrix), is2D: Cell::new(is2D), - } + } } // https://drafts.fxtf.org/geometry-1/#dom-dommatrixreadonly-dommatrixreadonly diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index 8baba9bb54d..8912abba6ae 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -2599,7 +2599,7 @@ impl Element { self.has_attribute(&local_name!("href")) }, _ => false, - } + } } /// Please call this method *only* for real click events diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs index e21dfa2bee0..ea0638a006f 100644 --- a/components/script/dom/node.rs +++ b/components/script/dom/node.rs @@ -2383,7 +2383,7 @@ impl NodeMethods for Node { // Step 2. Node::namespace_to_string(Node::locate_namespace(self, prefix)) - } + } // https://dom.spec.whatwg.org/#dom-node-isdefaultnamespace fn IsDefaultNamespace(&self, namespace: Option) -> bool { diff --git a/components/script/dom/treewalker.rs b/components/script/dom/treewalker.rs index 24925b9f0d9..2409d65af4f 100644 --- a/components/script/dom/treewalker.rs +++ b/components/script/dom/treewalker.rs @@ -457,7 +457,7 @@ impl<'a> Iterator for &'a TreeWalker { // will probably be using a native Rust filter, // which cannot produce an Err result. unreachable!() - } + } } } diff --git a/components/script/dom/webglrenderingcontext.rs b/components/script/dom/webglrenderingcontext.rs index f13f7f02d9b..8d99e1303d0 100644 --- a/components/script/dom/webglrenderingcontext.rs +++ b/components/script/dom/webglrenderingcontext.rs @@ -1795,7 +1795,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { // WebGl conformance expects error with null programs. Check tests in get-active-test.html self.webgl_error(InvalidValue); return None; - } + } }; match program.get_active_uniform(index) { @@ -1822,7 +1822,7 @@ impl WebGLRenderingContextMethods for WebGLRenderingContext { // WebGl conformance expects error with null programs. Check tests in get-active-test.html self.webgl_error(InvalidValue); return None; - } + } }; match program.get_active_attrib(index) { diff --git a/components/script_layout_interface/reporter.rs b/components/script_layout_interface/reporter.rs index 23e216341ee..12157e646a4 100644 --- a/components/script_layout_interface/reporter.rs +++ b/components/script_layout_interface/reporter.rs @@ -22,28 +22,28 @@ pub struct CSSErrorReporter { } impl ParseErrorReporter for CSSErrorReporter { - fn report_error(&self, - input: &mut Parser, - position: SourcePosition, - message: &str, - url: &ServoUrl, - line_number_offset: u64) { + fn report_error(&self, + input: &mut Parser, + position: SourcePosition, + message: &str, + url: &ServoUrl, + line_number_offset: u64) { let location = input.source_location(position); let line_offset = location.line + line_number_offset as usize; if log_enabled!(log::LogLevel::Info) { - info!("Url:\t{}\n{}:{} {}", - url.as_str(), - line_offset, - location.column, - message) + info!("Url:\t{}\n{}:{} {}", + url.as_str(), + line_offset, + location.column, + message) } - //TODO: report a real filename - let _ = self.script_chan.lock().unwrap().send( - ConstellationControlMsg::ReportCSSError(self.pipelineid, - "".to_owned(), - location.line, - location.column, - message.to_owned())); - } + //TODO: report a real filename + let _ = self.script_chan.lock().unwrap().send( + ConstellationControlMsg::ReportCSSError(self.pipelineid, + "".to_owned(), + location.line, + location.column, + message.to_owned())); + } } diff --git a/components/selectors/parser.rs b/components/selectors/parser.rs index bff599600e2..0431b75f484 100644 --- a/components/selectors/parser.rs +++ b/components/selectors/parser.rs @@ -427,7 +427,7 @@ impl ToCss for SimpleSelector { attr_selector_to_css(a, " = ", v, match case { CaseSensitivity::CaseSensitive => None, CaseSensitivity::CaseInsensitive => Some(" i"), - }, dest) + }, dest) } AttrDashMatch(ref a, ref v) => attr_selector_to_css(a, " |= ", v, None, dest), AttrIncludesNeverMatch(ref a, ref v) | diff --git a/components/style/animation.rs b/components/style/animation.rs index aeff683f407..fc4a4bd1f7a 100644 --- a/components/style/animation.rs +++ b/components/style/animation.rs @@ -381,7 +381,7 @@ pub fn start_transitions_if_applicable(new_animations_sender: &Sender // [1]: https://drafts.csswg.org/css-transitions/#starting if possibly_expired_animations.iter().any(|animation| { animation.has_the_same_end_value_as(&property_animation) - }) { + }) { continue } diff --git a/components/style/attr.rs b/components/style/attr.rs index 04d37a37a70..1989816329d 100644 --- a/components/style/attr.rs +++ b/components/style/attr.rs @@ -160,8 +160,8 @@ impl AttrValue { pub fn from_comma_separated_tokenlist(tokens: String) -> AttrValue { let atoms = split_commas(&tokens).map(Atom::from) .fold(vec![], |mut acc, atom| { - if !acc.contains(&atom) { acc.push(atom) } - acc + if !acc.contains(&atom) { acc.push(atom) } + acc }); AttrValue::TokenList(tokens, atoms) } diff --git a/components/style/values/specified/length.rs b/components/style/values/specified/length.rs index 1468eb5ce53..4d41be3e07e 100644 --- a/components/style/values/specified/length.rs +++ b/components/style/values/specified/length.rs @@ -1022,7 +1022,7 @@ impl ToCss for CalcLengthOrPercentage { } write!(dest, ")") - } + } } /// A percentage value. diff --git a/components/style/values/specified/mod.rs b/components/style/values/specified/mod.rs index 1852a10f370..e5d4693a2d3 100644 --- a/components/style/values/specified/mod.rs +++ b/components/style/values/specified/mod.rs @@ -554,7 +554,7 @@ impl HasViewportPercentage for BorderWidth { match *self { BorderWidth::Thin | BorderWidth::Medium | BorderWidth::Thick => false, BorderWidth::Width(ref length) => length.has_viewport_percentage() - } + } } } diff --git a/components/webdriver_server/lib.rs b/components/webdriver_server/lib.rs index 6fb494e0eef..210c5b0f9c8 100644 --- a/components/webdriver_server/lib.rs +++ b/components/webdriver_server/lib.rs @@ -95,7 +95,7 @@ pub fn start_server(port: u16, constellation_chan: Sender) { match server::start(SocketAddr::V4(address), handler, &extension_routes()) { Ok(listening) => info!("WebDriver server listening on {}", listening.socket), Err(_) => panic!("Unable to start WebDriver HTTPD server"), - } + } }).expect("Thread spawning failed"); } diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index 8c7d2175a16..3b9edccdde1 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -2023,90 +2023,90 @@ pub extern "C" fn Servo_StyleSet_FillKeyframesForName(raw_data: RawServoStyleSet let style = ComputedValues::as_arc(&style); if let Some(ref animation) = data.stylist.animations().get(&name) { - let global_style_data = &*GLOBAL_STYLE_DATA; - let guard = global_style_data.shared_lock.read(); - for step in &animation.steps { - // Override timing_function if the keyframe has animation-timing-function. - let timing_function = if let Some(val) = step.get_animation_timing_function(&guard) { - val.into() - } else { - *timing_function - }; + let global_style_data = &*GLOBAL_STYLE_DATA; + let guard = global_style_data.shared_lock.read(); + for step in &animation.steps { + // Override timing_function if the keyframe has animation-timing-function. + let timing_function = if let Some(val) = step.get_animation_timing_function(&guard) { + val.into() + } else { + *timing_function + }; - let keyframe = unsafe { - Gecko_AnimationAppendKeyframe(keyframes, - step.start_percentage.0 as f32, - &timing_function) - }; + let keyframe = unsafe { + Gecko_AnimationAppendKeyframe(keyframes, + step.start_percentage.0 as f32, + &timing_function) + }; - fn add_computed_property_value(keyframe: *mut Keyframe, - index: usize, - style: &ComputedValues, - property: &TransitionProperty, - shared_lock: &SharedRwLock) { - let block = style.to_declaration_block(property.clone().into()); - unsafe { - (*keyframe).mPropertyValues.set_len((index + 1) as u32); - (*keyframe).mPropertyValues[index].mProperty = property.clone().into(); - // FIXME. Do not set computed values once we handles missing keyframes - // with additive composition. - (*keyframe).mPropertyValues[index].mServoDeclarationBlock.set_arc_leaky( - Arc::new(shared_lock.wrap(block))); - } - } + fn add_computed_property_value(keyframe: *mut Keyframe, + index: usize, + style: &ComputedValues, + property: &TransitionProperty, + shared_lock: &SharedRwLock) { + let block = style.to_declaration_block(property.clone().into()); + unsafe { + (*keyframe).mPropertyValues.set_len((index + 1) as u32); + (*keyframe).mPropertyValues[index].mProperty = property.clone().into(); + // FIXME. Do not set computed values once we handles missing keyframes + // with additive composition. + (*keyframe).mPropertyValues[index].mServoDeclarationBlock.set_arc_leaky( + Arc::new(shared_lock.wrap(block))); + } + } - match step.value { - KeyframesStepValue::ComputedValues => { - for (index, property) in animation.properties_changed.iter().enumerate() { - add_computed_property_value( - keyframe, index, style, property, &global_style_data.shared_lock); - } - }, - KeyframesStepValue::Declarations { ref block } => { - let guard = block.read_with(&guard); - // Filter out non-animatable properties. - let animatable = - guard.declarations() - .iter() - .filter(|&&(ref declaration, _)| { - declaration.is_animatable() - }); + match step.value { + KeyframesStepValue::ComputedValues => { + for (index, property) in animation.properties_changed.iter().enumerate() { + add_computed_property_value( + keyframe, index, style, property, &global_style_data.shared_lock); + } + }, + KeyframesStepValue::Declarations { ref block } => { + let guard = block.read_with(&guard); + // Filter out non-animatable properties. + let animatable = + guard.declarations() + .iter() + .filter(|&&(ref declaration, _)| { + declaration.is_animatable() + }); - let mut seen = LonghandIdSet::new(); + let mut seen = LonghandIdSet::new(); - for (index, &(ref declaration, _)) in animatable.enumerate() { - unsafe { - let property = TransitionProperty::from_declaration(declaration).unwrap(); - (*keyframe).mPropertyValues.set_len((index + 1) as u32); - (*keyframe).mPropertyValues[index].mProperty = property.into(); - (*keyframe).mPropertyValues[index].mServoDeclarationBlock.set_arc_leaky( - Arc::new(global_style_data.shared_lock.wrap( - PropertyDeclarationBlock::with_one( - declaration.clone(), Importance::Normal - )))); - if step.start_percentage.0 == 0. || - step.start_percentage.0 == 1. { - seen.set_transition_property_bit(&property); - } - } - } + for (index, &(ref declaration, _)) in animatable.enumerate() { + unsafe { + let property = TransitionProperty::from_declaration(declaration).unwrap(); + (*keyframe).mPropertyValues.set_len((index + 1) as u32); + (*keyframe).mPropertyValues[index].mProperty = property.into(); + (*keyframe).mPropertyValues[index].mServoDeclarationBlock.set_arc_leaky( + Arc::new(global_style_data.shared_lock.wrap( + PropertyDeclarationBlock::with_one( + declaration.clone(), Importance::Normal + )))); + if step.start_percentage.0 == 0. || + step.start_percentage.0 == 1. { + seen.set_transition_property_bit(&property); + } + } + } - // Append missing property values in the initial or the finial keyframes. - if step.start_percentage.0 == 0. || - step.start_percentage.0 == 1. { - let mut index = unsafe { (*keyframe).mPropertyValues.len() }; - for property in animation.properties_changed.iter() { - if !seen.has_transition_property_bit(&property) { - add_computed_property_value( - keyframe, index, style, property, &global_style_data.shared_lock); - index += 1; - } - } - } - }, - } - } - return true + // Append missing property values in the initial or the finial keyframes. + if step.start_percentage.0 == 0. || + step.start_percentage.0 == 1. { + let mut index = unsafe { (*keyframe).mPropertyValues.len() }; + for property in animation.properties_changed.iter() { + if !seen.has_transition_property_bit(&property) { + add_computed_property_value( + keyframe, index, style, property, &global_style_data.shared_lock); + index += 1; + } + } + } + }, + } + } + return true } false } diff --git a/tests/unit/style/media_queries.rs b/tests/unit/style/media_queries.rs index ab04b8b07ee..f256d296c9f 100644 --- a/tests/unit/style/media_queries.rs +++ b/tests/unit/style/media_queries.rs @@ -283,12 +283,12 @@ fn test_mq_expressions() { #[test] fn test_to_css() { - test_media_rule("@media print and (width: 43px) { }", |list, _| { - let q = &list.media_queries[0]; - let mut dest = String::new(); - assert_eq!(Ok(()), q.to_css(&mut dest)); - assert_eq!(dest, "print and (width: 43px)"); - }); + test_media_rule("@media print and (width: 43px) { }", |list, _| { + let q = &list.media_queries[0]; + let mut dest = String::new(); + assert_eq!(Ok(()), q.to_css(&mut dest)); + assert_eq!(dest, "print and (width: 43px)"); + }); } #[test]