Skip to content

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
Member

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
Member

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
Member

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
Member

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
Member

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
Member

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?

case GenericImportOrExport() => Mode.ImportOrExport | Mode.Scope // import TrieMa@@
case BindMixedWithNamedPatterns() => Mode.None // case User(name = name, sur@@)
case untpd.Literal(Constants.Constant(_: String)) :: _ => Mode.Term | Mode.Scope // literal completions
// TODO case (_: tpd.Bind) :: _ => we should complete only when in backticks
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 intentionally left as TODO?

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