Merge #9505
9505: internal: ensure consistent passing for config params r=matklad a=matklad
bors r+
🤖
Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>
This commit is contained in:
commit
f9d20b6747
@ -46,8 +46,8 @@ pub struct AnnotationConfig {
|
||||
|
||||
pub(crate) fn annotations(
|
||||
db: &RootDatabase,
|
||||
config: &AnnotationConfig,
|
||||
file_id: FileId,
|
||||
config: AnnotationConfig,
|
||||
) -> Vec<Annotation> {
|
||||
let mut annotations = Vec::default();
|
||||
|
||||
@ -190,8 +190,7 @@ fn check(ra_fixture: &str, expect: Expect) {
|
||||
|
||||
let annotations: Vec<Annotation> = analysis
|
||||
.annotations(
|
||||
file_id,
|
||||
AnnotationConfig {
|
||||
&AnnotationConfig {
|
||||
binary_target: true,
|
||||
annotate_runnables: true,
|
||||
annotate_impls: true,
|
||||
@ -200,6 +199,7 @@ fn check(ra_fixture: &str, expect: Expect) {
|
||||
run: true,
|
||||
debug: true,
|
||||
},
|
||||
file_id,
|
||||
)
|
||||
.unwrap()
|
||||
.into_iter()
|
||||
|
@ -532,27 +532,27 @@ mod tests {
|
||||
|
||||
fn check_hover_no_result(ra_fixture: &str) {
|
||||
let (analysis, position) = fixture::position(ra_fixture);
|
||||
assert!(analysis
|
||||
let hover = analysis
|
||||
.hover(
|
||||
position,
|
||||
&HoverConfig {
|
||||
links_in_hover: true,
|
||||
documentation: Some(HoverDocFormat::Markdown)
|
||||
}
|
||||
documentation: Some(HoverDocFormat::Markdown),
|
||||
},
|
||||
position,
|
||||
)
|
||||
.unwrap()
|
||||
.is_none());
|
||||
.unwrap();
|
||||
assert!(hover.is_none());
|
||||
}
|
||||
|
||||
fn check(ra_fixture: &str, expect: Expect) {
|
||||
let (analysis, position) = fixture::position(ra_fixture);
|
||||
let hover = analysis
|
||||
.hover(
|
||||
position,
|
||||
&HoverConfig {
|
||||
links_in_hover: true,
|
||||
documentation: Some(HoverDocFormat::Markdown),
|
||||
},
|
||||
position,
|
||||
)
|
||||
.unwrap()
|
||||
.unwrap();
|
||||
@ -568,11 +568,11 @@ fn check_hover_no_links(ra_fixture: &str, expect: Expect) {
|
||||
let (analysis, position) = fixture::position(ra_fixture);
|
||||
let hover = analysis
|
||||
.hover(
|
||||
position,
|
||||
&HoverConfig {
|
||||
links_in_hover: false,
|
||||
documentation: Some(HoverDocFormat::Markdown),
|
||||
},
|
||||
position,
|
||||
)
|
||||
.unwrap()
|
||||
.unwrap();
|
||||
@ -588,11 +588,11 @@ fn check_hover_no_markdown(ra_fixture: &str, expect: Expect) {
|
||||
let (analysis, position) = fixture::position(ra_fixture);
|
||||
let hover = analysis
|
||||
.hover(
|
||||
position,
|
||||
&HoverConfig {
|
||||
links_in_hover: true,
|
||||
documentation: Some(HoverDocFormat::PlainText),
|
||||
},
|
||||
position,
|
||||
)
|
||||
.unwrap()
|
||||
.unwrap();
|
||||
@ -608,11 +608,11 @@ fn check_actions(ra_fixture: &str, expect: Expect) {
|
||||
let (analysis, position) = fixture::position(ra_fixture);
|
||||
let hover = analysis
|
||||
.hover(
|
||||
position,
|
||||
&HoverConfig {
|
||||
links_in_hover: true,
|
||||
documentation: Some(HoverDocFormat::Markdown),
|
||||
},
|
||||
position,
|
||||
)
|
||||
.unwrap()
|
||||
.unwrap();
|
||||
|
@ -488,7 +488,7 @@ fn check_chains(ra_fixture: &str) {
|
||||
fn check_with_config(config: InlayHintsConfig, ra_fixture: &str) {
|
||||
let (analysis, file_id) = fixture::file(&ra_fixture);
|
||||
let expected = extract_annotations(&*analysis.file_text(file_id).unwrap());
|
||||
let inlay_hints = analysis.inlay_hints(file_id, &config).unwrap();
|
||||
let inlay_hints = analysis.inlay_hints(&config, file_id).unwrap();
|
||||
let actual =
|
||||
inlay_hints.into_iter().map(|it| (it.range, it.label.to_string())).collect::<Vec<_>>();
|
||||
assert_eq!(expected, actual, "\nExpected:\n{:#?}\n\nActual:\n{:#?}", expected, actual);
|
||||
@ -496,7 +496,7 @@ fn check_with_config(config: InlayHintsConfig, ra_fixture: &str) {
|
||||
|
||||
fn check_expect(config: InlayHintsConfig, ra_fixture: &str, expect: Expect) {
|
||||
let (analysis, file_id) = fixture::file(&ra_fixture);
|
||||
let inlay_hints = analysis.inlay_hints(file_id, &config).unwrap();
|
||||
let inlay_hints = analysis.inlay_hints(&config, file_id).unwrap();
|
||||
expect.assert_debug_eq(&inlay_hints)
|
||||
}
|
||||
|
||||
|
@ -347,8 +347,8 @@ pub fn file_structure(&self, file_id: FileId) -> Cancellable<Vec<StructureNode>>
|
||||
/// Returns a list of the places in the file where type hints can be displayed.
|
||||
pub fn inlay_hints(
|
||||
&self,
|
||||
file_id: FileId,
|
||||
config: &InlayHintsConfig,
|
||||
file_id: FileId,
|
||||
) -> Cancellable<Vec<InlayHint>> {
|
||||
self.with_db(|db| inlay_hints::inlay_hints(db, file_id, config))
|
||||
}
|
||||
@ -417,8 +417,8 @@ pub fn find_all_methods(&self, file_id: FileId) -> Cancellable<Vec<FileRange>> {
|
||||
/// Returns a short text describing element at position.
|
||||
pub fn hover(
|
||||
&self,
|
||||
position: FilePosition,
|
||||
config: &HoverConfig,
|
||||
position: FilePosition,
|
||||
) -> Cancellable<Option<RangeInfo<HoverResult>>> {
|
||||
self.with_db(|db| hover::hover(db, position, config))
|
||||
}
|
||||
@ -649,10 +649,10 @@ pub fn structural_search_replace(
|
||||
|
||||
pub fn annotations(
|
||||
&self,
|
||||
config: &AnnotationConfig,
|
||||
file_id: FileId,
|
||||
config: AnnotationConfig,
|
||||
) -> Cancellable<Vec<Annotation>> {
|
||||
self.with_db(|db| annotations::annotations(db, file_id, config))
|
||||
self.with_db(|db| annotations::annotations(db, config, file_id))
|
||||
}
|
||||
|
||||
pub fn resolve_annotation(&self, annotation: Annotation) -> Cancellable<Annotation> {
|
||||
|
@ -871,7 +871,7 @@ pub(crate) fn handle_hover(
|
||||
) -> Result<Option<lsp_ext::Hover>> {
|
||||
let _p = profile::span("handle_hover");
|
||||
let position = from_proto::file_position(&snap, params.text_document_position_params)?;
|
||||
let info = match snap.analysis.hover(position, &snap.config.hover())? {
|
||||
let info = match snap.analysis.hover(&snap.config.hover(), position)? {
|
||||
None => return Ok(None),
|
||||
Some(info) => info,
|
||||
};
|
||||
@ -1136,8 +1136,7 @@ pub(crate) fn handle_code_lens(
|
||||
let lenses = snap
|
||||
.analysis
|
||||
.annotations(
|
||||
file_id,
|
||||
AnnotationConfig {
|
||||
&AnnotationConfig {
|
||||
binary_target: cargo_target_spec
|
||||
.map(|spec| {
|
||||
matches!(
|
||||
@ -1153,6 +1152,7 @@ pub(crate) fn handle_code_lens(
|
||||
run: lens_config.run,
|
||||
debug: lens_config.debug,
|
||||
},
|
||||
file_id,
|
||||
)?
|
||||
.into_iter()
|
||||
.map(|annotation| to_proto::code_lens(&snap, annotation).unwrap())
|
||||
@ -1253,7 +1253,7 @@ pub(crate) fn handle_inlay_hints(
|
||||
let line_index = snap.file_line_index(file_id)?;
|
||||
Ok(snap
|
||||
.analysis
|
||||
.inlay_hints(file_id, &snap.config.inlay_hints())?
|
||||
.inlay_hints(&snap.config.inlay_hints(), file_id)?
|
||||
.into_iter()
|
||||
.map(|it| to_proto::inlay_hint(&line_index, it))
|
||||
.collect())
|
||||
|
@ -489,42 +489,6 @@ fn foo(bar: Option<Bar>) { ... }
|
||||
Splitting the two different control flows into two functions simplifies each path, and remove cross-dependencies between the two paths.
|
||||
If there's common code between `foo` and `foo_with_bar`, extract *that* into a common helper.
|
||||
|
||||
## Avoid Monomorphization
|
||||
|
||||
Avoid making a lot of code type parametric, *especially* on the boundaries between crates.
|
||||
|
||||
```rust
|
||||
// GOOD
|
||||
fn frobnicate(f: impl FnMut()) {
|
||||
frobnicate_impl(&mut f)
|
||||
}
|
||||
fn frobnicate_impl(f: &mut dyn FnMut()) {
|
||||
// lots of code
|
||||
}
|
||||
|
||||
// BAD
|
||||
fn frobnicate(f: impl FnMut()) {
|
||||
// lots of code
|
||||
}
|
||||
```
|
||||
|
||||
Avoid `AsRef` polymorphism, it pays back only for widely used libraries:
|
||||
|
||||
```rust
|
||||
// GOOD
|
||||
fn frobnicate(f: &Path) {
|
||||
}
|
||||
|
||||
// BAD
|
||||
fn frobnicate(f: impl AsRef<Path>) {
|
||||
}
|
||||
```
|
||||
|
||||
**Rationale:** Rust uses monomorphization to compile generic code, meaning that for each instantiation of a generic functions with concrete types, the function is compiled afresh, *per crate*.
|
||||
This allows for exceptionally good performance, but leads to increased compile times.
|
||||
Runtime performance obeys 80%/20% rule -- only a small fraction of code is hot.
|
||||
Compile time **does not** obey this rule -- all code has to be compiled.
|
||||
|
||||
## Appropriate String Types
|
||||
|
||||
When interfacing with OS APIs, use `OsString`, even if the original source of data is utf-8 encoded.
|
||||
@ -617,6 +581,42 @@ pub fn reachable_nodes(node: Node) -> FxHashSet<Node> {
|
||||
|
||||
**Rationale:** re-use allocations, accumulator style is more concise for complex cases.
|
||||
|
||||
## Avoid Monomorphization
|
||||
|
||||
Avoid making a lot of code type parametric, *especially* on the boundaries between crates.
|
||||
|
||||
```rust
|
||||
// GOOD
|
||||
fn frobnicate(f: impl FnMut()) {
|
||||
frobnicate_impl(&mut f)
|
||||
}
|
||||
fn frobnicate_impl(f: &mut dyn FnMut()) {
|
||||
// lots of code
|
||||
}
|
||||
|
||||
// BAD
|
||||
fn frobnicate(f: impl FnMut()) {
|
||||
// lots of code
|
||||
}
|
||||
```
|
||||
|
||||
Avoid `AsRef` polymorphism, it pays back only for widely used libraries:
|
||||
|
||||
```rust
|
||||
// GOOD
|
||||
fn frobnicate(f: &Path) {
|
||||
}
|
||||
|
||||
// BAD
|
||||
fn frobnicate(f: impl AsRef<Path>) {
|
||||
}
|
||||
```
|
||||
|
||||
**Rationale:** Rust uses monomorphization to compile generic code, meaning that for each instantiation of a generic functions with concrete types, the function is compiled afresh, *per crate*.
|
||||
This allows for exceptionally good performance, but leads to increased compile times.
|
||||
Runtime performance obeys 80%/20% rule -- only a small fraction of code is hot.
|
||||
Compile time **does not** obey this rule -- all code has to be compiled.
|
||||
|
||||
# Style
|
||||
|
||||
## Order of Imports
|
||||
@ -780,6 +780,38 @@ impl Parent {
|
||||
**Rationale:** easier to get the sense of the API by visually scanning the file.
|
||||
If function bodies are folded in the editor, the source code should read as documentation for the public API.
|
||||
|
||||
## Context Parameters
|
||||
|
||||
Some parameters are threaded unchanged through many function calls.
|
||||
They determine the "context" of the operation.
|
||||
Pass such parameters first, not last.
|
||||
If there are several context parameters, consider packing them into a `struct Ctx` and passing it as `&self`.
|
||||
|
||||
```rust
|
||||
// GOOD
|
||||
fn dfs(graph: &Graph, v: Vertex) -> usize {
|
||||
let mut visited = FxHashSet::default();
|
||||
return go(graph, &mut visited, v);
|
||||
|
||||
fn go(graph: &Graph, visited: &mut FxHashSet<Vertex>, v: usize) -> usize {
|
||||
...
|
||||
}
|
||||
}
|
||||
|
||||
// BAD
|
||||
fn dfs(v: Vertex, graph: &Graph) -> usize {
|
||||
fn go(v: usize, graph: &Graph, visited: &mut FxHashSet<Vertex>) -> usize {
|
||||
...
|
||||
}
|
||||
|
||||
let mut visited = FxHashSet::default();
|
||||
go(v, graph, &mut visited)
|
||||
}
|
||||
```
|
||||
|
||||
**Rationale:** consistency.
|
||||
Context-first works better when non-context parameter is a lambda.
|
||||
|
||||
## Variable Naming
|
||||
|
||||
Use boring and long names for local variables ([yay code completion](https://github.com/rust-analyzer/rust-analyzer/pull/4162#discussion_r417130973)).
|
||||
@ -934,7 +966,6 @@ fn dfs(graph: &Graph, v: Vertex) -> usize {
|
||||
let mut visited = FxHashSet::default();
|
||||
go(graph, &mut visited, v)
|
||||
}
|
||||
|
||||
```
|
||||
|
||||
**Rationale:** consistency, improved top-down readability.
|
||||
|
Loading…
Reference in New Issue
Block a user