[REVERTED] Completion bugfix: Matcher should be able to match on name exactly

[tgodzik]

Previously, we would get completion candidates that would by filter by a wrong name if it was an alias (name of the type that was liased). Now, we find dealiased candidates later on, so no need to add the alias info.

The problem appeared since we use the name matcher to fuzzy match on the input string. If the name that we match on is wrong then the results will also be wrong.

Follow up from https://github.com/scala/scala/pull/9754/files

@SethTisue if it's a sensible change I could also revert the rest of the changes from the other PR. Or do you think we should keep it as is?

[SethTisue]

@jxnu-liguobin opinion here?

[jxnu-liguobin]

@jxnu-liguobin opinion here?

For my part, due to the usage of the alias, it may be easily or already confirmed by the user, so it is OK to not show the alias.

[tgodzik]

@jxnu-liguobin opinion here?

For my part, due to the usage of the alias, it may be easily or already confirmed by the user, so it is OK to not show the alias.

Actually this PR should not change the behaviour of the REPL, I just moved the functionality further down the chain, since it makes it complicated to filter out the initial members with the current approach.

[kubukoz]

hey, is there anything blocking this PR? It seems like we're waiting for it to get 2.13.10 which would fix a bincompat issue: scala/bug#12650

[tgodzik]

Och, I had no idea this was a blocker 😨 Should I remove alias: Option[String] = None that was added in the other PR since it's no longer used?

[kubukoz]

(I got that info from https://contributors.scala-lang.org/t/scala-2-13-10-release-planning/5943 btw)

[SethTisue]

I have been waffling about what to do about this PR: merge it? or merge a full reversion of #9754? or leave things alone until 2.13.11?

(I apologize for waffling privately rather than speaking up here sooner.)

On the one hand, this PR certainly seems safe, and it corrects a known regression in 2.13.9.

But on the other hand, tooling authors were able to work around the regression, and they will equally be able to work around it in 2.13.10, if we postpone any further changes.

Plus, there was some awkward timing and I unexpectedly ended up short on time to properly devote to this.

I've got to make some choice since it's now very pressing to get 2.13.10 out the door, so the decision is: let's leave this alone for now and do something for 2.13.11.

[tgodzik]

I work around the issue in 2.13.10, let me know if I need to do anything to push this forward 😄

[SethTisue]

I added commits based on the review feedback from Lukas.

The lack of any test coverage in-repo worries me a bit, though this is reassuring:

Actually this PR should not change the behaviour of the REPL, I just moved the functionality further down the chain

Perhaps there is sufficient test coverage in downstream repos (Metals?) and I shouldn't be worried 🤷

[SethTisue]

@tgodzik is this Metals test failure in the community build an expected consequence of this?

https://scala-ci.typesafe.com/job/scala-2.13.x-jdk11-integrate-community-build/4504/artifact/logs/metals-build.log

[metals] [info] tests.pc.CompletionIssueSuite.issue-749_2.13.11-bin-fd209dc started
[metals] [error] ==> X tests.pc.CompletionIssueSuite.issue-749_2.13.11-bin-fd209dc  0.04s munit.ComparisonFailException: /home/jenkins/workspace/scala-2.13.x-jdk11-integrate-community-build/target-0.9.20/project-builds/metals-d7c35065e6e279bc7b1afed6954ae07d362a6c3e/tests/cross/src/test/scala/tests/pc/CompletionIssueSuite.scala:38
[metals] [error] 37:
[metals] [error] 38:  check(
[metals] [error] 39:    "issue-749".tag(IgnoreScala3),
[metals] [error] diff assertion failed
[metals] [error] => Obtained
[metals] [error] "+(other: String): String"
[metals] [error] => Diff (- obtained, + expected)
[metals] [error] -+(other: String): String
[metals] [error] +Self[+T] = Main.this.stream.Self
[metals] [error]     at munit.Assertions.failComparison(Assertions.scala:274)

[tgodzik]

Not expected at all. We will need to dig in what's happening :/

This test wasn't really impacted previously, so I don't fully understand what is happening.

[SethTisue]

I'm going to revert this for 2.13.11. I invite you to submit a revised PR for 2.13.12.

I can run the community build on the revised PR prior to merge, and/or I can help you to do it yourself locally if you want a faster feedback cycle on making coordinated changes in the two repos.