Scaladoc: new heuristic for extension method parameter extraction #14810
Conversation
rochala
changed the title
| val method = sym.tree.asInstanceOf[DefDef] | ||
| (for { | ||
| defPosition <- method.symbol.pos | ||
| defStart <- scala.util.Try(defPosition.start).toOption |
There was a problem hiding this comment.
Does start here throw exceptions?
If so, I'm not sure if it's not a bug
There was a problem hiding this comment.
Yes it does. It fails assertions for some methods.
There was a problem hiding this comment.
@nicolasstucki is this intentional?
There was a problem hiding this comment.
What is the exception?
Does the position exist? If not, calling start should fail an assertion. It would be unexpected for the position to not
exist and I would say that is a bug.
There was a problem hiding this comment.
It's the assertion failure just as you said.
There was a problem hiding this comment.
Assertion fails for methods, which have position of type NoSourcePosition. These are mostly methods from inheritance such as productElementNames from Product trait. Is this a bug, as symbol.pos returns Option and logically it should return None for NoSourcePosition ?
| val method = sym.tree.asInstanceOf[DefDef] | ||
| (for { | ||
| defPosition <- method.symbol.pos | ||
| defStart <- scala.util.Try(defPosition.start).toOption |
There was a problem hiding this comment.
What is the exception?
Does the position exist? If not, calling start should fail an assertion. It would be unexpected for the position to not
exist and I would say that is a bug.
pikinier20
force-pushed
the
extension-method-doc-new-heuristic
branch
from
5d5f5d4 to
dbf84f5
Compare
|
@nicolasstucki Could you take a look at the last commit? |
Now that we know that that Scala 2 symbol did not have a position we should check why that was the case. Maybe there is a bug somewhere else and this is only hiding it. |
Co-authored-by: Nicolas Stucki <[email protected]>
pikinier20
force-pushed
the
extension-method-doc-new-heuristic
branch
from
87eea93 to
6cdc381
Compare
| def ffffff[B](b: B): (A, B) = ??? | ||
| extension (using String)(using Unit)(a: Animal)(using Int)(using Number) | ||
| def f11(b: Any)(c: Any): Any | ||
| = ??? |
There was a problem hiding this comment.
How do these tests work? I see test “fixtures” but no test specification.
There was a problem hiding this comment.
I had answered in person, but I'll put it here for posterity:
This works by comparing the text of the file with the output of scala-meta, similarly to what is done in the compiler with the tests/pos, tests/neg folders.
This is therefore both the declaration and the specification for the tests !
|
@pikinier20, I’ve re-requested a review from you because the content of the PR changed since your previous review. |
Duplicate of scala#14810, once merged, frop this
|
Hello, |
Duplicate of scala#14810, once merged, frop this
|
Hello, |
Sporarum
requested review from
Florian3k and
szymon-rd
and removed request for
pikinier20
There was a problem hiding this comment.
It looks good to me, but @Florian3k has the most expertise there right now.
Before this changes, it wouldn't generate due to incorrect extension parameters extractor functions.
It also implements new heuristic to distinct extensions and method parameters based on definition position.
Also fixed type parameter coloring by adding new css rule, to match doc style.
Based on #14321 , it should be closed after this PR is merged.
Documentation for test
tests.extensionParams