Inconsistent and incomplete completion results for Scala 2 and 3

[vzmerr]

Describe the bug

If you put this in CompletionSuite, you would see that there are differences between the results returned for scala 3 and 2

• some of the results are members of java._ so they should not differ between Scala 2 and 3.
• some of the results of Scala 2 such as String= seem to be incorrect for the position.
• a valid result such as StringIndexOutOfBoundsException, is omitted from Scala 3 result, although it is part of java api

check(
    "simple-short".only,
    """
      |object A {
      |  val a: Strin@@
      |}""".stripMargin,
    """|String java.lang
       |StringBuffer java.lang
       |StringContext scala
       |StringFormat scala.Predef
       |StringOps - scala.collection
       |StringView - scala.collection
       |FromString - scala.util.CommandLineParser
       |StringJoiner - java.util
       |StringReader - java.io
       |StringWriter - java.io
       |StringBuilder - java.lang
       |StringParsers - scala.collection
       |StringToExpr - scala.quoted.ToExpr
       |""".stripMargin,
    compat = Map(
      "2" -> """|StringBuilder scala.collection.mutable
                |String java.lang
                |String= String
                |StringBuffer java.lang
                |StringBuilder java.lang
                |StringBuilder= StringBuilder
                |StringContext scala
                |StringIndexOutOfBoundsException java.lang
                |StringIndexOutOfBoundsException= StringIndexOutOfBoundsException
                |StringFormat scala.Predef
                |String - scala.math.Equiv
                |String - scala.math.Ordering
                |StringOps - scala.collection
                |StringProp - scala.sys.Prop
                |StringView - scala.collection
                |StringFormat - scala.Predef: [A](self: A): StringFormat[A] <and> StringFormat.type
                |StringJoiner - java.util
                |StringReader - java.io
                |StringWriter - java.io
                |StringParsers - scala.collection
                |""".stripMargin
    ),
  )

Also please note that StringBufferInputStream in the below test is generated, while omitted from the results of the above tests for both Scala 2 and 3.

 check(
    "simple",
    """
      |object A {
      |  val a: StringBuffe@@
      |}""".stripMargin,
    """|StringBuffer java.lang
       |StringBufferInputStream - java.io
       |""".stripMargin,
  )

Expected behavior

Consistent and complete results for scala 2 and 3 except when the Scala API itself differs.

Operating system

No response

Editor/Extension

No response

Version of Metals

0.11.7+71-ae60fa38-SNAPSHOT

Extra context or search terms

Type Completion Scala 2 Scala 3

[tgodzik]

Looks like the issue with StringIndexOutOfBoundsException is related to the compiler. We are getting an object from the compiler :thinking:

Completions.scala:182 compilerCompletions: List(
  Completion(
    label = "StringIndexOutOfBoundsException",
    description = "StringIndexOutOfBoundsException",
    symbols = List(object StringIndexOutOfBoundsException)
  ),
  Completion(
    label = "StringBuffer",
    description = "StringBuffer",
    symbols = List(object StringBuffer)
  ),
  Completion(
    label = "StringContext",
    description = "StringContext",
    symbols = List(object StringContext)
  ),
  Completion(
    label = "StringFormat",
    description = "StringFormat",
    symbols = List(object StringFormat)
  ),
  Completion(
    label = "StringBuffer",
    description = "java.lang.StringBuffer",
    symbols = List(class StringBuffer)
  ),
  Completion(
    label = "StringContext",
    description = "scala.StringContext",
    symbols = List(class StringContext)
  ),
  Completion(label = "String", description = "String", symbols = List(object String)),
  Completion(
    label = "StringFormat",
    description = "scala.Predef.StringFormat",
    symbols = List(class StringFormat)
  )
)

[vzmerr]

Similar behaviour is observed with typing IndexedSeq, Future, Iterator, Iterable in different stages of typing; or in other words, how many characters we have typed; in the live demo.

[vzmerr]

Michal might know something about it @prolativ

[prolativ]

I'm not sure if I understood the issue correctly. In your initial example which of the completions exactly

1. are returned by the compiler but shouldn't be?
2. aren't returned by the compiler but should be?

Regarding StringIndexOutOfBoundsException I think the problem is actually not that an object is returned but that a class should be also returned but it isn't. In general representation of java types in scala in quite hacky I would say because we need to synthesize something pretending to be companion objects for java classes to be able to call static methods and invoke constructors via apply. And in general we need to complete objects when a type is expected because we need to be able to access types defined inside of objects. So the question is why the completion for the class of StringIndexOutOfBoundsException is missing. Actually it seems to be missing also for String but e.g. for StringBuffer both the class and the object are returned so this seems quite strange

