Stream: t-compiler/rust-analyzer

Topic: trait func completion does not fill in generics


Riccardo D'Ambrosio (Jun 16 2021 at 19:41, on Zulip):

Hello!

One particular problem in RA bugs me a bit, when a trait fn is autocompleted (like mul in Mul), it will not fill in the generics in the signature with the ones you have, so if you do impl Mul<usize>, it will not give you fn mul(self, rhs: usize) -> ..., it will give you fn mul(self, rhs: Rhs) -> .... This is kind of annoying because you need to go back and fill it in. Is this issue already recorded as an issue or fixed in a PR? id be wiling to submit a PR to fix this (with maybe some help in finding where the completion is done :) ).

Lukas Wirth (Jun 16 2021 at 19:58, on Zulip):

I don't think we have an issue tracking that yet, the completion for trait items lives in https://github.com/rust-analyzer/rust-analyzer/blob/master/crates/ide_completion/src/completions/trait_impl.rs

Lukas Wirth (Jun 16 2021 at 20:02, on Zulip):

Here to be precise is where the function is built https://github.com/rust-analyzer/rust-analyzer/blob/master/crates/ide_completion/src/completions/trait_impl.rs#L127-L165

Riccardo D'Ambrosio (Jun 16 2021 at 20:02, on Zulip):

Awesome

Riccardo D'Ambrosio (Jun 16 2021 at 20:02, on Zulip):

Ill file an issue later and see if i can fix it myself

Riccardo D'Ambrosio (Jun 16 2021 at 20:03, on Zulip):

im not sure if copy and pasting the generic from the impl sig to the right place is enough haha

Riccardo D'Ambrosio (Jun 16 2021 at 20:03, on Zulip):

I cant think of any examples where that wouldnt work

Lukas Wirth (Jun 16 2021 at 20:03, on Zulip):

Feel free to ask here if you got trouble with it/if somethings unclear

Lukas Wirth (Jun 16 2021 at 20:04, on Zulip):

Ye taking the generic from the trait in the impl header seems to be a good idea

Riccardo D'Ambrosio (Jun 16 2021 at 20:08, on Zulip):

Alright ill try that first then see if anyone has alternate ideas

Riccardo D'Ambrosio (Jun 16 2021 at 23:56, on Zulip):

@Lukas Wirth how would i go about replacing the types in the function? from what i know, this is cached in a salsa db, so is there some way to get an owned struct or something? kind of confused on how to go about this :sweat_smile:

Lukas Wirth (Jun 17 2021 at 00:02, on Zulip):

You dont need to replace anything, you only have to change the source creation in ehre https://github.com/rust-analyzer/rust-analyzer/blob/master/crates/ide_completion/src/completions/trait_impl.rs#L153-L160

Riccardo D'Ambrosio (Jun 17 2021 at 00:04, on Zulip):

so instead of modifying it in place i just add more edits to replace the types in the string?

Riccardo D'Ambrosio (Jun 17 2021 at 00:05, on Zulip):

im not sure if doing it on the type level or the source/ast level is easier

Lukas Wirth (Jun 17 2021 at 00:05, on Zulip):

Oh I msorry I misunderstood the code here

Lukas Wirth (Jun 17 2021 at 00:06, on Zulip):

Right you wanna clone_for_update() the function syntax node and then rewrite it you were right with that

Riccardo D'Ambrosio (Jun 17 2021 at 00:06, on Zulip):

cause from what i understand about RA's architecture, everything is either refcounted or its housed in a db, which makes getting owned stuff kinda rough :/

Riccardo D'Ambrosio (Jun 17 2021 at 00:07, on Zulip):

wouldnt working on the syntax node level be harder because the type info isnt there?

Lukas Wirth (Jun 17 2021 at 00:07, on Zulip):

Ye so for syntax trees we have that clone_for_update function which basically makes a mutable clone of it that you can rewrite

Riccardo D'Ambrosio (Jun 17 2021 at 00:07, on Zulip):

i see

Riccardo D'Ambrosio (Jun 17 2021 at 00:07, on Zulip):

so i need to identify what to replace in another pass, then rewrite the syntax nodes based on that, right?

Lukas Wirth (Jun 17 2021 at 00:07, on Zulip):

well all you really need is to copy the generic param node out of the impl/trait header and copy it into the function node's param type

Lukas Wirth (Jun 17 2021 at 00:07, on Zulip):

then you can just render that

