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

Fix double annotations in javadocs #9682

Closed
wants to merge 5 commits into from

Conversation

mfnalex
Copy link

@mfnalex mfnalex commented Sep 3, 2023

Hi, this pull requests adds the fix-javadoc-plugin to the "root" build file. It automatically adds a "finalizedBy" task for each task of the Javadoc type to get rid of the annoying "double annotations".

Before:
image

After:
image

@mfnalex mfnalex requested a review from a team as a code owner September 3, 2023 02:42
@mfnalex
Copy link
Author

mfnalex commented Sep 3, 2023

Here's a link to my fix-javadoc-plugin: https://github.com/mfnalex/gradle-fix-javadoc-plugin

I'm currently waiting to get it approved for the gradle plugin portal - until then, it'll only be available in my own repository.

@mfnalex mfnalex changed the title Add fix javadoc plugin Fix double annotations in javadocs Sep 3, 2023
@electronicboy
Copy link
Member

depending on a 3rd party repo is pretty much a no-go

as for the intent of this PR, I mean, post cleanup is nice, not something I personally care about enough to introduce a plugin and such for

@mfnalex
Copy link
Author

mfnalex commented Sep 3, 2023

As said, I'm waiting to get it approved in the gradlePluginPortal(). I could also PR the whole plugin into the paper repo instead? It's basically just one .kt class in buildSrc

@mfnalex
Copy link
Author

mfnalex commented Sep 3, 2023

Imho there's 5 solutions:

  1. We'll wait until it got approved for the gradlePluginPortal()
  2. I can upload it to repo.papermc.io/repository/maven-public/
  3. I add this plugin to the paper repo's buildSrc or similar
  4. I can upload it to maven central
  5. or we just forget about it and live with the double annotations

Please let me know if you're interested in adding this at all, because if not, I don't have to waste time into getting it approved for the plugin portal and/or uploading it to maven central

@MiniDigger
Copy link
Member

I like this on principle, but is using regex really the best approach here? Also, I guess ideally it would be fixed in the javadoc generator, anybody ever tried to chase that down?
Kinda related to stuff, I wanted to edit javadoc files anyways to add like a navigation to navigate projects and versions and add an alert if you aren't viewing latest, but I guess that would be suited better for the software who serves the javadocs

@mfnalex
Copy link
Author

mfnalex commented Sep 3, 2023

I know that regex isn‘t the best solution, I only wrote this for myself and it seemed to work just fine. I didn‘t dare to take a look at the actual javadoc generation code lol

@granny
Copy link
Contributor

granny commented Sep 3, 2023

JDK-8175533 and JDK-8278592 are the only references i can find with any info.

The problem occurs with any annotation that is both a declaration annotation and a type use annotation ... for example, an annotation on a field declaration that is both FIELD and TYPE_USE.

JDK-8278592#comment-14482841

At some level, the solution is to de-duplicate type annotations when used on the outermost TypeMirror of an Element.

That being said, the problem is exacerbated by the condition noted earlier, that declaration annotations are not displayed in the summary table, complicating the decision whether or not to display bimodal type annotations. The implementation code explicitly avoids displaying declaration annotations in the summary table, so it is probably an 8-year oversight that while declaration annotations may be suppressed in the summary table, type annotations are not.

JDK-8278592#comment-14482842

@mfnalex
Copy link
Author

mfnalex commented Sep 4, 2023

I don't think the bug is going to be fixed in the javadoc tool anytime soon.

I meanwhile did some improvements to the fix-javadoc-plugin. It doesn't just throw regex over the whole html file but instead parses only the relevant stuff using Jsoup.

I have let it generate the full paper API docs and uploaded them here (EDIT: See my reply below), in case anyone is bored feel free to check them out and let me know if you can find any issues (like e.g. still duplicated annotations, or maybe even missing correct annotations or anything).

As mentioned, I'd be happy to either pull request the whole plugin directly into the paper repo, or alternatively upload the plugin into the papermc repository if anyone would like that - the current docs are a pain to look at.

@mfnalex
Copy link
Author

mfnalex commented Sep 4, 2023

I noticed that by default, the annotations in method parameters appear on separate lines (at least that's what happens when using annotations-java5), so I made that configurable in the task (with newlines being the default).

In case anyone wants to take a look at the fixed paper docs:

@mfnalex
Copy link
Author

mfnalex commented Sep 5, 2023

Version 1.19 of the plugin is available in gradlePluginPortal(), so no separate repository is required anymore.

@mfnalex
Copy link
Author

mfnalex commented Sep 15, 2023

Any news on this? If there's still something that needs changes before being able to merge, I'd be happy to hear them.

@electronicboy
Copy link
Member

We've generally decided not to go forward with this change, there is generally little interest in pulling in 3rd party build tooling in order to fix what really amounts to an aesthetical grievance which generally has 0 side effects

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Closed
Development

Successfully merging this pull request may close these issues.

4 participants