From c9dafda03a4a143f4626726df7562077a7cdd64f Mon Sep 17 00:00:00 2001 From: Michael Droettboom Date: Mon, 18 Sep 2017 16:08:55 -0400 Subject: [PATCH] Make tidy aware of Rust multiline strings As a result of tighter and more correct handling of character literals, this now catches a few kinds of syntax involving lifetimes that were previously missed, so those have been updated. --- components/layout/generated_content.rs | 4 +-- components/layout/persistent_list.rs | 2 +- .../script_plugins/unrooted_must_root.rs | 2 +- python/tidy/servo_tidy/tidy.py | 29 ++++++++++++------- .../tidy/servo_tidy_tests/multiline_string.rs | 28 ++++++++++++++++++ python/tidy/servo_tidy_tests/test_tidy.py | 5 ++++ 6 files changed, 55 insertions(+), 15 deletions(-) create mode 100644 python/tidy/servo_tidy_tests/multiline_string.rs diff --git a/components/layout/generated_content.rs b/components/layout/generated_content.rs index d9adfabb832..3f9d314fa19 100644 --- a/components/layout/generated_content.rs +++ b/components/layout/generated_content.rs @@ -137,7 +137,7 @@ impl<'a> InorderFlowTraversal for ResolveGeneratedContent<'a> { } /// 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. @@ -148,7 +148,7 @@ struct ResolveGeneratedContentFragmentMutator<'a,'b:'a> { 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). diff --git a/components/layout/persistent_list.rs b/components/layout/persistent_list.rs index 210606f1d5d..e331550be60 100644 --- a/components/layout/persistent_list.rs +++ b/components/layout/persistent_list.rs @@ -69,7 +69,7 @@ impl Clone for PersistentList where T: Send + Sync { } } -pub struct PersistentListIterator<'a,T> where T: 'a + Send + Sync { +pub struct PersistentListIterator<'a, T> where T: 'a + Send + Sync { entry: Option<&'a PersistentListEntry>, } diff --git a/components/script_plugins/unrooted_must_root.rs b/components/script_plugins/unrooted_must_root.rs index f3f5e60ab62..b0b409862c5 100644 --- a/components/script_plugins/unrooted_must_root.rs +++ b/components/script_plugins/unrooted_must_root.rs @@ -163,7 +163,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnrootedPass { } } -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, } diff --git a/python/tidy/servo_tidy/tidy.py b/python/tidy/servo_tidy/tidy.py index c3c76edbb5f..e88f4dc1458 100644 --- a/python/tidy/servo_tidy/tidy.py +++ b/python/tidy/servo_tidy/tidy.py @@ -467,15 +467,6 @@ def check_rust(file_name, lines): 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) @@ -494,6 +485,14 @@ def check_rust(file_name, 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: @@ -504,9 +503,17 @@ def check_rust(file_name, lines): 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) diff --git a/python/tidy/servo_tidy_tests/multiline_string.rs b/python/tidy/servo_tidy_tests/multiline_string.rs new file mode 100644 index 00000000000..06f7fda7a96 --- /dev/null +++ b/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/"); +} diff --git a/python/tidy/servo_tidy_tests/test_tidy.py b/python/tidy/servo_tidy_tests/test_tidy.py index 73a37cd4612..4631d20ded7 100644 --- a/python/tidy/servo_tidy_tests/test_tidy.py +++ b/python/tidy/servo_tidy_tests/test_tidy.py @@ -266,6 +266,11 @@ class CheckTidiness(unittest.TestCase): 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