mirror of
https://github.com/rust-lang/rust.git
synced 2024-11-27 17:24:06 +00:00
internal: ensure consistent passing for config params
We pass "context" parametes first, so configs should be on the left. "Bigger" context wins, so configs goes after db.
This commit is contained in:
parent
b8a6ea5ab5
commit
0db4f3f6a4
@ -46,8 +46,8 @@ pub struct AnnotationConfig {
|
|||||||
|
|
||||||
pub(crate) fn annotations(
|
pub(crate) fn annotations(
|
||||||
db: &RootDatabase,
|
db: &RootDatabase,
|
||||||
|
config: &AnnotationConfig,
|
||||||
file_id: FileId,
|
file_id: FileId,
|
||||||
config: AnnotationConfig,
|
|
||||||
) -> Vec<Annotation> {
|
) -> Vec<Annotation> {
|
||||||
let mut annotations = Vec::default();
|
let mut annotations = Vec::default();
|
||||||
|
|
||||||
@ -190,8 +190,7 @@ mod tests {
|
|||||||
|
|
||||||
let annotations: Vec<Annotation> = analysis
|
let annotations: Vec<Annotation> = analysis
|
||||||
.annotations(
|
.annotations(
|
||||||
file_id,
|
&AnnotationConfig {
|
||||||
AnnotationConfig {
|
|
||||||
binary_target: true,
|
binary_target: true,
|
||||||
annotate_runnables: true,
|
annotate_runnables: true,
|
||||||
annotate_impls: true,
|
annotate_impls: true,
|
||||||
@ -200,6 +199,7 @@ mod tests {
|
|||||||
run: true,
|
run: true,
|
||||||
debug: true,
|
debug: true,
|
||||||
},
|
},
|
||||||
|
file_id,
|
||||||
)
|
)
|
||||||
.unwrap()
|
.unwrap()
|
||||||
.into_iter()
|
.into_iter()
|
||||||
|
@ -532,27 +532,27 @@ mod tests {
|
|||||||
|
|
||||||
fn check_hover_no_result(ra_fixture: &str) {
|
fn check_hover_no_result(ra_fixture: &str) {
|
||||||
let (analysis, position) = fixture::position(ra_fixture);
|
let (analysis, position) = fixture::position(ra_fixture);
|
||||||
assert!(analysis
|
let hover = analysis
|
||||||
.hover(
|
.hover(
|
||||||
position,
|
|
||||||
&HoverConfig {
|
&HoverConfig {
|
||||||
links_in_hover: true,
|
links_in_hover: true,
|
||||||
documentation: Some(HoverDocFormat::Markdown)
|
documentation: Some(HoverDocFormat::Markdown),
|
||||||
}
|
},
|
||||||
|
position,
|
||||||
)
|
)
|
||||||
.unwrap()
|
.unwrap();
|
||||||
.is_none());
|
assert!(hover.is_none());
|
||||||
}
|
}
|
||||||
|
|
||||||
fn check(ra_fixture: &str, expect: Expect) {
|
fn check(ra_fixture: &str, expect: Expect) {
|
||||||
let (analysis, position) = fixture::position(ra_fixture);
|
let (analysis, position) = fixture::position(ra_fixture);
|
||||||
let hover = analysis
|
let hover = analysis
|
||||||
.hover(
|
.hover(
|
||||||
position,
|
|
||||||
&HoverConfig {
|
&HoverConfig {
|
||||||
links_in_hover: true,
|
links_in_hover: true,
|
||||||
documentation: Some(HoverDocFormat::Markdown),
|
documentation: Some(HoverDocFormat::Markdown),
|
||||||
},
|
},
|
||||||
|
position,
|
||||||
)
|
)
|
||||||
.unwrap()
|
.unwrap()
|
||||||
.unwrap();
|
.unwrap();
|
||||||
@ -568,11 +568,11 @@ mod tests {
|
|||||||
let (analysis, position) = fixture::position(ra_fixture);
|
let (analysis, position) = fixture::position(ra_fixture);
|
||||||
let hover = analysis
|
let hover = analysis
|
||||||
.hover(
|
.hover(
|
||||||
position,
|
|
||||||
&HoverConfig {
|
&HoverConfig {
|
||||||
links_in_hover: false,
|
links_in_hover: false,
|
||||||
documentation: Some(HoverDocFormat::Markdown),
|
documentation: Some(HoverDocFormat::Markdown),
|
||||||
},
|
},
|
||||||
|
position,
|
||||||
)
|
)
|
||||||
.unwrap()
|
.unwrap()
|
||||||
.unwrap();
|
.unwrap();
|
||||||
@ -588,11 +588,11 @@ mod tests {
|
|||||||
let (analysis, position) = fixture::position(ra_fixture);
|
let (analysis, position) = fixture::position(ra_fixture);
|
||||||
let hover = analysis
|
let hover = analysis
|
||||||
.hover(
|
.hover(
|
||||||
position,
|
|
||||||
&HoverConfig {
|
&HoverConfig {
|
||||||
links_in_hover: true,
|
links_in_hover: true,
|
||||||
documentation: Some(HoverDocFormat::PlainText),
|
documentation: Some(HoverDocFormat::PlainText),
|
||||||
},
|
},
|
||||||
|
position,
|
||||||
)
|
)
|
||||||
.unwrap()
|
.unwrap()
|
||||||
.unwrap();
|
.unwrap();
|
||||||
@ -608,11 +608,11 @@ mod tests {
|
|||||||
let (analysis, position) = fixture::position(ra_fixture);
|
let (analysis, position) = fixture::position(ra_fixture);
|
||||||
let hover = analysis
|
let hover = analysis
|
||||||
.hover(
|
.hover(
|
||||||
position,
|
|
||||||
&HoverConfig {
|
&HoverConfig {
|
||||||
links_in_hover: true,
|
links_in_hover: true,
|
||||||
documentation: Some(HoverDocFormat::Markdown),
|
documentation: Some(HoverDocFormat::Markdown),
|
||||||
},
|
},
|
||||||
|
position,
|
||||||
)
|
)
|
||||||
.unwrap()
|
.unwrap()
|
||||||
.unwrap();
|
.unwrap();
|
||||||
|
@ -488,7 +488,7 @@ mod tests {
|
|||||||
fn check_with_config(config: InlayHintsConfig, ra_fixture: &str) {
|
fn check_with_config(config: InlayHintsConfig, ra_fixture: &str) {
|
||||||
let (analysis, file_id) = fixture::file(&ra_fixture);
|
let (analysis, file_id) = fixture::file(&ra_fixture);
|
||||||
let expected = extract_annotations(&*analysis.file_text(file_id).unwrap());
|
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 =
|
let actual =
|
||||||
inlay_hints.into_iter().map(|it| (it.range, it.label.to_string())).collect::<Vec<_>>();
|
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);
|
assert_eq!(expected, actual, "\nExpected:\n{:#?}\n\nActual:\n{:#?}", expected, actual);
|
||||||
@ -496,7 +496,7 @@ mod tests {
|
|||||||
|
|
||||||
fn check_expect(config: InlayHintsConfig, ra_fixture: &str, expect: Expect) {
|
fn check_expect(config: InlayHintsConfig, ra_fixture: &str, expect: Expect) {
|
||||||
let (analysis, file_id) = fixture::file(&ra_fixture);
|
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)
|
expect.assert_debug_eq(&inlay_hints)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -347,8 +347,8 @@ impl Analysis {
|
|||||||
/// Returns a list of the places in the file where type hints can be displayed.
|
/// Returns a list of the places in the file where type hints can be displayed.
|
||||||
pub fn inlay_hints(
|
pub fn inlay_hints(
|
||||||
&self,
|
&self,
|
||||||
file_id: FileId,
|
|
||||||
config: &InlayHintsConfig,
|
config: &InlayHintsConfig,
|
||||||
|
file_id: FileId,
|
||||||
) -> Cancellable<Vec<InlayHint>> {
|
) -> Cancellable<Vec<InlayHint>> {
|
||||||
self.with_db(|db| inlay_hints::inlay_hints(db, file_id, config))
|
self.with_db(|db| inlay_hints::inlay_hints(db, file_id, config))
|
||||||
}
|
}
|
||||||
@ -417,8 +417,8 @@ impl Analysis {
|
|||||||
/// Returns a short text describing element at position.
|
/// Returns a short text describing element at position.
|
||||||
pub fn hover(
|
pub fn hover(
|
||||||
&self,
|
&self,
|
||||||
position: FilePosition,
|
|
||||||
config: &HoverConfig,
|
config: &HoverConfig,
|
||||||
|
position: FilePosition,
|
||||||
) -> Cancellable<Option<RangeInfo<HoverResult>>> {
|
) -> Cancellable<Option<RangeInfo<HoverResult>>> {
|
||||||
self.with_db(|db| hover::hover(db, position, config))
|
self.with_db(|db| hover::hover(db, position, config))
|
||||||
}
|
}
|
||||||
@ -649,10 +649,10 @@ impl Analysis {
|
|||||||
|
|
||||||
pub fn annotations(
|
pub fn annotations(
|
||||||
&self,
|
&self,
|
||||||
|
config: &AnnotationConfig,
|
||||||
file_id: FileId,
|
file_id: FileId,
|
||||||
config: AnnotationConfig,
|
|
||||||
) -> Cancellable<Vec<Annotation>> {
|
) -> 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> {
|
pub fn resolve_annotation(&self, annotation: Annotation) -> Cancellable<Annotation> {
|
||||||
|
@ -871,7 +871,7 @@ pub(crate) fn handle_hover(
|
|||||||
) -> Result<Option<lsp_ext::Hover>> {
|
) -> Result<Option<lsp_ext::Hover>> {
|
||||||
let _p = profile::span("handle_hover");
|
let _p = profile::span("handle_hover");
|
||||||
let position = from_proto::file_position(&snap, params.text_document_position_params)?;
|
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),
|
None => return Ok(None),
|
||||||
Some(info) => info,
|
Some(info) => info,
|
||||||
};
|
};
|
||||||
@ -1136,8 +1136,7 @@ pub(crate) fn handle_code_lens(
|
|||||||
let lenses = snap
|
let lenses = snap
|
||||||
.analysis
|
.analysis
|
||||||
.annotations(
|
.annotations(
|
||||||
file_id,
|
&AnnotationConfig {
|
||||||
AnnotationConfig {
|
|
||||||
binary_target: cargo_target_spec
|
binary_target: cargo_target_spec
|
||||||
.map(|spec| {
|
.map(|spec| {
|
||||||
matches!(
|
matches!(
|
||||||
@ -1153,6 +1152,7 @@ pub(crate) fn handle_code_lens(
|
|||||||
run: lens_config.run,
|
run: lens_config.run,
|
||||||
debug: lens_config.debug,
|
debug: lens_config.debug,
|
||||||
},
|
},
|
||||||
|
file_id,
|
||||||
)?
|
)?
|
||||||
.into_iter()
|
.into_iter()
|
||||||
.map(|annotation| to_proto::code_lens(&snap, annotation).unwrap())
|
.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)?;
|
let line_index = snap.file_line_index(file_id)?;
|
||||||
Ok(snap
|
Ok(snap
|
||||||
.analysis
|
.analysis
|
||||||
.inlay_hints(file_id, &snap.config.inlay_hints())?
|
.inlay_hints(&snap.config.inlay_hints(), file_id)?
|
||||||
.into_iter()
|
.into_iter()
|
||||||
.map(|it| to_proto::inlay_hint(&line_index, it))
|
.map(|it| to_proto::inlay_hint(&line_index, it))
|
||||||
.collect())
|
.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.
|
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.
|
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
|
## Appropriate String Types
|
||||||
|
|
||||||
When interfacing with OS APIs, use `OsString`, even if the original source of data is utf-8 encoded.
|
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.
|
**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
|
# Style
|
||||||
|
|
||||||
## Order of Imports
|
## Order of Imports
|
||||||
@ -780,6 +780,38 @@ impl Parent {
|
|||||||
**Rationale:** easier to get the sense of the API by visually scanning the file.
|
**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.
|
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
|
## Variable Naming
|
||||||
|
|
||||||
Use boring and long names for local variables ([yay code completion](https://github.com/rust-analyzer/rust-analyzer/pull/4162#discussion_r417130973)).
|
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();
|
let mut visited = FxHashSet::default();
|
||||||
go(graph, &mut visited, v)
|
go(graph, &mut visited, v)
|
||||||
}
|
}
|
||||||
|
|
||||||
```
|
```
|
||||||
|
|
||||||
**Rationale:** consistency, improved top-down readability.
|
**Rationale:** consistency, improved top-down readability.
|
||||||
|
Loading…
Reference in New Issue
Block a user