Skip to content
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

Add basic support of @author, @since, @return tags for methods and classes for javadoc (#1770) #2967

Merged

Conversation

irina-turova
Copy link
Contributor

This draft PR adds @author, @SInCE, and @return tags support for Javadoc output.

Here is an example:

/**
 * This class is for test purposes
 * @author Class Author
 * @since 18 april 2023
 * @see String
 */
class TestClass {

    /**
     * Simple test function
     * @param s simple String parameter
     * @param n simple Int parameter
     * @author Initial Author
     * @author First Contributor
     * @author Second Contributor
     * @since 20.04.2023
     * @throws NullPointerException
     * @see s
     * @return An empty list for now
     */
    fun testFunction(s: String, n: Int): List<String> {
        return emptyList()
    }

image

The PR may need some improvements, and there are some questions I'd like to clarify before publishing the PR for review.

  1. The original issue (dokkaJavadoc does not recognize @return @since @author #1770) was about the tags for methods. I've also added @author and @SInCE tags for classes. Do we need that? Do we need that for properties also and other node types? KDoc documentation doesn't specify which code entities can have which tags (as far as I know).
  2. Do you think the tag order is appropriate?
  3. The tag values are wrapped in HTML paragraph tags <p>, and since the margin value isn't inherited, it causes larger gaps between list elements than in the parameters section. I will fix that in the following commits, but it's unclear what's the proper way to deal with that.
  4. I've also found a way to provide authors information like that in tests: @author(name = "Franklin D. Roosevelt"). The current straightforward realization will render it as (name = "Franklin D. Roosevelt"). Do we need to process this syntax also?
  5. The main question to discuss is the realization. I approached that, having in mind all the other tag types. So I planned the interface (plugins/javadoc/src/main/kotlin/org/jetbrains/dokka/javadoc/pages/JavadocPageNodes.kt)
interface WithCommonTags {
    val authors: List<List<ContentNode>>
    val since: List<ContentNode>
}

to contain also @sample and @see tags, for example, and to implement that in Classes, Functions, and Properties nodes.
And the interface

interface WithExecutableTags {
    val `return`: List<ContentNode>
}

was planned to include @receiver and @throws tags.
Do you think this structure is appropriate, or should it be reworked?
6. Regarding the @receiver tag for extension functions. I've noticed that in Javadoc style it's rendered as final List<String> testExtension(String $self, Integer n) and is missing in Dokka style. I think it's a discussion for another issue, but I feel encouraged to help if needed.

Pay attention that this causes API changes.

Thanks for your feedback in advance.

@IgnatBeresnev IgnatBeresnev self-requested a review April 18, 2023 12:07
@IgnatBeresnev
Copy link
Member

IgnatBeresnev commented Apr 18, 2023

Well done! Very impressive for the first contribution to Dokka.

The questions seem very reasonable, I'll write out detailed answers as well as do a very high-level (API) review by the end of the week for sure, but hopefully much sooner than that

@IgnatBeresnev IgnatBeresnev linked an issue Apr 18, 2023 that may be closed by this pull request
…om margins of nested elements collapse, it makes the line gaps equal.
@irina-turova
Copy link
Contributor Author

I've managed to fix unequal gaps between lines. All the <p> tags now have the same top and bottom margins as the parent <dt> and <dd> elements. Other tags inside <dt> and <dd> will have the default browser styles. Given that the paragraph is the default entity in Markdown, I assume it's the point where we can stop customizing styles.
image

@IgnatBeresnev
Copy link
Member

IgnatBeresnev commented Apr 25, 2023

I've finally gotten to writing out my thoughts

The original issue was about the tags for methods. I've also added @author and @since tags for classes. Do we need that?

Do you think the tag order is appropriate?

The answer to both questions: Dokka's javadoc format is basically a copycat of Java's Javadoc format. So for questions like that, we can just generate the true native javadocs, and see what it outputs.

Let's look at what it generates for the example you gave:

/**
 * @since 2021.03.10
 * @author test author
 */
public class TestClass { }

2023-04-25_19-27-58

/**
 * Simple test function
 * @param s simple String parameter
 * @param n simple Int parameter
 * @author Initial Author
 * @author First Contributor
 * @author Second Contributor
 * @since 20.04.2023
 * @throws NullPointerException description
 * @see List for information
 * @return an empty list for now
 */
public String testFunction(String s, int n) {
    return "Test";
}

2023-04-25_19-27-09

As you can see, it renders @since and @author for classes just fine. However, the @author tag is not rendered for the function. So it looks like our support for the @author tag in functions also needs to be removed.

And the order should be like in the true Javadocs: parameters, return, since, see also, author (author is for classes only)

I've also found a way to provide authors information like that in tests: https://github.com/author(name = "Franklin D. Roosevelt").

What you found are probably these tests. If that's the case, the @Author here is an annotation, not a javadoc/kdoc tag, so it's a completely different thing. No need to do anything about it :)

The main question to discuss is the realization. I approached that, having in mind all the other tag types. .. Do you think this structure is appropriate, or should it be reworked?

I'll leave a review comment for this piece of code so that there's a thread in which we can discuss it.

Regarding the @receiver tag for extension functions. I've noticed that in Javadoc style it's rendered as final List testExtension(String $self, Integer n) and is missing in Dokka style.

Not sure what exactly you mean. Could you clarify, what exactly is missing and where?

Copy link
Member

@IgnatBeresnev IgnatBeresnev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well done! Looks like I only have two comments, and I'd be happy to merge the PR after they are resolved :)

upd: oh, forgot about the order of tags - I mentioned it in the previous comment. I don't think it matters much really, but having the same order as in Java's javadocs would be nice

@irina-turova
Copy link
Contributor Author

Thank you for your feedback. I agree with all the points.

The only thing I'd like to clarify is the purpose of the Javadoc format. I was sure it should be exactly the same as the Dokka default HTML but in a familiar Javadoc HTML layout. For example, in Dokka HTML, the @author tags are rendered for functions (and the author of the issue wanted @author for functions in their example - I think that's where my confusion started from). Kotlin documentation doesn't specify the types of entities that can have the @author tag:

@author
Specifies the author of the element being documented.

So we render KDoc according to KDoc rules in the Javadoc HTML layout, don't we? I think that's what the Dokka users want.
That's only my point of view. If there is no room for discussion, I'll accept that)