Lukas Wirth (Jun 17 2021 at 00:07, on Zulip):

if I see this right

Riccardo D'Ambrosio (Jun 17 2021 at 00:07, on Zulip):

right but i need to check what types include generics

Riccardo D'Ambrosio (Jun 17 2021 at 00:08, on Zulip):

which might also be recursive

Riccardo D'Ambrosio (Jun 17 2021 at 00:08, on Zulip):

such as Vec<T>as a param instead of just T

Lukas Wirth (Jun 17 2021 at 00:08, on Zulip):

But T should already be in scope in that case so it doesnt matter just grabbing it should be fine

Lukas Wirth (Jun 17 2021 at 00:08, on Zulip):

(deleted)

Lukas Wirth (Jun 17 2021 at 00:09, on Zulip):

(deleted)

Riccardo D'Ambrosio (Jun 17 2021 at 00:09, on Zulip):

is there an easy way to pull the references to it inside that specific function?

Lukas Wirth (Jun 17 2021 at 00:09, on Zulip):

what references do you mean

Riccardo D'Ambrosio (Jun 17 2021 at 00:09, on Zulip):

pulling all the references to each generic declared in the impl

Riccardo D'Ambrosio (Jun 17 2021 at 00:10, on Zulip):

like if the function takes T as a param, pulling that reference from the declared T in the impl

Riccardo D'Ambrosio (Jun 17 2021 at 00:13, on Zulip):

not even quite sure how to get the actual impl :sweat_smile:

Lukas Wirth (Jun 17 2021 at 00:13, on Zulip):

Mmh ye I just realized its not that simple as I was hoping

Lukas Wirth (Jun 17 2021 at 00:13, on Zulip):

But

Lukas Wirth (Jun 17 2021 at 00:14, on Zulip):

we have an assist that does something similar I just realized

Lukas Wirth (Jun 17 2021 at 00:14, on Zulip):

so I think you can take inspiration from that

Riccardo D'Ambrosio (Jun 17 2021 at 00:14, on Zulip):

ooh

Lukas Wirth (Jun 17 2021 at 00:14, on Zulip):

add_missing_impl_members

Lukas Wirth (Jun 17 2021 at 00:14, on Zulip):

The fill_in_type_params_1 and fill_in_type_params_2 tests seems to do pretty similar things

Riccardo D'Ambrosio (Jun 17 2021 at 00:14, on Zulip):

where is that assist?

Lukas Wirth (Jun 17 2021 at 00:14, on Zulip):

So I think you should be able to copy whatever its doing there

Riccardo D'Ambrosio (Jun 17 2021 at 00:15, on Zulip):

nvm found it

Lukas Wirth (Jun 17 2021 at 00:15, on Zulip):

crates\ide_assists\src\handlers\add_missing_impl_members.rs

Riccardo D'Ambrosio (Jun 17 2021 at 00:16, on Zulip):

hmmm cant quite tell how its doing it

Riccardo D'Ambrosio (Jun 17 2021 at 00:19, on Zulip):

it seems to defer to add_trait_assoc_items_to_impl, but it doesnt seem to do anything for that, and it doesnt seem to be done before that

Lukas Wirth (Jun 17 2021 at 00:24, on Zulip):

The assist first fetches all missing assoc items, which in your case here would be the function we want to complete, then add_trait_assoc_items_to_impl takes those and adds them to the passed impl and returns a new impl. Then the assist replaces the old impl block with the new one

Riccardo D'Ambrosio (Jun 17 2021 at 00:24, on Zulip):

it calls get_missing_assoc_items to get the items, but that function seems to do nothing for that

Lukas Wirth (Jun 17 2021 at 00:24, on Zulip):

The replacement we dont wanna do here of course

Riccardo D'Ambrosio (Jun 17 2021 at 00:25, on Zulip):

hows that function substituting the generics however?

Lukas Wirth (Jun 17 2021 at 00:26, on Zulip):

Substitution happens inside add_trait_assoc_items_to_impl via the PathTransform object in there

Riccardo D'Ambrosio (Jun 17 2021 at 00:26, on Zulip):

ah hmmm

Riccardo D'Ambrosio (Jun 17 2021 at 00:26, on Zulip):

ohh i see

Riccardo D'Ambrosio (Jun 17 2021 at 00:26, on Zulip):

yeah thats kind of obscure but i can prob just use that

