objects completion in New and Type positions

[vzmerr]

It partially addresses #4213 ✅ allowing Objects for type completion in New and Type positions.

TODO(Out of Scope)

• filtering out objects that are private members and inaccessible in scope. The catch is that even the java objects with only private and protected constructors can have a member that can be instantiated, so we should still include them in new positions.
• filtering out objects with no member that can be instantiated in new position.
• filtering out java objects, aka classes, with private and protected constructors in new and type positions; because they cannot be instantiated in Scala code. The catch is that java objects with protected constructors can be obtained as a member or return type of a method in another class or object located in the same package as them, such as the so called "builder" ones.

Update

When the class/trait does not take any type parameters, it is useless to offer both the companion object and class/trait in the type position. Because both are going to generate the same edit. lets say

case class Foo()
object Foo:
  ???

object Main:
  val a: Foo@@

Update

The PR now introduces four additional changes:

✅fixing the symName in SemanticdbSymbols, by adding # to the descriptor of classes and traits to mark them as types ✅filtering objects only when type parameter is present ✅ allowing dot in the index id of object symbols for differentiation from class ✅ covering the case for extends in detecting type position

Update

TODO (Out of the Scope of the PR):

• We need to filter out all the case classes, and classes and traits with final identifier from type completions after extends and with
• We need to filter out all the classes and traits with sealed identifier not defined in the same file, from type completions after extends and with

Update:

Done:

✅ allowing JavaModule to be suggested where objects are not allowed, which is where type parameters are present, beacuse Java objects are actually classes

Done:

✅ not allowing classes in new position. This fix, although at first glance is unrelated to the main topic of the PR, is done for getting the tests to pass. ✅ allowing duplicate class and companion object where the class takes type parameters ✅allowing duplicate trait and companion object where the trait takes type parameters or curly braces.

TODO(Out of Scope):

• fixing the printed label of classes/traits to include type parameters

[tgodzik]

filtering out objects that are private members and inaccessible in scope

That could be done separately in another PR.

filtering out objects with no member that can be instantiated in new position.

I don't think we should do that, it would most likely be inefficient.

detecting java objects, which are actually classes, and removing the . suffix from them.

We should not add . suffix to them in the first place i think.

filtering out java objects, aka classes, with private and protected constructors in new and type positions; because they cannot be instantiated in Scala code.

This could also be another PR

[vzmerr]

We should add . suffix to them in the first place i think.

should or should not?

[tgodzik]

We should add . suffix to them in the first place i think.

should or should not?

Shouldn not :sweat_smile: I updated the comment

[vzmerr]

Let's include it into release. @vzmerr there is conflict - could you rebase it on the main?

@dos65 sure, there is also an additional problem: currently the index used in filterInteresting is using symbolName as the key, which causes the objects which are now not filtered out to shade and block the classes and traits with the same key from entering the index :( Some tests are failing specifically because of that. I am trying to come up with a better key for the index! Any idea?

Update

It seems to me that the issue should be fixed in the addDescriptor(sym: Symbol) in SemanticdbSymbols where we do not differentiate between objects, classes, and traits.

Update

there, in else if sym.isType || sym.isAllOf(JavaModule) then b.append('#') I want to add || sym.info.typeSymbol.is(Trait) || sym.isClass because classes and traits are types too. This would create differentiation between type/class/trait category and object. Also, there is no trait that has a companion class or type, while traits and classes can have companion objects.

Update

I just don't get what ModuleClass stands for in if sym.is(ModuleClass) then addDescriptor(sym.sourceModule) and whether this has any overlap with the sym.isClass

Also, it is not documented which overlaps we're eliminated by removing the end . in filterInteresting in case sym.isClass || sym.is(Module) ? Was it because the existing addDescriptor(sym: Symbol) method was adding the same descriptor . to both objects and classes, and we did not want both an object and class in the results? If so, now in this PR, we do want differentiated results for a class potentially suffixed with [] and its companion object which syntactically would not take type parameters.

[dos65]

@vzmerr

sure, there is also an additional problem: currently the index used in filterInteresting is using symbolName as the key, which causes the objects which are now not filtered out to shade and block the classes and traits with the same key from entering the index :(

yep. there is a such thing - we needed it to avoid duplication in completions items as at that time we didn't have a suffix ([] | ()) support for scala3. Now we can just remove this hack from filterInteresting and there is no need to touch SemanticdbSymbols

[vzmerr]

and there is no need to touch SemanticdbSymbols

oh, I changed that, and it started working like a charm. Indeed, as a result, all the tests in which the classes were shaded and filtered out, now are behaving exactly as expected. I would push it for you in an hour or so, so that you would be able to check.

I mean the addition of the dot was not solving the shading problem, because both the classes and their companion objects had them :(

[vzmerr]

@dos65 Hey Vadim, please feel free to take on this PR and change it as you please to fit in the frame of the new rework. Please feel free to let me know if the explanations are insufficient or you need help ;)

[tanishiking]

closing for now as it would be easier to re-work based on https://github.com/scalameta/metals/pull/4234, but anyone wants to work from this, feel free to re-open :)