-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix issue with pc breaking in requiredMethod on newly overloaded valueOf #23708
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
Conversation
// NOTE(olafur) IntelliJ does not complete the root package and without this filter | ||
// then `_root_` would appear as a completion result in the code `foobar(_<COMPLETE>)` | ||
defn.RootPackage, | ||
// NOTE(gabro) valueOf was added as a Predef member in 2.13. We filter it out since is a niche | ||
// use case and it would appear upon typing 'val' | ||
defn.ValueOfClass.info.member(nme.valueOf).symbol, |
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.
ValueOf.valueOf does not exist and going by the comment, it seems that the ValueOf class was meant to be filtered here instead, so I changed this.
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.
LGTM! Thanks!
bae0924
to
a4d07aa
Compare
…eOf (#23708) We recently started having issues when running completions in metals, with the presentation compiler crashing: ```scala dotty.tools.dotc.core.TypeError$.apply(TypeErrors.scala:54) dotty.tools.dotc.core.Denotations$MultiDenotation.suchThat(Denotations.scala:1280) dotty.tools.dotc.core.Denotations$Denotation.requiredSymbol(Denotations.scala:305) dotty.tools.dotc.core.Denotations$Denotation.requiredMethod(Denotations.scala:321) dotty.tools.pc.completions.Completions.isUninterestingSymbol$lzyINIT1(Completions.scala:744) dotty.tools.pc.completions.Completions.isUninterestingSymbol(Completions.scala:725) dotty.tools.pc.completions.Completions.includeSymbol(Completions.scala:90) dotty.tools.pc.completions.Completions.visit$3(Completions.scala:697) dotty.tools.pc.completions.Completions.filterInteresting$$anonfun$1(Completions.scala:715) scala.collection.immutable.List.foreach(List.scala:334) dotty.tools.pc.completions.Completions.filterInteresting(Completions.scala:715) dotty.tools.pc.completions.Completions.enrichedCompilerCompletions(Completions.scala:118) dotty.tools.pc.completions.Completions.completions(Completions.scala:136) dotty.tools.pc.completions.CompletionProvider.completions(CompletionProvider.scala:139) dotty.tools.pc.ScalaPresentationCompiler.complete$$anonfun$1(ScalaPresentationCompiler.scala:194) dotty.tools.dotc.core.TypeError$$anon$1: Failure to disambiguate overloaded reference with method valueOf in object Predef: [T]: T and method valueOf in object Predef: [T](implicit vt: ValueOf[T]): T ``` This happened after the recent changes to the stdlib, with the, I believe, newly added valueOf overload, and the previously used `requiredMethod` by design not handling these cases. I also noticed that in the same piece of code we had a bit of an empty Symbol factory situation going on, with MultiDenotation being changed into a Symbol (always resulting in an empty symbol), and only later flattened with the `alternatives`, so I changed that too. I can't really test this properly, as the pc tests seem to use an older stdlib, but at least the `wait` methods do resolve properly after these changes, so I have no reason to think the valueOf methods would be any different. [Cherry-picked 1b5e93b]
We recently started having issues when running completions in metals, with the presentation compiler crashing:
This happened after the recent changes to the stdlib, with the, I believe, newly added valueOf overload, and the previously used
requiredMethod
by design not handling these cases.I also noticed that in the same piece of code we had a bit of an empty Symbol factory situation going on, with MultiDenotation being changed into a Symbol (always resulting in an empty symbol), and only later flattened with the
alternatives
, so I changed that too.I can't really test this properly, as the pc tests seem to use an older stdlib, but at least the
wait
methods do resolve properly after these changes, so I have no reason to think the valueOf methods would be any different.