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

Generating external link should depend on the module it refers to #3368

Open
vmishenev opened this issue Nov 23, 2023 · 1 comment
Open

Generating external link should depend on the module it refers to #3368

vmishenev opened this issue Nov 23, 2023 · 1 comment
Labels
bug kdoc-spec An issue that requires clarification from the KDoc spec's perspective

Comments

@vmishenev
Copy link
Contributor

vmishenev commented Nov 23, 2023

Context

Dokka uses package-list text files to build external links (see External documentation links configuration).
This behaviour is similar to Javadoc's package-list / element-list files.

Since Java 10, Javadoc has started producing a modular package-list named element-list (see JDK-8191363).

In addition to what package-list does, element-list groups the packages by modules. You can compare Java 9 package-list vs Java 10 element-list.

Dokka's package-list also groups the packages by modules, even though the file name is still package-list (not element-list). See this example.

Currently, Dokka generates links to external documentation only based on the package these links point to, so it doesn't take modules into account.

Bug

In Dokka, a link is represented as DRI (Dokka Resource Identifier). DRI does not have any information about modules. So Dokka determines the module that a link refers to only by the package name:

This behaviour is incorrect, it causes bugs like #2272, where a link points to a symbol in a module that doesn't exist (it exists in a different module under the same package name).

Note: unlike Dokka, Javadoc has the information about which module a link refers to, so Javadoc doesn't have that problem

Proposal

We should keep the information about Kotlin/Java modules in DRI, and use it to find the correct package in package-list to generate external links.

Note: to cover #2913, Dokka might need to be able to differentiate Java and Kotlin sources.

Open questions

What name should be used for the module name?

Currently, Dokka uses the project name as the module name written to package-list:

However, moduleName is a user-defined property that doesn't have any guarantees, so it can be changed often and unexpectedly, which would lead to outdated package lists and broken links.

Kotlin has a concept of modules, but the module name doesn't seem to be defined.

How can Dokka get the name of a symbol's module to create a DRI?

Kotlin: Presumably published jar/klib have .kotlin_module files. We can get the name from it? Maybe KGP or the Kotlin compiler can generate something additionally for us as part of the KDoc spec?

Java: module-info.class

Is Kotlin's module name unique between libraries? Stable?

There is a situation when two libraries can have the same module name, e.g. core. Also, it is possible to have it in a project due to hierarchy, e.g. (a:foo and b:foo).

Can we get the module name via Kotlin Analysis API?

Needs to be researched.

Additional notes

This issue focuses on generating valid links that point to external documentation. For example, when a 3rd party library you are using and linking to also has non-unique package names in two or more modules.

This issue also existed for local user projects that don't use any external documentation, but it was fixed in #2272.

@vmishenev vmishenev added the bug label Nov 23, 2023
@IgnatBeresnev IgnatBeresnev added the kdoc-spec An issue that requires clarification from the KDoc spec's perspective label Jan 10, 2024
vmishenev added a commit that referenced this issue Jan 11, 2024
Fixes #2272. There is a situation where 2 (or more) gradle modules have the same package and dokka doesn’t know where to link.
E.g.
The class com.example.classA is in moduleA
and com.example.classB is in moduleB
moduleC depends on moduleA and moduleB.
In moduleC: there is a link [com.example.classB]. Having only package-list, dokka doesn’t know where to link, since the package com.example is in both modules.

This is short-term and temporary solution of #3368 to cover a case of links between only local modules in a project.
It is based on #2901 from @EddieRingle. It generates an external link depending on a check if the local link exists. The check works only when the number of possible resolved links are more than one.



* all-modules-page: Validate cross-module links during resolution

The existing behavior was just taking the first module that includes the
package name of a given reference, so this resulted in broken links when
a package is split across multiple modules.

This change checks to ensure the resolved file exists before choosing it
as the target for the resolved link.

Signed-off-by: Eddie Ringle <[email protected]>

* Check links only if resolved links has more than one link

* Add integration test

* Refactor

* Add comment

---------

Signed-off-by: Eddie Ringle <[email protected]>
Co-authored-by: Eddie Ringle <[email protected]>
@IgnatBeresnev
Copy link
Member

Related: #2913

