Stream: t-compiler/wg-rls-2.0

Topic: completion label filtering


Laurențiu Nicola (Feb 12 2019 at 17:52, on Zulip):

This could be an issue, but I wanted to try out Zulip.

rust-analyzer filters out comments in the function body for the completion detail area, but it keeps the attributes, and I'm not sure how useful they are, see below:

pasted image

Would it be a good idea to filter them out?

matklad (Feb 12 2019 at 17:53, on Zulip):

Yeah, I think it's a good idea to remove the attributes.

matklad (Feb 12 2019 at 17:54, on Zulip):

In general, I think we'll soon need to create a "more enterpriesy" module for rendering defs as completion items. Currently, completion code contains way to many "rendering" logic

Laurențiu Nicola (Feb 12 2019 at 17:55, on Zulip):

there's already a body range check, but it doesn't seem to work (or maybe it's doing something else)

Laurențiu Nicola (Feb 12 2019 at 18:05, on Zulip):
info: caused by: No space left on device (os error 28)
test check_code_formatting ... FAILED

Well, I didn't expect that one...

Laurențiu Nicola (Feb 12 2019 at 18:11, on Zulip):
thread 'completion::complete_dot::tests::test_method_body_filtering' panicked at 'byte index 14 is out of bounds of ``', src/libcore/str/mod.rs:2026:9
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
             at src/libstd/sys/unix/backtrace/tracing/gcc_s.rs:39
   1: std::sys_common::backtrace::_print
             at src/libstd/sys_common/backtrace.rs:70
   2: std::panicking::default_hook::{{closure}}
             at src/libstd/sys_common/backtrace.rs:58
             at src/libstd/panicking.rs:200
   3: std::panicking::default_hook
             at src/libstd/panicking.rs:209
   4: std::panicking::rust_panic_with_hook
             at src/libstd/panicking.rs:478
   5: std::panicking::continue_panic_fmt
             at src/libstd/panicking.rs:385
   6: rust_begin_unwind
             at src/libstd/panicking.rs:312
   7: core::panicking::panic_fmt
             at src/libcore/panicking.rs:85
   8: core::str::slice_error_fail
             at src/libcore/str/mod.rs:0
   9: core::str::traits::<impl core::slice::SliceIndex<str> for core::ops::range::RangeFrom<usize>>::index::{{closure}}
             at /rustc/4b1e39b7b36d677803e40130ea29ee6d300abf6e/src/libcore/str/mod.rs:1893
  10: <core::option::Option<T>>::unwrap_or_else
             at /rustc/4b1e39b7b36d677803e40130ea29ee6d300abf6e/src/libcore/option.rs:386
  11: core::str::traits::<impl core::slice::SliceIndex<str> for core::ops::range::RangeFrom<usize>>::index
             at /rustc/4b1e39b7b36d677803e40130ea29ee6d300abf6e/src/libcore/str/mod.rs:1893
  12: core::str::traits::<impl core::ops::index::Index<I> for str>::index
             at /rustc/4b1e39b7b36d677803e40130ea29ee6d300abf6e/src/libcore/str/mod.rs:1632
  13: <alloc::string::String as core::ops::index::Index<core::ops::range::RangeFrom<usize>>>::index
             at /rustc/4b1e39b7b36d677803e40130ea29ee6d300abf6e/src/liballoc/string.rs:1973
  14: insta::runtime::format_rust_expression
             at /home/grayshade/.cargo/registry/src/github.com-1ecc6299db9ec823/insta-0.6.2/src/runtime.rs:62
  15: insta::runtime::print_changeset
             at /home/grayshade/.cargo/registry/src/github.com-1ecc6299db9ec823/insta-0.6.2/src/runtime.rs:174
  16: insta::runtime::print_snapshot_diff
             at /home/grayshade/.cargo/registry/src/github.com-1ecc6299db9ec823/insta-0.6.2/src/runtime.rs:298
  17: insta::runtime::print_snapshot_diff_with_title
             at /home/grayshade/.cargo/registry/src/github.com-1ecc6299db9ec823/insta-0.6.2/src/runtime.rs:317
  18: insta::runtime::assert_snapshot
             at /home/grayshade/.cargo/registry/src/github.com-1ecc6299db9ec823/insta-0.6.2/src/runtime.rs:404
  19: ra_ide_api::completion::completion_item::check_completion
             at crates/ra_ide_api/src/completion/completion_item.rs:363
  20: ra_ide_api::completion::complete_dot::tests::check_ref_completion
             at crates/ra_ide_api/src/completion/complete_dot.rs:88
  21: ra_ide_api::completion::complete_dot::tests::test_method_body_filtering
             at crates/ra_ide_api/src/completion/complete_dot.rs:184
  22: ra_ide_api::completion::complete_dot::tests::test_method_body_filtering::{{closure}}
             at crates/ra_ide_api/src/completion/complete_dot.rs:183
  23: core::ops::function::FnOnce::call_once
             at /rustc/4b1e39b7b36d677803e40130ea29ee6d300abf6e/src/libcore/ops/function.rs:231
  24: <F as alloc::boxed::FnBox<A>>::call_box
             at src/libtest/lib.rs:1474
             at /rustc/4b1e39b7b36d677803e40130ea29ee6d300abf6e/src/libcore/ops/function.rs:231
             at /rustc/4b1e39b7b36d677803e40130ea29ee6d300abf6e/src/liballoc/boxed.rs:734
  25: __rust_maybe_catch_panic
             at src/libpanic_unwind/lib.rs:92
  26: test::run_test::run_test_inner::{{closure}}
             at /rustc/4b1e39b7b36d677803e40130ea29ee6d300abf6e/src/libstd/panicking.rs:276
             at /rustc/4b1e39b7b36d677803e40130ea29ee6d300abf6e/src/libstd/panic.rs:388
             at src/libtest/lib.rs:1429

Is insta crashing?

matklad (Feb 12 2019 at 18:11, on Zulip):

seems like this, yeah

Laurențiu Nicola (Feb 12 2019 at 18:12, on Zulip):
    #[test]
    fn test_method_body_filtering() {
        check_ref_completion(
            "method_body_filtering",
            r"
            struct A {}
            impl A {
                fn the_method() {
                    let x = 1;
                    let x = 2;
                }
            }
            fn foo(a: A) {
               a.<|>
            }
            ",
        );
    }

I wonder if I'm doing something wrong..

matklad (Feb 12 2019 at 18:14, on Zulip):

I'd say this is a bug in insta. Perhaps removing existing snapshot would work?

Laurențiu Nicola (Feb 12 2019 at 18:16, on Zulip):
[pid  9365] write(2, "Unrecognized option: 'edition'", 30Unrecognized option: 'edition') = 30

wat

Maybe my toolchain is screwed up. Yeah.

Jeremy Kolb (Feb 12 2019 at 18:47, on Zulip):

Yeah the comments filtering was for a quick and dirty solution. There's a lot of room for improvement. There are other items like that too

Laurențiu Nicola (Feb 12 2019 at 18:49, on Zulip):

https://github.com/rust-analyzer/rust-analyzer/pull/811 should fix this particular instance

Laurențiu Nicola (Feb 12 2019 at 18:53, on Zulip):

though maybe it should check for ast::FnDef instead?

Jeremy Kolb (Feb 12 2019 at 19:02, on Zulip):

An alternative may be to reconstruct the signature based on it's visibility, name and param list. iirc that's what rls does

Laurențiu Nicola (Feb 12 2019 at 19:04, on Zulip):

but the signature is already available somewhere else (it gets passed to CompletionItem::new), so I'm not sure what's supposed to happen here

Jeremy Kolb (Feb 12 2019 at 19:04, on Zulip):

That way we could also format type parameters or something so that they always use the where syntax (or not). sort of a normalized presentation format.

Laurențiu Nicola (Feb 12 2019 at 19:05, on Zulip):
impl CompletionItem {
    pub(crate) fn new(
        completion_kind: CompletionKind,
        replace_range: TextRange,
        label: impl Into<String>,
    ) -> Builder {
        let label = label.into();
    }
}
Laurențiu Nicola (Feb 12 2019 at 19:05, on Zulip):

oh..

Laurențiu Nicola (Feb 12 2019 at 19:05, on Zulip):

so the signature is stored as the label, then it gets overwritten

Laurențiu Nicola (Feb 12 2019 at 19:07, on Zulip):

never mind, one is label, and the other one is detail

Jeremy Kolb (Feb 12 2019 at 19:07, on Zulip):

yeah detail is the fn sig

Jeremy Kolb (Feb 12 2019 at 19:08, on Zulip):

label is the function name iirc

Laurențiu Nicola (Feb 12 2019 at 19:09, on Zulip):

right

Jeremy Kolb (Feb 12 2019 at 19:09, on Zulip):

so the function that fills in detail also fills in the signature when you're autocompleting a function call

Jeremy Kolb (Feb 12 2019 at 19:09, on Zulip):

(see call_info.rs)

Laurențiu Nicola (Feb 12 2019 at 19:11, on Zulip):

I guess this is because FnSignature isn't Display?

Jeremy Kolb (Feb 12 2019 at 19:18, on Zulip):

I don't know if that's quite where we would put it. for instance the function signature help also wants access to either a substring or a range of offsets representing each parameter in the list so that it can underline when typing out a call (though there is no reason that whatever does that + Display wouldn't use the same code to figure that out

Laurențiu Nicola (Feb 12 2019 at 19:19, on Zulip):

good point

Last update: Nov 12 2019 at 17:00UTC