lightbend / genjavadoc

A compiler plugin for generating doc’able Java source from Scala source
Other
58 stars 32 forks source link

Support java Deprecated on Scala symbols #348

Closed lrytz closed 6 months ago

lrytz commented 6 months ago

Fixes https://github.com/lightbend/genjavadoc/issues/347

lrytz commented 6 months ago

I don't seem to understand how these 2.12.x.patch files work.

lrytz commented 6 months ago

Not sure what's going on. When running test using 2.13.12 with the original patch file I see the following output:

diff -wurN -I ^ *// -I ^ *private  java\.lang\.Object readResolve target/expected_output/basic/akka/rk/buh/is/it/DontTouchThis.java /var/folders/8d/8tnybb497mbgq2v08vgcr5c80000gn/T/genjavadoc-java2488064058609746431/akka/rk/buh/is/it/DontTouchThis.java
--- target/expected_output/basic/akka/rk/buh/is/it/DontTouchThis.java   2024-03-04 16:44:23
+++ /var/folders/8d/8tnybb497mbgq2v08vgcr5c80000gn/T/genjavadoc-java2488064058609746431/akka/rk/buh/is/it/DontTouchThis.java    2024-03-04 16:44:23
@@ -12,8 +12,6 @@
   public void alreadyDeprecatedInComment () { throw new RuntimeException(); }
   /**
    * buh!
-   *
-   * @deprecated
    */
   public  void javaDeprecatedThingie ()  { throw new RuntimeException(); }
   /**
[error] Test com.typesafe.genjavadoc.BasicSpec.compileSourcesAndGenerateExpectedOutput failed: java.lang.AssertionError: assertion failed, took 0.509 sec
[error]     at scala.Predef$.assert(Predef.scala:264)
[error]     at com.typesafe.genjavadoc.util.CompilerSpec.$anonfun$lines$2(CompilerSpec.scala:47)

So I added that patch to 2.13.12.patch. Now running test (still on 2.13.12) gives

diff -wurN -I ^ *// -I ^ *private  java\.lang\.Object readResolve target/expected_output/basic/akka/rk/buh/is/it/DontTouchThis.java.orig /var/folders/8d/8tnybb497mbgq2v08vgcr5c80000gn/T/genjavadoc-java10649405226718870908/akka/rk/buh/is/it/DontTouchThis.java.orig
--- target/expected_output/basic/akka/rk/buh/is/it/DontTouchThis.java.orig  2024-03-04 16:46:13
+++ /var/folders/8d/8tnybb497mbgq2v08vgcr5c80000gn/T/genjavadoc-java10649405226718870908/akka/rk/buh/is/it/DontTouchThis.java.orig  1970-01-01 01:00:00
@@ -1,25 +0,0 @@
-package akka.rk.buh.is.it;
-/**
- * Don't touch this!
- *
- * @deprecated This is replaced by TouchThisInstead. Since now.
- */
-public class DontTouchThis {
-  public   DontTouchThis () { throw new RuntimeException(); }
-  /**
-   * @deprecated This is already deprecated. Since now.
-   */
-  public void alreadyDeprecatedInComment () { throw new RuntimeException(); }
-  /**
-   * buh!
-   *
-   * @deprecated
-   */
-  public void javaDeprecatedThingie () { throw new RuntimeException(); }
-  /**
-   * Some methods are forever.
-   *
-   * @deprecated This is replaced by someDiamondsAre. Since now.
-   */
-  public void orNotSoMuch () { throw new RuntimeException(); }
-}
[error] Test com.typesafe.genjavadoc.BasicSpec.compileSourcesAndGenerateExpectedOutput failed: java.lang.AssertionError: assertion failed, took 0.528 sec
[error]     at scala.Predef$.assert(Predef.scala:264)
[error]     at com.typesafe.genjavadoc.util.CompilerSpec.$anonfun$lines$2(CompilerSpec.scala:47)
raboof commented 6 months ago

Not sure what's going on. When running test using 2.13.12 with the original patch file I see the following output:

diff -wurN -I ^ *// -I ^ *private  java\.lang\.Object readResolve target/expected_output/basic/akka/rk/buh/is/it/DontTouchThis.java /var/folders/8d/8tnybb497mbgq2v08vgcr5c80000gn/T/genjavadoc-java2488064058609746431/akka/rk/buh/is/it/DontTouchThis.java
--- target/expected_output/basic/akka/rk/buh/is/it/DontTouchThis.java 2024-03-04 16:44:23
+++ /var/folders/8d/8tnybb497mbgq2v08vgcr5c80000gn/T/genjavadoc-java2488064058609746431/akka/rk/buh/is/it/DontTouchThis.java  2024-03-04 16:44:23
@@ -12,8 +12,6 @@
   public void alreadyDeprecatedInComment () { throw new RuntimeException(); }
   /**
    * buh!
-   *
-   * @deprecated
    */
   public  void javaDeprecatedThingie ()  { throw new RuntimeException(); }
   /**
[error] Test com.typesafe.genjavadoc.BasicSpec.compileSourcesAndGenerateExpectedOutput failed: java.lang.AssertionError: assertion failed, took 0.509 sec
[error]     at scala.Predef$.assert(Predef.scala:264)
[error]     at com.typesafe.genjavadoc.util.CompilerSpec.$anonfun$lines$2(CompilerSpec.scala:47)

So I added that patch to 2.13.12.patch. Now running test (still on 2.13.12) gives


diff -wurN -I ^ *// -I ^ *private  java\.lang\.Object readResolve target/expected_output/basic/akka/rk/buh/is/it/DontTouchThis.java.orig /var/folders/8d/8tnybb497mbgq2v08vgcr5c80000gn/T/genjavadoc-java10649405226718870908/akka/rk/buh/is/it/DontTouchThis.java.orig
--- target/expected_output/basic/akka/rk/buh/is/it/DontTouchThis.java.orig    2024-03-04 16:46:13
+++ /var/folders/8d/8tnybb497mbgq2v08vgcr5c80000gn/T/genjavadoc-java10649405226718870908/akka/rk/buh/is/it/DontTouchThis.java.orig    1970-01-01 01:00:00

The patch applied, but with 'fuzz', leaving behind an .orig, which is what you're seeing in this diff. Not sure where the 'fuzz' is coming from, but it seems convenient to allow, so https://github.com/lrytz/genjavadoc/pull/1 might help

lightbend-cla-validator commented 6 months ago

Hi @raboof,

Thank you for your contribution! We really value the time you've taken to put this together.

We see that you have signed the Lightbend Contributors License Agreement before, however, the CLA has changed since you last signed it. Please review the new CLA and sign it before we proceed with reviewing this pull request:

http://www.lightbend.com/contribute/cla

lrytz commented 6 months ago

Thank you!!!

raboof commented 6 months ago

We see that you have signed the Lightbend Contributors License Agreement before, however, the CLA has changed since you last signed it.

Is there a diff somewhere?

(the commit is trivial enough that you can also just squash it in, I don't think it's copyrightable)

SethTisue commented 6 months ago

@JustinPihony the CLA-changed notice is new to me... any idea what that's about?

SethTisue commented 6 months ago

Is there a diff somewhere?

I'm inquiring internally.

I agree that the change you contributed (thank you!) is too small for a CLA issue to block merging.