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

lots of scaladoc warnings in build #353

Open
Tracked by #1052
pjfanning opened this issue May 27, 2023 · 13 comments
Open
Tracked by #1052

lots of scaladoc warnings in build #353

pjfanning opened this issue May 27, 2023 · 13 comments
Labels
good first issue Good for newcomers help wanted Extra attention is needed

Comments

@pjfanning
Copy link
Contributor

Check out git repo and run sbt unidoc and you will lots of warnings like Could not find any member to link for (and others).

It would be great if anyone who want to help can look at these and submit some PRs.

@pjfanning pjfanning added good first issue Good for newcomers help wanted Extra attention is needed labels May 27, 2023
@ydash
Copy link
Contributor

ydash commented Jun 20, 2023

@pjfanning
May I work on it?

@pjfanning
Copy link
Contributor Author

@ydash - this is still an issue. If you have time to contribute some PRs that would be great.

We are looking to do a release at the moment so we would like to minimise disruption. Could you keep any PRs small? Not one change in each PR but a relatively limited number of changes in each PR.

@ydash
Copy link
Contributor

ydash commented Jun 22, 2023

It seems to appear two types of warn log, Could not find any member to link and The link target is ambiguous.

One of Could not find any member to link logs is following:

`[warn] \path\incubator-pekko\cluster-sharding-typed\src\main\scala\org\apache\pekko\cluster\sharding\typed\testkit\javadsl\EntityRef.scala:22:1: Could not find any member to link for "EntityRef".`

This problem can be resolved by explicitly writing a full path of an entity:

-  * For testing purposes this `EntityRef` can be used in place of a real [[EntityRef]]. 
+  * For testing purposes this `EntityRef` can be used in place of a real [[org.apache.pekko.cluster.sharding.typed.javadsl.EntityRef EntityRef]].

https://github.com/apache/incubator-pekko/blob/966204814ecac09b9d7cc6e866fe2a71a660ab05/cluster-sharding-typed/src/main/scala/org/apache/pekko/cluster/sharding/typed/testkit/javadsl/EntityRef.scala#L23

Following is an example of The link target is ambiguous logs:

[warn] \path\incubator-pekko\actor-testkit-typed\src\main\scala\org\apache\pekko\actor\testkit\typed\javadsl\TestProbe.scala:70:1: The link target "ActorTestKit#createTestProbe" is ambiguous. Several members fit the target:
[warn] [M](name: String, clazz: Class[M]): org.apache.pekko.actor.testkit.typed.javadsl.TestProbe[M] in class ActorTestKit [chosen]
[warn] [M](name: String): org.apache.pekko.actor.testkit.typed.javadsl.TestProbe[M] in class ActorTestKit
[warn] [M](clazz: Class[M]): org.apache.pekko.actor.testkit.typed.javadsl.TestProbe[M] in class ActorTestKit
[warn] [M](): org.apache.pekko.actor.testkit.typed.javadsl.TestProbe[M] in class ActorTestKit

If a link target is ambiguous, scaladoc seems to automatically generate a link for one candidate.
So, I will resolve this problem by explicitly writing link of a candidate automatically chosen:

