2962: Differentiate underscore alias from named aliases r=matklad a=zombiefungus
pre for Fixing Issue 2736
edited to avoid autoclosing the issue
Co-authored-by: zombiefungus <divmermarlav@gmail.com>
2959: Rework how we send diagnostics to client r=matklad a=kiljacken
The previous way of sending from the thread pool suffered from stale diagnostics due to being canceled before we could clear the old ones.
The key change is moving to sending diagnostics from the main loop thread, but doing all the hard work in the thread pool. This should provide the best of both worlds, with little to no of the downsides.
This should hopefully fix a lot of issues, but we'll need testing in each individual issue to be sure.
Co-authored-by: Emil Lauridsen <mine809@gmail.com>
The previous way of sending from the thread pool suffered from stale
diagnostics due to being canceled before we could clear the old ones.
The key change is moving to sending diagnostics from the main loop
thread, but doing all the hard work in the thread pool. This should
provide the best of both worlds, with little to no of the downsides.
This should hopefully fix a lot of issues, but we'll need testing in
each individual issue to be sure.
The extra allocation for message should not matter here at all, but
using a static string is just as ergonomic, if not more, and there's
no reason to write deliberately slow code
By default, `spawn` inherits stderr/stdout/stderr of the parent
process, and so, if child, for example does fcntl(O_NONBLOCK), weird
stuff happens to us.
Closes https://github.com/rust-analyzer/lsp-server/pull/10
2920: Better handle illformed node id from metadata r=matklad a=edwin0cheng
In some rare cases, deps node-id from cargo-metadata do not match its version-id, which cause a panic in `cargo-workspace.rs`. This PR try to ignore these ill-formed node id from `cargo-metadata`. An alternative is return `Err` in these cases but I think make it resilience is a better choice here.
Related #2767
Co-authored-by: Edwin Cheng <edwin0cheng@gmail.com>
2895: Rewrite ra_prof's profile printing r=michalt a=michalt
This changes the way we print things to first construct a mapping from
events to the children and uses that mapping to actually print things.
It should not change the actual output that we produce.
The new approach two benefits:
* It avoids a potential quadratic behavior of the previous approach.
For instance, for a vector of N elements:
```
[Message{level: (N - 1)}, ..., Message{level: 1}, Message{level: 0}]
```
we would first do a linear scan to find entry with level 0, then
another scan to find one with level 1, etc.
* It makes it much easier to improve the output in the future, because
we now pre-compute the children for each entry and can easily take
that into account when printing.
Signed-off-by: Michal Terepeta <michal.terepeta@gmail.com>
Co-authored-by: Michal Terepeta <michal.terepeta@gmail.com>
We previously used serde's stream deserializer to read json blobs from
the cargo output. It has an issue though: If the deserializer encounters
invalid input, it gets stuck reporting the same error again and again
because it is unable to foward over the input until it reaches a new
valid object.
Reading a line at a time and manually deserializing fixes this issue,
because cargo makes sure to only outpu one json blob per line, so should
we encounter invalid input, we can just skip a line and continue.
The main reason this would happen is stray printf-debugging in
procedural macros, so we still report that an error occured, but we
handle it gracefully now.
Fixes#2935
2931: Added documentation to test_utils r=matklad a=Veetaha
Added some doc comments to test_utils functions while studying this crate. They should be all stable enough to document them.
Also some minor code relocation in `parse_fixture()` closer to its usage according to the advice of @matklad.
Co-authored-by: Veetaha <gerzoh1@gmail.com>
2924: Modify ordering of drops in check watcher to only ever have one cargo r=matklad a=kiljacken
Due to the way drops are ordered when assigning to a mutable variable we
were launching a new cargo sub-process before letting the old one quite.
By explicitly replacing the original watcher with a dummy first, we
ensure it is dropped and the process is completed, before we start the
new process.
Co-authored-by: Emil Lauridsen <mine809@gmail.com>
Due to the way drops are ordered when assigning to a mutable variable we
were launching a new cargo sub-process before letting the old one quite.
By explicitly replacing the original watcher with a dummy first, we
ensure it is dropped and the process is completed, before we start the
new process.
2810: Improves reference search by StructLiteral r=mikhail-m1 a=mikhail-m1
Hey, I've made some changes to improve search for struct literals, now it works for `struct Foo<|> {`, `struct Foo <|>{`, `struct Foo<|>(`. Unfortunately tuple creation is represented as a call expression, so for tuples it works only is search is started in a tuple declaration. It leads to incorrect classification of function calls during search phase, but from user perspective it's not visible and works as expected. May be it worth to add a comment or rename it to remove this misleading classification. Issue #2549.
Co-authored-by: Mikhail Modin <mikhailm1@gmail.com>
2887: Initial auto import action implementation r=matklad a=SomeoneToIgnore
Closes https://github.com/rust-analyzer/rust-analyzer/issues/2180
Adds an auto import action implementation.
This implementation is not ideal and has a few limitations:
* The import search functionality should be moved into a separate crate accessible from ra_assists.
This requires a lot of changes and a preliminary design.
Currently the functionality is provided as a trait impl, more on that here: https://github.com/rust-analyzer/rust-analyzer/issues/2180#issuecomment-575690942
* Due to the design desicion from the previous item, no doctests are run for the new aciton (look for a new FIXME in the PR)
* For the same reason, I have to create the mock trait implementaion to test the assist
* Ideally, I think we should have this feature as a diagnostics (that detects an absense of an import) that has a corresponding quickfix action that gets evaluated on demand.
Curretly we perform the import search every time we resolve the import which looks suboptimal.
This requires `classify_name_ref` to be moved from ra_ide, so not done currently.
A few improvements to the imports mechanism to be considered later:
* Constants like `ra_syntax::SyntaxKind::NAME` are not imported, because they are not present in the database
* Method usages are not imported, they are found in the database, but `find_use_path` does not return any import paths for them
* Some import paths returned by the `find_use_path` method end up in `core::` or `alloc::` instead of `std:`, for example: `core::fmt::Debug` instead of `std::fmt::Debug`.
This is not an error techically, but still looks weird.
* No detection of cases where a trait should be imported in order to be able to call a method
* Improve `auto_import_text_edit` functionality: refactor it and move away from the place it is now, add better logic for merging the new import with already existing imports
Co-authored-by: Kirill Bulatov <mail4score@gmail.com>
2899: Provide more runners for potential tests r=matklad a=SomeoneToIgnore
Based on the https://rust-lang.zulipchat.com/#narrow/stream/185405-t-compiler.2Fwg-rls-2.2E0/topic/Runners.20for.20custom.20test.20annotations discussion.
Adds a test runner for every method that has an annotation that contains `test` word in it, allowing to run tests annotated with custom testing annotations such as `#[tokio::test]`, `#[test_case(...)]` and others at costs of potentially emitting some false-positives.
Co-authored-by: Kirill Bulatov <mail4score@gmail.com>
@matklad mentioned this might be a good idea.
So the general idea is that we don't really need the lock, as we can
just clone the check watcher state when creating a snapshot. We can then
use `Arc::get_mut` to get mutable access to the state from `WorldState`
when needed.
Running with this it seems to improve responsiveness a bit while cargo
is running, but I have no hard numbers to prove it. In any case, a
serialization point less is always better when we're trying to be
responsive.
This changes the way we print things to first construct a mapping from
events to the children and uses that mapping to actually print things.
It should not change the actual output that we produce.
The new approach two benefits:
* It avoids a potential quadratic behavior of the previous approach.
For instance, for a vector of N elements:
```
[Message{level: (N - 1)}, ..., Message{level: 1}, Message{level: 0}]
```
we would first do a linear scan to find entry with level 0, then
another scan to find one with level 1, etc.
* It makes it much easier to improve the output in the future, because
we now pre-compute the children for each entry and can easily take
that into account when printing.
Signed-off-by: Michal Terepeta <michal.terepeta@gmail.com>
2877: "Insert explicit type " assist fix#2869, fix typo r=matklad a=TomasKralCZ
So this was quite straightforward. I basically looked at how the other assists work and tried doing something simillar. I also fixed a typo in the other assist.
Co-authored-by: TomasKralCZ <tomas@kral.hk>
Previously `ra_prof` wouldn't actually print the unaccounted time in
some cases.
We would print, for instance, this:
```
5ms - foo
2ms - bar
```
instead of:
```
5ms - foo
2ms - bar
3ms - ???
```
The fix is to properly handle the case when an entry has 0 children
instead of using the `last` variable.
Signed-off-by: Michal Terepeta <michal.terepeta@gmail.com>
2865: fix(mixed): fixed a couple of typos and added a todo r=kjeremy a=Veetaha
Fixed a couple of typos and added a todo while studying the codebase.
Co-authored-by: Veetaha <gerzoh1@gmail.com>
2827: Fix array element attribute position r=matklad a=edwin0cheng
This PR fixed a bug which an ATTR node insert in the wrong place in array element. ~~And introduce `precede_next` for allow outer attributes to insert into a parsed `expr`.~~
related #2783
Co-authored-by: Edwin Cheng <edwin0cheng@gmail.com>
2837: Accidentally quadratic r=matklad a=matklad
Our syntax highlighting is accdentally quadratic. Current state of the PR fixes it in a pretty crude way, looks like for the proper fix we need to redo how source-analyzer works.
**NB:** don't be scared by diff stats, that's mostly a test-data file
Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>
2844: Use dummy value for line! and column! macro r=matklad a=edwin0cheng
Use dummy value `0` for line! and column! macro.
Co-authored-by: Edwin Cheng <edwin0cheng@gmail.com>
2834: refactor(ra_syntax.validation): removed code duplication from validate_literal() r=kiljacken a=Veetaha
Hi! This is my first ever contribution to this project.
I've taken some dirty job from issue #223
This is a simple atomic PR to remove code duplication according to FIXME comment in the function that is the main focus of the further development.
I just didn't want to mix refactoring with the implementation of new features...
I am not sure whether you prefer such atomic PRs here or you'd rather have a single PR that contains all atomic commits inside of it?
So if you want me to add all that validation in one PR I'll mark this one as WIP and update it when the work is finished, otherwise, I'll go with the option of creating separate PRs per each feature of validation of strings, numbers, and comments respectively.
### Comments about refactoring
Yeah, reducing the duplication is quite hard here, extracting into stateless functions could be another option but the number of their arguments would be very big and repeated across char and string implementations so that just writing their types and names would become cumbersome.
I tried the option of having everything captured implicitly in the closure but failed since rust doesn't have templated (or generic) closures as C++ does, this is needed because `unescape_byte*()` and `unescape_char|str()` have different return types...
Maybe I am missing something here? I may be wrong because I am not enough experienced in Rust...
Well, I am awaiting any kind of feedback!
Co-authored-by: Veetaha <gerzoh1@gmail.com>
2807: Use attr location for builtin derive in goto-implementation r=matklad a=edwin0cheng
This PR is use attribute location for builtin derive in `ImplBlock`'s NavigationTarget such that the goto-implementation will goto to a correct position.
Related to #2531
Co-authored-by: Edwin Cheng <edwin0cheng@gmail.com>
2803: Fix various names, e.g. Iterator not resolving in core prelude r=matklad a=flodiebold
Basically, `Iterator` is re-exported via several steps, which happened to not be
resolved yet when we got to the prelude import, but since the name resolved to
the reexport from `core::iter` (just to no actual items), we gave up trying to
resolve it further.
Maybe part of the problem is that we can have
`PartialResolvedImport::Unresolved` or `PartialResolvedImport::Indeterminate`
with `None` in all namespaces, and handle them differently.
Fixes#2683.
Co-authored-by: Florian Diebold <flodiebold@gmail.com>
Basically, `Iterator` is re-exported via several steps, which happened to not be
resolved yet when we got to the prelude import, but since the name resolved to
the reexport from `core::iter` (just to no actual items), we gave up trying to
resolve it further.
Maybe part of the problem is that we can have
`PartialResolvedImport::Unresolved` or `PartialResolvedImport::Indeterminate`
with `None` in all namespaces, and handle them differently.
Fixes#2683.
2791: Slightly more robust cargo watcher root search r=kiljacken a=kiljacken
Fixes#2780 (hopefully).
Use the already painstakingly found `workspaces` instead of naively using `folder_roots` from editor.
Co-authored-by: Emil Lauridsen <mine809@gmail.com>
2795: Use dummy value for macro file in bulitin macros r=matklad a=edwin0cheng
This PR skip the actual line and column computation for `MacroFile` and return a dummy value instead.
Related to #2794
Co-authored-by: Edwin Cheng <edwin0cheng@gmail.com>
2790: Add test for macro expansion in various expressions r=edwin0cheng a=flodiebold
cc @edwin0cheng
Co-authored-by: Florian Diebold <flodiebold@gmail.com>
2786: Proper handling local in hover r=flodiebold a=edwin0cheng
This PR implement back the `Local` hover information generation, which is fall back to a general case catch previously :
9a44f627be/crates/ra_ide/src/hover.rs (L173-L182)
Co-authored-by: Edwin Cheng <edwin0cheng@gmail.com>
2772: Actually test references r=kjeremy a=kjeremy
This will be a little more work when `ReferenceSearchResults` change but I think it's easier to maintain in the end. It also follows a similar pattern to navigation targets and call hierarchy.
Co-authored-by: kjeremy <kjeremy@gmail.com>
Co-authored-by: Jeremy Kolb <kjeremy@gmail.com>
2771: Remove the Default impl for SourceRoot r=matklad a=michalt
Let's be always explicit whether we create a library (i.e., an immutable
dependency) or a local `SourceRoot`, since it can have a large impact on
the validation performance in salsa. (we found it the hard way recently,
where the `Default` instance made it quite tricky to spot a bug)
Signed-off-by: Michal Terepeta <michal.terepeta@gmail.com>
Co-authored-by: Michal Terepeta <michal.terepeta@gmail.com>
Let's be always explicit whether we create a library (i.e., an immutable
dependency) or a local `SourceRoot`, since it can have a large impact on
the validation performance in salsa. (we found it the hard way recently,
where the `Default` instance made it quite tricky to spot a bug)
Signed-off-by: Michal Terepeta <michal.terepeta@gmail.com>
When processing a change with added libraries, we used
`Default::default` for `SourceRoot` which sets `is_library` to false.
Since we use `is_library` to decide whether to use low or high
durability, I believe that this caused us to mark many library
dependencies as having low durability and thus increased the size of the
graph that salsa needed to verify on every change.
Based on my initial tests this speeds up the `CrateDefMapQuery` on
rust-analyzer from about ~64ms to ~14ms and reduces the number of
validations for the query from over 60k to about 7k.
Signed-off-by: Michal Terepeta <michal.terepeta@gmail.com>
2726: Improve profiling output when duration filter is specified r=matklad a=michalt
In particular:
- Use strict inequality for comparisons, since that's what the filter
syntax supports.
- Convert to millis for comparisons, since that's the unit used both
for the filter and when printing.
Now something like `RA_PROFILE='*>0'` will only print things that took
at least 1ms (when rounded to millis).
Signed-off-by: Michal Terepeta <michal.terepeta@gmail.com>
Co-authored-by: Michal Terepeta <michal.terepeta@gmail.com>
The `-` turned into a `+` during a refactoring.
The original issue was caused by `Read` resolving wrongly to a trait without
type parameters instead of a struct with one parameter; this only fixes the
crash, not the wrong resolution.
This change:
- introduces `compute_crate_def_map` query and renames
`CrateDefMap::crate_def_map_query` for consistency,
- annotates `crate_def_map` as `salsa::transparent` and adds a
top-level `crate_def_map` wrapper function around that starts the
profiler and immediately calls into `compute_crate_def_map` query.
This allows us to better understand where we spent the time, in
particular, how much is spent in the recomputaiton and how much in
salsa.
Example output (where we don't actually re-compute anything, but the
query still takes a non-trivial amount of time):
```
211ms - handle_inlay_hints
150ms - get_inlay_hints
150ms - SourceAnalyzer::new
65ms - def_with_body_from_child_node
65ms - analyze_container
65ms - analyze_container
65ms - Module::from_definition
65ms - Module::from_file
65ms - crate_def_map
1ms - parse_macro_query (6 calls)
0ms - raw_items_query (1 calls)
64ms - ???
```
Signed-off-by: Michal Terepeta <michal.terepeta@gmail.com>
In particular:
- Use strict inequality for comparisons, since that's what the filter
syntax supports.
- Convert to millis for comparisons, since that's the unit used both
for the filter and when printing.
Now something like `RA_PROFILE='*>0'` will only print things that took
at least 1ms (when rounded to millis).
Signed-off-by: Michal Terepeta <michal.terepeta@gmail.com>
2681: cargo-watcher: Resolve macro call site in more cases r=matklad a=kiljacken
This resolves the actual macro call site in a few more cases, f.x. when a macro invokes `compile_error!` (I'm looking at you `ra_hir_def::path::__path`).
Co-authored-by: Emil Lauridsen <mine809@gmail.com>
2668: In-server cargo check watching r=matklad a=kiljacken
Opening a draft now so people can follow the progress, and comment if they spot something stupid.
Things that need doing:
- [x] Running cargo check on save
- [x] Pipe through configuration options from client
- [x] Tests for parsing behavior
- [x] Remove existing cargo watch support from VSCode extension
- [x] Progress notification in VSCode extension using LSP 3.15 `$/progress` notification
- [ ] ~~Rework ra-ide diagnostics to support secondary messages~~
- [ ] ~~Make cargo-check watcher use ra-ide diagnostics~~
~~I'd love some input on whether to try to keep the status bar progress thingy for VSCode? It will require some plumbing, and maintaining yet another rust-analyzer specific LSP notification, which I'm not sure we want to.~~
Fixes#1894
Co-authored-by: Emil Lauridsen <mine809@gmail.com>
2667: Visibility r=matklad a=flodiebold
This adds the infrastructure for handling visibility (for fields and methods, not in name resolution) in the HIR and code model, and as a first application hides struct fields from completions if they're not visible from the current module. (We might want to relax this again later, but I think it's ok for now?)
Co-authored-by: Florian Diebold <flodiebold@gmail.com>
2657: Omit closure parameters in closure type display strings r=flodiebold a=SomeoneToIgnore
Part of https://github.com/rust-analyzer/rust-analyzer/issues/1946
I wonder, should we display the the closure trait (Fn/FnMut/FnOnce) in inlay hints instead of `|...|` at all?
Co-authored-by: Kirill Bulatov <mail4score@gmail.com>