IgnatBeresnev pushed a commit that referenced this issue Jan 11, 2024
Fixes #2272. There is a situation where 2 (or more) gradle modules have the same package and dokka doesn’t know where to link.
E.g.
The class com.example.classA is in moduleA
and com.example.classB is in moduleB
moduleC depends on moduleA and moduleB.
In moduleC: there is a link [com.example.classB]. Having only package-list, dokka doesn’t know where to link, since the package com.example is in both modules.

This is short-term and temporary solution of #3368 to cover a case of links between only local modules in a project.
It is based on #2901 from @EddieRingle. It generates an external link depending on a check if the local link exists. The check works only when the number of possible resolved links are more than one.

* all-modules-page: Validate cross-module links during resolution

The existing behavior was just taking the first module that includes the
package name of a given reference, so this resulted in broken links when
a package is split across multiple modules.

This change checks to ensure the resolved file exists before choosing it
as the target for the resolved link.

Signed-off-by: Eddie Ringle <[email protected]>

* Check links only if resolved links has more than one link

* Add integration test

* Refactor

* Add comment

---------

Signed-off-by: Eddie Ringle <[email protected]>
Co-authored-by: Eddie Ringle <[email protected]>
(cherry picked from commit 6904571)
hoc081098 added a commit to hoc081098/solivagant that referenced this issue Feb 7, 2024
hoc081098 added a commit to hoc081098/solivagant that referenced this issue Feb 7, 2024
* lifecycle: supports targets
  tvosX64()
  tvosSimulatorArm64()
  tvosArm64()
  watchosArm32()
  watchosArm64()
  watchosX64()
  watchosSimulatorArm64()

* lifecycle: supports targets
  tvosX64()
  tvosSimulatorArm64()
  tvosArm64()
  watchosArm32()
  watchosArm64()
  watchosX64()
  watchosSimulatorArm64()

* lifecycle: supports targets
  tvosX64()
  tvosSimulatorArm64()
  tvosArm64()
  watchosArm32()
  watchosArm64()
  watchosX64()
  watchosSimulatorArm64()

* add rememberCloseableOnRoute

* add rememberCloseableOnRoute

* add rememberCloseableOnRoute

* replace DestinationId with StackEntryId

* fix dokka

* org.gradle.jvmargs=-Xmx8g

* POM_INCEPTION_YEAR=2024

* fully qualified import for WeakReference
Kotlin/dokka#3403

* fix dokka

* dokka: suppress all internal packages

* dokka: suppress all internal packages

* LifecycleControllerEffect to non-internal pkg

* externalDocumentationLink {
        url = URL("https://hoc081098.github.io/kmp-viewmodel/docs/0.x/API/")
        packageListUrl = URL("https://hoc081098.github.io/kmp-viewmodel/docs/0.x/API/package-list")
      }

* fix dokka

* LifecycleDestroyedException

* LifecycleDestroyedException

* license

* license

* docs

* org.gradle.jvmargs=-Xmx4g

* update CI

* dokka

* InternalNavigationApi

* InternalNavigationApi

* InternalNavigationApi

* dump

* dump

* detekt

* detekt

* update ci [skip ci]

* TODO: Issue Kotlin/dokka#3368 and Kotlin/dokka#2037

* changelog [skip ci]
vmishenev added a commit that referenced this issue Mar 20, 2024
Fixes #2272. There is a situation where 2 (or more) gradle modules have the same package and dokka doesn’t know where to link.
E.g.
The class com.example.classA is in moduleA
and com.example.classB is in moduleB
moduleC depends on moduleA and moduleB.
In moduleC: there is a link [com.example.classB]. Having only package-list, dokka doesn’t know where to link, since the package com.example is in both modules.

This is short-term and temporary solution of #3368 to cover a case of links between only local modules in a project.
It is based on #2901 from @EddieRingle. It generates an external link depending on a check if the local link exists. The check works only when the number of possible resolved links are more than one.



* all-modules-page: Validate cross-module links during resolution

The existing behavior was just taking the first module that includes the
package name of a given reference, so this resulted in broken links when
a package is split across multiple modules.

This change checks to ensure the resolved file exists before choosing it
as the target for the resolved link.

Signed-off-by: Eddie Ringle <[email protected]>

* Check links only if resolved links has more than one link

* Add integration test

* Refactor

* Add comment

---------

Signed-off-by: Eddie Ringle <[email protected]>
Co-authored-by: Eddie Ringle <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug kdoc-spec An issue that requires clarification from the KDoc spec's perspective
Projects
None yet
Development

No branches or pull requests

2 participants