Lukas Wirth (Jun 17 2021 at 00:26, on Zulip):

So thats the component you certainly want to reuse

Riccardo D'Ambrosio (Jun 17 2021 at 00:26, on Zulip):

can i access that from ide_completion?

Riccardo D'Ambrosio (Jun 17 2021 at 00:27, on Zulip):

cause its a separate crate

Lukas Wirth (Jun 17 2021 at 00:27, on Zulip):

Nope you will have to move that to ide_db most likely

Riccardo D'Ambrosio (Jun 17 2021 at 00:27, on Zulip):

whew, thats going to be some work

Riccardo D'Ambrosio (Jun 17 2021 at 00:28, on Zulip):

where would you suggest i put it? just in assists.rs?

Riccardo D'Ambrosio (Jun 17 2021 at 00:28, on Zulip):

actually, i can prob just copy over path_transform.rs

Lukas Wirth (Jun 17 2021 at 00:29, on Zulip):

I would just move the ide_assists/src/path_transform.rs to ide_db/src/path_transform.rs for now

Lukas Wirth (Jun 17 2021 at 00:29, on Zulip):

ye

Riccardo D'Ambrosio (Jun 17 2021 at 00:29, on Zulip):

yeah thats what i did

Riccardo D'Ambrosio (Jun 17 2021 at 00:30, on Zulip):

getting a lot of unresolved imports for things like ide_db, is that normal?

Riccardo D'Ambrosio (Jun 17 2021 at 00:30, on Zulip):

from RA, it compiles fine though

Lukas Wirth (Jun 17 2021 at 00:31, on Zulip):

you mean rustc is complaining?

Lukas Wirth (Jun 17 2021 at 00:31, on Zulip):

oh

Lukas Wirth (Jun 17 2021 at 00:31, on Zulip):

nvm

Lukas Wirth (Jun 17 2021 at 00:31, on Zulip):

I imagine its related to the Databases?

Riccardo D'Ambrosio (Jun 17 2021 at 00:31, on Zulip):

yeah it seems to be

Riccardo D'Ambrosio (Jun 17 2021 at 00:31, on Zulip):

oh think i found it

Lukas Wirth (Jun 17 2021 at 00:31, on Zulip):

Your proc-macro server might have crashed if you are on a recent nightly

Riccardo D'Ambrosio (Jun 17 2021 at 00:31, on Zulip):

proc macro server crashed, i forgot i enabled the proc macro experimental thing some time ago lol

Riccardo D'Ambrosio (Jun 17 2021 at 00:31, on Zulip):

or is that not related to that?

Riccardo D'Ambrosio (Jun 17 2021 at 00:32, on Zulip):

yeah im on the latest nightly, i could switch to stable though

Lukas Wirth (Jun 17 2021 at 00:32, on Zulip):

ye if crashes with the experimental feature turned on all items with attributes disappear to RA

Lukas Wirth (Jun 17 2021 at 00:32, on Zulip):

well either switch to stable or turn off the experimental feature

Riccardo D'Ambrosio (Jun 17 2021 at 00:33, on Zulip):

what should the scopes in the path transform be in this case?

Lukas Wirth (Jun 17 2021 at 00:33, on Zulip):

uuh

Riccardo D'Ambrosio (Jun 17 2021 at 00:35, on Zulip):

the assist seems to use the impl def as the target and ctx.sema's scope for the source

Riccardo D'Ambrosio (Jun 17 2021 at 00:35, on Zulip):

im not sure what ctx.sema is in this case, im guessing prob just the scope inside the impl block?

Lukas Wirth (Jun 17 2021 at 00:35, on Zulip):

sema = Semantics, is just something that binds syntax to the hir in a sense

Lukas Wirth (Jun 17 2021 at 00:36, on Zulip):

so you wanna use the same scopes here

Lukas Wirth (Jun 17 2021 at 00:36, on Zulip):

that is

Riccardo D'Ambrosio (Jun 17 2021 at 00:36, on Zulip):

alright

Lukas Wirth (Jun 17 2021 at 00:36, on Zulip):

get the scope for the trait def as source

Lukas Wirth (Jun 17 2021 at 00:36, on Zulip):

and the scope of the impl for the target

Riccardo D'Ambrosio (Jun 17 2021 at 00:36, on Zulip):

to get the impl def the assist's approach of ctx.find_node_at_offset::<ast::Impl>()? should also work, or is this in a different context?

