Add option to skip trivial cases

Veykril · Veykril · commit 7ab0aaa82a24 · 2022-03-19T19:01:19.000+01:00
diff --git a/crates/ide/src/inlay_hints.rs b/crates/ide/src/inlay_hints.rs
@@ -4,7 +4,7 @@ use ide_db::{
     base_db::FileRange, famous_defs::FamousDefs, syntax_helpers::node_ext::walk_ty, RootDatabase,
 };
 use itertools::Itertools;
-use rustc_hash::FxHashSet;
+use rustc_hash::FxHashMap;
 use stdx::to_lower_snake_case;
 use syntax::{
     ast::{self, AstNode, HasArgList, HasGenericParams, HasName, UnaryOp},
@@ -20,13 +20,19 @@ pub struct InlayHintsConfig {
     pub parameter_hints: bool,
     pub chaining_hints: bool,
     pub closure_return_type_hints: bool,
-    // FIXME: ternary option here, on off non-noisy
-    pub lifetime_elision_hints: bool,
+    pub lifetime_elision_hints: LifetimeElisionHints,
     pub param_names_for_lifetime_elision_hints: bool,
     pub hide_named_constructor_hints: bool,
     pub max_length: Option<usize>,
 }
 
+#[derive(Clone, Debug, PartialEq, Eq)]
+pub enum LifetimeElisionHints {
+    Always,
+    SkipTrivial,
+    Never,
+}
+
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub enum InlayKind {
     TypeHint,
@@ -58,6 +64,7 @@ pub struct InlayHint {
 // Optionally, one can enable additional hints for
 //
 // * return types of closure expressions with blocks
+// * elided lifetimes
 //
 // **Note:** VS Code does not have native support for inlay hints https://github.com/microsoft/vscode/issues/16221[yet] and the hints are implemented using decorations.
 // This approach has limitations, the caret movement and bracket highlighting near the edges of the hint may be weird:
@@ -132,24 +139,24 @@ fn lifetime_hints(
     config: &InlayHintsConfig,
     func: ast::Fn,
 ) -> Option<()> {
-    if !config.lifetime_elision_hints {
+    if config.lifetime_elision_hints == LifetimeElisionHints::Never {
         return None;
     }
     let param_list = func.param_list()?;
     let generic_param_list = func.generic_param_list();
     let ret_type = func.ret_type();
     let self_param = param_list.self_param().filter(|it| it.amp_token().is_some());
 
-    let used_names: FxHashSet<SmolStr> = generic_param_list
+    let mut used_names: FxHashMap<SmolStr, usize> = generic_param_list
         .iter()
-        .filter(|_| !config.param_names_for_lifetime_elision_hints)
+        .filter(|_| config.param_names_for_lifetime_elision_hints)
         .flat_map(|gpl| gpl.lifetime_params())
         .filter_map(|param| param.lifetime())
-        .map(|lt| SmolStr::from(lt.text().as_str()))
+        .filter_map(|lt| Some((SmolStr::from(lt.text().as_str().get(1..)?), 0)))
         .collect();
 
     let mut allocated_lifetimes = vec![];
-    let mut gen_name = {
+    let mut gen_idx_name = {
         let mut gen = (0u8..).map(|idx| match idx {
             idx if idx < 10 => SmolStr::from_iter(['\'', (idx + 48) as char]),
             idx => format!("'{idx}").into(),
@@ -158,19 +165,27 @@ fn lifetime_hints(
     };
 
     let mut potential_lt_refs: Vec<_> = vec![];
-    param_list.params().filter_map(|it| Some((it.pat(), it.ty()?))).for_each(|(pat, ty)| {
-        // FIXME: check path types
-        walk_ty(&ty, &mut |ty| match ty {
-            ast::Type::RefType(r) => potential_lt_refs.push((
-                pat.as_ref().and_then(|it| match it {
-                    ast::Pat::IdentPat(p) => p.name(),
-                    _ => None,
-                }),
-                r,
-            )),
-            _ => (),
+    param_list
+        .params()
+        .filter_map(|it| {
+            Some((
+                config.param_names_for_lifetime_elision_hints.then(|| it.pat()).flatten(),
+                it.ty()?,
+            ))
         })
-    });
+        .for_each(|(pat, ty)| {
+            // FIXME: check path types
+            walk_ty(&ty, &mut |ty| match ty {
+                ast::Type::RefType(r) => potential_lt_refs.push((
+                    pat.as_ref().and_then(|it| match it {
+                        ast::Pat::IdentPat(p) => p.name(),
+                        _ => None,
+                    }),
+                    r,
+                )),
+                _ => (),
+            })
+        });
 
     enum LifetimeKind {
         Elided,
@@ -195,25 +210,28 @@ fn lifetime_hints(
     if let Some(self_param) = &self_param {
         if is_elided(self_param.lifetime()) {
             allocated_lifetimes.push(if config.param_names_for_lifetime_elision_hints {
+                // self can't be used as a lifetime, so no need to check for collisions
                 "'self".into()
             } else {
-                gen_name()
+                gen_idx_name()
             });
         }
     }
     potential_lt_refs.iter().for_each(|(name, it)| {
         if is_elided(it.lifetime()) {
-            allocated_lifetimes.push(
-                name.as_ref()
-                    .filter(|it| {
-                        config.param_names_for_lifetime_elision_hints
-                            && !used_names.contains(it.text().as_str())
-                    })
-                    .map_or_else(
-                        || gen_name(),
-                        |it| SmolStr::from_iter(["\'", it.text().as_str()]),
-                    ),
-            );
+            let name = match name {
+                Some(it) => {
+                    if let Some(c) = used_names.get_mut(it.text().as_str()) {
+                        *c += 1;
+                        SmolStr::from(format!("'{text}{c}", text = it.text().as_str()))
+                    } else {
+                        used_names.insert(it.text().as_str().into(), 0);
+                        SmolStr::from_iter(["\'", it.text().as_str()])
+                    }
+                }
+                _ => gen_idx_name(),
+            };
+            allocated_lifetimes.push(name);
         }
     });
 
@@ -236,8 +254,21 @@ fn lifetime_hints(
         }
     };
 
-    // apply hints
+    if allocated_lifetimes.is_empty() && output.is_none() {
+        return None;
+    }
 
+    let skip_due_trivial_single = config.lifetime_elision_hints
+        == LifetimeElisionHints::SkipTrivial
+        && (allocated_lifetimes.len() == 1)
+        && generic_param_list.as_ref().map_or(true, |it| it.lifetime_params().next().is_none());
+
+    if skip_due_trivial_single {
+        cov_mark::hit!(lifetime_hints_single);
+        return None;
+    }
+
+    // apply hints
     // apply output if required
     match (&output, ret_type) {
         (Some(output_lt), Some(r)) => {
@@ -800,14 +831,14 @@ mod tests {
     use syntax::{TextRange, TextSize};
     use test_utils::extract_annotations;
 
-    use crate::{fixture, inlay_hints::InlayHintsConfig};
+    use crate::{fixture, inlay_hints::InlayHintsConfig, LifetimeElisionHints};
 
     const DISABLED_CONFIG: InlayHintsConfig = InlayHintsConfig {
         render_colons: false,
         type_hints: false,
         parameter_hints: false,
         chaining_hints: false,
-        lifetime_elision_hints: false,
+        lifetime_elision_hints: LifetimeElisionHints::Never,
         hide_named_constructor_hints: false,
         closure_return_type_hints: false,
         param_names_for_lifetime_elision_hints: false,
@@ -818,7 +849,7 @@ mod tests {
         parameter_hints: true,
         chaining_hints: true,
         closure_return_type_hints: true,
-        lifetime_elision_hints: true,
+        lifetime_elision_hints: LifetimeElisionHints::Always,
         ..DISABLED_CONFIG
     };
 
@@ -2037,6 +2068,47 @@ impl () {
     // ^^^<'0, '1>
         // ^'0       ^'1     ^'0
 }
+"#,
+        );
+    }
+
+    #[test]
+    fn hints_lifetimes_named() {
+        check_with_config(
+            InlayHintsConfig { param_names_for_lifetime_elision_hints: true, ..TEST_CONFIG },
+            r#"
+fn nested_in<'named>(named: &        &X<      &()>) {}
+//          ^'named1, 'named2, 'named3, $
+                          //^'named1 ^'named2 ^'named3
+"#,
+        );
+    }
+
+    #[test]
+    fn hints_lifetimes_skingle_skip() {
+        cov_mark::check!(lifetime_hints_single);
+        check_with_config(
+            InlayHintsConfig {
+                lifetime_elision_hints: LifetimeElisionHints::SkipTrivial,
+                ..TEST_CONFIG
+            },
+            r#"
+fn single(a: &()) -> &() {}
+
+fn double(a: &(), b: &()) {}
+// ^^^^^^<'0, '1>
+          // ^'0     ^'1
+fn partial<'a>(a: &'a (), b: &()) {}
+        //^'0, $             ^'0
+fn partial2<'a>(a: &'a ()) -> &() {}
+                            //^'a
+
+impl () {
+    fn foo(&self) -> &() {}
+    fn foo(&self, a: &()) -> &() {}
+    // ^^^<'0, '1>
+        // ^'0       ^'1     ^'0
+}
 "#,
         );
     }
diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs
@@ -81,7 +81,7 @@ pub use crate::{
     folding_ranges::{Fold, FoldKind},
     highlight_related::{HighlightRelatedConfig, HighlightedRange},
     hover::{HoverAction, HoverConfig, HoverDocFormat, HoverGotoTypeData, HoverResult},
-    inlay_hints::{InlayHint, InlayHintsConfig, InlayKind},
+    inlay_hints::{InlayHint, InlayHintsConfig, InlayKind, LifetimeElisionHints},
     join_lines::JoinLinesConfig,
     markup::Markup,
     moniker::{MonikerKind, MonikerResult, PackageInformation},
diff --git a/crates/ide/src/static_index.rs b/crates/ide/src/static_index.rs
@@ -12,11 +12,14 @@ use ide_db::{
 use rustc_hash::FxHashSet;
 use syntax::{AstNode, SyntaxKind::*, SyntaxToken, TextRange, T};
 
-use crate::moniker::{crate_for_file, def_to_moniker, MonikerResult};
 use crate::{
     hover::hover_for_definition, Analysis, Fold, HoverConfig, HoverDocFormat, HoverResult,
     InlayHint, InlayHintsConfig, TryToNav,
 };
+use crate::{
+    moniker::{crate_for_file, def_to_moniker, MonikerResult},
+    LifetimeElisionHints,
+};
 
 /// A static representation of fully analyzed source code.
 ///
@@ -110,7 +113,7 @@ impl StaticIndex<'_> {
                     parameter_hints: true,
                     chaining_hints: true,
                     closure_return_type_hints: true,
-                    lifetime_elision_hints: false,
+                    lifetime_elision_hints: LifetimeElisionHints::Never,
                     hide_named_constructor_hints: false,
                     param_names_for_lifetime_elision_hints: false,
                     max_length: Some(25),
diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs
@@ -12,7 +12,8 @@ use std::{ffi::OsString, iter, path::PathBuf};
 use flycheck::FlycheckConfig;
 use ide::{
     AssistConfig, CompletionConfig, DiagnosticsConfig, ExprFillDefaultMode, HighlightRelatedConfig,
-    HoverConfig, HoverDocFormat, InlayHintsConfig, JoinLinesConfig, Snippet, SnippetScope,
+    HoverConfig, HoverDocFormat, InlayHintsConfig, JoinLinesConfig, LifetimeElisionHints, Snippet,
+    SnippetScope,
 };
 use ide_db::{
     imports::insert_use::{ImportGranularity, InsertUseConfig, PrefixKind},
@@ -248,19 +249,19 @@ config_data! {
         inlayHints_maxLength: Option<usize>                = "25",
         /// Whether to show function parameter name inlay hints at the call
         /// site.
-        inlayHints_parameterHints: bool                    = "true",
+        inlayHints_parameterHints: bool                     = "true",
         /// Whether to show inlay type hints for variables.
-        inlayHints_typeHints: bool                         = "true",
+        inlayHints_typeHints: bool                          = "true",
         /// Whether to show inlay type hints for method chains.
-        inlayHints_chainingHints: bool                     = "true",
+        inlayHints_chainingHints: bool                      = "true",
         /// Whether to show inlay type hints for return types of closures with blocks.
-        inlayHints_closureReturnTypeHints: bool            = "false",
+        inlayHints_closureReturnTypeHints: bool             = "false",
         /// Whether to show inlay type hints for elided lifetimes in function signatures.
-        inlayHints_lifetimeElisionHints: bool              = "false",
+        inlayHints_lifetimeElisionHints: LifetimeElisionDef = "\"never\"",
         /// Whether to show prefer using parameter names as the name for elided lifetime hints.
-        inlayHints_paramNamesForLifetimeElisionHints: bool = "false",
+        inlayHints_paramNamesForLifetimeElisionHints: bool  = "false",
         /// Whether to hide inlay hints for constructors.
-        inlayHints_hideNamedConstructorHints: bool         = "false",
+        inlayHints_hideNamedConstructorHints: bool          = "false",
 
         /// Join lines inserts else between consecutive ifs.
         joinLines_joinElseIf: bool = "true",
@@ -859,7 +860,11 @@ impl Config {
             parameter_hints: self.data.inlayHints_parameterHints,
             chaining_hints: self.data.inlayHints_chainingHints,
             closure_return_type_hints: self.data.inlayHints_closureReturnTypeHints,
-            lifetime_elision_hints: self.data.inlayHints_lifetimeElisionHints,
+            lifetime_elision_hints: match self.data.inlayHints_lifetimeElisionHints {
+                LifetimeElisionDef::Always => LifetimeElisionHints::Always,
+                LifetimeElisionDef::Never => LifetimeElisionHints::Never,
+                LifetimeElisionDef::SkipTrivial => LifetimeElisionHints::SkipTrivial,
+            },
             hide_named_constructor_hints: self.data.inlayHints_hideNamedConstructorHints,
             param_names_for_lifetime_elision_hints: self
                 .data
@@ -1133,6 +1138,16 @@ enum ImportGranularityDef {
     Module,
 }
 
+#[derive(Deserialize, Debug, Clone)]
+#[serde(rename_all = "snake_case")]
+enum LifetimeElisionDef {
+    #[serde(alias = "true")]
+    Always,
+    #[serde(alias = "false")]
+    Never,
+    SkipTrivial,
+}
+
 #[derive(Deserialize, Debug, Clone)]
 #[serde(rename_all = "snake_case")]
 enum ImportPrefixDef {
@@ -1385,7 +1400,16 @@ fn field_props(field: &str, ty: &str, doc: &[&str], default: &str) -> serde_json
             "minimum": 0,
             "maximum": 255
         },
-        _ => panic!("{}: {}", ty, default),
+        "LifetimeElisionDef" => set! {
+            "type": "string",
+            "enum": ["always", "never", "skip_trivial"],
+            "enumDescriptions": [
+                "Always show lifetime elision hints.",
+                "Never show lifetime elision hints.",
+                "Always show lifetime elision hints but skip them for trivial single input to output mapping."
+            ],
+        },
+        _ => panic!("missing entry for {}: {}", ty, default),
     }
 
     map.into()
diff --git a/docs/user/generated_config.adoc b/docs/user/generated_config.adoc
@@ -378,6 +378,16 @@ Whether to show inlay type hints for method chains.
 --
 Whether to show inlay type hints for return types of closures with blocks.
 --
+[[rust-analyzer.inlayHints.lifetimeElisionHints]]rust-analyzer.inlayHints.lifetimeElisionHints (default: `"never"`)::
++
+--
+Whether to show inlay type hints for elided lifetimes in function signatures.
+--
+[[rust-analyzer.inlayHints.paramNamesForLifetimeElisionHints]]rust-analyzer.inlayHints.paramNamesForLifetimeElisionHints (default: `false`)::
++
+--
+Whether to show prefer using parameter names as the name for elided lifetime hints.
+--
 [[rust-analyzer.inlayHints.hideNamedConstructorHints]]rust-analyzer.inlayHints.hideNamedConstructorHints (default: `false`)::
 +
 --
diff --git a/editors/code/package.json b/editors/code/package.json
