Repository: rust-analyzer/rust-analyzer
Pull request #8922 (2021-05-22 13:53:59)

Add more docs
bors r+
🤖


Patch:
diff --git a/crates/hir/src/db.rs b/crates/hir/src/db.rs
index ffc8155b94..19898f6839 100644
--- a/crates/hir/src/db.rs
+++ b/crates/hir/src/db.rs
@@ -1,5 +1,8 @@
-//! FIXME: write short doc here
-
+//! Re-exports various subcrates databases so that the calling code can depend
+//! only on `hir`. This breaks abstraction boundary a bit, it would be cool if
+//! we didn't do that.
+//!
+//! But we need this for at least LRU caching at the query level.
 pub use hir_def::db::*;
 pub use hir_expand::db::{
     AstDatabase, AstDatabaseStorage, AstIdMapQuery, HygieneFrameQuery, InternMacroQuery,
diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs
index b1ebba5164..414c3f35ee 100644
--- a/crates/hir/src/diagnostics.rs
+++ b/crates/hir/src/diagnostics.rs
@@ -1,4 +1,8 @@
-//! FIXME: write short doc here
+//! Re-export diagnostics such that clients of `hir` don't have to depend on
+//! low-level crates.
+//!
+//! This probably isn't the best way to do this -- ideally, diagnistics should
+//! be expressed in terms of hir types themselves.
 pub use hir_def::diagnostics::{
     InactiveCode, UnresolvedMacroCall, UnresolvedModule, UnresolvedProcMacro,
 };
diff --git a/crates/hir_ty/src/infer/unify.rs b/crates/hir_ty/src/infer/unify.rs
index f8233cac39..21d3fb54e9 100644
--- a/crates/hir_ty/src/infer/unify.rs
+++ b/crates/hir_ty/src/infer/unify.rs
@@ -435,7 +435,7 @@ mod resolve {
     use super::InferenceTable;
     use crate::{
         ConcreteConst, Const, ConstData, ConstValue, DebruijnIndex, GenericArg, InferenceVar,
-        Interner, Lifetime, Ty, TyVariableKind, VariableKind,
+        Interner, Ty, TyVariableKind, VariableKind,
     };
     use chalk_ir::{
         cast::Cast,
@@ -524,17 +524,5 @@ mod resolve {
             };
             Ok(result)
         }
-
-        fn fold_inference_lifetime(
-            &mut self,
-            _var: InferenceVar,
-            _outer_binder: DebruijnIndex,
-        ) -> Fallible<Lifetime> {
-            // fall back all lifetimes to 'static -- currently we don't deal
-            // with any lifetimes, but we can sometimes get some lifetime
-            // variables through Chalk's unification, and this at least makes
-            // sure we don't leak them outside of inference
-            Ok(crate::static_lifetime())
-        }
     }
 }
diff --git a/crates/ide/src/display/navigation_target.rs b/crates/ide/src/display/navigation_target.rs
index 2079c22a37..b75ec411cf 100644
--- a/crates/ide/src/display/navigation_target.rs
+++ b/crates/ide/src/display/navigation_target.rs
@@ -1,4 +1,4 @@
-//! FIXME: write short doc here
+//! See [`NavigationTarget`].
 
 use std::fmt;
 
diff --git a/crates/ide/src/folding_ranges.rs b/crates/ide/src/folding_ranges.rs
index a039887783..b893c1c541 100755
--- a/crates/ide/src/folding_ranges.rs
+++ b/crates/ide/src/folding_ranges.rs
@@ -1,5 +1,3 @@
-//! FIXME: write short doc here
-
 use rustc_hash::FxHashSet;
 
 use syntax::{
@@ -29,6 +27,10 @@ pub struct Fold {
     pub kind: FoldKind,
 }
 
+// Feature: Folding
+//
+// Defines folding regions for curly braced blocks, runs of consecutive import
+// statements, and `region` / `endregion` comment markers.
 pub(crate) fn folding_ranges(file: &SourceFile) -> Vec<Fold> {
     let mut res = vec![];
     let mut visited_comments = FxHashSet::default();
diff --git a/crates/ide_assists/src/assist_context.rs b/crates/ide_assists/src/assist_context.rs
index 20754a02ad..8fc40f9bdf 100644
--- a/crates/ide_assists/src/assist_context.rs
+++ b/crates/ide_assists/src/assist_context.rs
@@ -1,4 +1,4 @@
-//! See `AssistContext`
+//! See [`AssistContext`].
 
 use std::mem;
 
diff --git a/crates/ide_assists/src/handlers/fill_match_arms.rs b/crates/ide_assists/src/handlers/fill_match_arms.rs
index 97435f0211..58b0010505 100644
--- a/crates/ide_assists/src/handlers/fill_match_arms.rs
+++ b/crates/ide_assists/src/handlers/fill_match_arms.rs
@@ -1,4 +1,4 @@
-use std::iter::{self, Peekable};
+use std::iter;
 
 use either::Either;
 use hir::{Adt, HasSource, ModuleDef, Semantics};
@@ -63,61 +63,50 @@ pub(crate) fn fill_match_arms(acc: &mut Assists, ctx: &AssistContext) -> Option<
 
     let module = ctx.sema.scope(expr.syntax()).module()?;
 
-    let mut missing_pats: Peekable<Box<dyn Iterator<Item = ast::Pat>>> = if let Some(enum_def) =
-        resolve_enum_def(&ctx.sema, &expr)
-    {
+    let missing_arms: Vec<MatchArm> = if let Some(enum_def) = resolve_enum_def(&ctx.sema, &expr) {
         let variants = enum_def.variants(ctx.db());
 
-        let missing_pats = variants
+        let mut variants = variants
             .into_iter()
             .filter_map(|variant| build_pat(ctx.db(), module, variant))
-            .filter(|variant_pat| is_variant_missing(&top_lvl_pats, variant_pat));
-
-        let missing_pats: Box<dyn Iterator<Item = _>> = if Some(enum_def)
-            == FamousDefs(&ctx.sema, Some(module.krate())).core_option_Option().map(lift_enum)
+            .filter(|variant_pat| is_variant_missing(&top_lvl_pats, variant_pat))
+            .map(|pat| make::match_arm(iter::once(pat), make::expr_empty_block()))
+            .map(|it| it.clone_for_update())
+            .collect::<Vec<_>>();
+        if Some(enum_def)
+            == FamousDefs(&ctx.sema, Some(module.krate()))
+                .core_option_Option()
+                .map(|x| lift_enum(x))
         {
             // Match `Some` variant first.
             cov_mark::hit!(option_order);
-            Box::new(missing_pats.rev())
-        } else {
-            Box::new(missing_pats)
-        };
-        missing_pats.peekable()
+            variants.reverse()
+        }
+        variants
     } else if let Some(enum_defs) = resolve_tuple_of_enum_def(&ctx.sema, &expr) {
-        let mut n_arms = 1;
-        let variants_of_enums: Vec<Vec<ExtendedVariant>> = enum_defs
-            .into_iter()
-            .map(|enum_def| enum_def.variants(ctx.db()))
-            .inspect(|variants| n_arms *= variants.len())
-            .collect();
-
         // When calculating the match arms for a tuple of enums, we want
         // to create a match arm for each possible combination of enum
         // values. The `multi_cartesian_product` method transforms
         // Vec<Vec<EnumVariant>> into Vec<(EnumVariant, .., EnumVariant)>
         // where each tuple represents a proposed match arm.
-
-        // A number of arms grows very fast on even a small tuple of large enums.
-        // We skip the assist beyond an arbitrary threshold.
-        if n_arms > 256 {
-            return None;
-        }
-        let missing_pats = variants_of_enums
+        enum_defs
             .into_iter()
+            .map(|enum_def| enum_def.variants(ctx.db()))
             .multi_cartesian_product()
-            .inspect(|_| cov_mark::hit!(fill_match_arms_lazy_computation))
             .map(|variants| {
                 let patterns =
                     variants.into_iter().filter_map(|variant| build_pat(ctx.db(), module, variant));
                 ast::Pat::from(make::tuple_pat(patterns))
             })
-            .filter(|variant_pat| is_variant_missing(&top_lvl_pats, variant_pat));
-        (Box::new(missing_pats) as Box<dyn Iterator<Item = _>>).peekable()
+            .filter(|variant_pat| is_variant_missing(&top_lvl_pats, variant_pat))
+            .map(|pat| make::match_arm(iter::once(pat), make::expr_empty_block()))
+            .map(|it| it.clone_for_update())
+            .collect()
     } else {
         return None;
     };
 
-    if missing_pats.peek().is_none() {
+    if missing_arms.is_empty() {
         return None;
     }
 
@@ -128,9 +117,6 @@ pub(crate) fn fill_match_arms(acc: &mut Assists, ctx: &AssistContext) -> Option<
         target,
         |builder| {
             let new_match_arm_list = match_arm_list.clone_for_update();
-            let missing_arms = missing_pats
-                .map(|pat| make::match_arm(iter::once(pat), make::expr_empty_block()))
-                .map(|it| it.clone_for_update());
 
             let catch_all_arm = new_match_arm_list
                 .arms()
@@ -181,13 +167,13 @@ fn does_pat_match_variant(pat: &Pat, var: &Pat) -> bool {
     }
 }
 
-#[derive(Eq, PartialEq, Clone, Copy)]
+#[derive(Eq, PartialEq, Clone)]
 enum ExtendedEnum {
     Bool,
     Enum(hir::Enum),
 }
 
-#[derive(Eq, PartialEq, Clone, Copy)]
+#[derive(Eq, PartialEq, Clone)]
 enum ExtendedVariant {
     True,
     False,
@@ -199,7 +185,7 @@ fn lift_enum(e: hir::Enum) -> ExtendedEnum {
 }
 
 impl ExtendedEnum {
-    fn variants(self, db: &RootDatabase) -> Vec<ExtendedVariant> {
+    fn variants(&self, db: &RootDatabase) -> Vec<ExtendedVariant> {
         match self {
             ExtendedEnum::Enum(e) => {
                 e.variants(db).into_iter().map(|x| ExtendedVariant::Variant(x)).collect::<Vec<_>>()
@@ -280,9 +266,7 @@ fn build_pat(db: &RootDatabase, module: hir::Module, var: ExtendedVariant) -> Op
 mod tests {
     use ide_db::helpers::FamousDefs;
 
-    use crate::tests::{
-        check_assist, check_assist_not_applicable, check_assist_target, check_assist_unresolved,
-    };
+    use crate::tests::{check_assist, check_assist_not_applicable, check_assist_target};
 
     use super::fill_match_arms;
 
@@ -1061,19 +1045,4 @@ fn foo(t: Test) {
 }"#,
         );
     }
-
-    #[test]
-    fn lazy_computation() {
-        // Computing a single missing arm is enough to determine applicability of the assist.
-        cov_mark::check_count!(fill_match_arms_lazy_computation, 1);
-        check_assist_unresolved(
-            fill_match_arms,
-            r#"
-enum A { One, Two, }
-fn foo(tuple: (A, A)) {
-    match $0tuple {};
-}
-"#,
-        );
-    }
 }
diff --git a/crates/ide_assists/src/path_transform.rs b/crates/ide_assists/src/path_transform.rs
index 6ec318c4ce..48a7fa06a2 100644
--- a/crates/ide_assists/src/path_transform.rs
+++ b/crates/ide_assists/src/path_transform.rs
@@ -1,4 +1,5 @@
-//! See `PathTransform`
+//! See [`PathTransform`].
+
 use hir::{HirDisplay, SemanticsScope};
 use ide_db::helpers::mod_path_to_ast;
 use rustc_hash::FxHashMap;
diff --git a/crates/profile/src/lib.rs b/crates/profile/src/lib.rs
index a31fb8f439..7cd01a7dfc 100644
--- a/crates/profile/src/lib.rs
+++ b/crates/profile/src/lib.rs
@@ -124,5 +124,5 @@ impl Drop for CpuSpan {
 }
 
 pub fn memory_usage() -> MemoryUsage {
-    MemoryUsage::current()
+    MemoryUsage::now()
 }
diff --git a/crates/profile/src/memory_usage.rs b/crates/profile/src/memory_usage.rs
index cb4e544477..2917ded60f 100644
--- a/crates/profile/src/memory_usage.rs
+++ b/crates/profile/src/memory_usage.rs
@@ -1,4 +1,6 @@
-//! FIXME: write short doc here
+//! Like [`std::time::Instant`], but for memory.
+//!
+//! Measures the total size of all currently allocated objects.
 use std::fmt;
 
 use cfg_if::cfg_if;
@@ -22,7 +24,7 @@ impl std::ops::Sub for MemoryUsage {
 }
 
 impl MemoryUsage {
-    pub fn current() -> MemoryUsage {
+    pub fn now() -> MemoryUsage {
         cfg_if! {
             if #[cfg(all(feature = "jemalloc", not(target_env = "msvc")))] {
                 jemalloc_ctl::epoch::advance().unwrap();
diff --git a/crates/profile/src/stop_watch.rs b/crates/profile/src/stop_watch.rs
index cb6915d458..112d03a9c7 100644
--- a/crates/profile/src/stop_watch.rs
+++ b/crates/profile/src/stop_watch.rs
@@ -44,7 +44,7 @@ impl StopWatch {
     }
     pub fn memory(mut self, yes: bool) -> StopWatch {
         if yes {
-            self.memory = Some(MemoryUsage::current());
+            self.memory = Some(MemoryUsage::now());
         }
         self
     }
@@ -58,7 +58,7 @@ impl StopWatch {
         #[cfg(not(target_os = "linux"))]
         let instructions = None;
 
-        let memory = self.memory.map(|it| MemoryUsage::current() - it);
+        let memory = self.memory.map(|it| MemoryUsage::now() - it);
         StopWatchSpan { time, instructions, memory }
     }
 }
diff --git a/crates/project_model/src/cargo_workspace.rs b/crates/project_model/src/cargo_workspace.rs
index 4a4996cf4d..ad705c7527 100644
--- a/crates/project_model/src/cargo_workspace.rs
+++ b/crates/project_model/src/cargo_workspace.rs
@@ -1,4 +1,4 @@
-//! FIXME: write short doc here
+//! See [`CargoWorkspace`].
 
 use std::path::PathBuf;
 use std::{convert::TryInto, ops, process::Command, sync::Arc};
@@ -12,10 +12,9 @@ use rustc_hash::FxHashMap;
 use serde::Deserialize;
 use serde_json::from_value;
 
-use crate::build_data::BuildDataConfig;
-use crate::utf8_stdout;
+use crate::{build_data::BuildDataConfig, utf8_stdout};
 
-/// `CargoWorkspace` represents the logical structure of, well, a Cargo
+/// [`CargoWorkspace`] represents the logical structure of, well, a Cargo
 /// workspace. It pretty closely mirrors `cargo metadata` output.
 ///
 /// Note that internally, rust analyzer uses a different structure:
diff --git a/crates/project_model/src/lib.rs b/crates/project_model/src/lib.rs
index a5b35ed953..8c6cf94c24 100644
--- a/crates/project_model/src/lib.rs
+++ b/crates/project_model/src/lib.rs
@@ -1,4 +1,19 @@
-//! FIXME: write short doc here
+//! In rust-analyzer, we maintain a strict separation between pure abstract
+//! semantic project model and a concrete model of a particular build system.
+//!
+//! Pure model is represented by the [`base_db::CrateGraph`] from another crate.
+//!
+//! In this crate, we are conserned with "real world" project models.
+//!
+//! Specifically, here we have a representation for a Cargo project
+//! ([`CargoWorkspace`]) and for manually specified layout ([`ProjectJson`]).
+//!
+//! Roughly, the things we do here are:
+//!
+//! * Project discovery (where's the relevant Cargo.toml for the current dir).
+//! * Custom build steps (`build.rs` code generation and compilation of
+//!   procedural macros).
+//! * Lowering of concrete model to a [`base_db::CrateGraph`]
 
 mod cargo_workspace;
 mod cfg_flag;
diff --git a/crates/project_model/src/project_json.rs b/crates/project_model/src/project_json.rs
index 41a2ac03e0..e8f1aca61b 100644
--- a/crates/project_model/src/project_json.rs
+++ b/crates/project_model/src/project_json.rs
@@ -1,4 +1,9 @@
-//! FIXME: write short doc here
+//! `rust-project.json` file format.
+//!
+//! This format is spiritually a serialization of [`base_db::CrateGraph`]. The
+//! idea here is that people who do not use Cargo, can instead teach their build
+//! system to generate `rust-project.json` which can be ingested by
+//! rust-analyzer.
 
 use std::path::PathBuf;
 
diff --git a/crates/syntax/fuzz/fuzz_targets/parser.rs b/crates/syntax/fuzz/fuzz_targets/parser.rs
index 386768343b..f80e130021 100644
--- a/crates/syntax/fuzz/fuzz_targets/parser.rs
+++ b/crates/syntax/fuzz/fuzz_targets/parser.rs
@@ -1,4 +1,4 @@
-//! FIXME: write short doc here
+//! Fuzzing for from-scratch parsing.
 
 #![no_main]
 use libfuzzer_sys::fuzz_target;
diff --git a/crates/syntax/fuzz/fuzz_targets/reparse.rs b/crates/syntax/fuzz/fuzz_targets/reparse.rs
index 5ac99fdafd..f865ce8d67 100644
--- a/crates/syntax/fuzz/fuzz_targets/reparse.rs
+++ b/crates/syntax/fuzz/fuzz_targets/reparse.rs
@@ -1,4 +1,4 @@
-//! FIXME: write short doc here
+//! Fuzzing for incremental parsing.
 
 #![no_main]
 use libfuzzer_sys::fuzz_target;
diff --git a/crates/syntax/src/algo.rs b/crates/syntax/src/algo.rs
index 241713c48e..8d7a77b507 100644
--- a/crates/syntax/src/algo.rs
+++ b/crates/syntax/src/algo.rs
@@ -1,4 +1,4 @@
-//! FIXME: write short doc here
+//! Collection of assorted algorithms for syntax trees.
 
 use std::{hash::BuildHasherDefault, ops::RangeInclusive};
 
diff --git a/crates/syntax/src/fuzz.rs b/crates/syntax/src/fuzz.rs
index aa84239d2f..256999fe09 100644
--- a/crates/syntax/src/fuzz.rs
+++ b/crates/syntax/src/fuzz.rs
@@ -1,4 +1,6 @@
-//! FIXME: write short doc here
+//! Some infrastructure for fuzzy testing.
+//!
+//! We don't normally run fuzzying, so this is hopelessly bitrotten :(
 
 use std::{
     convert::TryInto,
diff --git a/crates/syntax/src/parsing/text_tree_sink.rs b/crates/syntax/src/parsing/text_tree_sink.rs
index d63ec080b4..25bfd41a16 100644
--- a/crates/syntax/src/parsing/text_tree_sink.rs
+++ b/crates/syntax/src/parsing/text_tree_sink.rs
@@ -1,4 +1,4 @@
-//! FIXME: write short doc here
+//! See [`TextTreeSink`].
 
 use std::mem;
 
diff --git a/crates/syntax/src/ptr.rs b/crates/syntax/src/ptr.rs
index d3fb7a5d98..195d2251b1 100644
--- a/crates/syntax/src/ptr.rs
+++ b/crates/syntax/src/ptr.rs
@@ -1,4 +1,13 @@
-//! FIXME: write short doc here
+//! In rust-analyzer, syntax trees are transient objects.
+//!
+//! That means that we create trees when we need them, and tear them down to
+//! save memory. In this architecture, hanging on to a particular syntax node
+//! for a long time is ill-advisable, as that keeps the whole tree resident.
+//!
+//! Instead, we provide a [`SyntaxNodePtr`] type, which stores information about
+//! *location* of a particular syntax node in a tree. Its a small type which can
+//! be cheaply stored, and which can be resolved to a real [`SyntaxNode`] when
+//! necessary.
 
 use std::{
     hash::{Hash, Hasher},
diff --git a/crates/tt/src/buffer.rs b/crates/tt/src/buffer.rs
index 3606c887dc..e0021039ae 100644
--- a/crates/tt/src/buffer.rs
+++ b/crates/tt/src/buffer.rs
@@ -1,5 +1,6 @@
-//! FIXME: write short doc here
-
+//! Stateful iteration over token trees.
+//!
+//! We use this as the source of tokens for parser.
 use crate::{Leaf, Subtree, TokenTree};
 
 #[derive(Copy, Clone, Debug, Eq, PartialEq)]
@@ -104,7 +105,7 @@ impl<'t> TokenBuffer<'t> {
 
     /// Creates a cursor referencing the first token in the buffer and able to
     /// traverse until the end of the buffer.
-    pub fn begin(&self) -> Cursor {
+    pub fn begin(&self) -> Cursor<'_> {
         Cursor::create(self, EntryPtr(EntryId(0), 0))
     }
 
diff --git a/xtask/src/tidy.rs b/xtask/src/tidy.rs
index 6c55823eb0..e31cc2a673 100644
--- a/xtask/src/tidy.rs
+++ b/xtask/src/tidy.rs
@@ -392,18 +392,7 @@ impl TidyDocs {
             )
         }
 
-        let poorly_documented = [
-            "hir",
-            "hir_expand",
-            "ide",
-            "mbe",
-            "parser",
-            "profile",
-            "project_model",
-            "syntax",
-            "tt",
-            "hir_ty",
-        ];
+        let poorly_documented = ["hir_expand", "mbe", "parser", "hir_ty"];
 
         let mut has_fixmes =
             poorly_documented.iter().map(|it| (*it, false)).collect::<HashMap<&str, bool>>();


Test patch:
diff --git a/crates/ide_assists/src/tests.rs b/crates/ide_assists/src/tests.rs
index 6a9231e078..1739302bf0 100644
--- a/crates/ide_assists/src/tests.rs
+++ b/crates/ide_assists/src/tests.rs
@@ -65,12 +65,6 @@ pub(crate) fn check_assist_not_applicable(assist: Handler, ra_fixture: &str) {
     check(assist, ra_fixture, ExpectedResult::NotApplicable, None);
 }
 
-/// Check assist in unresolved state. Useful to check assists for lazy computation.
-#[track_caller]
-pub(crate) fn check_assist_unresolved(assist: Handler, ra_fixture: &str) {
-    check(assist, ra_fixture, ExpectedResult::Unresolved, None);
-}
-
 #[track_caller]
 fn check_doc_test(assist_id: &str, before: &str, after: &str) {
     let after = trim_indent(after);
@@ -107,7 +101,6 @@ fn check_doc_test(assist_id: &str, before: &str, after: &str) {
 
 enum ExpectedResult<'a> {
     NotApplicable,
-    Unresolved,
     After(&'a str),
     Target(&'a str),
 }
@@ -122,11 +115,7 @@ fn check(handler: Handler, before: &str, expected: ExpectedResult, assist_label:
     let sema = Semantics::new(&db);
     let config = TEST_CONFIG;
     let ctx = AssistContext::new(sema, &config, frange);
-    let resolve = match expected {
-        ExpectedResult::Unresolved => AssistResolveStrategy::None,
-        _ => AssistResolveStrategy::All,
-    };
-    let mut acc = Assists::new(&ctx, resolve);
+    let mut acc = Assists::new(&ctx, AssistResolveStrategy::All);
     handler(&mut acc, &ctx);
     let mut res = acc.finish();
 
@@ -174,14 +163,8 @@ fn check(handler: Handler, before: &str, expected: ExpectedResult, assist_label:
             let range = assist.target;
             assert_eq_text!(&text_without_caret[range], target);
         }
-        (Some(assist), ExpectedResult::Unresolved) => assert!(
-            assist.source_change.is_none(),
-            "unresolved assist should not contain source changes"
-        ),
         (Some(_), ExpectedResult::NotApplicable) => panic!("assist should not be applicable!"),
-        (None, ExpectedResult::After(_))
-        | (None, ExpectedResult::Target(_))
-        | (None, ExpectedResult::Unresolved) => {
+        (None, ExpectedResult::After(_)) | (None, ExpectedResult::Target(_)) => {
             panic!("code action is not applicable")
         }
         (None, ExpectedResult::NotApplicable) => (),