Lukas Wirth (Jun 17 2021 at 00:37, on Zulip):

You already have the Impl node in the completion

Riccardo D'Ambrosio (Jun 17 2021 at 00:37, on Zulip):

oh i do? haha i must have missed it

Lukas Wirth (Jun 17 2021 at 00:37, on Zulip):

the context there is different

Lukas Wirth (Jun 17 2021 at 00:37, on Zulip):

Ye you will ahve to thread it through to your function call

Lukas Wirth (Jun 17 2021 at 00:37, on Zulip):

But it already exists in the complete_trait_impl call

Riccardo D'Ambrosio (Jun 17 2021 at 00:38, on Zulip):

through the context?

Riccardo D'Ambrosio (Jun 17 2021 at 00:38, on Zulip):

nvm im blind

Lukas Wirth (Jun 17 2021 at 00:39, on Zulip):

the tricky part is to get the trait def node of the impl which you will need the semantics for

Riccardo D'Ambrosio (Jun 17 2021 at 00:41, on Zulip):

so, to get the trait i need to go through resolve_target_trait, but that wants the sema, which is different in this case

Lukas Wirth (Jun 17 2021 at 00:41, on Zulip):

Ah ye there is a function for this already sweet

Lukas Wirth (Jun 17 2021 at 00:41, on Zulip):

so the contexts are different here because one is an AssistContext and the other a CompletionContext

Lukas Wirth (Jun 17 2021 at 00:41, on Zulip):

both have a Semantics though

Lukas Wirth (Jun 17 2021 at 00:41, on Zulip):

no problem there then

Riccardo D'Ambrosio (Jun 17 2021 at 00:41, on Zulip):

yeah, and i presume the assistcontext is "inside" the impl block?

Riccardo D'Ambrosio (Jun 17 2021 at 00:41, on Zulip):

oh but even if it is, the sema should be the same, nvm

Lukas Wirth (Jun 17 2021 at 00:42, on Zulip):

no the contexts is not linked to the syntax nodes in anyway

Riccardo D'Ambrosio (Jun 17 2021 at 00:42, on Zulip):

oh it isnt? odd

Lukas Wirth (Jun 17 2021 at 00:42, on Zulip):

well the AssistContext knows about the file and can look through it to find nodes, not linked in any way is a wrong term sorry

Lukas Wirth (Jun 17 2021 at 00:43, on Zulip):

The completioncontext does some more analysis on the syntax

Riccardo D'Ambrosio (Jun 17 2021 at 00:43, on Zulip):

should i also call remove_attrs_and_docs like the assist does?

Riccardo D'Ambrosio (Jun 17 2021 at 00:44, on Zulip):

nvm that was a stupid question

Riccardo D'Ambrosio (Jun 17 2021 at 01:00, on Zulip):

for some reason its also failing to check because its trying to build tikv-jemalloc-sys, but im on msvc, and Cargo.toml says it shouldnt be built for msvc

Riccardo D'Ambrosio (Jun 17 2021 at 01:00, on Zulip):

rustc builds it fine though

Lukas Wirth (Jun 17 2021 at 01:03, on Zulip):

That seems odd

Riccardo D'Ambrosio (Jun 17 2021 at 01:04, on Zulip):

Screenshot_228.png really weird

Riccardo D'Ambrosio (Jun 17 2021 at 18:20, on Zulip):

Yeah its really weird, even after deleting the jemalloc features, RA doesnt seem to identify errors when saving and running cargo check, but running cargo check manually shows them

Lukas Wirth (Jun 17 2021 at 18:23, on Zulip):

Have you tried cargo clean and then rebuilding yet?

Riccardo D'Ambrosio (Jun 17 2021 at 18:23, on Zulip):

haha i was just about to say that fixed it

Riccardo D'Ambrosio (Jun 17 2021 at 18:23, on Zulip):

although the tikv-jemalloc-sys issue still exists, i had to remove the feature manually

Lukas Wirth (Jun 17 2021 at 18:25, on Zulip):

hmm thats quite odd honestly, im on windwos as well and for me everything builds fine

Riccardo D'Ambrosio (Jun 17 2021 at 18:26, on Zulip):

im on 1.53 btw

Lukas Wirth (Jun 17 2021 at 18:26, on Zulip):

that should be fine

Riccardo D'Ambrosio (Jun 17 2021 at 18:26, on Zulip):

i dont have any exotic RA settings that might mess with it i dont think