-  * or via [[ActorTestKit#createTestProbe]].
+  * or via [[ActorTestKit#createTestProbe[M](name:String,clazz:Class[M])* ActorTestKit#createTestProbe]].

https://github.com/apache/incubator-pekko/blob/966204814ecac09b9d7cc6e866fe2a71a660ab05/actor-testkit-typed/src/main/scala/org/apache/pekko/actor/testkit/typed/javadsl/TestProbe.scala#L72

@pjfanning
What do you think about the above plan?

@pjfanning
Copy link
Contributor Author

@ydash seems ok to me - but my point in an earlier comment still stands - can we keep any PRs a manageable size - to make them easier to review?

@ydash
Copy link
Contributor

ydash commented Jun 23, 2023

@pjfanning

can we keep any PRs a manageable size - to make them easier to review?

Probably yes, we can.
I will divide into two PRs based on the warning log type.
One will have about 10 lines modification and the other will have abount 80 lines modification.

@ydash
Copy link
Contributor

ydash commented Jun 28, 2023

I've found links which refer to a private type or member. Those links cause Could not find any member to link warn logs.
For example:
https://github.com/apache/incubator-pekko/blob/4ce1095b4b48f5cdd31342ab9f0950bd44a2829a/actor/src/main/scala/org/apache/pekko/dispatch/Dispatchers.scala#L326

Unless specified by -private option, scaladoc generate only public and protected types and members.
https://github.com/scala/scala/blob/93d6281876ae53d7eb0d1f1e8369437f0a260015/src/scaladoc/scala/tools/nsc/doc/Settings.scala#L241-L244

@pjfanning
Which solution do you think is better:

  1. Add -private option to generate scaladoc of private types and members.
  2. Disable links by removing square brackets (e.g. [[pekko.dispatch.BalancingDispatcher]] to pekko.dispatch.BalancingDispatcher).
  3. Ignore warn log. Those links work well in Intellij IDEA. Developers can jump to private members and types under coding.
  4. Something else.

@pjfanning
Copy link
Contributor Author

I've found links which refer to a private type or member. Those links cause Could not find any member to link warn logs. For example:

https://github.com/apache/incubator-pekko/blob/4ce1095b4b48f5cdd31342ab9f0950bd44a2829a/actor/src/main/scala/org/apache/pekko/dispatch/Dispatchers.scala#L326

Unless specified by -private option, scaladoc generate only public and protected types and members. https://github.com/scala/scala/blob/93d6281876ae53d7eb0d1f1e8369437f0a260015/src/scaladoc/scala/tools/nsc/doc/Settings.scala#L241-L244

@pjfanning Which solution do you think is better:

  1. Add -private option to generate scaladoc of private types and members.
  2. Disable links by removing square brackets (e.g. [[pekko.dispatch.BalancingDispatcher]] to pekko.dispatch.BalancingDispatcher).
  3. Ignore warn log. Those links work well in Intellij IDEA. Developers can jump to private members and types under coding.
  4. Something else.

3. Ignore warn log seems best for now

@ydash
Copy link
Contributor

ydash commented Jun 29, 2023

Links for java external library also cause Could not find any member to link warn logs.
For example.: https://github.com/apache/incubator-pekko/blob/4ce1095b4b48f5cdd31342ab9f0950bd44a2829a/slf4j/src/main/scala/org/apache/pekko/event/slf4j/Slf4jLogger.scala#L200

Similar problem was reported in stackoverflow.
Probably, scaladoc can not generate links for java external library.

@pjfanning
Which solution do you think is better?:

  1. Ignore warn logs. Those links work well in Intellij IDEA.
  2. Use URL links (e.g. rewrite [[org.slf4j.Marker]] to [[https://javadoc.io/static/org.slf4j/slf4j-api/1.7.36/org/slf4j/Marker.html org.slf4j.Marker]]).
    • It can generate valid links in scaladoc, but developers can't jump to library sources in Intellij IDEA.
    • I think It has worse maintainability because of URL links contain library version.

@pjfanning
Copy link
Contributor Author

@ydash Don't do 2.

Just do best effort to fix whatever scaladoc warnings are easily fixed.

I think there is a way to get the external links to slf4j to work. I've done it before but I don't have time today.

Here is an example from another project I work on.

https://github.com/FasterXML/jackson-module-scala/blob/2.16/build.sbt#L19-L36

@ydash
Copy link
Contributor

ydash commented Jul 4, 2023

I tried to add settings to Doc.scala like following, but (Compile / fullClasspath).value can not get subproject's jar paths in unidoc task.

https://github.com/apache/incubator-pekko/blob/88bf6329f193eedd45091f4f9a515943bd8ecb23/project/Doc.scala#L159

       Seq(
         ScalaUnidoc / unidocProjectFilter := unidocRootProjectFilter(unidocRootIgnoreProjects.value),
         JavaUnidoc / unidocProjectFilter := unidocRootProjectFilter(unidocRootIgnoreProjects.value),
-        ScalaUnidoc / apiMappings := (Compile / doc / apiMappings).value) ++
+        ScalaUnidoc / apiMappings := (Compile / doc / apiMappings).value,
+        ScalaUnidoc / apiMappings ++= {
+          def mappingsFor(organization: String, names: List[String], location: String, revision: (String) => String = identity): Seq[(File, URL)] =
+            for {
+              entry: Attributed[File] <- (Compile / fullClasspath).value
+              _ = println(s"entry: ${entry}")
+              module: ModuleID <- entry.get(moduleID.key)
+              if module.organization == organization
+              if names.exists(module.name.startsWith)
+            } yield entry.data -> url(location.format(revision(module.revision)))
+
+          val mappings: Seq[(File, URL)] =
+            mappingsFor("org.slf4j", List("slf4j-api"), "https://javadoc.io/doc/org.slf4j/slf4j-api/%s/")
+          mappings.toMap
+        }
+      ) ++
       UnidocRoot.CliOptions.genjavadocEnabled

@pjfanning
Would you suggest any solutions?

@pjfanning
Copy link
Contributor Author

@ydash I'm busy with release stuff so can't help much here.

I would have thought that Scala would handle api mappings for the various submodules of Pekko and that the only projects that we need to help Scala with are providing javadoc links for 3rd party libs like slf4j-api.

@pjfanning
Copy link
Contributor Author

@ydash if you feel like it would be easier to start with more straightforward issues, maybe you could start with the doc issues where the Pekko class can't be found (because the doc reference doesn't include enough of the package name)

ydash pushed a commit to ydash/incubator-pekko that referenced this issue Jul 10, 2023
ydash added a commit to ydash/incubator-pekko that referenced this issue Jul 10, 2023
ydash added a commit to ydash/incubator-pekko that referenced this issue Jul 10, 2023
@ydash
Copy link
Contributor

ydash commented Jul 10, 2023

I submitted a PR(#477) which solves following problem:

#353 (comment)

@pjfanning
Could you see my PR?

pjfanning pushed a commit that referenced this issue Jul 10, 2023
* make unidoc generate external javadoc links (#353)

* Move code of updating apiMappings
ydash pushed a commit to ydash/incubator-pekko that referenced this issue Sep 22, 2023
pjfanning pushed a commit that referenced this issue Sep 23, 2023
ydash pushed a commit to ydash/incubator-pekko that referenced this issue Sep 25, 2023
* Add enough package name
* Fix invalid syntax of links to a method
* Remove invalid newlines after a javadoc's link tag
ydash pushed a commit to ydash/incubator-pekko that referenced this issue Sep 25, 2023
* Add enough package name
* Fix invalid syntax of links to a method
He-Pin pushed a commit that referenced this issue Sep 26, 2023
* Add enough package name
* Fix invalid syntax of links to a method
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants