avoid allocating an unnecessary intermediate vector & not traverse multiple times

crates/ra_editor/src/diagnostics.rs

@@ -7,6 +7,7 @@ use ra_syntax::{
     SyntaxKind,
     TextRange,
 };
+use ra_syntax::SyntaxNodeRef;
 use ra_text_edit::{
     TextEdit,
     TextEditBuilder,
@@ -37,44 +38,44 @@ pub fn diagnostics(file: &SourceFileNode) -> Vec<Diagnostic> {
         })
         .collect();
 
-    errors.extend(check_unnecessary_braces_in_use_statement(file));
-    errors.extend(check_struct_shorthand_initialization(file));
+    for node in file.syntax().descendants() {
+        check_unnecessary_braces_in_use_statement(&mut errors, node);
+        check_struct_shorthand_initialization(&mut errors, node);
+    }
+
     errors
 }
 
-fn check_unnecessary_braces_in_use_statement(file: &SourceFileNode) -> Vec<Diagnostic> {
-    let mut diagnostics = Vec::new();
-    for use_tree_list in file
-        .syntax()
-        .descendants()
-        .filter_map(ast::UseTreeList::cast)
-    {
-        if let Some((single_use_tree,)) = use_tree_list.use_trees().collect_tuple() {
-            let range = use_tree_list.syntax().range();
-            let edit =
-                text_edit_for_remove_unnecessary_braces_with_self_in_use_statement(single_use_tree)
-                    .unwrap_or_else(|| {
-                        let to_replace = single_use_tree.syntax().text().to_string();
-                        let mut edit_builder = TextEditBuilder::new();
-                        edit_builder.delete(range);
-                        edit_builder.insert(range.start(), to_replace);
-                        edit_builder.finish()
-                    });
+fn check_unnecessary_braces_in_use_statement(
+    acc: &mut Vec<Diagnostic>,
+    node: SyntaxNodeRef,
+) -> Option<()> {
+    let use_tree_list = ast::UseTreeList::cast(node)?;
+    if let Some((single_use_tree,)) = use_tree_list.use_trees().collect_tuple() {
+        let range = use_tree_list.syntax().range();
+        let edit =
+            text_edit_for_remove_unnecessary_braces_with_self_in_use_statement(single_use_tree)
+                .unwrap_or_else(|| {
+                    let to_replace = single_use_tree.syntax().text().to_string();
+                    let mut edit_builder = TextEditBuilder::new();
+                    edit_builder.delete(range);
+                    edit_builder.insert(range.start(), to_replace);
+                    edit_builder.finish()
+                });
 
-            diagnostics.push(Diagnostic {
-                range,
-                msg: format!("Unnecessary braces in use statement"),
-                severity: Severity::WeakWarning,
-                fix: Some(LocalEdit {
-                    label: "Remove unnecessary braces".to_string(),
-                    edit,
-                    cursor_position: None,
-                }),
-            })
-        }
+        acc.push(Diagnostic {
+            range,
+            msg: format!("Unnecessary braces in use statement"),
+            severity: Severity::WeakWarning,
+            fix: Some(LocalEdit {
+                label: "Remove unnecessary braces".to_string(),
+                edit,
+                cursor_position: None,
+            }),
+        });
     }
 
-    diagnostics
+    Some(())
 }
 
 fn text_edit_for_remove_unnecessary_braces_with_self_in_use_statement(
@@ -99,36 +100,36 @@ fn text_edit_for_remove_unnecessary_braces_with_self_in_use_statement(
     None
 }
 
-fn check_struct_shorthand_initialization(file: &SourceFileNode) -> Vec<Diagnostic> {
-    let mut diagnostics = Vec::new();
-    for struct_lit in file.syntax().descendants().filter_map(ast::StructLit::cast) {
-        if let Some(named_field_list) = struct_lit.named_field_list() {
-            for named_field in named_field_list.fields() {
-                if let (Some(name_ref), Some(expr)) = (named_field.name_ref(), named_field.expr()) {
-                    let field_name = name_ref.syntax().text().to_string();
-                    let field_expr = expr.syntax().text().to_string();
-                    if field_name == field_expr {
-                        let mut edit_builder = TextEditBuilder::new();
-                        edit_builder.delete(named_field.syntax().range());
-                        edit_builder.insert(named_field.syntax().range().start(), field_name);
-                        let edit = edit_builder.finish();
+fn check_struct_shorthand_initialization(
+    acc: &mut Vec<Diagnostic>,
+    node: SyntaxNodeRef,
+) -> Option<()> {
+    let struct_lit = ast::StructLit::cast(node)?;
+    let named_field_list = struct_lit.named_field_list()?;
+    for named_field in named_field_list.fields() {
+        if let (Some(name_ref), Some(expr)) = (named_field.name_ref(), named_field.expr()) {
+            let field_name = name_ref.syntax().text().to_string();
+            let field_expr = expr.syntax().text().to_string();
+            if field_name == field_expr {
+                let mut edit_builder = TextEditBuilder::new();
+                edit_builder.delete(named_field.syntax().range());
+                edit_builder.insert(named_field.syntax().range().start(), field_name);
+                let edit = edit_builder.finish();
 
-                        diagnostics.push(Diagnostic {
-                            range: named_field.syntax().range(),
-                            msg: format!("Shorthand struct initialization"),
-                            severity: Severity::WeakWarning,
-                            fix: Some(LocalEdit {
-                                label: "use struct shorthand initialization".to_string(),
-                                edit,
-                                cursor_position: None,
-                            }),
-                        });
-                    }
-                }
+                acc.push(Diagnostic {
+                    range: named_field.syntax().range(),
+                    msg: format!("Shorthand struct initialization"),
+                    severity: Severity::WeakWarning,
+                    fix: Some(LocalEdit {
+                        label: "use struct shorthand initialization".to_string(),
+                        edit,
+                        cursor_position: None,
+                    }),
+                });
             }
         }
     }
-    diagnostics
+    Some(())
 }
 
 #[cfg(test)]
@@ -137,15 +138,24 @@ mod tests {
 
     use super::*;
 
-    fn check_not_applicable(code: &str, func: fn(file: &SourceFileNode) -> Vec<Diagnostic>) {
+    type DiagnosticChecker = fn(&mut Vec<Diagnostic>, SyntaxNodeRef) -> Option<()>;
+
+    fn check_not_applicable(code: &str, func: DiagnosticChecker) {
         let file = SourceFileNode::parse(code);
-        let diagnostics = func(&file);
+        let mut diagnostics = Vec::new();
+        for node in file.syntax().descendants() {
+            func(&mut diagnostics, node);
+        }
         assert!(diagnostics.is_empty());
     }
 
-    fn check_apply(before: &str, after: &str, func: fn(file: &SourceFileNode) -> Vec<Diagnostic>) {
+    fn check_apply(before: &str, after: &str, func: DiagnosticChecker) {
         let file = SourceFileNode::parse(before);
-        let diagnostic = func(&file)
+        let mut diagnostics = Vec::new();
+        for node in file.syntax().descendants() {
+            func(&mut diagnostics, node);
+        }
+        let diagnostic = diagnostics
             .pop()
             .unwrap_or_else(|| panic!("no diagnostics for:\n{}\n", before));
         let fix = diagnostic.fix.unwrap();