Riccardo D'Ambrosio (Jun 17 2021 at 18:27, on Zulip):

lol nevermind i have all features active :face_palm:

Riccardo D'Ambrosio (Jun 17 2021 at 18:27, on Zulip):

spoke too soon

Lukas Wirth (Jun 17 2021 at 18:27, on Zulip):

well there we go

Lukas Wirth (Jun 17 2021 at 18:27, on Zulip):

:big_smile:

Riccardo D'Ambrosio (Jun 17 2021 at 18:27, on Zulip):

haha does cargo not have a way to not offer a feature for a target

Riccardo D'Ambrosio (Jun 17 2021 at 18:28, on Zulip):

huh but it should be disabled for this workspace as the settings say

Riccardo D'Ambrosio (Jun 17 2021 at 18:28, on Zulip):

thats weird, turning it on and off fixed it, anyways it works haha

Riccardo D'Ambrosio (Jun 17 2021 at 18:34, on Zulip):

also, is there a way to easily transform the hir version of something to its ast version? other than going through the syntax node then to the ast node

Riccardo D'Ambrosio (Jun 17 2021 at 18:34, on Zulip):

like hir::Impl -> ast::Impl

Riccardo D'Ambrosio (Jun 17 2021 at 18:35, on Zulip):

ah nvm

Lukas Wirth (Jun 17 2021 at 18:36, on Zulip):

there is a source function implemented on most hir things if needed

Lukas Wirth (Jun 17 2021 at 18:36, on Zulip):

HasSource being the trait i think

Riccardo D'Ambrosio (Jun 17 2021 at 18:37, on Zulip):

yeah i realized i can just go through source then value

Riccardo D'Ambrosio (Jun 17 2021 at 18:38, on Zulip):

everything working off a big database is interesting, its nice because you can cheaply pass around stuff but also kind of weird because u need to go through the db to get anything meaningful

Riccardo D'Ambrosio (Jun 17 2021 at 18:39, on Zulip):

also a bit weird because theres things that modify structures but dont take mutable refs, because they modify it in the db, so its sometimes hard to deduce what is changing what

Lukas Wirth (Jun 17 2021 at 18:40, on Zulip):

The stuff inside the Database is immutable as far as I know

Riccardo D'Ambrosio (Jun 17 2021 at 18:42, on Zulip):

when should i be using clone_for_update? im making an AssocItem from the value of an hir Function, then applying the transform to that, then the edit, but in other places, the edit calls clone_subtree then clone_for_update

Lukas Wirth (Jun 17 2021 at 18:43, on Zulip):

Basically the syntax trees are immutable, clone_for_update gives you a separate copy that can be modified though. clone_subtree clones a tree discarding the mutability "flag" again which is used there so clone_for_update doesnt panic should the tree already be mutable

Riccardo D'Ambrosio (Jun 17 2021 at 18:43, on Zulip):

sorry for all the questions, im very familiar with the parser and syntax part of RA but ive never messed with higher level stuff :sweat_smile:

Lukas Wirth (Jun 17 2021 at 18:43, on Zulip):

so usually you wanna call clone_for_update on a tree if you want to mutate (a copy of) it

Riccardo D'Ambrosio (Jun 17 2021 at 18:44, on Zulip):

so i should be cloning for update here so i can modify it let assoc_item = ast::AssocItem::Fn(func.source(ctx.db)?.value);

Lukas Wirth (Jun 17 2021 at 18:45, on Zulip):

yes

Riccardo D'Ambrosio (Jun 17 2021 at 18:45, on Zulip):

how come remove_attrs_and_docs doesnt seem to modify in place then :thinking:

Lukas Wirth (Jun 17 2021 at 18:47, on Zulip):

It just hasnt been rewritten yet most likely. The inplace modification api is rather new

Lukas Wirth (Jun 17 2021 at 18:47, on Zulip):

before you had to reconstruct a tree with your changes instead

Riccardo D'Ambrosio (Jun 17 2021 at 18:47, on Zulip):

oh is this the new rowan experiment i saw a while ago?

Lukas Wirth (Jun 17 2021 at 18:47, on Zulip):

Yep

Riccardo D'Ambrosio (Jun 17 2021 at 18:47, on Zulip):

oh sweet

Lukas Wirth (Jun 17 2021 at 18:47, on Zulip):

Immutable-mutable syntax trees

Riccardo D'Ambrosio (Jun 17 2021 at 18:48, on Zulip):

