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

Presentation compiler: Add completions for named patterns #22251

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

rochala
Copy link
Contributor

@rochala rochala commented Dec 19, 2024

This PR adds completion capabilities for named patterns.

Also adds comment that we should add backtick term completion in Bind position but only when we are inside backticks.

@rochala rochala requested a review from kasiaMarek December 19, 2024 22:40
@rochala rochala requested a review from natsukagami December 20, 2024 09:13
@rochala rochala force-pushed the named-pattern-completions branch from 8aa208f to cc0c444 Compare December 20, 2024 14:06
Comment on lines +168 to +171
def unapply(path: List[untpd.Tree]): Boolean =
path match
case (_: untpd.Ident) :: (fn0: untpd.Apply) :: untpd.CaseDef(fn1, _, _) :: _
if fn1 == fn0 && fn0.args.exists(_.isInstanceOf[untpd.NamedArg]) => true
Copy link
Contributor

Choose a reason for hiding this comment

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

What if it's nested:

case Some(User(nam@@)) =>

or is this covered by some other case?

Comment on lines +41 to +42
rest.collectFirst: // We can't complete names without knowing the type of selector
case Match(selector, _) => selector
Copy link
Contributor

Choose a reason for hiding this comment

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

Won't this fail for nested named tuples?

Comment on lines +68 to +72
private object NamedTupleUnapplyResultType:
def unapply(tree: Type)(using Context): Option[Type] = tree match
case AppliedType(TypeRef(_, cls), (namedTuple @ defn.NamedTuple(_, _)) :: Nil)
if (cls == ctx.definitions.OptionClass || cls == ctx.definitions.SomeClass) => Some(namedTuple)
case _ => None
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this used?

Comment on lines +83 to +87
val maybeApplied = tree match
// The check for case flag is necessary to filter introduced type bounds T$1..n
case tpeApply @ TypeApply(_, args) if !args.exists(_.symbol.flags.is(Flags.Case)) =>
apply.info.appliedTo(args.map(_.tpe))
case _ => apply.info
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you simplify and always do:

apply.info.appliedTo(args.map(_.tpe))

// introduced in this commit
)

// TODO Leaving this test as I want to make it work in the future, same for named args
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you create an issue and relate this to the issue?

)

// TODO completion for binds but only in backtics
// @Test def `named-tuples-bind-with-named-patterns-3` =
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you create an issue and relate this to the issue?

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.

2 participants