servo: Merge #18553 - Make tidy aware of Rust multiline strings (from mdboom:tidy-support-multiline-strings); r=wafflespeanut
authorMichael Droettboom <mdboom@gmail.com>
Thu, 21 Sep 2017 23:38:29 -0500
changeset 382404 56facbd1d507619532f85af1d6ac24bfab2c9842
parent 382403 804199553c13fd14c266f08cb77816863e60d7f0
child 382405 d2a032f9e2561bfd3baaac404a9eb4c124051266
push id32558
push userkwierso@gmail.com
push dateFri, 22 Sep 2017 21:29:46 +0000
treeherdermozilla-central@61e58a7d800b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerswafflespeanut
milestone58.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
servo: Merge #18553 - Make tidy aware of Rust multiline strings (from mdboom:tidy-support-multiline-strings); r=wafflespeanut <!-- Please describe your changes on the following line: --> This makes the internal tidy script properly ignore the contents of Rust multiline strings. --- <!-- 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 #18551 (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. --> Source-Repo: https://github.com/servo/servo Source-Revision: a8a25dac5226a12916c8fe17155d1dbb3b6cb565
servo/components/layout/generated_content.rs
servo/components/layout/persistent_list.rs
servo/components/script_plugins/unrooted_must_root.rs
servo/python/tidy/servo_tidy/tidy.py
servo/python/tidy/servo_tidy_tests/multiline_string.rs
servo/python/tidy/servo_tidy_tests/test_tidy.py
--- a/servo/components/layout/generated_content.rs
+++ b/servo/components/layout/generated_content.rs
@@ -132,28 +132,28 @@ impl<'a> InorderFlowTraversal for Resolv
     #[inline]
     fn should_process_subtree(&mut self, flow: &mut Flow) -> bool {
         flow::base(flow).restyle_damage.intersects(RESOLVE_GENERATED_CONTENT) ||
             flow::base(flow).flags.intersects(AFFECTS_COUNTERS | HAS_COUNTER_AFFECTING_CHILDREN)
     }
 }
 
 /// The object that mutates the generated content fragments.
-struct ResolveGeneratedContentFragmentMutator<'a,'b:'a> {
+struct ResolveGeneratedContentFragmentMutator<'a, 'b: 'a> {
     /// The traversal.
     traversal: &'a mut ResolveGeneratedContent<'b>,
     /// The level we're at in the flow tree.
     level: u32,
     /// Whether this flow is a block flow.
     is_block: bool,
     /// Whether we've incremented the counter yet.
     incremented: bool,
 }
 
-impl<'a,'b> ResolveGeneratedContentFragmentMutator<'a,'b> {
+impl<'a, 'b> ResolveGeneratedContentFragmentMutator<'a, 'b> {
     fn mutate_fragment(&mut self, fragment: &mut Fragment) {
         // We only reset and/or increment counters once per flow. This avoids double-incrementing
         // counters on list items (once for the main fragment and once for the marker).
         if !self.incremented {
             self.reset_and_increment_counters_as_necessary(fragment);
         }
 
         let mut list_style_type = fragment.style().get_list().list_style_type;
--- a/servo/components/layout/persistent_list.rs
+++ b/servo/components/layout/persistent_list.rs
@@ -64,17 +64,17 @@ impl<T> Clone for PersistentList<T> wher
         // its head.
         PersistentList {
             head: self.head.clone(),
             length: self.length,
         }
     }
 }
 
-pub struct PersistentListIterator<'a,T> where T: 'a + Send + Sync {
+pub struct PersistentListIterator<'a, T> where T: 'a + Send + Sync {
     entry: Option<&'a PersistentListEntry<T>>,
 }
 
 impl<'a, T> Iterator for PersistentListIterator<'a, T> where T: Send + Sync + 'static {
     type Item = &'a T;
 
     #[inline]
     fn next(&mut self) -> Option<&'a T> {
--- a/servo/components/script_plugins/unrooted_must_root.rs
+++ b/servo/components/script_plugins/unrooted_must_root.rs
@@ -158,17 +158,17 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> fo
         let mut visitor = FnDefVisitor {
             cx: cx,
             in_new_function: in_new_function,
         };
         visit::walk_expr(&mut visitor, &body.value);
     }
 }
 
-struct FnDefVisitor<'a, 'b: 'a, 'tcx: 'a+'b> {
+struct FnDefVisitor<'a, 'b: 'a, 'tcx: 'a + 'b> {
     cx: &'a LateContext<'b, 'tcx>,
     in_new_function: bool,
 }
 
 impl<'a, 'b, 'tcx> visit::Visitor<'tcx> for FnDefVisitor<'a, 'b, 'tcx> {
     fn visit_expr(&mut self, expr: &'tcx hir::Expr) {
         let cx = self.cx;
 
--- a/servo/python/tidy/servo_tidy/tidy.py
+++ b/servo/python/tidy/servo_tidy/tidy.py
@@ -462,25 +462,16 @@ def check_rust(file_name, lines):
     decl_found = "\n\t\033[91mfound: {}\033[0m"
 
     for idx, original_line in enumerate(lines):
         # simplify the analysis
         line = original_line.strip()
         prev_indent = indent
         indent = len(original_line) - len(line)
 
-        # Hack for components/selectors/build.rs
-        if multi_line_string:
-            if line.startswith('"#'):
-                multi_line_string = False
-            else:
-                continue
-        if line.endswith('r#"'):
-            multi_line_string = True
-
         is_attribute = re.search(r"#\[.*\]", line)
         is_comment = re.search(r"^//|^/\*|^\*", line)
 
         # Simple heuristic to avoid common case of no comments.
         if '/' in line:
             comment_depth += line.count('/*')
             comment_depth -= line.count('*/')
 
@@ -489,29 +480,45 @@ def check_rust(file_name, lines):
             continue
         if comment_depth:
             merged_lines += line
             continue
         if merged_lines:
             line = merged_lines + line
             merged_lines = ''
 
+        if multi_line_string:
+            line, count = re.subn(
+                r'^(\\.|[^"\\])*?"', '', line, count=1)
+            if count == 1:
+                multi_line_string = False
+            else:
+                continue
+
         # Ignore attributes, comments, and imports
         # Keep track of whitespace to enable checking for a merged import block
         if import_block:
             if not (is_comment or is_attribute or line.startswith("use ")):
                 whitespace = line == ""
 
                 if not whitespace:
                     import_block = False
 
         # get rid of strings and chars because cases like regex expression, keep attributes
-        if not is_attribute:
+        if not is_attribute and not is_comment:
             line = re.sub(r'"(\\.|[^\\"])*?"', '""', line)
-            line = re.sub(r"'(\\.|[^\\'])*?'", "''", line)
+            line = re.sub(
+                r"'(\\.|[^\\']|(\\x[0-9a-fA-F]{2})|(\\u{[0-9a-fA-F]{1,6}}))'",
+                "''", line)
+            # If, after parsing all single-line strings, we still have
+            # an odd number of double quotes, this line starts a
+            # multiline string
+            if line.count('"') % 2 == 1:
+                line = re.sub(r'"(\\.|[^\\"])*?$', '""', line)
+                multi_line_string = True
 
         # get rid of comments
         line = re.sub('//.*?$|/\*.*?$|^\*.*?$', '//', line)
 
         # get rid of attributes that do not contain =
         line = re.sub('^#[A-Za-z0-9\(\)\[\]_]*?$', '#[]', line)
 
         # flag this line if it matches one of the following regular expressions
new file mode 100644
--- /dev/null
+++ b/servo/python/tidy/servo_tidy_tests/multiline_string.rs
@@ -0,0 +1,28 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+
+// This puts a "multi-line string
+// inside of a comment" and then subsequently has a hyphenated-phrase
+
+
+const FOO: &'static str = "Do not confuse 'apostrophes',
+    They can be 'lifetimes' or 'characters'";
+
+
+fn main() {
+    assert!(foo("test
+                 foo-bar"));
+
+    assert!(foo("test
+                 test2 \"
+                 foo-bar"));
+
+    assert!(foo("test
+                 test2 \
+                 foo-bar"));
+
+    println!("This is a multiline string with a URL, which kinda, \
+             sorta looks like a comment https://github.com/servo/servo/");
+}
--- a/servo/python/tidy/servo_tidy_tests/test_tidy.py
+++ b/servo/python/tidy/servo_tidy_tests/test_tidy.py
@@ -261,11 +261,16 @@ class CheckTidiness(unittest.TestCase):
         file_list = tidy.FileList(base_path, only_changed_files=False, exclude_dirs=[])
         lst = list(file_list)
         self.assertEqual([os.path.join(base_path, 'whee', 'test.rs'), os.path.join(base_path, 'whee', 'foo', 'bar.rs')], lst)
         file_list = tidy.FileList(base_path, only_changed_files=False,
                                   exclude_dirs=[os.path.join(base_path, 'whee', 'foo')])
         lst = list(file_list)
         self.assertEqual([os.path.join(base_path, 'whee', 'test.rs')], lst)
 
+    def test_multiline_string(self):
+        errors = tidy.collect_errors_for_files(iterFile('multiline_string.rs'), [], [tidy.check_rust], print_text=True)
+        self.assertNoMoreErrors(errors)
+
+
 def do_tests():
     suite = unittest.TestLoader().loadTestsFromTestCase(CheckTidiness)
     return 0 if unittest.TextTestRunner(verbosity=2).run(suite).wasSuccessful() else 1