Mirrors/servo
1
0
Fork 0
mirror of https://github.com/servo/servo.git synced 2025-07-22 14:53:49 +01:00
Code Issues Projects Releases Packages Wiki Activity

Add a tidy check for problematic match cases in script_thread.rs

Browse source
This commit is contained in:
Josh Matthews 2016-12-16 11:36:47 -05:00
parent a74cbbeb1a
commit 14d8ae2478
4 changed files with 41 additions and 14 deletions
Download patch file Download diff file Expand all files Collapse all files

28
components/script/script_thread.rs
View file

@ -1175,7 +1175,7 @@ impl ScriptThread {
fn handle_set_scroll_state(&self, fn handle_set_scroll_state(&self,
id: PipelineId, id: PipelineId,
scroll_states: &[(UntrustedNodeAddress, Point2D<f32>)]) { scroll_states: &[(UntrustedNodeAddress, Point2D<f32>)]) {
let window = match self.documents.borrow().find_window(id) { let window = match { self.documents.borrow().find_window(id) } {
Some(window) => window, Some(window) => window,
None => return warn!("Set scroll state message sent to nonexistent pipeline: {:?}", id), None => return warn!("Set scroll state message sent to nonexistent pipeline: {:?}", id),
}; };
@ -1243,7 +1243,7 @@ impl ScriptThread {
} }
fn handle_loads_complete(&self, pipeline: PipelineId) { fn handle_loads_complete(&self, pipeline: PipelineId) {
let doc = match self.documents.borrow().find_document(pipeline) { let doc = match { self.documents.borrow().find_document(pipeline) } {
Some(doc) => doc, Some(doc) => doc,
None => return warn!("Message sent to closed pipeline {}.", pipeline), None => return warn!("Message sent to closed pipeline {}.", pipeline),
}; };
@ -1408,7 +1408,7 @@ impl ScriptThread {
parent_pipeline_id: PipelineId, parent_pipeline_id: PipelineId,
frame_id: Option<FrameId>, frame_id: Option<FrameId>,
event: MozBrowserEvent) { event: MozBrowserEvent) {
let doc = match self.documents.borrow().find_document(parent_pipeline_id) { let doc = match { self.documents.borrow().find_document(parent_pipeline_id) } {
None => return warn!("Mozbrowser event after pipeline {:?} closed.", parent_pipeline_id), None => return warn!("Mozbrowser event after pipeline {:?} closed.", parent_pipeline_id),
Some(doc) => doc, Some(doc) => doc,
}; };
@ -1496,7 +1496,7 @@ impl ScriptThread {
Some(r) => r, Some(r) => r,
None => return None => return
}; };
let window = match self.documents.borrow().find_window(pipeline_id) { let window = match { self.documents.borrow().find_window(pipeline_id) } {
Some(window) => window, Some(window) => window,
None => return warn!("Registration failed for {}", scope), None => return warn!("Registration failed for {}", scope),
}; };
@ -1554,7 +1554,7 @@ impl ScriptThread {
/// Handles a request for the window title. /// Handles a request for the window title.
fn handle_get_title_msg(&self, pipeline_id: PipelineId) { fn handle_get_title_msg(&self, pipeline_id: PipelineId) {
let document = match self.documents.borrow().find_document(pipeline_id) { let document = match { self.documents.borrow().find_document(pipeline_id) } {
Some(document) => document, Some(document) => document,
None => return warn!("Message sent to closed pipeline {}.", pipeline_id), None => return warn!("Message sent to closed pipeline {}.", pipeline_id),
}; };
@ -1611,7 +1611,7 @@ impl ScriptThread {
/// Handles when layout thread finishes all animation in one tick /// Handles when layout thread finishes all animation in one tick
fn handle_tick_all_animations(&self, id: PipelineId) { fn handle_tick_all_animations(&self, id: PipelineId) {
let document = match self.documents.borrow().find_document(id) { let document = match { self.documents.borrow().find_document(id) } {
Some(document) => document, Some(document) => document,
None => return warn!("Message sent to closed pipeline {}.", id), None => return warn!("Message sent to closed pipeline {}.", id),
}; };
@ -1661,7 +1661,7 @@ impl ScriptThread {
/// Notify a window of a storage event /// Notify a window of a storage event
fn handle_storage_event(&self, pipeline_id: PipelineId, storage_type: StorageType, url: ServoUrl, fn handle_storage_event(&self, pipeline_id: PipelineId, storage_type: StorageType, url: ServoUrl,
key: Option<String>, old_value: Option<String>, new_value: Option<String>) { key: Option<String>, old_value: Option<String>, new_value: Option<String>) {
let window = match self.documents.borrow().find_window(pipeline_id) { let window = match { self.documents.borrow().find_window(pipeline_id) } {
None => return warn!("Storage event sent to closed pipeline {}.", pipeline_id), None => return warn!("Storage event sent to closed pipeline {}.", pipeline_id),
Some(window) => window, Some(window) => window,
}; };
@ -1921,7 +1921,7 @@ impl ScriptThread {
} }
MouseMoveEvent(point) => { MouseMoveEvent(point) => {
let document = match self.documents.borrow().find_document(pipeline_id) { let document = match { self.documents.borrow().find_document(pipeline_id) } {
Some(document) => document, Some(document) => document,
None => return warn!("Message sent to closed pipeline {}.", pipeline_id), None => return warn!("Message sent to closed pipeline {}.", pipeline_id),
}; };
@ -1993,7 +1993,7 @@ impl ScriptThread {
} }
TouchpadPressureEvent(point, pressure, phase) => { TouchpadPressureEvent(point, pressure, phase) => {
let doc = match self.documents.borrow().find_document(pipeline_id) { let doc = match { self.documents.borrow().find_document(pipeline_id) } {
Some(doc) => doc, Some(doc) => doc,
None => return warn!("Message sent to closed pipeline {}.", pipeline_id), None => return warn!("Message sent to closed pipeline {}.", pipeline_id),
}; };
@ -2001,7 +2001,7 @@ impl ScriptThread {
} }
KeyEvent(ch, key, state, modifiers) => { KeyEvent(ch, key, state, modifiers) => {
let document = match self.documents.borrow().find_document(pipeline_id) { let document = match { self.documents.borrow().find_document(pipeline_id) } {
Some(document) => document, Some(document) => document,
None => return warn!("Message sent to closed pipeline {}.", pipeline_id), None => return warn!("Message sent to closed pipeline {}.", pipeline_id),
}; };
@ -2015,7 +2015,7 @@ impl ScriptThread {
mouse_event_type: MouseEventType, mouse_event_type: MouseEventType,
button: MouseButton, button: MouseButton,
point: Point2D<f32>) { point: Point2D<f32>) {
let document = match self.documents.borrow().find_document(pipeline_id) { let document = match { self.documents.borrow().find_document(pipeline_id) } {
Some(document) => document, Some(document) => document,
None => return warn!("Message sent to closed pipeline {}.", pipeline_id), None => return warn!("Message sent to closed pipeline {}.", pipeline_id),
}; };
@ -2028,7 +2028,7 @@ impl ScriptThread {
identifier: TouchId, identifier: TouchId,
point: Point2D<f32>) point: Point2D<f32>)
-> TouchEventResult { -> TouchEventResult {
let document = match self.documents.borrow().find_document(pipeline_id) { let document = match { self.documents.borrow().find_document(pipeline_id) } {
Some(document) => document, Some(document) => document,
None => { None => {
warn!("Message sent to closed pipeline {}.", pipeline_id); warn!("Message sent to closed pipeline {}.", pipeline_id);
@ -2061,7 +2061,7 @@ impl ScriptThread {
} }
fn handle_resize_event(&self, pipeline_id: PipelineId, new_size: WindowSizeData, size_type: WindowSizeType) { fn handle_resize_event(&self, pipeline_id: PipelineId, new_size: WindowSizeData, size_type: WindowSizeType) {
let document = match self.documents.borrow().find_document(pipeline_id) { let document = match { self.documents.borrow().find_document(pipeline_id) } {
Some(document) => document, Some(document) => document,
None => return warn!("Message sent to closed pipeline {}.", pipeline_id), None => return warn!("Message sent to closed pipeline {}.", pipeline_id),
}; };
@ -2144,7 +2144,7 @@ impl ScriptThread {
} }
fn handle_parsing_complete(&self, id: PipelineId) { fn handle_parsing_complete(&self, id: PipelineId) {
let document = match self.documents.borrow().find_document(id) { let document = match { self.documents.borrow().find_document(id) } {
Some(document) => document, Some(document) => document,
None => return, None => return,
}; };

3
python/tidy/servo_tidy/tidy.py
View file

@ -542,6 +542,9 @@ def check_rust(file_name, lines):
lambda match, line: line.startswith('use ')), lambda match, line: line.startswith('use ')),
(r"^\s*else {", "else braces should be on the same line", no_filter), (r"^\s*else {", "else braces should be on the same line", no_filter),
(r"[^$ ]\([ \t]", "extra space after (", no_filter), (r"[^$ ]\([ \t]", "extra space after (", no_filter),
# This particular pattern is not reentrant-safe in script_thread.rs
(r"match self.documents.borrow", "use a separate variable for the match expression",
lambda match, line: file_name.endswith('script_thread.rs')),
] ]
for pattern, message, filter_func in regex_rules: for pattern, message, filter_func in regex_rules:

18
python/tidy/servo_tidy_tests/script_thread.rs Normal file
View file

@ -0,0 +1,18 @@
fn main() {
// This should trigger an error.
match self.documents.borrow_mut() {
_ => {}
}
// This should trigger an error.
match self.documents.borrow() {
_ => {}
}
// This should not trigger an error.
match { self.documents.borrow().find_window(id) } {
=> {}
}
// This should not trigger an error.
match self.documents_status.borrow() {
=> {}
}
}

6
python/tidy/servo_tidy_tests/test_tidy.py
View file

@ -141,6 +141,12 @@ class CheckTidiness(unittest.TestCase):
self.assertEqual('method declared in webidl is missing a comment with a specification link', errors.next()[2]) self.assertEqual('method declared in webidl is missing a comment with a specification link', errors.next()[2])
self.assertNoMoreErrors(errors) self.assertNoMoreErrors(errors)
def test_script_thread(self):
errors = tidy.collect_errors_for_files(iterFile('script_thread.rs'), [], [tidy.check_rust], print_text=False)
self.assertEqual('use a separate variable for the match expression', errors.next()[2])
self.assertEqual('use a separate variable for the match expression', errors.next()[2])
self.assertNoMoreErrors(errors)
def test_webidl(self): def test_webidl(self):
errors = tidy.collect_errors_for_files(iterFile('spec.webidl'), [tidy.check_webidl_spec], [], print_text=False) errors = tidy.collect_errors_for_files(iterFile('spec.webidl'), [tidy.check_webidl_spec], [], print_text=False)
self.assertEqual('No specification link found.', errors.next()[2]) self.assertEqual('No specification link found.', errors.next()[2])