-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
base: main
Are you sure you want to change the base?
Conversation
8aa208f
to
cc0c444
Compare
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 |
There was a problem hiding this comment.
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?
rest.collectFirst: // We can't complete names without knowing the type of selector | ||
case Match(selector, _) => selector |
There was a problem hiding this comment.
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?
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this used?
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 |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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` = |
There was a problem hiding this comment.
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?
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.