Skip to content

Add config for disabling hover memory layout data #14758

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion crates/ide/src/hover.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ use crate::{
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct HoverConfig {
pub links_in_hover: bool,
pub memory_layout: bool,
pub documentation: bool,
pub keywords: bool,
pub format: HoverDocFormat,
Expand Down Expand Up @@ -226,7 +227,7 @@ fn hover_simple(
return None;
}
let c = token.parent().and_then(|x| x.parent()).and_then(ast::ClosureExpr::cast)?;
render::closure_expr(sema, c)
render::closure_expr(sema, config, c)
})
});

Expand Down
26 changes: 15 additions & 11 deletions crates/ide/src/hover/render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,17 @@ pub(super) fn type_info_of(

pub(super) fn closure_expr(
sema: &Semantics<'_, RootDatabase>,
config: &HoverConfig,
c: ast::ClosureExpr,
) -> Option<HoverResult> {
let ty = &sema.type_of_expr(&c.into())?.original;
let layout = ty
.layout(sema.db)
.map(|x| format!(" // size = {}, align = {}", x.size.bytes(), x.align.abi.bytes()))
.unwrap_or_default();
let layout = if config.memory_layout {
ty.layout(sema.db)
.map(|x| format!(" // size = {}, align = {}", x.size.bytes(), x.align.abi.bytes()))
.unwrap_or_default()
} else {
String::default()
};
let c = ty.as_closure()?;
let mut captures = c
.captured_items(sema.db)
Expand Down Expand Up @@ -415,7 +419,7 @@ pub(super) fn definition(
let mod_path = definition_mod_path(db, &def);
let (label, docs) = match def {
Definition::Macro(it) => label_and_docs(db, it),
Definition::Field(it) => label_and_layout_info_and_docs(db, it, |&it| {
Definition::Field(it) => label_and_layout_info_and_docs(db, it, config, |&it| {
let var_def = it.parent_def(db);
let id = it.index();
let layout = it.layout(db).ok()?;
Expand All @@ -435,7 +439,7 @@ pub(super) fn definition(
}),
Definition::Module(it) => label_and_docs(db, it),
Definition::Function(it) => label_and_docs(db, it),
Definition::Adt(it) => label_and_layout_info_and_docs(db, it, |&it| {
Definition::Adt(it) => label_and_layout_info_and_docs(db, it, config, |&it| {
let layout = it.layout(db).ok()?;
Some(format!("size = {}, align = {}", layout.size.bytes(), layout.align.abi.bytes()))
}),
Expand Down Expand Up @@ -473,7 +477,7 @@ pub(super) fn definition(
}),
Definition::Trait(it) => label_and_docs(db, it),
Definition::TraitAlias(it) => label_and_docs(db, it),
Definition::TypeAlias(it) => label_and_layout_info_and_docs(db, it, |&it| {
Definition::TypeAlias(it) => label_and_layout_info_and_docs(db, it, config, |&it| {
let layout = it.ty(db).layout(db).ok()?;
Some(format!("size = {}, align = {}", layout.size.bytes(), layout.align.abi.bytes()))
}),
Expand Down Expand Up @@ -577,17 +581,17 @@ where
fn label_and_layout_info_and_docs<D, E, V>(
db: &RootDatabase,
def: D,
config: &HoverConfig,
value_extractor: E,
) -> (String, Option<hir::Documentation>)
where
D: HasAttrs + HirDisplay,
E: Fn(&D) -> Option<V>,
V: Display,
{
let label = if let Some(value) = value_extractor(&def) {
format!("{} // {value}", def.display(db))
} else {
def.display(db).to_string()
let label = match value_extractor(&def) {
Some(value) if config.memory_layout => format!("{} // {value}", def.display(db)),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately closure hover doesn't use label_and_layout_info_and_docs, but it shows some layout info which needs to be disabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, I missed that closures use a different implementation.
I've added an additional test for closures.

_ => def.display(db).to_string(),
};
let docs = def.attrs(db).docs();
(label, docs)
Expand Down
56 changes: 56 additions & 0 deletions crates/ide/src/hover/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use crate::{fixture, HoverConfig, HoverDocFormat};

const HOVER_BASE_CONFIG: HoverConfig = HoverConfig {
links_in_hover: false,
memory_layout: true,
documentation: true,
format: HoverDocFormat::Markdown,
keywords: true,
Expand Down Expand Up @@ -57,6 +58,23 @@ fn check_hover_no_links(ra_fixture: &str, expect: Expect) {
expect.assert_eq(&actual)
}

fn check_hover_no_memory_layout(ra_fixture: &str, expect: Expect) {
let (analysis, position) = fixture::position(ra_fixture);
let hover = analysis
.hover(
&HoverConfig { memory_layout: false, ..HOVER_BASE_CONFIG },
FileRange { file_id: position.file_id, range: TextRange::empty(position.offset) },
)
.unwrap()
.unwrap();

let content = analysis.db.file_text(position.file_id);
let hovered_element = &content[hover.range];

let actual = format!("*{hovered_element}*\n{}\n", hover.info.markup);
expect.assert_eq(&actual)
}

fn check_hover_no_markdown(ra_fixture: &str, expect: Expect) {
let (analysis, position) = fixture::position(ra_fixture);
let hover = analysis
Expand Down Expand Up @@ -1745,6 +1763,44 @@ pub fn fo$0o() {}
);
}

#[test]
fn test_hover_no_memory_layout() {
check_hover_no_memory_layout(
r#"struct Foo { fiel$0d_a: u8, field_b: i32, field_c: i16 }"#,
expect![[r#"
*field_a*

```rust
test::Foo
```

```rust
field_a: u8
```
"#]],
);

check_hover_no_memory_layout(
r#"
//- minicore: copy
fn main() {
let x = 2;
let y = $0|z| x + z;
}
"#,
expect![[r#"
*|*
```rust
{closure#0}
impl Fn(i32) -> i32
```

## Captures
* `x` by immutable borrow
"#]],
);
}

#[test]
fn test_hover_macro_generated_struct_fn_doc_comment() {
cov_mark::check!(hover_macro_generated_struct_fn_doc_comment);
Expand Down
1 change: 1 addition & 0 deletions crates/ide/src/static_index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ impl StaticIndex<'_> {
});
let hover_config = HoverConfig {
links_in_hover: true,
memory_layout: true,
documentation: true,
keywords: true,
format: crate::HoverDocFormat::Markdown,
Expand Down
5 changes: 4 additions & 1 deletion crates/rust-analyzer/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,8 +313,10 @@ config_data! {
/// Whether to show keyword hover popups. Only applies when
/// `#rust-analyzer.hover.documentation.enable#` is set.
hover_documentation_keywords_enable: bool = "true",
/// Use markdown syntax for links in hover.
/// Use markdown syntax for links on hover.
hover_links_enable: bool = "true",
/// Whether to show memory layout data on hover.
hover_memoryLayout_enable: bool = "true",

/// Whether to enforce the import granularity setting for all files. If set to false rust-analyzer will try to keep import styles consistent per file.
imports_granularity_enforce: bool = "false",
Expand Down Expand Up @@ -1472,6 +1474,7 @@ impl Config {
pub fn hover(&self) -> HoverConfig {
HoverConfig {
links_in_hover: self.data.hover_links_enable,
memory_layout: self.data.hover_memoryLayout_enable,
documentation: self.data.hover_documentation_enable,
format: {
let is_markdown = try_or_def!(self
Expand Down
7 changes: 6 additions & 1 deletion docs/user/generated_config.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,12 @@ Whether to show keyword hover popups. Only applies when
[[rust-analyzer.hover.links.enable]]rust-analyzer.hover.links.enable (default: `true`)::
+
--
Use markdown syntax for links in hover.
Use markdown syntax for links on hover.
--
[[rust-analyzer.hover.memoryLayout.enable]]rust-analyzer.hover.memoryLayout.enable (default: `true`)::
+
--
Whether to show memory layout data on hover.
--
[[rust-analyzer.imports.granularity.enforce]]rust-analyzer.imports.granularity.enforce (default: `false`)::
+
Expand Down
7 changes: 6 additions & 1 deletion editors/code/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -955,7 +955,12 @@
"type": "boolean"
},
"rust-analyzer.hover.links.enable": {
"markdownDescription": "Use markdown syntax for links in hover.",
"markdownDescription": "Use markdown syntax for links on hover.",
"default": true,
"type": "boolean"
},
"rust-analyzer.hover.memoryLayout.enable": {
"markdownDescription": "Whether to show memory layout data on hover.",
"default": true,
"type": "boolean"
},
Expand Down