Add completion for expressions

[vinistock]

First step for #59

This PR adds a completion API with support for expression completion. That means, completion when there's nothing before the cursor.

The idea of the implementation is to have an enum to describe all of the possible contexts we may see completion being triggered since they all collect slightly different declarations. I tried to document all scenarios, despite having only implemented expression.

Notes

The expression completion is currently missing keywords. I'll propose something for those in a different PR.

[vinistock]

• Add completion for expressions #585 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[soutaro]

Looks good to me. 👍

I have a few thoughts related to this PR:

• I’m assuming shortening candidate names will be implemented in another layer, right?
• I’m not sure filtering out nested constants from completion makes sense. I often just type CONST and hope completion inserts Foo::CONST depending on the context. I think this is more useful because we usually focus on the last part of the qualified name and sometimes forget the prefix. (Steep filters out nested names in Ruby code completions but keeps them in RBS completions, and the latter feels better to me.)

[Morriar]

Let's add a doc comment here to clarify the purpose. Also, should we validate that we eventually find a non-singleton class?

Suggested change

	pub fn attached_object<'a>(&'a self, maybe_singleton: &'a Namespace) -> &'a Namespace {
	/// Searches for the initial attached object for an arbitrarily nested singleton class.
	/// Walks up the owner chain until finding a non-singleton namespace.
	///
	/// # Example
	/// For `Foo::<Foo>::<<Foo>>`, returns `Foo`
	///
	/// # Panics
	///
	/// Panics if we attached a singleton class to something that isn't a namespace
	/// or if the owner chain is broken.
	#[must_use]
	pub fn attached_object<'a>(&'a self, maybe_singleton: &'a Namespace) -> &'a Namespace {

My only concern is - what happens if we have a broken chain where the loop never exits? Should we add a safety check or assert that attached_object is actually different from the input?

[vinistock]

I added the docs you suggested. Regarding the broken chain, I'm trying to think if we can add some debug asserts to verify that owners are always namespaces and ultimately connected to Object.

[vinistock]

I think we can have a sanity check about the resolution data, so that we can avoid paying the price of checking in the completion loop. I'll prototype this in a different PR.

[vinistock]

#592

[Morriar]

I wonder if we could make these errors more structured?

Right now we're using string-based errors which makes it harder to handle specific cases. Any thoughts on creating an enum for completion errors?

pub enum CompletionError {
    NameNotFound(NameId),
    UnresolvedName(NameId),
    InvalidDeclarationType,
}

This would make error handling more explicit and easier to test. We can do this later if needed 👍

[vinistock]

Added another task in #59. I agree, we should use specific errors.

[vinistock]

I’m assuming shortening candidate names will be implemented in another layer, right?

Yes, the name shortening will happen on the LSP side.

I’m not sure filtering out nested constants from completion makes sense.

Do you mean always showing all constants available regardless of lexical scope? This is an interesting question. The behaviour distinction would be:

• Returning everything in one go gives you a gigantic list, but you can see everything immediately
• Filtering based on lexical scope means you see constants in parts. For example:

module Foo
  CONST = 1
end

# completion here does not show Foo::CONST, only Foo
# Immediately after accepting `Foo` and adding `::`, another completion is triggered
# and you see all constants inside of `Foo`

What do people prefer in terms of experience?

[soutaro]

Returning everything in one go gives you a gigantic list, but you can see everything immediately

True... I haven't had it my mind. Like in Shopify codebase, it must be unrealistic.

Filtering based on lexical scope means you see constants in parts

Yes. I often remember the base name of the constant, CONST, but forget the prefixes.