Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Interactive] Include scope completions for synthic select tree #13515

Merged
merged 1 commit into from
Sep 14, 2021

Conversation

dos65
Copy link
Contributor

@dos65 dos65 commented Sep 13, 2021

Previously, in case if query matches on existing class member
the returned completions were not returning symbols from scope.

Fixes the following case:

class Y {
  def bar: Unit =
   val argument: Int = 42
   arg@@   // should return both `local.argument` and `this.arg`
           // but tree looks like select -  `Select(This(Ident(Y)), Ident("arg"))`

  def arg: String = ???
}

@dos65 dos65 force-pushed the completion_synthetic_select_fix branch from 802facd to 8a560b9 Compare September 13, 2021 11:44
@@ -112,6 +112,9 @@ object Completion {
val completer = new Completer(mode, prefix, pos)

val completions = path match {
case Select(qual @ This(_), _) :: _ =>
val scope = if (qual.span.isSynthetic) completer.scopeCompletions else Map.empty
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest moving this logic into the body of selectionCompletions, leaving this match block here as it was - just as simple dispatching

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I simplified it a bit. Would it be ok to leave it here in this version?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

@dos65 dos65 force-pushed the completion_synthetic_select_fix branch from 8a560b9 to 5707140 Compare September 13, 2021 14:06
@dos65 dos65 requested a review from prolativ September 13, 2021 14:07
case Import(expr, _) :: _ => completer.directMemberCompletions(expr)
case (_: untpd.ImportSelector) :: Import(expr, _) :: _ => completer.directMemberCompletions(expr)
case _ => completer.scopeCompletions
case Select(qual @ This(_), _) :: _ if qual.span.isSynthetic => completer.scopeCompletions
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since it's non-obvious why this would be needed, I suggest adding a comment with an example here (or a reference to the added test case).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added

@dos65 dos65 force-pushed the completion_synthetic_select_fix branch from 5707140 to 08aae18 Compare September 13, 2021 16:37
Previously, in case if query matches on existing class member
the returned completions were not returning symbols from scope.

Fixes the following case:
```scala
class Y {
  def bar: Unit =
   val argument: Int = 42
   arg@@   // should return both `local.argument` and `this.arg`
           // but tree looks like select -  `Select(This(Ident(Y)), Ident("arg"))`

  def arg: String = ???
}
```
@dos65 dos65 force-pushed the completion_synthetic_select_fix branch from 08aae18 to 494094a Compare September 13, 2021 16:38
@dos65 dos65 requested a review from smarter September 14, 2021 09:21
@prolativ prolativ merged commit 01be968 into scala:master Sep 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants