From 724059569b4c775ee4723640e0eaabe0da7cdeaf Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 20 Jan 2021 14:30:50 +0300 Subject: [PATCH 1/2] Don't show runnable suggestions for other files It't be actually great to have these once we have run anything dialog, but for run the thing at point it makes sense to show a limited set. --- crates/ide/src/runnables.rs | 35 ++++++++++++++++++++++++++++++++--- docs/dev/style.md | 3 +++ 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/crates/ide/src/runnables.rs b/crates/ide/src/runnables.rs index 8976f1080f..13582e61f6 100644 --- a/crates/ide/src/runnables.rs +++ b/crates/ide/src/runnables.rs @@ -9,6 +9,7 @@ use syntax::{ ast::{self, AstNode, AttrsOwner}, match_ast, SyntaxNode, }; +use test_utils::mark; use crate::{ display::{ToNav, TryToNav}, @@ -96,7 +97,7 @@ impl Runnable { pub(crate) fn runnables(db: &RootDatabase, file_id: FileId) -> Vec { let sema = Semantics::new(db); let module = match sema.to_module_def(file_id) { - None => return vec![], + None => return Vec::new(), Some(it) => it, }; @@ -128,8 +129,14 @@ fn runnables_mod(sema: &Semantics, module: hir::Module) -> Vec runnables_mod(sema, it), - _ => vec![], + hir::ModuleDef::Module(submodule) => match submodule.definition_source(sema.db).value { + hir::ModuleSource::SourceFile(_) => { + mark::hit!(dont_recurse_in_outline_submodules); + Vec::new() + } + hir::ModuleSource::Module(_) => runnables_mod(sema, submodule), + }, + _ => Vec::new(), })); res @@ -326,6 +333,7 @@ fn has_test_function_or_multiple_test_submodules( #[cfg(test)] mod tests { use expect_test::{expect, Expect}; + use test_utils::mark; use crate::fixture; @@ -1050,4 +1058,25 @@ mod tests { "#]], ); } + + #[test] + fn dont_recurse_in_outline_submodules() { + mark::check!(dont_recurse_in_outline_submodules); + check( + r#" +//- /lib.rs +$0 +mod m; +//- /m.rs +mod tests { + #[test] + fn t() {} +} +"#, + &[], + expect![[r#" + [] + "#]], + ); + } } diff --git a/docs/dev/style.md b/docs/dev/style.md index 21330948ba..aed15cee93 100644 --- a/docs/dev/style.md +++ b/docs/dev/style.md @@ -280,6 +280,9 @@ Prefer `Default` even it has to be implemented manually. **Rationale:** less typing in the common case, uniformity. +Use `Vec::new` rather than `vec![]`. **Rationale:** uniformity, strength +reduction. + ## Functions Over Objects Avoid creating "doer" objects. From 74f8201586435a7a2e7f8fd49c7eb0750a089180 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Wed, 20 Jan 2021 14:47:42 +0300 Subject: [PATCH 2/2] Avoid intermediate collections --- crates/ide/src/runnables.rs | 46 ++++++++++++++++--------------------- docs/dev/style.md | 34 ++++++++++++++++++++++++++- 2 files changed, 53 insertions(+), 27 deletions(-) diff --git a/crates/ide/src/runnables.rs b/crates/ide/src/runnables.rs index 13582e61f6..47a85dc456 100644 --- a/crates/ide/src/runnables.rs +++ b/crates/ide/src/runnables.rs @@ -101,24 +101,22 @@ pub(crate) fn runnables(db: &RootDatabase, file_id: FileId) -> Vec { Some(it) => it, }; - runnables_mod(&sema, module) + let mut res = Vec::new(); + runnables_mod(&sema, &mut res, module); + res } -fn runnables_mod(sema: &Semantics, module: hir::Module) -> Vec { - let mut res: Vec = module - .declarations(sema.db) - .into_iter() - .filter_map(|def| { - let runnable = match def { - hir::ModuleDef::Module(it) => runnable_mod(&sema, it), - hir::ModuleDef::Function(it) => runnable_fn(&sema, it), - _ => None, - }; - runnable.or_else(|| module_def_doctest(&sema, def)) - }) - .collect(); +fn runnables_mod(sema: &Semantics, acc: &mut Vec, module: hir::Module) { + acc.extend(module.declarations(sema.db).into_iter().filter_map(|def| { + let runnable = match def { + hir::ModuleDef::Module(it) => runnable_mod(&sema, it), + hir::ModuleDef::Function(it) => runnable_fn(&sema, it), + _ => None, + }; + runnable.or_else(|| module_def_doctest(&sema, def)) + })); - res.extend(module.impl_defs(sema.db).into_iter().flat_map(|it| it.items(sema.db)).filter_map( + acc.extend(module.impl_defs(sema.db).into_iter().flat_map(|it| it.items(sema.db)).filter_map( |def| match def { hir::AssocItem::Function(it) => { runnable_fn(&sema, it).or_else(|| module_def_doctest(&sema, it.into())) @@ -128,18 +126,14 @@ fn runnables_mod(sema: &Semantics, module: hir::Module) -> Vec match submodule.definition_source(sema.db).value { - hir::ModuleSource::SourceFile(_) => { - mark::hit!(dont_recurse_in_outline_submodules); - Vec::new() + for def in module.declarations(sema.db) { + if let hir::ModuleDef::Module(submodule) = def { + match submodule.definition_source(sema.db).value { + hir::ModuleSource::Module(_) => runnables_mod(sema, acc, submodule), + hir::ModuleSource::SourceFile(_) => mark::hit!(dont_recurse_in_outline_submodules), } - hir::ModuleSource::Module(_) => runnables_mod(sema, submodule), - }, - _ => Vec::new(), - })); - - res + } + } } pub(crate) fn runnable_fn(sema: &Semantics, def: hir::Function) -> Option { diff --git a/docs/dev/style.md b/docs/dev/style.md index aed15cee93..3896493980 100644 --- a/docs/dev/style.md +++ b/docs/dev/style.md @@ -421,12 +421,44 @@ fn frobnicate(s: &str) { **Rationale:** reveals the costs. It is also more efficient when the caller already owns the allocation. -## Collection types +## Collection Types Prefer `rustc_hash::FxHashMap` and `rustc_hash::FxHashSet` instead of the ones in `std::collections`. **Rationale:** they use a hasher that's significantly faster and using them consistently will reduce code size by some small amount. +## Avoid Intermediate Collections + +When writing a recursive function to compute a sets of things, use an accumulator parameter instead of returning a fresh collection. +Accumulator goes first in the list of arguments. + +```rust +// GOOD +pub fn reachable_nodes(node: Node) -> FxHashSet { + let mut res = FxHashSet::default(); + go(&mut res, node); + res +} +fn go(acc: &mut FxHashSet, node: Node) { + acc.insert(node); + for n in node.neighbors() { + go(acc, n); + } +} + +// BAD +pub fn reachable_nodes(node: Node) -> FxHashSet { + let mut res = FxHashSet::default(); + res.insert(node); + for n in node.neighbors() { + res.extend(reachable_nodes(n)); + } + res +} +``` + +**Rational:** re-use allocations, accumulator style is more concise for complex cases. + # Style ## Order of Imports