ive used rowan a ton for my own projects, thankfully i didnt really need tree mutation

Riccardo D'Ambrosio (Jun 17 2021 at 18:51, on Zulip):

hmm whats the opposite conversion method? ast -> hir ?

Lukas Wirth (Jun 17 2021 at 18:51, on Zulip):

sema.to_def

Riccardo D'Ambrosio (Jun 17 2021 at 18:51, on Zulip):

thanks :)

Riccardo D'Ambrosio (Jun 17 2021 at 19:04, on Zulip):

nice it seems to work

    #[test]
    fn generics_are_inlined() {
        check_edit(
            "function",
            r#"
trait Foo<T> {
    fn function(bar: T);
}
struct Bar;

impl Foo<u32> for Bar {
    fn f$0
}
"#,
            r#"
trait Foo<T> {
    fn function(bar: T);
}
struct Bar;

impl Foo<u32> for Bar {
    fn function(bar: u32) {
    $0
}
}
"#,
        )
    }
Riccardo D'Ambrosio (Jun 17 2021 at 19:07, on Zulip):

just going to add some more tests and ill submit the PR

Lukas Wirth (Jun 17 2021 at 19:08, on Zulip):

perfect :thumbs_up:

Riccardo D'Ambrosio (Jun 17 2021 at 19:09, on Zulip):

are there any other special cases to test other than parameter inlining and return type inlining?

Lukas Wirth (Jun 17 2021 at 19:10, on Zulip):

the type of associated consts

Riccardo D'Ambrosio (Jun 17 2021 at 19:11, on Zulip):

ohh yes

Lukas Wirth (Jun 17 2021 at 19:11, on Zulip):

but aside from that i dont think so

Lukas Wirth (Jun 17 2021 at 19:11, on Zulip):

or well technically the where clauses of functions could reference the generic parameters as well

Lukas Wirth (Jun 17 2021 at 19:11, on Zulip):

so those have also be checked

Riccardo D'Ambrosio (Jun 17 2021 at 19:12, on Zulip):

oh think i found a bug, its not inlining multiple generics

Riccardo D'Ambrosio (Jun 17 2021 at 19:13, on Zulip):
Left:
trait Foo<T, U, V> {
    fn function(bar: Vec<T>, baz: U) -> Arc<Vec<V>>;
}
struct Bar;

impl Foo<u32, Vec<usize>, u8> for Bar {
    fn function(bar: Vec<u32, baz: Vec<usize>>) -> Arc<Vec<u8>> {
    $0
}
}


Right:
trait Foo<T, U, V> {
    fn function(bar: Vec<T>, baz: U) -> Arc<Vec<V>>;
}
struct Bar;

impl Foo<u32> for Bar {
    fn function(bar: Vec<u32>, baz: U) -> Arc<Vec<V>> {
    $0
}
}
Riccardo D'Ambrosio (Jun 17 2021 at 19:13, on Zulip):

thats odd, the path transform should already be doing this

Riccardo D'Ambrosio (Jun 17 2021 at 19:14, on Zulip):

oh wait i missed the other ones in the check lmao

Riccardo D'Ambrosio (Jun 17 2021 at 19:17, on Zulip):

what does the f$0 represent? the f specifically, is $0 enough if i want to test the const/function name?

Lukas Wirth (Jun 17 2021 at 19:24, on Zulip):

The $0 is the cursor position in the test

Lukas Wirth (Jun 17 2021 at 19:25, on Zulip):

you probably need to put at least one character after the fn because completion might not work otherwise in that position

Lukas Wirth (Jun 17 2021 at 19:25, on Zulip):

as in fn $0 might not trigger it

Lukas Wirth (Jun 17 2021 at 19:25, on Zulip):

so the f there is just a character

Riccardo D'Ambrosio (Jun 17 2021 at 19:28, on Zulip):

ah i see

Riccardo D'Ambrosio (Jun 18 2021 at 00:05, on Zulip):

hmm i just realized that PathTransform does not work on const generics

Riccardo D'Ambrosio (Jun 18 2021 at 00:09, on Zulip):

submitted the pr :tada: https://github.com/rust-analyzer/rust-analyzer/pull/9321

Riccardo D'Ambrosio (Jun 18 2021 at 00:25, on Zulip):

hmm CI is failing because of trailing whitespace, but im not sure what it's complaining about, running cargo fmt check locally doesnt trigger it