[vzmerr]

And in general we need to complete objects when a type is expected because we need to be able to access types defined inside of objects.

Interesting 🧐 because the current implementation does not return object completions when type is expected.

Any way to filter whether an object has type members or not?

Perhaps, when we complete with objects in type positions, we should suffix the object with . so as to hint towards selecting a type member defined inside of it.

[vzmerr]

are returned by the compiler but shouldn't be?

String= for Scala 2 because it is the equals method

aren't returned by the compiler but should be?

In Scala 3: StringBufferInputStream and StringIndexOutOfBoundsException

In Scala 2:

StringIndexOutOfBoundsException

[prolativ]

I would argue that it's correct that StringBufferInputStream is missing - it's not in scope by default unless you import it, e.g.

object A {
  val a: StringBufferInputStream = ???
}

doesn't compile

[prolativ]

I'm not sure if it really makes sense to filter out objects without members. Actually for true scala objects you always have .type as a possible member. For java types - the line below compiles in scala 2 but not i scala 3

type A = StringIndexOutOfBoundsException.type

[tgodzik]

Actually for true scala objects you always have .type as a possible member.

Sure, but it's rarely used in reality.

The main issue seems to be that we only get an object for StringIndexOutOfBoundsException, but since we don't actually have objects in Java and you always need a class, maybe we could allow Java objects @vzmerr ?

[vzmerr]

So what we would do for Scala 3 is this:

• Allow objects coming from Scala that have type members other than .type
• Allow all objects coming from java

[tgodzik]

Allow objects coming from Scala that have type members other than .type

This one is harder to do efficiently, so I would not do that unless anyone raises an issue.

Allow objects coming from Scala that have type members other than .type

I think that should be fine.

[vzmerr]

I guess you meant

Allow all objects coming from java I think that should be fine

👍

[prolativ]

We need objects with .type as the only type members because something like Foo.type might actually be what the user wanted to type. In case of traits and types that don't have a companion objects (classes). Actually it's java pseudo-objects that we might want to exclude if they don't have any nested types

[vzmerr]

it's java pseudo-objects that we might want to exclude if they don't have any nested types

Thank you for this. So how should we recognise them? Through flags?

[vzmerr]

Also, can you please check why specifically StringBufferInputStream is appearing when typing StringBuffe but it does not appear when we type Stri?

Because in both these cases, it is not imported into scope, as a member of java.io

[prolativ]

I'm afraid there are no flags on a class indicating if it has any nested type members. But if you have a symbol and you know it's a class you should be able to list all of its members and check if there are any type symbols among them

[vzmerr]

So what is a java pseudo-object? one with no explicitly defined constructor and only static methods?

[vzmerr]

But java pseudo-objects can be instantiated too. So they are valid types.

[tgodzik]

Actually it's java pseudo-objects that we might want to exclude if they don't have any nested types

It's the only completion we are getting, so we can't exclude them.

We need objects with .type as the only type members because something like Foo.type might actually be what the user wanted to type. In case of traits and types that don't have a companion objects (classes).

This might be what the user wanted, sure, but since it's a rare scenario we don't want to show it in type positions. At least that's what we are doing in Scala 2 and that never was a problem. This is a bit of conflict between usability and correctness.

[tgodzik]

This might be what the user wanted, sure, but since it's a rare scenario we don't want to show it in type positions. At least that's what we are doing in Scala 2 and that never was a problem. This is a bit of conflict between usability and correctness.

Actually, I might be wrong about that, we do suggest objects where type is applicable. It seems we just exclude any advanced completions there :thinking:

[vzmerr]

This might be what the user wanted, sure, but since it's a rare scenario we don't want to show it in type positions. At least that's what we are doing in Scala 2 and that never was a problem. This is a bit of conflict between usability and correctness.

Actually, I might be wrong about that, we do suggest objects where type is applicable. It seems we just exclude any advanced completions there 🤔

Oh no, we don't suggest objects currently in type completions as in val a: Str@@. This something to be fixed.

[kasiaMarek]

I think https://github.com/scalameta/metals/commit/6ccc512d53f231e906b9fb99a74cc774efd64fc9 already addresses the issue here.