Auto merge of #16495 - coalman:tidy-brace-alignment, r=emilio

Make tidy check that opening and closing braces that begin a line do …

…so with proper alignment.

---
<!-- 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
- [x] `./mach test-tidy` does not report any errors
- [X] These changes fix #16486 (github issue number if applicable).

<!-- Either: -->
- [X] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- 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/16495)
<!-- Reviewable:end -->

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;

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))
        })
     }
 

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 "
-           }
+            }
         }
     }
 

components/script/dom/bindings/trace.rs

@@ -728,7 +728,7 @@ impl<T: JSTraceable> RootableVec<T> {
         RootableVec {
             v: vec![],
         }
-   }
+    }
 }
 
 /// A vector of items that are rooted for the lifetime 'a.

components/script/dom/blob.rs

@@ -353,12 +353,12 @@ pub fn blob_parts_to_bytes(blobparts: Vec<BlobOrString>) -> Result<Vec<u8>, ()>
 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

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

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

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<DOMString>) -> bool {

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!()
-       }
+        }
     }
 }
 

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) {

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()));
+    }
 }

components/selectors/parser.rs

@@ -530,7 +530,7 @@ impl<Impl: SelectorImpl> ToCss for SimpleSelector<Impl> {
                 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) |

components/style/animation.rs

@@ -381,7 +381,7 @@ pub fn start_transitions_if_applicable(new_animations_sender: &Sender<Animation>
             // [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
             }
 

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)
     }

components/style/values/specified/length.rs

@@ -1022,7 +1022,7 @@ impl ToCss for CalcLengthOrPercentage {
         }
 
         write!(dest, ")")
-     }
+    }
 }
 
 /// A percentage value.

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()
-         }
+        }
     }
 }
 

components/webdriver_server/lib.rs

@@ -95,7 +95,7 @@ pub fn start_server(port: u16, constellation_chan: Sender<ConstellationMsg>) {
         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");
 }
 

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
 }

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]
 

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
 }

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)

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]