Incorrect flags for some completion items

[vzmerr]

Compiler version

3.1.3

Minimized code

This code is from CompletionSnippetSuite in Metals which is run by the specified scala version.
@@ represents the cursor position.

object Main {
    new scala.Iterabl@@
}

Output

calling Completion.completions(pos) from Dotty API, produces

List(
  Completion(
    label = "Iterable",
    description = "=> collection.Iterable.type",
    symbols = List(method Iterable)
  ),
  Completion(
    label = "IterableOnce",
    description = "scala.IterableOnce",
    symbols = List(type IterableOnce)
  ),
  Completion(label = "Iterable", description = "scala.Iterable", symbols = List(type Iterable))
) 

Now the problem with the flags is that, for the last two items on the list, which are traits actually,

• the symbol.is(Flags.Trait) returns false;
• while symbol.info.typeSymbol.is(Flags.Trait) and symbol.info.typeSymbol.is(Trait) return true.

This is problematic for using the API for completions, such as when deciding whether to insert {} or when filtering results.

You might be interested in taking a look at these comments on the this PR for Scala 3 type completions in Metals.

Expectation

It is better if the flags are correctly set and are consistent.

[KacperFKorban]

I think that the behavior is correct. Seems like those items are type aliases. see here

[vzmerr]

I think that the behavior is correct. Seems like those items are type aliases. see here

Thank you for the reply; but why are the values of symbol.is(Flags.Trait) and symbol.info.typeSymbol.is(Flags.Trait) different?

Also, it is unsuspected that symbol.info.typeSymbol.is(Trait) has a different value to symbol.is(Flags.Trait).

Any chance the api can change to become a little bit user-friendlier? Almost everyone on our team got a little bit confused about the expected values of the flags in this case.

Basically we want to detect abstract types for the purpose of deciding where the completion should be allowed or for inserting {} in order for the user to implement the abstract members inside.

Which flag combination do you recommend? We need to test them for various use-cases to make sure that they behave the expected way and valid results do not get incorrectly filtered out.

[vzmerr]

Another inconsistency case:

In this code, from metals completion test cases,

      object A {
        scala.concurrent.ExecutionContext.Implicits.global@@
      }

we have:

• sym.info.typeSymbol.is(Trait): true
• sym.is(Trait): false
  for "method global"

is this expected? Why is that? How can we predict the flag values?

[prolativ]

I think @KacperFKorban is right. The flags of a symbol depend on how something is defined syntactically, not on the actual semantics and this is expected, e.g. in

trait Foo

the symbol Foo represents a trait but in

type Bar = Foo

Bar is defined as a type not a trait since it's not

trait Bar = Foo

(which is not syntactically correct scala code).
The right hand side doesn't really matter here when the symbol itself is considered.
On the other hand , the RHS does influence the type of a symbol because of type inference, so for

val x = 2

the type of symbol x is Int because this gets desugared to

val x: Int = 2

[vzmerr]

@prolativ thank you for the elaboration. But still, this does not explain the inconsistencies.

[vzmerr]

@prolativ I think it would be more helpful to the consumers of the api, to set the flag values according to the semantics though. We definitely need that for completions in Metals.

[prolativ]

In your second example scala.concurrent.ExecutionContext.Implicits.global is defined as def so it's a method symbol (not a trait) but it has type scala.concurrent.ExecutionContext, and ExecutionContext is defined as a trait so its symbol is a trait.
So the results you get are correct

[vzmerr]

So the values on the typeSymbol are semantically set?

[vzmerr]

So generally, if the method flag is set, the type symbol flags show the category of the return type, right?

[prolativ]

Not really. typeSymbol also returns a symbol
Let's consider a simple scenario:

trait Foo[A] // sym1
object Foo // sym2
def foo = new Foo[Int] {} // sym3
type Bar[A] = Foo[A] // sym4
val Bar = Foo // sym5
def bar = new Bar[Int] {} // sym6

This snippet declares 6 symbols.
Here only sym1 is a trait symbol so sym1.is(Trait) should be true.
sym4 is declared as type so sym4.is(Trait) is false. However as type Bar proxies to Foo I believe sym4.info.typeSymbol should return sym1 (and so should sym1.info.typeSymbol) so if you want to check if a type symbol sym is semantically a trait I think something like sym.info.typeSymbol.is(Trait) should do the work (although I haven't tested it).
sym3 is a term symbol representing a method and sym3.info is the type Foo[Int] so its type symbol sym3.info.typeSymbol is also sym1.

In your particular case you should however note that new Fo@@ should not always directly complete Foo[] as if some type was declared inside of Foo's companion object the user might rather prefer to complete just new Foo so that the user continues typing like new Foo.Quux

[vzmerr]

@prolativ awesome elaboration. Thank you so much 😊