According to the spec we should return ServerNotInitialized if the server is waiting for an initialize request and something else comes in.
Upgrading to lsp-server 0.5.1 will do this and retry until the initialize request comes in.
Fixes#8581
8540: Prevent being able to rename items that are not part of the workspace r=Veykril a=Veykril
This change causes renames that happen on items coming from crates outside the workspace to fail. I believe this should be the right approach, but usage of cargo's workspace might not be entirely correct for preventing these kinds of refactoring from touching things they shouldn't. I'm not entirely sure?
cc #6623, this is one of the bigger footguns when it comes to refactoring, especially in combination with import aliases people tend to rename items coming from a crates dependency which this prevents.
Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
8354: Distinguishing between different operators in semantic highlighting r=matklad a=chetankhilosiya
Co-authored-by: Chetan Khilosiya <chetan.khilosiya@gmail.com>
Conceptually, using a *message* here is wrong, because this is a
"status", rather than "point in time" thing. But statuses are an LSP
extension, while messages are stable. As a compromise, send message only
for more critical `metadata` failures, and only once per state change.
8410: Use CompletionTextEdit::InsertAndReplace if supported by the client r=Veykril a=Veykril
Fixes#8404, Fixes#3130
Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
8375: feat: show errors from `cargo metadata` and initial `cargo check` in the status bar r=matklad a=matklad
bors r+
🤖
Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>
We have a CLI for benchmarking, but no one actually uses it it seems.
Let's try switching to "internal" benchmarks, implemented as rust tests.
They should be easier to "script" to automate tracking of perf
regressions.
8132: Add `'` to trigger_characters, allowing more direct lifetime completions r=Veykril a=Veykril
Fixes having to type a character after `'` to complete lifetimes and labels
Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
8021: Enable searching for builtin types r=matklad a=Veykril
Not too sure how useful this is for reference search overall, but for completeness sake it should be there
Also enables document highlighting for them.
8022: some clippy::performance fixes r=matklad a=matthiaskrgr
use vec![] instead of Vec::new() + push()
avoid redundant clones
use chars instead of &str for single char patterns in ends_with() and starts_with()
allocate some Vecs with capacity to avoid unnecessary resizing
Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
Co-authored-by: Matthias Krüger <matthias.krueger@famsik.de>
use vec![] instead of Vec::new() + push()
avoid redundant clones
use chars instead of &str for single char patterns in ends_with() and starts_with()
allocate some Vecs with capacity to avoid unneccessary resizing
7873: Consider unresolved qualifiers during flyimport r=matklad a=SomeoneToIgnore
Closes https://github.com/rust-analyzer/rust-analyzer/issues/7679
Takes unresolved qualifiers into account, providing better completions (or none, if the path is resolved or do not match).
Does not handle cases when both path qualifier and some trait has to be imported: there are many extra issues with those (such as overlapping imports, for instance) that will require large diffs to address.
Also does not do a fuzzy search on qualifier, that requires some adjustments in `import_map` for better queries and changes to the default replace range which also seems relatively big to include here.
7933: Improve compilation speed r=matklad a=matklad
bors r+
🤖
Co-authored-by: Kirill Bulatov <mail4score@gmail.com>
Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>
7891: Improve handling of rustc_private r=matklad a=DJMcNab
This PR changes how `rust-analyzer` handles `rustc_private`. In particular, packages now must opt-in to using `rustc_private` in `Cargo.toml`, by adding:
```toml
[package.metadata.rust-analyzer]
rustc_private=true
```
This means that depending on crates which also use `rustc_private` will be significantly improved, since their dependencies on the `rustc_private` crates will be resolved properly.
A similar approach could be used in #6714 to allow annotating that your package uses the `test` crate, although I have not yet handled that in this PR.
Additionally, we now only index the crates which are transitive dependencies of `rustc_driver` in the `rustcSource` directory. This should not cause any change in behaviour when using `rustcSource: "discover"`, as the source used then will only be a partial clone. However, if `rustcSource` pointing at a local checkout of rustc, this should significantly improve the memory usage and lower indexing time. This is because we avoids indexing all crates in `src/tools/`, which includes `rust-analyzer` itself.
Furthermore, we also prefer named dependencies over dependencies from `rustcSource`. This ensures that feature resolution for crates which are depended on by both `rustc` and your crate uses the correct set for analysing your crate.
See also [introductory zulip stream](https://rust-lang.zulipchat.com/#narrow/stream/185405-t-compiler.2Fwg-rls-2.2E0/topic/Fixed.20crate.20graphs.20and.20optional.20builtin.20crates/near/229086673)
I have tested this in [priroda](https://github.com/oli-obk/priroda/), and it provides a significant improvement to the development experience (once I give `miri` the required data in `Cargo.toml`)
Todo:
- [ ] Documentation
This is ready to review, and I will add documentation if this would be accepted (or if I get time to do so anyway)
Co-authored-by: Daniel McNab <36049421+DJMcNab@users.noreply.github.com>
7335: added region folding r=matklad a=LucianoBestia
Regions of code that you'd like to be folded can be wrapped with `// #region` and `// #endregion` line comments.
This is called "Region Folding". It is originally available for many languages in VSCode. But Rust-analyzer has its own folding function and this is missing.
With this Pull Request I am suggesting a simple solution.
The regions are a special kind of comments, so I added a bit of code in the comment folding function.
The regex to match are: `^\s*//\s*#?region\b` and `^\s*//\s*#?endregion\b`.
The number of space characters is not important. There is an optional # character. The line can end with a name of the region.
Example:
```rust
// 1. some normal comment
// region: test
// 2. some normal comment
calling_function(x,y);
// endregion: test
```
I added a test for this new functionality in `folding_ranges.rs`.
Please, take a look and comment.
I found that these exact regexes are already present in the file `language-configuration.json`, but I don't find a way to read this configuration. So my regex is hardcoded in the code.
7691: Suggest name in extract variable r=matklad a=cpud36
Generate better default name in extract variable assist as was mentioned in issue #1587
# Currently supported
(in order of declining precedence)
1. Expr is argument to a function; use corresponding parameter name
2. Expr is result of a function or method call; use this function/method's name
3. Use expr type name (if possible)
4. Fallback to `var_name` otherwise
# Showcase
# Questions
* Should we more aggressively strip known types? E.g. we already strip `&T -> T`; should we strip `Option<T> -> T`, `Result<T, E> -> T`, and others?
* Integers and floats use `var_name` by default. Should we introduce a name, like `i`, `f` etc?
* Can we return a list and suggest a name when renaming(like IntelliJ does)?
* Should we add counters to remove duplicate variables? E.g. `type`, `type1`, type2`, etc.
Co-authored-by: Luciano Bestia <LucianoBestia@gmail.com>
Co-authored-by: Luciano <31509965+LucianoBestia@users.noreply.github.com>
Co-authored-by: Vladyslav Katasonov <cpud47@gmail.com>
Reading through the code for diagnostics and observing debug logs, I noticed
that diagnostics are transmitted after every change for every opened file,
even if they haven't changed (especially visible for files with no diagnostics).
This change avoids marking files as "changed" if diagnostics are the same to what
was already sent before. This will only work if diagnostics are always produced in
the same order, but from my limited testing it seems this is the case.
7690: Extract `fn load_workspace(…)` from `fn load_cargo(…)` r=matklad a=regexident
Unfortunately in https://github.com/rust-analyzer/rust-analyzer/pull/7595 I forgot to `pub use` (rather than just `use`) the newly introduced `LoadCargoConfig`.
So this PR fixes this now.
It also:
- splits up `fn load_cargo` into a "workspace loading" and a "project loading" phase
- adds a `progress: &dyn Fn(String)` to allow third-parties to provide CLI progress updates, too
The motivation behind both of these is the fact that rust-analyzer currently does not support caching.
As such any third-party making use of `ra_ap_…` needs to providing a caching layer itself.
Unlike for rust-analyzer itself however a common use-pattern of third-parties is to analyze a specific target (`--lib`/`--bin <BIN>`/…) from a specific package (`--package`). The targets/packages of a crate can be obtained via `ProjectWorkspace::load(…)`, which currently is performed inside of `fn load_cargo`, effectively making the returned `ProjectWorkspace` inaccessible to the outer caller. With this information one can then provide early error handling via CLI (in case of ambiguities or invalid arguments, etc), instead of `fn load_cargo` failing with a possibly obscure error message. It also allows for annotating the persisted caches with its specific associated package/target selector and short-circuit quickly if a matching cache is found on disk, significantly cutting load times.
Before:
```rust
pub struct LoadCargoConfig {
pub cargo_config: &CargoConfig,
pub load_out_dirs_from_check: bool,
pub with_proc_macro: bool,
}
pub fn load_cargo(
root: &Path,
config: &LoadCargoConfig
) -> Result<(AnalysisHost, vfs::Vfs)> {
// ...
}
```
After:
```rust
pub fn load_workspace(
root: &Path,
config: &CargoConfig,
progress: &dyn Fn(String),
) -> Result<ProjectWorkspace> {
// ...
}
pub struct LoadCargoConfig {
pub load_out_dirs_from_check: bool,
pub with_proc_macro: bool,
}
pub fn load_cargo(
ws: ProjectWorkspace,
config: &LoadCargoConfig,
progress: &dyn Fn(String),
) -> Result<(AnalysisHost, vfs::Vfs)> {
// ...
}
```
Co-authored-by: Vincent Esche <regexident@gmail.com>
7657: utf8 r=matklad a=matklad
- Prepare for utf-8 offsets
- reduce code duplication in tests
- Make utf8 default, implement utf16 in terms of it
- Make it easy to add additional context for offset conversion
- Implement utf8 offsets
closes#7453
Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>
7661: Start LSP 3.17 support r=kjeremy a=kjeremy
Companion to https://github.com/gluon-lang/lsp-types/pull/199 which <strike>has not been merged yet</strike> has been merged.
This doesn't opt into any 3.17 functionality yet.
Co-authored-by: Jeremy Kolb <kjeremy@gmail.com>
7643: Automatically detect the rustc-src directory (fixes#3517) r=matklad a=bnjbvr
If the configured rustcSource was not set, then try to automatically
detect a source for the sysroot rustc directory.
I wasn't sure how to do it in the case of the project.json file, though.
7663: Tolerate spaces in nix binary patching r=matklad a=CertainLach
If path to original file contains space (I.e on code insiders, where
default data directory is ~/Code - Insiders/), then there is syntax
error evaluating src arg.
Instead pass path as str, and coerce to path back in nix expression
Co-authored-by: Benjamin Bouvier <public@benj.me>
Co-authored-by: Yaroslav Bolyukin <iam@lach.pw>
In some situations we reloaded the workspace in the tests after having reported
to be ready. There's two fixes here:
1. Add a version to the VFS config and include that version in progress reports,
so that we don't think we're done prematurely;
2. Delay status transitions until after changes are applied. Otherwise the last
change during loading can potentially trigger a workspace reload, if it contains
interesting changes.
I've noticed that there are various suggestions that rust-analyzer seems
to filter out, even if they make sense.
Here's an example of where it seems like there should be a suggestion,
but there isn't:
It turns out that this specific suggestion is not considered
`MachineApplicable`, which are the only suggestions that rust-analyzer
accepts. However if you read the documentation for `MachineApplicable`,
b3897e3d13/compiler/rustc_lint_defs/src/lib.rs (L27-L29)
then you realize that these are specifically only those suggestions that
rust-analyzer could even automatically apply (in some distant future,
behind some setting or so). Other suggestions that may have some
semantic impact do not use `MachineApplicable`. So all other suggestions
are still intended to be suggested to the user, just not automatically
applied without the user being consulted.
b3897e3d13/compiler/rustc_lint_defs/src/lib.rs (L22-L24)
So with that in mind, rust-analyzer should almost definitely not filter
out `MaybeIncorrect` (which honestly is named horribly, it just means
that it's a semantic change, not just a syntactical one).
Then there's `HasPlaceholders` which basically is just another semantic
one, but with placeholders. The user will have to make some adjustments,
but the suggestion still is perfectly valid. rust-analyzer could
probably detect those placeholders and put proper "tab through" markers
there for the IDE, but that's not necessary for now.
Then the last one is `Unspecified` which is so unknown that I don't even
know how to judge it, meaning that the suggestion should probably also
just be suggested to the user and then they can decide.
So with all that in mind, I'm proposing to get rid of the check for
Applicability entirely.
7457: Add no-buffering file logging and wait for a debugger option. r=vsrs a=vsrs
Adds two command line flags: `--no-buffering` and `--wait-dbg`.
Not sure if someone else needs this, but personally I found both flags extremely useful trying to figure out why RA does not work with Visual Studio. Or better to say why Visual Studio does not work with RA.
Co-authored-by: vsrs <vit@conrlab.com>
7353: Add LifetimeParam and ConstParam to CompletionItemKind r=matklad a=Veykril
Adds `LifetimeParam` and `ConstParam` to `CompletionItemKind` and maps them both to `TypeParam` in the protocol conversion as there are no equivalents, so nothing really changes there.
`ConstParam` could be mapped to `Const` I guess but I'm split on whether that would be better?
Additions were solely inspired by (the single) test output for const params.
Also sorts the variants of `CompletionItemKind` and its to_proto match.
Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
I've noticed a bunch of "main loop too long" warnings in console when
typing in Cargo.toml. Profiling showed that the culprit is `rustc
--print cfg` call.
I moved it to the background project loading phase, where it belongs.
This highlighted a problem: we generally use single `cfg`, while it
really should be per crate.
7220: same level folder rename for will_rename_files r=kjeremy a=ShuiRuTian
use tricky way to support folder rename.
Another step after #7009 and for #4471
Co-authored-by: ShuiRuTian <158983297@qq.com>
Co-authored-by: Song Gao <158983297@qq.com>
7218: Fix typos r=Veykril a=regexident
Apart from the very last commit on this PR (which fixes a public type's name) all changes are non-breaking.
Co-authored-by: Vincent Esche <regexident@gmail.com>
After we started reporting progress when running cargo check during
loading, it is possible to crash the client with two identical progress
tokens.
This points to a deeper issue: we might be running several cargo checks
concurrently, which doesn't make sense.
This commit linearizes all workspace fetches, making sure no updates are
lost.
As an additional touch, it also normalizes progress & result reporting,
to make sure they stand in sync.
This leaks a lot of LSP details into ide layer, which we want to avoid:
c9cec381bc/docs/dev (lsp-independence)
Additionally, all what this infra does is providing a toggle for
auto-import completion, but we already have one!
Rather than eagerly converting JSON, we losslessly keep it as is, and
change the shape of user-submitted data at the last moment.
This also allows us to remove a bunch of wrong Defaults
7068: Add VSCode command to view the hir of a function body r=theotherphil a=theotherphil
Will fix https://github.com/rust-analyzer/rust-analyzer/issues/7061. Very rough initial version just to work out where I needed to wire everything up.
@matklad would you be happy merging a hir visualiser of some kind? If so, do you have any thoughts on what you'd like it show, and how?
I've spent very little time on this thus far, so I'm fine with throwing away the contents of this PR, but I want to avoid taking the time to make this more polished/interactive/useful only to discover that no-one else has any interest in this functionality.
Co-authored-by: Phil Ellison <phil.j.ellison@gmail.com>
7115: Migrate HasSource::source to return Option r=matklad a=nick96
I've made a start on fixing #6913 based on the provided work plan, migrating `HasSource::source` to return an `Option`. The simple cases are migrated but there are a few that I'm unsure exactly how they should be handled:
- Logging the processing of functions in `AnalysisStatsCmd::run`: In verbose mode it includes the path to the module containing the function and the syntax range. I've handled this with an if-let but would it be better to blow up here with `expect`? I'm not 100% on the code paths but if we're processing a function definition then the source should exist.
I've handled `source()` in all code paths as `None` being a valid return value but are there some cases where we should just blow up? Also, all I've done is bubble up the returned `None`s, there may be some places where we can recover and still provide something.
Co-authored-by: Nick Spain <nicholas.spain@stileeducation.com>
Co-authored-by: Nick Spain <nicholas.spain96@gmail.com>
In an attempt to fix#6052 and #4249 this attempts to detect
if rustfmt is a rustup proxy which isn't installed, and reports
the error message to the user for them to fix.
In theory this ought to be memoised but for now it'll do as-is.
Future work might be to ask the user if they would like us to
trigger the installation (if possible).
Signed-off-by: Daniel Silverstone <dsilvers@digital-scurf.org>
Assist vs UnresolvedAssist split doesn't really pull its weight. This
is especially bad if we want to include `Assist` as a field of
diagnostics, where we'd have to make the thing generic.
7030: Support labels in reference search r=matklad a=Veykril
Implements general navigation for labels, goto def, rename and gives labels their own semantic highlighting class.
Fixes#6966
Co-authored-by: Lukas Wirth <lukastw97@gmail.com>
Avoid mutation of snapshot's config -- that's spooky action at a
distance. Instead, copy it over to a local variable.
This points out a minor architecture problem, which we won't fix right
away.
Various `ide`-level config structs, like `AssistConfig`, are geared
towards one-shot use when calling a specific methods. On the other
hand, the large `Config` struct in `rust-analyzer` is a long-term
config store.
The fact that `Config` stores `AssistConfig` is accidental -- a better
design would probably be to just store `ConfigData` inside `Config`
and create various `Config`s on the fly out of it.
6993: Clean up descriptions for settings r=matklad a=rherrmann
Use two consecutive newlines (`\n\n`) to actually continue text on a
new line.
Use proper markup to reference related settings.
Consistently format references to files, command line arguments, etc.
as `code`. Format mentions of UI elements in _italic_.
Fix typos, add missing full-stops, add missing default values.
Co-authored-by: Rüdiger Herrmann <ruediger.herrmann@gmx.de>
Use two consecutive newlines (`\n\n`) to actually continue text on a
new line.
Use proper markup to reference related settings.
Consistently format references to files, editor commands, command line
arguments, files, etc. as `code`.
Fix typos, add missing full-stops, add missing default values.
Curiously, LSP uses different enums for those, and unsurprising and
annoyingly, there are things which exist in one but not in the other.
Let's not repeat the mistake and unify the two things
6785: Fix "no value set for FileTextQuery(FileId(..))" r=jonas-schievink a=jonas-schievink
Fixes https://github.com/rust-analyzer/rust-analyzer/issues/6622
Let's hope I got it right this time, but I feel like I slowly begin to understand the main loop logic.
bors r+
Co-authored-by: Jonas Schievink <jonasschievink@gmail.com>
6761: Make config.rs a single source of truth for configuration. r=matklad a=matklad
Configuration is editor-independent. For this reason, we pick
JSON-schema as the repr of the source of truth. We do specify it using
rust-macros and some quick&dirty hackery though.
The idea for syncing truth with package.json is to just do that
manually, but there's a test to check that they are actually synced.
I'll add something like `rust-analyzer --config-schema` in a follow-up
commit.
Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>
Configuration is editor-independent. For this reason, we pick
JSON-schema as the repr of the source of truth. We do specify it using
rust-macros and some quick&dirty hackery though.
The idea for syncing truth with package.json is to just do that
manually, but there's a test to check that they are actually synced.
There's CLI to print config's json schema:
$ rust-analyzer --print-config-schema
We go with a CLI rather than LSP request/response to make it easier to
incorporate the thing into extension's static config. This is roughtly
how we put the thing in package.json.
6650: Make completion and assists module independent r=matklad a=SomeoneToIgnore
A follow-up of https://github.com/rust-analyzer/rust-analyzer/pull/6553#discussion_r524402907
Move the common code for both assists and completion modules into a separate crate.
Co-authored-by: Kirill Bulatov <mail4score@gmail.com>
6553: Auto imports in completion r=matklad a=SomeoneToIgnore
Closes https://github.com/rust-analyzer/rust-analyzer/issues/1062 but does not handle the completion order, since it's a separate task for https://github.com/rust-analyzer/rust-analyzer/issues/4922 , https://github.com/rust-analyzer/rust-analyzer/issues/4922 and maybe something else.
2 quirks in the current implementation:
* traits are not auto imported during method completion
If I understand the current situation right, we cannot search for traits by a **part** of a method name, we need a full name with correct case to get a trait for it.
* VSCode (?) autocompletion is not as rigid as in Intellij Rust as you can notice on the animation.
Intellij is able to refresh the completions on every new symbol added, yet VS Code does not query the completions on every symbol for me.
With a few debug prints placed in RA, I've observed the following behaviour: after the first set of completion suggestions is received, next symbol input does not trigger a server request, if the completions contain this symbol.
When more symbols added, the existing completion suggestions are filtered out until none are left and only then, on the next symbol it queries for completions.
It seems like the only alternative to get an updated set of results is to manually retrigger it with Esc and Ctrl + Space.
Despite the eerie latter bullet, the completion seems to work pretty fine and fast nontheless, but if you have any ideas on how to make it more smooth, I'll gladly try it out.
Co-authored-by: Kirill Bulatov <mail4score@gmail.com>
6524: Add support for loading rustc private crates r=matklad a=xldenis
This PR presents a solution to the problem of making`rustc_private` crates visible to `rust-analyzer`.
Currently developers add dependencies to those crates behind a `cfg(NOT_A_TARGET)` target to prevent `cargo` from building them.
This solution is unsatisfactory since it requires modifying `Cargo.toml` and causes problems for collaboration or CI.
The proposed solution suggested by @matklad is to allow users to give RA a path where the `rustc` sources could be found and then load that like a normal workspace.
This PR implements this solution by adding a `rustcSource` configuration item and adding the cargo metadata to the crate graph if it is provided.
------
I have no idea how this should be tested, or if this code is generally tested at all. I've locally run the extension with these changes and it correctly loads the relevant crates on a `rustc_private` project of mine.
Co-authored-by: Xavier Denis <xldenis@gmail.com>
Historically, we intentinally violated JSON-RPC spec here by hard
crashing. The idea was to poke both the clients and servers to fix
stuff.
However, this is confusing for server implementors, and falls down in
one important place -- protocol extension are not always backwards
compatible, which causes crashes simply due to version mismatch. We
had once such case with our own extension, and one for semantic
tokens.
So let's be less adventerous and just err on the err side!
6331: correct hover text for items with doc attribute with raw strings r=matklad a=JoshMcguigan
Fixes#6300 by improving the handling of raw string literals in attribute style doc comments.
This still has a bug where it could consume too many `"` at the start or end of the comment text, just as the original code had. Not sure if we want to fix that as part of this PR or not? If so, I think I'd prefer to add a unit test for either the `as_simple_key_value` function (I'm not exactly sure where this would belong / how to set this up) or create a `fn(&SmolStr) -> &SmolStr` to unit test by factoring out the `trim` operations from `as_simple_key_value`. Thoughts on this?
6342: Shorter dependency chain r=matklad a=popzxc
Continuing implementing suggestions from the `Completion refactoring` zulip thread.
This PR does the following:
- Removes dependency of `completions` on `assists` by moving required functionality into `ide_db`.
- Moves completely `call_info` crate into `ide_db` as it looks like it fits perfect there.
- Adds a bunch of new tests and docs.
- Adds the re-export of `base_db` to the `ide_db` and removes direct dependency on `base_db` from other crates.
The last point is controversial, I guess, but I noticed that in places where `ide_db` is used, `base_db` is also *always* used. Thus I think the dependency on the `base_db` is implied by the fact of `ide_db` interfaces, and thus it makes sense to just provide `base_db` out of the box.
Co-authored-by: Josh Mcguigan <joshmcg88@gmail.com>
Co-authored-by: Igor Aleksanov <popzxc@yandex.ru>
