-
Notifications
You must be signed in to change notification settings - Fork 408
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
Support parsing javadoc @param
for classes
#3391
Conversation
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.
👍 👍
} | ||
|
||
private fun customDocTag(text: String): CustomDocTag { | ||
return CustomDocTag(listOf(P(listOf(Text(text)))), name = "MARKDOWN_FILE") |
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.
Did you have a chance to see if MARKDOWN_FILE
here is at all needed? In other words, does the test fail if it's not specified / some other name is used? Collecting use cases for #3366 😅
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.
So, MARKDOWN_FILE
is needed here just because it's already created in other code — here it's needed only to test that the content is the same.
But overall, I've tracked why this was introduced and will write some info into #3366
// can be a PsiClass if @param is referencing class generics, like here: | ||
// https://github.com/biojava/biojava/blob/2417c230be36e4ba73c62bb3631b60f876265623/biojava-core/src/main/java/org/biojava/nbio/core/alignment/SimpleProfilePair.java#L43 | ||
// not supported at the moment |
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.
Shoot, I think it's one of my comments that I didn't turn into a TODO/issue, so I forgot to fix it before the merge 😅
import kotlin.test.Test | ||
import kotlin.test.assertEquals | ||
|
||
class JavadocAnalysisTest { |
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.
Could you please add a single test for @param Bar
on a class/function (not <Bar>
, but Bar
)? To verify that it either resolves Bar
correctly, or it doesn't resolve it at all.
It's highlighted in red by IDEA, but Dokka 1.8.20 resolves it correctly in the resulting documentation. Whether it's right by specification and whether Dokka should support it is a different question, I'm not proposing to change the resolution logic now, I want the test to only "freeze" this behaviour so that we are notified in case it changes. And once/if it does change - we'll figure out what to do with it
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.
Ok, I will add.
But, as far as I remember, during implementation, I found that both java and kotlin analysis for param
tag doesn't validate if such parameter exists. So f.e., if you will have function like this (kotlin):
/**
* @param non_existent documentation
**/
fun something(param: String) {}
In the end, documentation for non_existent
param will be present in generated HTML file. Same for Java.
Also, there will be no links for params - as they have nowhere to go - as parameters are already shown on the same page under this documentation
You can take a look here on what I mean: https://kotlinlang.org/api/kotlinx.coroutines/kotlinx-coroutines-core/kotlinx.coroutines/launch.html (parameters are underlined but have no links)
c406b03
to
d80a633
Compare
Fixes #3199