Allow import <ident> to show completions

[tgodzik]

Previously, there would be no completions after simple import annot. Now, we make sure that the completions mode is set correctly for that case.

Additionally, whenever both java.lang and scala imports conflict we choose scala one.

Connected to #13623

But it will not work with package objects yet. We should most likely open up a separate issue for that?

[prolativ]

We shouldn't assume that in case of a conflict there would be only 2 colliding items.
Consider this code (which should be added as a test case)

import java.lang.annotation; import annot${m1}

Here the completion should be ("annotation", Module, "java.lang.annotation") but currently no completion is returned.

[tgodzik]

Ach damn, so maybe:
denotss.forall(d => isScalaPackage(d) || isJavaLangPackage(d))
would work?

[tgodzik]

Should be good now!

[mtk]

i'm confused by this. if i recreate that example, "annotation" is the completion after the java import. i have no idea what module the repl thinks it is but does it matter?

[prolativ]

@mtk Do you mean you are actually getting any completion suggestion for the example I gave? Which version of the compiler are you using?
Anyway even though the REPL currently provides completions without showing their exact types this is something that would be actually quite easy to change, simply nobody has time to work on the improvement.
Secondly, if Metals rely on completions logic implemented here, you would also get confusing results in your IDE if this is done wrong.

[mtk]

yes, i get a completion for your example. i'm using an up-to-date branch with tomasz's PR on top of it. hence my confusion :-). /mark

…

On Thu, Dec 9, 2021 at 5:17 AM Michał Pałka ***@***.***> wrote: @mtk <https://github.com/mtk> Do you mean you are actually getting any completion suggestion for the example I gave? Which version of the compiler are you using? Anyway even though the REPL currently provides completions without showing their exact types this is something that would be actually quite easy to change, simply nobody has time to work on the improvement. Secondly, if Metals rely on completions logic implemented here, you would also get confusing results in your IDE if this is done wrong. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#14069 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAYG2PQNKH6KB7DD2QQ2G3UQB6YZANCNFSM5JR2NXSQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[prolativ]

That's really strange. Indeed this seems to work in the REPL but when I created a test case in CompletionTest.scala

  @Test def importAnnotationAfterImport : Unit =
    code"""import java.lang.annotation; import annot${m1}"""
          .withSource
          .completion(m1,
            Set(
              ("annotation", Module, "scala.annotation")
            )
          )

it fails with no completions

[mtk]

i don't know enough about the testing framework to be sure (even though i worked with tomasz on this) but i'm guessing that the test is too specific. it doesn't want to just verify that the completion of 'anno' is 'tation'. it wants to verify that the completion came from a specific namespace. and in the case of annotation, it is provided by both 'scala' and 'java.lang' but the test is demanding that it comes from only 'scala'. does this make sense to the two of you?

[prolativ]

@mtk I would say when something gets completed automatically for you it's usually better to know what this actually is exactly than just have to guess, especially if you don't have to check it manually but the tooling does this for you. Different definitions with the same name can have completely different semantics.
And as mentioned before, this additional information is not currently available in the REPL but it could be exposed there (if you'd like to work on that I can give you some tips) and it's already used by Metals