Laurențiu (Jun 18 2021 at 06:03, on Zulip):

Ugh. There's a space at the end of line 966 (do they start from 0?), but the test fails if you remove it.

Laurențiu (Jun 18 2021 at 06:04, on Zulip):

Try:

diff --git i/crates/ide_completion/src/completions/trait_impl.rs w/crates/ide_completion/src/completions/trait_impl.rs
index 1f6b959af..37622aaad 100644
--- i/crates/ide_completion/src/completions/trait_impl.rs
+++ w/crates/ide_completion/src/completions/trait_impl.rs
@@ -953,7 +953,7 @@ trait Foo<T> {
 struct Bar;

 impl Foo<u32> for Bar {
-    const B$0
+    const B$0;
 }
 "#,
             r#"
@@ -963,7 +963,7 @@ trait Foo<T> {
 struct Bar;

 impl Foo<u32> for Bar {
-    const BAR: u32 =
+    const BAR: u32 = ;
 }
 "#,
         )
Riccardo D'Ambrosio (Jun 18 2021 at 06:14, on Zulip):

That worked, i pushed the changes

Laurențiu (Jun 18 2021 at 06:15, on Zulip):

I think there were other errors on CI, but I couldn't reproduce them locally.

Laurențiu (Jun 18 2021 at 06:15, on Zulip):

If you run into this again, try setting RUN_SLOW_TESTS=1

Riccardo D'Ambrosio (Jun 18 2021 at 06:17, on Zulip):

I think you need to approve a rerun of the workflows

Riccardo D'Ambrosio (Jun 18 2021 at 06:17, on Zulip):

the errors in CI were just the formatting

Laurențiu (Jun 18 2021 at 06:18, on Zulip):

Approved

Riccardo D'Ambrosio (Jun 18 2021 at 06:18, on Zulip):

thanks :smile:

Riccardo D'Ambrosio (Jun 18 2021 at 06:23, on Zulip):

CI passes :tada:

Laurențiu (Jun 18 2021 at 11:48, on Zulip):

Filed https://github.com/rust-analyzer/rust-analyzer/pull/9326 for the confusing error

Riccardo D'Ambrosio (Jun 18 2021 at 16:50, on Zulip):

@Lukas Wirth i think you need to manually approve the bors workflow because im a first time contributor

Riccardo D'Ambrosio (Jun 18 2021 at 16:51, on Zulip):

kind of annoying, github didnt used to require this :thinking:

Lukas Wirth (Jun 18 2021 at 16:51, on Zulip):

bors runs it on its own, as long the bors run works it'll merge

Riccardo D'Ambrosio (Jun 18 2021 at 16:51, on Zulip):

ah ok

Lukas Wirth (Jun 18 2021 at 16:51, on Zulip):

Its required now because people started abusing workflows of other repos for crypto mining

Riccardo D'Ambrosio (Jun 18 2021 at 16:51, on Zulip):

yeah makes sense

Laurențiu (Jun 18 2021 at 16:52, on Zulip):

Once someone approved the workflow for a contributor (or a PR, at least), it should probably work the second time

Riccardo D'Ambrosio (Jun 18 2021 at 16:52, on Zulip):

but it still runs it when you submit the pr, it just doesnt run it multiple times

Laurențiu (Jun 18 2021 at 16:52, on Zulip):

But it looks like it doesn't

Riccardo D'Ambrosio (Jun 18 2021 at 16:52, on Zulip):

yeah

Riccardo D'Ambrosio (Jun 18 2021 at 16:52, on Zulip):

really weird

Lukas Wirth (Jun 18 2021 at 16:52, on Zulip):

Laurențiu said:

Once someone approved the workflow for a contributor (or a PR, at least), it should probably work the second time

you always have to reapprove after a commit unfortunately...

Riccardo D'Ambrosio (Jun 18 2021 at 16:57, on Zulip):

im not sure how new features in releases work, does this get added to the monday release or is there an experimental version it goes through first?

Lukas Wirth (Jun 18 2021 at 16:58, on Zulip):

We do a stable release every monday, and a nightly release every day

Riccardo D'Ambrosio (Jun 18 2021 at 16:59, on Zulip):

awesome

Riccardo D'Ambrosio (Jun 18 2021 at 17:05, on Zulip):

@Lukas Wirth thanks for all the help btw! :heart:

Last update: Jul 29 2021 at 09:00UTC