Repository: scalameta/scalameta (scala)

Issue:
Looking up some JDK symbols fails with JDK21
Lookups that work with JDK20 do not work with JDK21:
```
scala.meta.internal.symtab.GlobalSymbolTable(scala.meta.Classpath(Nil), true).info("java/lang/System#lineSeparator().")
scala.meta.internal.symtab.GlobalSymbolTable(scala.meta.Classpath(Nil), true).info("java/nio/file/LinkOption#NOFOLLOW_LINKS.")
```

```
java.lang.NullPointerException: Cannot invoke "String.isEmpty()" because "$this" is null
  at scala.collection.StringOps$.head$extension(StringOps.scala:1124)
  at scala.meta.internal.semanticdb.Scala$Names$.encode(Scala.scala:181)
  at scala.meta.internal.semanticdb.Scala$Descriptor.toString(Scala.scala:147)
  at scala.meta.internal.semanticdb.Scala$Descriptor.toString$(Scala.scala:140)
  at scala.meta.internal.semanticdb.Scala$Descriptor$Parameter.toString(Scala.scala:158)
  at scala.meta.internal.semanticdb.Scala$Symbols$.Global(Scala.scala:14)
  at scala.meta.internal.javacp.Javacp$.$anonfun$sinfos$13(Javacp.scala:175)
  at scala.collection.immutable.List.map(List.scala:246)
  at scala.meta.internal.javacp.Javacp$.$anonfun$sinfos$9(Javacp.scala:169)
  at scala.collection.IterableOnceOps.foreach(IterableOnce.scala:576)
  at scala.collection.IterableOnceOps.foreach$(IterableOnce.scala:574)
  at scala.collection.AbstractIterable.foreach(Iterable.scala:933)
  at scala.meta.internal.javacp.Javacp$.sinfos(Javacp.scala:135)
  at scala.meta.internal.javacp.Javacp$.$anonfun$sinfos$17(Javacp.scala:233)
  at scala.collection.IterableOnceOps.foreach(IterableOnce.scala:576)
  at scala.collection.IterableOnceOps.foreach$(IterableOnce.scala:574)
  at scala.collection.AbstractIterable.foreach(Iterable.scala:933)
  at scala.meta.internal.javacp.Javacp$.sinfos(Javacp.scala:225)
  at scala.meta.internal.javacp.Javacp$.$anonfun$sinfos$17(Javacp.scala:233)
  at scala.collection.IterableOnceOps.foreach(IterableOnce.scala:576)
  at scala.collection.IterableOnceOps.foreach$(IterableOnce.scala:574)
  at scala.collection.AbstractIterable.foreach(Iterable.scala:933)
  at scala.meta.internal.javacp.Javacp$.sinfos(Javacp.scala:225)
  at scala.meta.internal.javacp.Javacp$.parse(Javacp.scala:26)
  at scala.meta.internal.metacp.ClassfileInfos$.fromClassNode(ClassfileInfos.scala:55)
  at scala.meta.internal.symtab.GlobalSymbolTable.loadSymbol(GlobalSymbolTable.scala:39)
  at scala.meta.internal.symtab.GlobalSymbolTable.info(GlobalSymbolTable.scala:70)
  ... 57 elided
```

Found through https://github.com/scalacenter/scalafix/pull/1810

Pull request description:
Follows discussions in https://github.com/scalameta/scalameta/pull/3385

