aboutsummaryrefslogtreecommitdiff
path: root/crates
Commit message (Collapse)AuthorAgeFilesLines
* Merge #7535bors[bot]2021-02-054-1/+2189
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 7535: Extract function assist r=cpud36 a=cpud36 This PR adds `extract function/method` assist. closes #5409. # Supported features Assist should support extracting from expressions(`1`, `2 + 2`, `loop { }`) and from a series of statements, e.g.: ```rust foo(); $0bar(); baz();$0 quix(); ``` Assist also supports extracting parameters, like: ```rust fn foo() -> i32 { let n = 1; $0n + 1$0 } // - fn foo() -> i32 { let n = 1; fun_name(n) } fn fun_name(n: i32) -> i32 { n + 1 } ``` Extracting methods also generally works. Assist allows referencing outer variables, both mutably and immutably, and handles handles access to variables local to extracted function: ```rust fn foo() { let mut n = 1; let mut m = 2; let mut moved_v = Vec::new(); let mut ref_mut_v = Vec::new(); $0 n += 1; let k = 1; moved_v.push(n); let r = &mut m; ref_mut_v.push(*r); let h = 3; $0 n = ref_mut_v.len() + k; n -= h + m; } // - fn foo() { let mut n = 1; let mut m = 2; let mut moved_v = Vec::new(); let mut ref_mut_v = Vec::new(); let (k, h) = fun_name(&mut n, moved_v, &mut m, &mut ref_mut_v); n = ref_mut_v.len() + k; n -= h + m; } fn fun_name(n: &mut i32, mut moved_v: Vec<i32>, m: &mut i32, ref_mut_v: &mut Vec<i32>) -> (i32, i32) { *n += 1; let k = 1; moved_v.push(*n); let r = m; ref_mut_v.push(*r); let h = 3; (k, h) } ``` So we handle both input and output paramters # Showcase ![extract_cursor_in_range_3](https://user-images.githubusercontent.com/4218373/106980190-c9870800-6770-11eb-83d9-3d36b2550ff6.gif) ![fill_match_arms_discard_wildcard](https://user-images.githubusercontent.com/4218373/106980197-cbe96200-6770-11eb-96b0-14c27894fac0.gif) ![ide_db_helpers_handle_kind](https://user-images.githubusercontent.com/4218373/106980201-cdb32580-6770-11eb-9e6e-6ac8155d65ac.gif) ![ide_db_imports_location_local_query](https://user-images.githubusercontent.com/4218373/106980205-cf7ce900-6770-11eb-8516-653c8fcca807.gif) # Working with non-`Copy` types Consider the following example: ```rust fn foo() { let v = Vec::new(); $0 let n = v.len(); $0 let is_empty = v.is_empty(); } ``` `v` must be a parameter to extracted function. The question is, what type should it have. It could be `v: Vec<i32>`, or `v: &Vec<i32>`. The former is incorrect for `Vec<i32>`, but the later is silly for `i32`. To resolve this we need to know if the type implements `Copy` trait. I didn't find any api available from assists to query this. `hir_ty::method_resolution::implements` seems relevant, but is isn't publicly re-exported from `hir`. # Star(`*`) token and pointer dereference If I understand correctly, in order to create expression like `*p`, one should use `ast::make::expr_prefix(T![*], ...)`, which in turn calls `token(T![*])`. `token` does not have star in `tokens::SOURCE_FILE`, so this panics. I had to add `*` to `SOURCE_FILE` to make it work. Correct me if this is not intended way to do this. # Lowering access `value -> mut ref -> shared ref` Consider the following example: ```rust fn foo() { let v = Vec::new(); $0 let n = v.len(); $0 } ``` `v` is not used after extracted function body, so both `v: &Vec<i32>` and `v: Vec<i32>` would work. Currently the later would be chosen. We can however check the body of extracted function and conclude that `v: &Vec<i32>` is sufficient. Using `v: &Vec<i32>`(that is a minimal required access level) might be a better default. I am unsure. # Cleanup The assist seems to be reasonably handling most of common cases. If there are no concerns with code it produces(i.e. with test cases), I will start cleaning up [edit] added showcase Co-authored-by: Vladyslav Katasonov <[email protected]>
| * allow extracted body to be indented(dedent it)Vladyslav Katasonov2021-02-051-13/+101
| |
| * allow transitive `&mut` access for fields in extract_functionVladyslav Katasonov2021-02-051-27/+92
| |
| * add tests for extracting if/match/while/for exprsVladyslav Katasonov2021-02-041-0/+120
| |
| * document extract_function assist implementationVladyslav Katasonov2021-02-041-22/+126
| |
| * use `&T` for non copy params of extracted functionVladyslav Katasonov2021-02-041-2/+55
| | | | | | | | Use shared ref if param is not `T: Copy` and is used after body
| * split extract_function into pieces and order themVladyslav Katasonov2021-02-041-380/+510
| |
| * remove ignored test for downgrading mut to sharedVladyslav Katasonov2021-02-031-30/+0
| |
| * allow calling `&mut` methods on outer vars when extracing functionVladyslav Katasonov2021-02-031-0/+116
| |
| * allow `&mut param` when extracting functionVladyslav Katasonov2021-02-031-3/+107
| | | | | | | | | | | | Recognise &mut as variable modification. This allows extracting functions with `&mut var` with `var` being in outer scope
| * allow modifications of vars from outer scope inside extracted functionVladyslav Katasonov2021-02-032-46/+337
| | | | | | | | | | It currently allows only directly setting variable. No `&mut` references or methods.
| * allow local variables to be used after extracted bodyVladyslav Katasonov2021-02-031-41/+183
| | | | | | | | | | when variable is defined inside extracted body export this variable to original scope via return value(s)
| * change TODO to FIXMEVladyslav Katasonov2021-02-031-2/+2
| |
| * disable test for downgrading mutability on extractVladyslav Katasonov2021-02-031-0/+3
| |
| * convert IdentPat to Pat via IntoVladyslav Katasonov2021-02-031-5/+5
| | | | | | | | before child getter was used
| * support extracting methods; no mut loweringVladyslav Katasonov2021-02-031-37/+191
| | | | | | | | | | currently mut refernce will *not* be downgraded to shared if it is sufficient(see relevant test for example)
| * initial version of extract function assistVladyslav Katasonov2021-02-033-0/+848
| | | | | | | | | | | | | | | | there are a few currently limitations: * no modifications of function body * does not handle mutability and references * no method support * may produce incorrect results
* | Avoid using ModPath's fields directlyJonas Schievink2021-02-043-8/+9
| |
* | Make `ModPath`'s representation privateJonas Schievink2021-02-0418-51/+70
| |
* | Intern `TypeRef`s in the containing `ItemTree`Jonas Schievink2021-02-044-21/+67
| |
* | Expander: store a LocalModuleId, not ModuleIdJonas Schievink2021-02-042-14/+11
| | | | | | | | | | It already stores the DefMap containing the module, so having a full ModuleId is unnecessary and makes it easier to mix things up
* | Don't keep the parent DefMap alive via ArcJonas Schievink2021-02-045-36/+67
| | | | | | | | | | This seems like it could easily leak a lot of memory since we don't currently run GC
* | Split out ItemScope::dump from DefMap::dumpJonas Schievink2021-02-032-21/+26
| |
* | Merge #7546bors[bot]2021-02-032-0/+11
|\ \ | | | | | | | | | | | | | | | | | | | | | 7546: Add newline between block and crate maps r=jonas-schievink a=jonas-schievink bors r+ Co-authored-by: Jonas Schievink <[email protected]>
| * | Add newline between block and crate mapsJonas Schievink2021-02-032-0/+11
| | |
* | | Add a FIXME to ItemTreeJonas Schievink2021-02-031-0/+1
|/ /
* | Update `DefMap` and `block_def_map` docsJonas Schievink2021-02-032-1/+25
| |
* | Merge #7541bors[bot]2021-02-0312-100/+211
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 7541: Use block_def_map in body lowering (third time's the charm) r=jonas-schievink a=jonas-schievink After https://github.com/rust-analyzer/rust-analyzer/pull/7380 and https://github.com/rust-analyzer/rust-analyzer/pull/7506 both had to be reverted, this should have finally resolved all remaining bugs. Most importantly, the optimization to skip `block_def_map` computation when the block contains no inner items was fixed (which fortunately was simpler than expected). I've ran `analysis-stats` on libstd locally, which works fine, and also ran this PR locally for a short while without issues. Note that this *still* has no (or almost no) user-facing impact, because the rest of r-a still relies on some local item support hacks. bors r+ Co-authored-by: Jonas Schievink <[email protected]>
| * | Test for name resolution with DefMap shortcutJonas Schievink2021-02-031-0/+33
| | |
| * | Shortcut `block_def_map` if there's no inner itemsJonas Schievink2021-02-035-11/+26
| | | | | | | | | | | | | | | This previously didn't work, but apparently only because of the wonky test setup
| * | Use body lowering for block_def_map testsJonas Schievink2021-02-033-68/+117
| | | | | | | | | | | | Removes the hacky and buggy custom lowering code
| * | Use block_def_map in body loweringJonas Schievink2021-02-036-26/+40
| | |
* | | Add cargo file tidy testEdwin Cheng2021-02-031-2/+2
|/ /
* / Make sure normal dependencies always have versionPavan Kumar Sunkara2021-02-031-1/+1
|/
* Fix resolution of `crate` paths from within blocksJonas Schievink2021-02-022-2/+7
| | | | | They resolve to the crate root, not the DefMap's root module (which can be a block)
* Bump rustc_lexerLaurențiu Nicola2021-02-021-1/+1
|
* Bump chalkLaurențiu Nicola2021-02-022-5/+5
|
* Use non-deprecated memmap2 cratekjeremy2021-02-022-2/+2
| | | | | | | `cargo audit` complains that `memmap` is unmaintained so switch to RazrFalcon's maintained version. Removes yet another edge on winapi
* Show alias underlying typelumenian2021-02-021-1/+5
|
* Use the right `DefMap` when looking up modulesJonas Schievink2021-02-023-2/+20
|
* Revert "Use block_def_map in body lowering"Jonas Schievink2021-02-0210-161/+96
|
* Only allow one proc-macro processEdwin Cheng2021-02-011-14/+16
|
* Reap proc macro server instancesLaurențiu Nicola2021-02-012-8/+4
|
* Update Test DataChristopher Serr2021-02-011-1/+46
|
* Don't filter code suggestions on ApplicabilityChristopher Serr2021-02-011-4/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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: ![https://i.imgur.com/wsjM6iz.png](https://i.imgur.com/wsjM6iz.png) 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`, https://github.com/rust-lang/rust/blob/b3897e3d1302391ed02efbac1dce8073646b8173/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. https://github.com/rust-lang/rust/blob/b3897e3d1302391ed02efbac1dce8073646b8173/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.
* Shortcut `block_def_map` if there's no inner itemsJonas Schievink2021-02-011-2/+4
| | | | | This previously didn't work, but apparently only because of the wonky test setup
* Use body lowering for block_def_map testsJonas Schievink2021-02-013-68/+117
| | | | Removes the hacky and buggy custom lowering code
* Use block_def_map in body loweringJonas Schievink2021-02-016-26/+40
|
* Merge #7503bors[bot]2021-01-312-1/+25
|\ | | | | | | | | | | | | | | 7503: Return inner attributes of outline mod declarations in `attrs_query` r=jonas-schievink a=Veykril Co-authored-by: Lukas Wirth <[email protected]>
| * Return inner attributes of outline mod declarations in `attrs_query`Lukas Wirth2021-01-312-1/+25
| |