As for the @receiver tag, I meant not the @receiver tag itself but documentation for extension functions. Looking for a reference for rendering this tag content, I've noticed that Dokka default HTML misses extension functions.
And the Javadoc format for extension functions also doesn't look familiar to Kotlin users:

class TestClass {

    fun String.testExtension(n: Int): List<String> {
        return emptyList()
    }
}

image

@irina-turova
Copy link
Contributor Author

@IgnatBeresnev I don't mean to bother you, but have you been able to look through my thoughts in the comment above?
I am ready to go ahead after your decision

@IgnatBeresnev
Copy link
Member

IgnatBeresnev commented May 18, 2023

Hi! Yeah, sorry, we've been busy with preparations for Kotlin 2.0 these the last couple of weeks, but you now have my full attention :)

The only thing I'd like to clarify is the purpose of the Javadoc format. I was sure it should be exactly the same as the Dokka default HTML but in a familiar Javadoc HTML layout.

If you have a mixed codebase with both Kotlin and Java, Java's javadoc tool does not build documentation for Kotlin classes by default. But you might want to document your Kotlin classes. If your code is written primarily in Java, it makes sense to go with the format that is familiar, and you will be calling Kotlin code from your Java code, so Kotlin code needs to be translated - this is where Dokka's Javadoc format comes into play. So Dokka's Javadoc format is basically a clone of Java's Javadoc format, but with Kotlin support, and it translates Kotlin signatures to be like Java (for interop); no more, no less

If you have a Kotlin-only codebase, or if Kotlin is the primary/dominant language, there's really no reason to be using the Javadoc format, it's not the use case we want to support as we'd end up with a fork of the Javadoc format that we'd need to modify and support - for instance, because there are no top-level declarations in Java, there's no place for them on the Javadoc pages, so we'd need to add something custom. Little reason to do that if we already have our own primary HTML format

So we render KDoc according to KDoc rules in the Javadoc HTML layout, don't we?

