Help implement Metals' infer expected type feature #21390
Conversation
dwijnand
force-pushed
the
infer-expected-type
branch
from
d75e0b2 to
58cec81
Compare
| @@ -198,6 +199,15 @@ case class ScalaPresentationCompiler( | |||
| .asJava | |||
| } | |||
|
|
|||
| def inferExpectedType(params: OffsetParams): CompletableFuture[ju.Optional[String]] = | |||
There was a problem hiding this comment.
There is already an existing method insertInferredType wouldn't that be the same thing?
There was a problem hiding this comment.
This is just for the purpose of testing. The method will be used only inside of pc by such things like completions. This covers more cases then insertInferredType but doesn't change it into textEdit. We could have insertInferredType cover those cases too but I don't think it's ever useful.
| import scala.meta.pc.OffsetParams | ||
| import scala.meta.pc.SymbolSearch | ||
|
|
||
| class InferExpectedType( |
There was a problem hiding this comment.
Should we instead reuse this or parts of it in InferredTypeProvider?
There was a problem hiding this comment.
It seems that InferredTypeProvider still processes path to figure out where to add edit and if/what parenthesis should be added. So I'm not sure there is any benefit in this.
This comment was marked as resolved.
This comment was marked as resolved.
dwijnand
force-pushed
the
infer-expected-type
branch
from
58cec81 to
53ba3c8
Compare
|
@dwijnand Thanks for adding |
TypeComparer.explaining is like TypeComparer.explained, but instead of just returning the trace, returns the result, still allowing the trace to be accessed via .lastTrace, as exemplified by implementing TypeComparer.explained in terms of TypeComparer.explaining. Also add, but leave commented out the call of, a trace.dumpStack, which is like Thread.dumpStack(), but outputing to System.out, like all our tracing does - so the two don't interact when unbuffering onto the terminal. Also, we can do customisations like filtering out stack elements, limiting the stack.
But keep the extraction of the instDecision logic, and keep the tests cases I used in studying this change. Also simplify and update InstantiateModel. Martin had tweaked my i14218 fix to make it more conservative: in the (NN, UU) case (i.e. no inferred bounds, only a UU upper bound declared) for covariant type parameters, revert back to minimising to Nothing rather than maximising to the declared bound.
dwijnand
force-pushed
the
infer-expected-type
branch
from
5615ef1 to
43fc10c
Compare
There was a problem hiding this comment.
I don't know the typer deeply enough to be fully confident this is mergeable. But the changes do seem clear and good to me, well-separated into independent commits, with thorough testing, so I'm hoping it will be merged — especially since this is important stuff. Everyday users of Metals will notice these improvements; this is high-value work (once carried through in the Metals repo).
Apart from helping to implement the new InferExpectedType feature in Metals, I fixed a bug in Applications, which is actually a re-implementation of my fix in #18269, except now it also applies to varargs arguments. Previously, in #18269, I checked the typed args, but varargs specifically removes (dropInPlace) the - possibly error-typed! - var arg args, wrapping them in an not error-typed SeqLiteral tree. Now we record those failures instantly so we don't lose them.