* Fix incorrect explicit param name lookup in constructors of inner classes compiled with `-parameters`
* Fix exception in global symbol table when loading inner classes generated without `-parameters` on JDK21 https://github.com/scalameta/scalameta/issues/3376
* Fix, starting JDK21, a regression caused by [an optimization in JDK18 and later](https://bugs.openjdk.org/browse/JDK-8271623): the implicit parameter for constructors of inner classes not referencing their outer classes was no longer skipped

See corresponding failing tests in JDK8 & JDK21 before the actual fix (last commit) in https://github.com/scalameta/scalameta/actions/runs/6800563920

Patch:
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index bb2738cb5a..da86ecf5dd 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -40,7 +40,7 @@ jobs:
           - '2.13.12'
         java:
           - '8'
-          - '11'
+          - '21'
     steps:
       - uses: actions/checkout@v4
         with:
diff --git a/docs/semanticdb/specification.md b/docs/semanticdb/specification.md
index 53fb6192aa..6a9845946e 100644
--- a/docs/semanticdb/specification.md
+++ b/docs/semanticdb/specification.md
@@ -3622,7 +3622,9 @@ improve on this in the future.
   </tr>
   <tr>
     <td><code>Annotation(TypeRef(None, &lt;scala/annotation/strictfp&gt;, List()))</code></td>
-    <td> Declared <code>strictfp</code>; floating-point mode is FP-strict e.g. <code>strictfp class MyClass</code>.</td>
+    <td>
+      Declared <code>strictfp</code>; floating-point mode is FP-strict e.g. <code>strictfp class MyClass</code>. Note that this is the default mode as of JDK17, so this deprecated annotation is no longer emitted for JDK17+ bytecode.
+    </td>
   </tr>
   <tr>
     <td>Not supported</td>
diff --git a/semanticdb/integration/src/main/java/com/javacp/Outer.java b/semanticdb/integration/src/main/java/com/javacp/Outer.java
new file mode 100644
index 0000000000..44197512ad
--- /dev/null
+++ b/semanticdb/integration/src/main/java/com/javacp/Outer.java
@@ -0,0 +1,48 @@
+package com.javacp;
+
+class Outer {
+
+    class Inner {
+        // the compiler injects a first implicit param `this$` which is flagged mandated
+        // - only when using `-parameters` before JDK21
+        // - always as of JDK21
+        Inner(Integer... values) {}
+
+        public void printOuter() {
+            // For all JDKs, the generated code relies on an implicit class field `this$`
+            // holding a reference to the outer class.
+            System.out.println(Outer.this);
+        }
+
+        class InnerInner {
+            InnerInner(Integer... values) {}
+
+            public void printOuters() {
+                System.out.println(Outer.this);
+                System.out.println(Outer.Inner.this);
+            }
+        }
+    }
+
+    class InnerUnusedOuter {
+        // the compiler injects a first implicit param `this$` which is flagged mandated
+        // - only when using `-parameters` before JDK21
+        // - always as of JDK21
+        InnerUnusedOuter(Integer... values) {}
+
+        // Before JDK18, the `this$` synthetic field holding the reference to the outer
+        // class was synthetized. As of JDK18, that field is not present if unused. 
+    }
+
+    private class PrivateInnerUnusedOuter {
+        // the compiler injects a first implicit param `this$` which is flagged synthetic
+        // - only when using `-parameters` before JDK21
+        // - always as of JDK21
+        PrivateInnerUnusedOuter(Integer... values) {}
+
+        // Before JDK18, the `this$` synthetic field holding the reference to the outer
+        // class was synthetized. As of JDK18, that field is not present if unused. 
+    }
+
+}
+
diff --git a/semanticdb/metacp/src/main/scala/scala/meta/internal/javacp/Javacp.scala b/semanticdb/metacp/src/main/scala/scala/meta/internal/javacp/Javacp.scala
index d85cc65226..69be9dcf72 100644
--- a/semanticdb/metacp/src/main/scala/scala/meta/internal/javacp/Javacp.scala
+++ b/semanticdb/metacp/src/main/scala/scala/meta/internal/javacp/Javacp.scala
@@ -63,6 +63,7 @@ object Javacp {
     val classDisplayName = sdisplayName(node.name)
     val classAccess = node.access | access
     val hasOuterClassReference = node.fields.asScala.exists(isOuterClassReference)
+    val isEnum = node.access.hasFlag(o.ACC_ENUM)
 
     val classKind =
       if (classAccess.hasFlag(o.ACC_INTERFACE)) k.INTERFACE
@@ -155,23 +156,49 @@ object Javacp {
         }
         methodTypeParameters.foreach(buf += _)
 
-        val params =
-          if (isConstructor && hasOuterClassReference &&
-            // Guard against an empty parameter list, which seems to only happen
-            // in the JDK for java/util/regex/Pattern.class
-            method.signature.params.nonEmpty) {
-            // Drop the constructor argument that holds the reference to the outer class.
-            method.signature.params.tail
-          } else {
-            method.signature.params
-          }
+        val collectNonMandatedParams
+            : Tuple2[JavaTypeSignature, Int] => Option[(JavaTypeSignature, Option[String])] = {
+          case (_, 0) if isConstructor && hasOuterClassReference =>
+            // For JDK <21, implicit params holding the reference to the outer class
+            // in constructors can only detected through flags if `-parameters` was passed
+            // to `javac`, so we use the presence of a field as a heuristic proxy to
+            // know when the first param is that reference and should therefore be skipped.
+            //
+            // Note that in JDK >=18, the heuristic is unreliable, as `javac` does not
+            // synthetize the reference field if not needed, even though the constructor
+            // still has an (unused) implicit param that we should skip.
+            //
+            // That means that inner classes not referencing their outer class will have
+            // their implicit constructor param visible if compiled without `-parameters`
+            // on JDK 18, 19 or 20.
+            None
+          case (sigParam, i) =>
+            val nodeParamOpt =
+              Option(method.node.parameters).flatMap(xs => Option(xs.get(i)))
+
+            nodeParamOpt match {
+              case Some(nodeParam)
+                  if (isConstructor && !isEnum &&
+                    (nodeParam.access.hasFlag(o.ACC_MANDATED) ||
+                      nodeParam.access.hasFlag(o.ACC_SYNTHETIC))) =>
+                // Remove params holding the reference to the outer class in constructors
+                None
+              case Some(nodeParam) =>
+                // For JDK >=21, null-named parameters may exist when `-parameters`
+                // was not passed to `javac` and there is a mandated param.
+                // See https://github.com/openjdk/jdk/pull/9862.
+                Some((sigParam, Option(nodeParam.name)))
+              case _ =>
+                Some((sigParam, None))
+            }
+        }
+
+        val params = method.signature.params.zipWithIndex
+          .collect(Function.unlift(collectNonMandatedParams))
 
         val parameters: List[s.SymbolInformation] = params.zipWithIndex.map {
-          case (param: JavaTypeSignature, i) =>
-            val paramDisplayName = {
-              if (method.node.parameters == null) "param" + i
-              else method.node.parameters.get(i).name
-            }
+          case ((param: JavaTypeSignature, paramDisplayNameOpt), i) =>
+            val paramDisplayName = paramDisplayNameOpt.getOrElse("param" + i)
             val paramSymbol = Symbols.Global(methodSymbol, d.Parameter(paramDisplayName))
             val isRepeatedType = method.node.access.hasFlag(o.ACC_VARARGS) && i == params.length - 1
             val paramTpe =


Test patch:
diff --git a/tests/jvm/src/test/resources/metacp.expect_2.12 b/tests/jvm/src/test/resources/metacp.expect_2.12
index 95adfbbde1..20f53dec51 100644
--- a/tests/jvm/src/test/resources/metacp.expect_2.12
+++ b/tests/jvm/src/test/resources/metacp.expect_2.12
@@ -1227,6 +1227,62 @@ com/javacp/NonGeneric#`<init>`(). => ctor <init>()
 com/javacp/NonGeneric#flush(). => method flush(): Unit
   Unit => scala/Unit#
 
+com/javacp/Outer.class
+----------------------
+
+Summary:
+Schema => SemanticDB v4
+Uri => com/javacp/Outer.class
+Text => empty
+Language => Java
+Symbols => 16 entries
+
+Symbols:
+com/javacp/Outer# => private[javacp] class Outer extends Object { +4 decls }
+  javacp => com/javacp/
+  Object => java/lang/Object#
+com/javacp/Outer#Inner# => private[javacp] class Inner extends Object { +3 decls }
+  javacp => com/javacp/
+  Object => java/lang/Object#
+com/javacp/Outer#Inner#InnerInner# => private[javacp] class InnerInner extends Object { +2 decls }
+  javacp => com/javacp/
+  Object => java/lang/Object#
+com/javacp/Outer#Inner#InnerInner#`<init>`(). => private[javacp] ctor <init>(values: Integer*)
+  javacp => com/javacp/
+  values => com/javacp/Outer#Inner#InnerInner#`<init>`().(values)
+  Integer => java/lang/Integer#
+com/javacp/Outer#Inner#InnerInner#`<init>`().(values) => param values: Integer*
+  Integer => java/lang/Integer#
+com/javacp/Outer#Inner#InnerInner#printOuters(). => method printOuters(): Unit
+  Unit => scala/Unit#
+com/javacp/Outer#Inner#`<init>`(). => private[javacp] ctor <init>(values: Integer*)
+  javacp => com/javacp/
+  values => com/javacp/Outer#Inner#`<init>`().(values)
+  Integer => java/lang/Integer#
+com/javacp/Outer#Inner#`<init>`().(values) => param values: Integer*
+  Integer => java/lang/Integer#
+com/javacp/Outer#Inner#printOuter(). => method printOuter(): Unit
+  Unit => scala/Unit#
+com/javacp/Outer#InnerUnusedOuter# => private[javacp] class InnerUnusedOuter extends Object { +1 decls }
+  javacp => com/javacp/
+  Object => java/lang/Object#
+com/javacp/Outer#InnerUnusedOuter#`<init>`(). => private[javacp] ctor <init>(values: Integer*)
+  javacp => com/javacp/
+  values => com/javacp/Outer#InnerUnusedOuter#`<init>`().(values)
+  Integer => java/lang/Integer#
+com/javacp/Outer#InnerUnusedOuter#`<init>`().(values) => param values: Integer*
+  Integer => java/lang/Integer#
+com/javacp/Outer#PrivateInnerUnusedOuter# => private class PrivateInnerUnusedOuter extends Object { +1 decls }
+  Object => java/lang/Object#
+com/javacp/Outer#PrivateInnerUnusedOuter#`<init>`(). => private[javacp] ctor <init>(values: Integer*)
+  javacp => com/javacp/
+  values => com/javacp/Outer#PrivateInnerUnusedOuter#`<init>`().(values)
+  Integer => java/lang/Integer#
+com/javacp/Outer#PrivateInnerUnusedOuter#`<init>`().(values) => param values: Integer*
+  Integer => java/lang/Integer#
+com/javacp/Outer#`<init>`(). => private[javacp] ctor <init>()
+  javacp => com/javacp/
+
 com/javacp/Recursive.class
 --------------------------
 
diff --git a/tests/jvm/src/test/resources/metacp.expect_2.13 b/tests/jvm/src/test/resources/metacp.expect_2.13
index b98b9b1047..2aa4057fc4 100644
--- a/tests/jvm/src/test/resources/metacp.expect_2.13
+++ b/tests/jvm/src/test/resources/metacp.expect_2.13
@@ -1244,6 +1244,62 @@ com/javacp/NonGeneric#`<init>`(). => ctor <init>()
 com/javacp/NonGeneric#flush(). => method flush(): Unit
   Unit => scala/Unit#
 
+com/javacp/Outer.class
+----------------------
+
+Summary:
+Schema => SemanticDB v4
+Uri => com/javacp/Outer.class
+Text => empty
+Language => Java
+Symbols => 16 entries
+
+Symbols:
+com/javacp/Outer# => private[javacp] class Outer extends Object { +4 decls }
+  javacp => com/javacp/
+  Object => java/lang/Object#
+com/javacp/Outer#Inner# => private[javacp] class Inner extends Object { +3 decls }
+  javacp => com/javacp/
+  Object => java/lang/Object#
+com/javacp/Outer#Inner#InnerInner# => private[javacp] class InnerInner extends Object { +2 decls }
+  javacp => com/javacp/
+  Object => java/lang/Object#
+com/javacp/Outer#Inner#InnerInner#`<init>`(). => private[javacp] ctor <init>(values: Integer*)
+  javacp => com/javacp/
+  values => com/javacp/Outer#Inner#InnerInner#`<init>`().(values)
+  Integer => java/lang/Integer#
+com/javacp/Outer#Inner#InnerInner#`<init>`().(values) => param values: Integer*
+  Integer => java/lang/Integer#
+com/javacp/Outer#Inner#InnerInner#printOuters(). => method printOuters(): Unit
+  Unit => scala/Unit#
+com/javacp/Outer#Inner#`<init>`(). => private[javacp] ctor <init>(values: Integer*)
+  javacp => com/javacp/
+  values => com/javacp/Outer#Inner#`<init>`().(values)
+  Integer => java/lang/Integer#
+com/javacp/Outer#Inner#`<init>`().(values) => param values: Integer*
+  Integer => java/lang/Integer#
+com/javacp/Outer#Inner#printOuter(). => method printOuter(): Unit
+  Unit => scala/Unit#
+com/javacp/Outer#InnerUnusedOuter# => private[javacp] class InnerUnusedOuter extends Object { +1 decls }
+  javacp => com/javacp/
+  Object => java/lang/Object#
+com/javacp/Outer#InnerUnusedOuter#`<init>`(). => private[javacp] ctor <init>(values: Integer*)
+  javacp => com/javacp/
+  values => com/javacp/Outer#InnerUnusedOuter#`<init>`().(values)
+  Integer => java/lang/Integer#
+com/javacp/Outer#InnerUnusedOuter#`<init>`().(values) => param values: Integer*
+  Integer => java/lang/Integer#
+com/javacp/Outer#PrivateInnerUnusedOuter# => private class PrivateInnerUnusedOuter extends Object { +1 decls }
+  Object => java/lang/Object#
+com/javacp/Outer#PrivateInnerUnusedOuter#`<init>`(). => private[javacp] ctor <init>(values: Integer*)
+  javacp => com/javacp/
+  values => com/javacp/Outer#PrivateInnerUnusedOuter#`<init>`().(values)
+  Integer => java/lang/Integer#
+com/javacp/Outer#PrivateInnerUnusedOuter#`<init>`().(values) => param values: Integer*
+  Integer => java/lang/Integer#
+com/javacp/Outer#`<init>`(). => private[javacp] ctor <init>()
+  javacp => com/javacp/
+
 com/javacp/Recursive.class
 --------------------------
 
diff --git a/tests/jvm/src/test/scala/scala/meta/tests/semanticdb/ExpectSuite.scala b/tests/jvm/src/test/scala/scala/meta/tests/semanticdb/ExpectSuite.scala
index a8818e70e1..ea30edeb17 100644
--- a/tests/jvm/src/test/scala/scala/meta/tests/semanticdb/ExpectSuite.scala
+++ b/tests/jvm/src/test/scala/scala/meta/tests/semanticdb/ExpectSuite.scala
@@ -20,6 +20,7 @@ import munit.FunSuite
 import munit.internal.difflib.DiffUtils
 import scala.meta.tests.metacp.Library
 import scala.meta.tests.metacp.MetacpOps
+import scala.util.Properties
 
 class ExpectSuite extends FunSuite {
 
@@ -31,7 +32,19 @@ class ExpectSuite extends FunSuite {
   }
   test("metacp.expect") {
     import MetacpExpect._
-    this.assertNoDiff(loadObtained, loadExpected)
+
+    val javaVersion = Properties.javaVersion.takeWhile(_.isDigit).toInt
+    val expected =
+      if (javaVersion >= 17)
+        // strictfp is deprecated since JDK 17 and therefore ACC_STRICT is no longer emitted.
+        // See https://openjdk.org/jeps/306.
+        loadExpected.replace(
+          """|com/javacp/Test#strictfpMethod(). => @strictfp private[javacp] method strictfpMethod(): Unit
+             |  strictfp => scala/annotation/strictfp#""".stripMargin,
+          "com/javacp/Test#strictfpMethod(). => private[javacp] method strictfpMethod(): Unit".stripMargin
+        )
+      else loadExpected
+    this.assertNoDiff(loadObtained, expected)
   }
   test("metac.expect") {
     import MetacExpect._