Scaladoc tool: render @deprecated correctly even when named arguments weren't used
#21925
Conversation
|
I'm busy at the Lightbend company meeting the whole rest of the week, then I have some vacation days, so the earliest I could return to it is around Nov 21. @HarrisL2 feel free to run with it if you like. if you end up not having time either, I'll come back to it eventually |
SethTisue
changed the title
@deprecated correctly even when named arguments weren't used
SethTisue
added
the
backport:nominated
|
I'm not able to write a test case for this change without making really specific checks. Ideally it would be a general test framework like SignatureTest.scala, where we can check the generated attributes of each member with an expected result. I don't think this framework exists at the moment. There are test cases about annotations, but those are for annotations that affect the generated signature, whereas the I have added test cases to scaladoc-testcases/src/tests/deprecated.scala, but it should be noted that it currently does not check the behavior of annotations. It only checks signatures through Deprecated in TranslatableSignaturesTestCases.scala. |
SethTisue
force-pushed
the
20118
branch
2 times, most recently
from
cdea6b8 to
61dc746
Compare
|
I hope this PR will be considered mergeable even though our ambition level stopped short of proper automated testing for it. A JSoup-based test would be annoying fragile, I think. I've rebased it and marked it ready for review. @KacperFKorban I'm not sure if you're currently available for reviewing this sort of PR; let me know if you think we should try to find a different reviewer. |
| val message: Option[Annotation.AnnotationParameter] = | ||
| a.params.filter(p => p.name.isEmpty || p.name.get != "since").lift(0) | ||
| val since: Option[Annotation.AnnotationParameter] = | ||
| a.params.filter(_.name.nonEmpty).find(_.name.get == "since").orElse(a.params.lift(1)) |
There was a problem hiding this comment.
I find this logic somewhat hard to read and dependent on the definition of deprecated (which doesn't have to be bad).
Could you instead do a two step processing? i.e.
- Collect all named arguments into a map
- For all non-named arguments, map them to the remaining names positionally
It's not a hard preference though, so just let me know if you prefer to rephrase it or stay with the current approach.
There was a problem hiding this comment.
I've adjusted the syntax, hopefully it makes the logic clearer.
Fixes #20118
Still need a test case for this issue using jsoup