That could be the case if the Javadoc format had Kotlin signatures and features, but as you may have noticed, all Kotlin code is transformed to look like Java code. So fun foo() becomes public void foo(). Some features that Kotlin has, but Java doesn't (like extension functions), are also translated in a way that it can be called from Java, or are not even displayed (like inline functions or reified generics). KDoc has the @sample tag which doesn't exist in the original Javadoc format, so it is also not displayed (otherwise, what should it look like?).

Following this logic, I'd say the Kotlin code needs to be rendered using the same rules as the Java code, so KDoc should be adapter to be like Javadoc

Looking for a reference for rendering this tag content, I've noticed that Dokka default HTML misses extension functions. And the Javadoc format for extension functions also doesn't look familiar to Kotlin users:

I'm still not sure what you mean, sorry :(

Kotlin has extension functions, these are displayed properly (just as the Kotlin code) in Dokka's own HTML format, example here.

Java doesn't have extension functions as a concept.

But Kotlin and Java have interoperability, so you should be able to call Kotlin functions from Java code. Kotlin's extension functions are compiled as static Java functions, where the receiver is converted to be the first argument of the static function. If you try calling a Kotlin extension from Java, you should see that.

Dokka's Javadoc format translates Kotlin signatures to look like Java signatures. And because Java doesn't have extension functions, it translates them into static functions. Basically what you're seeing on your screenshot:

image

So it looks like the signature is correct. The only problem I see is the documentation for @receiver could be lost (need to check), when it could be converted to be the @param documentation of the first argument ($self).

And the Javadoc format for extension functions also doesn't look familiar to Kotlin users

Right, and it shouldn't - in the Javadoc format it should look familiar to Java users :)

Hopefully I was able to clear some things up

@irina-turova
Copy link
Contributor Author

Hi! Wow! You wrote kind of a book chapter for me.
Thank you for the extensive explanation. It covers all my questions. If I continue improving Dokka's Javadoc format, I'll definitely refer to it in case of confusion.

It seems I've fixed the things we had discussed. I also checked the Javadoc behavior for the @since tag and add support for its multiple occurrences.

It seems, that @throw and @see tags are a bit harder to support, but I can approach them later if needed.

Current result:
image

image

Please, notify me if I missed something

@irina-turova irina-turova marked this pull request as ready for review May 26, 2023 11:20
@irina-turova
Copy link
Contributor Author

Oh, I forgot to update api( Will be done in a minute

@IgnatBeresnev
Copy link
Member

Thank you, especially for the comparison screenshots :) I don't think I have anything else to add, well done! 👍

Could you please run ./gradlew apiDump though, to update the API file? This will fix the failing API check

After you do that, I'll start longer integration tests just in case, and will merge the PR after they pass

@IgnatBeresnev
Copy link
Member

The tests are looking good!

There's only a slight problem with the empty "Since" tag, I left a comment. As soon as it's fixed, I'm ready to merge it :)

@irina-turova
Copy link
Contributor Author

Oh, right, I'd missed that. Fixed it
By the way, do I need to provide some integration tests to check the final html?

@IgnatBeresnev
Copy link
Member

IgnatBeresnev commented May 31, 2023

Thanks for fixing it and thank you for the contribution! Looks like it's ready to be merged, and it will be released in Dokka 1.9.0 :) and sorry for the delay with review

By the way, do I need to provide some integration tests to check the final html?

No need now, the unit tests are good enough. Currently, the integration tests do not have such specific asserts - they just generally check that the documentation was built successfully, that all types were resolved and some similar common things, and they also upload the resulting output to the web somewhere so that we can open it in a browser and have a look (that's how I noticed the bug with the empty tag)

@irina-turova
Copy link
Contributor Author

Thank you for guiding me on my way to the first contribution to Dokka!
You are very supportive, and that is very encouraging to continue! ☺
Waiting for the release!

@IgnatBeresnev IgnatBeresnev merged commit f457506 into Kotlin:master May 31, 2023
6 checks passed
Abrdjalw added a commit to Abrdjalw/dokka that referenced this pull request May 31, 2023
eunwoop pushed a commit to eunwoop/dokka that referenced this pull request Jul 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dokkaJavadoc does not recognize @return @since @author
2 participants