Auto merge of #17749 - SergeevPavel:extra-space-check, r=jdm

Extra space check

<!-- Please describe your changes on the following line: -->
Add tidy check for keywords with more than one space afterwards.

---
<!-- 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 #17700 (github issue number if applicable).

<!-- Either: -->
- [ ] 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/17749)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2017-07-19 16:03:51 -07:00 committed by GitHub
commit 7d95fb8e49
7 changed files with 18 additions and 4 deletions

View file

@ -2037,7 +2037,7 @@ impl<Message, LTF, STF> Constellation<Message, LTF, STF>
let visibility_msg = ConstellationControlMsg::NotifyVisibilityChange(parent_pipeline_id, let visibility_msg = ConstellationControlMsg::NotifyVisibilityChange(parent_pipeline_id,
browsing_context_id, browsing_context_id,
visibility); visibility);
let result = match self.pipelines.get(&parent_pipeline_id) { let result = match self.pipelines.get(&parent_pipeline_id) {
None => return warn!("Parent pipeline {:?} closed", parent_pipeline_id), None => return warn!("Parent pipeline {:?} closed", parent_pipeline_id),
Some(parent_pipeline) => parent_pipeline.event_loop.send(visibility_msg), Some(parent_pipeline) => parent_pipeline.event_loop.send(visibility_msg),
}; };

View file

@ -960,7 +960,7 @@ impl Document {
let line = click_pos - last_pos; let line = click_pos - last_pos;
let dist = (line.dot(line) as f64).sqrt(); let dist = (line.dot(line) as f64).sqrt();
if now.duration_since(last_time) < DBL_CLICK_TIMEOUT && if now.duration_since(last_time) < DBL_CLICK_TIMEOUT &&
dist < DBL_CLICK_DIST_THRESHOLD as f64 { dist < DBL_CLICK_DIST_THRESHOLD as f64 {
// A double click has occurred if this click is within a certain time and dist. of previous click. // A double click has occurred if this click is within a certain time and dist. of previous click.
let click_count = 2; let click_count = 2;

View file

@ -176,7 +176,7 @@ impl ServiceWorkerManager {
} }
} }
} else { } else {
let _ = mediator.response_chan.send(None); let _ = mediator.response_chan.send(None);
} }
} else { } else {
let _ = mediator.response_chan.send(None); let _ = mediator.response_chan.send(None);

View file

@ -584,6 +584,12 @@ def check_rust(file_name, lines):
# -> () is unnecessary # -> () is unnecessary
(r"-> \(\)", "encountered function signature with -> ()", no_filter), (r"-> \(\)", "encountered function signature with -> ()", no_filter),
] ]
keywords = ["if", "let", "mut", "extern", "as", "impl", "fn", "struct", "enum", "pub", "mod",
"use", "in", "ref", "type", "where", "trait"]
extra_space_after = lambda key: (r"(?<![A-Za-z0-9\-_]){key} ".format(key=key),
"extra space after {key}".format(key=key),
lambda match, line: not is_attribute)
regex_rules.extend(map(extra_space_after, keywords))
for pattern, message, filter_func in regex_rules: for pattern, message, filter_func in regex_rules:
for match in re.finditer(pattern, line): for match in re.finditer(pattern, line):

View file

@ -72,4 +72,10 @@ impl test {
} else { // Should not trigger } else { // Should not trigger
"false" "false"
} // Should not trigger } // Should not trigger
if true { // Double space after keyword
42
} else {
let xif = 42 in { xif } // Should not trigger
}
} }

View file

@ -95,6 +95,7 @@ class CheckTidiness(unittest.TestCase):
def test_rust(self): def test_rust(self):
errors = tidy.collect_errors_for_files(iterFile('rust_tidy.rs'), [], [tidy.check_rust], print_text=False) errors = tidy.collect_errors_for_files(iterFile('rust_tidy.rs'), [], [tidy.check_rust], print_text=False)
self.assertEqual('extra space after use', errors.next()[2])
self.assertEqual('extra space after {', errors.next()[2]) self.assertEqual('extra space after {', errors.next()[2])
self.assertEqual('extra space before }', errors.next()[2]) self.assertEqual('extra space before }', errors.next()[2])
self.assertEqual('use statement spans multiple lines', errors.next()[2]) self.assertEqual('use statement spans multiple lines', errors.next()[2])
@ -133,6 +134,7 @@ class CheckTidiness(unittest.TestCase):
self.assertEqual('no = in the beginning of line', 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.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.assertEqual('extra space after if', errors.next()[2])
self.assertNoMoreErrors(errors) self.assertNoMoreErrors(errors)
feature_errors = tidy.collect_errors_for_files(iterFile('lib.rs'), [], [tidy.check_rust], print_text=False) feature_errors = tidy.collect_errors_for_files(iterFile('lib.rs'), [], [tidy.check_rust], print_text=False)

View file

@ -943,7 +943,7 @@ fn test_load_follows_a_redirect() {
} }
#[test] #[test]
fn test_redirect_from_x_to_y_provides_y_cookies_from_y() { fn test_redirect_from_x_to_y_provides_y_cookies_from_y() {
let shared_url_y = Arc::new(Mutex::new(None::<ServoUrl>)); let shared_url_y = Arc::new(Mutex::new(None::<ServoUrl>));
let shared_url_y_clone = shared_url_y.clone(); let shared_url_y_clone = shared_url_y.clone();
let handler = move |request: HyperRequest, mut response: HyperResponse| { let handler = move |request: HyperRequest, mut response: HyperResponse| {