Skip to content

Commit f5af01f

Browse files
jupblbjupblb
committed
Pin JDK version in stdlib SCIP symbols via scip.jdk.version system property
Replaces the regex-based 'jdk N' / 'jdk .' snapshot normalization with a proper plumbing knob: JdkPackage.forRuntime() now honors the scip.jdk.version system property before falling back to Runtime.version().feature(). The unit and snapshots test projects set -Dscip.jdk.version=11, so: - regenerated goldens contain a real 'jdk 11' instead of a placeholder - the on-disk snapshots match across JDK 11/17/21 without any comparison-time normalization - SnapshotNormalizer and the save/assert-side mutation can be deleted
1 parent d7e1f23 commit f5af01f

29 files changed

Lines changed: 357 additions & 296 deletions

‎FOLLOW_UP.md‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
# Follow-ups
2+
3+
## Derive JDK version in stdlib symbols from the bytecode target, not the runtime
4+
5+
### Current state
6+
7+
`JdkPackage.forRuntime()` embeds `Runtime.version().feature()` into every SCIP
8+
symbol that references a JRT classfile, e.g.:
9+
10+
```
11+
semanticdb maven jdk 11 java/lang/String#
12+
```
13+
14+
This means the JDK version baked into the symbol is whichever JVM happens to
15+
run the indexer — not a property of the project being indexed. As a workaround
16+
for cross-JDK snapshot stability we expose a `scip.jdk.version` system property
17+
(see `JdkPackage.forRuntime()`) that overrides the runtime value.
18+
19+
### Why this is the wrong source of truth
20+
21+
A repo whose bytecode targets Java 11 will produce different SCIP symbols
22+
depending on whether CI happens to run scip-java on JDK 11, 17, or 21. That
23+
silently breaks cross-repo navigation against the standard library.
24+
25+
scip-go does something analogous (it embeds the Go version into stdlib
26+
symbols, e.g. `scip-go gomod github.com/golang/go/src go1.22 fmt/Println().`)
27+
but takes the version from the project's `go.mod` `go` directive, **not** from
28+
the Go toolchain that runs the indexer. That's the principled design: the
29+
embedded version is a property of the project, so two repos that declare the
30+
same version interop regardless of where they were indexed.
31+
32+
### Proposed direction
33+
34+
Source the JDK version from the project's bytecode target rather than the
35+
running JVM:
36+
37+
- Maven: `maven.compiler.release` / `maven.compiler.target`
38+
- Gradle: the toolchain `languageVersion` for the compile task
39+
- Bazel: the `java_toolchain` `--source`/`--target` or the classfile major
40+
version embedded in produced `.class` files
41+
- Fallback: read the major version directly from one of the produced
42+
classfiles in the targetroot (bytes 6-7 of any `.class` file, mapped via
43+
`major - 44`).
44+
45+
Plumb the resulting value into `ScipSemanticdbOptions` (e.g. a
46+
`JdkPackage jdkPackage` field), use it in `PackageTable` in place of the
47+
current `JdkPackage.forRuntime()` call, and expose it on
48+
`IndexSemanticdbCommand` as `--jdk-version=<N>` for callers that want to set
49+
it explicitly.
50+
51+
### Expected outcome
52+
53+
- Two repos that compile against Java 11 always emit
54+
`semanticdb maven jdk 11 java/lang/String#`, regardless of which JDK runs
55+
scip-java in CI.
56+
- The `scip.jdk.version` system-property escape hatch and the snapshot test's
57+
reliance on it can both be removed.
58+
- Cross-repo "jump to definition" against the standard library becomes
59+
stable across CI upgrades.
60+
61+
### Open questions
62+
63+
- What should the default be when no bytecode target can be inferred? Probably
64+
`Runtime.version().feature()` (current behavior), to avoid regressions.
65+
- Should the version be per-document (a single targetroot can in principle
66+
mix targets) or per-index? Per-index matches the current model and is
67+
almost always correct in practice.

‎build.sbt‎

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -594,19 +594,23 @@ def javacModuleOptions = List(
594594
lazy val unit = project
595595
.in(file("tests/unit"))
596596
.settings(
597-
testSettings,
598-
// javaOptions ++= Seq( "-Xdebug", "-Xrunjdwp:transport=dt_socket,server=y,suspend=y,address=5005"),
599-
buildInfoKeys :=
600-
Seq[BuildInfoKey](
601-
version,
602-
scalaVersion,
603-
"temporaryDirectory" -> target.value / "tmpdir",
604-
"sourceroot" -> (ThisBuild / baseDirectory).value,
605-
"minimizedJavaSourceDirectory" -> minimizedSourceDirectory,
606-
"minimizedJavaTargetroot" ->
607-
(minimized / Compile / semanticdbTargetRoot).value
608-
),
609-
buildInfoPackage := "tests"
597+
testSettings,
598+
// javaOptions ++= Seq( "-Xdebug", "-Xrunjdwp:transport=dt_socket,server=y,suspend=y,address=5005"),
599+
// Pin the JDK version embedded in stdlib SCIP symbols (e.g. `jdk 11
600+
// java/lang/String#`) so snapshots stay stable regardless of which JDK
601+
// runs the tests.
602+
Test / javaOptions += "-Dscip.jdk.version=11",
603+
buildInfoKeys :=
604+
Seq[BuildInfoKey](
605+
version,
606+
scalaVersion,
607+
"temporaryDirectory" -> target.value / "tmpdir",
608+
"sourceroot" -> (ThisBuild / baseDirectory).value,
609+
"minimizedJavaSourceDirectory" -> minimizedSourceDirectory,
610+
"minimizedJavaTargetroot" ->
611+
(minimized / Compile / semanticdbTargetRoot).value
612+
),
613+
buildInfoPackage := "tests"
610614
)
611615
.dependsOn(javacPlugin, cli)
612616
.enablePlugins(BuildInfoPlugin)
@@ -639,6 +643,12 @@ lazy val snapshots = project
639643
.in(file("tests/snapshots"))
640644
.settings(
641645
testSettings,
646+
// Pin the JDK version embedded in stdlib SCIP symbols so both the
647+
// assert path (`snapshots/test`) and the regenerate path
648+
// (`snapshots/run`) produce stable output across JDKs.
649+
Test / javaOptions += "-Dscip.jdk.version=11",
650+
run / fork := true,
651+
run / javaOptions += "-Dscip.jdk.version=11",
642652
buildInfoKeys :=
643653
Seq[BuildInfoKey](
644654
"snapshotDirectory" -> (Compile / sourceDirectory).value / "generated"

‎scip-semanticdb/src/main/java/com/sourcegraph/scip_semanticdb/JdkPackage.java‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,16 @@ public JdkPackage(String version) {
77
this.version = version;
88
}
99

10-
/** Returns a {@link JdkPackage} for the currently running JVM. */
10+
/**
11+
* Returns a {@link JdkPackage} for the currently running JVM, or the value of the {@code
12+
* scip.jdk.version} system property if set. The override exists so that tests can produce
13+
* JDK-independent snapshots.
14+
*/
1115
public static JdkPackage forRuntime() {
16+
String override = System.getProperty("scip.jdk.version");
17+
if (override != null && !override.isEmpty()) {
18+
return new JdkPackage(override);
19+
}
1220
return new JdkPackage(Integer.toString(Runtime.version().feature()));
1321
}
1422

‎tests/snapshots/src/main/generated/tests/minimized/src/main/java/minimized/AbstractClasses.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ public abstract class AbstractClasses {
1212
// kind Constructor
1313
// ⌄ enclosing_range_start semanticdb maven . . minimized/AbstractClasses#defaultImplementation().
1414
public String defaultImplementation() {
15-
// ^^^^^^ reference semanticdb maven jdk . java/lang/String#
15+
// ^^^^^^ reference semanticdb maven jdk 11 java/lang/String#
1616
// ^^^^^^^^^^^^^^^^^^^^^ definition semanticdb maven . . minimized/AbstractClasses#defaultImplementation().
1717
// display_name defaultImplementation
1818
// signature_documentation java public String defaultImplementation()
@@ -23,7 +23,7 @@ public String defaultImplementation() {
2323

2424
// ⌄ enclosing_range_start semanticdb maven . . minimized/AbstractClasses#abstractImplementation().
2525
public abstract String abstractImplementation();
26-
// ^^^^^^ reference semanticdb maven jdk . java/lang/String#
26+
// ^^^^^^ reference semanticdb maven jdk 11 java/lang/String#
2727
// ^^^^^^^^^^^^^^^^^^^^^^ definition semanticdb maven . . minimized/AbstractClasses#abstractImplementation().
2828
// display_name abstractImplementation
2929
// signature_documentation java public abstract String abstractImplementation()

‎tests/snapshots/src/main/generated/tests/minimized/src/main/java/minimized/AnnotationParameters.java‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
// display_name Bar
77
// signature_documentation java @interface Bar
88
// kind Interface
9-
// relationship is_implementation semanticdb maven jdk . java/lang/annotation/Annotation#
9+
// relationship is_implementation semanticdb maven jdk 11 java/lang/annotation/Annotation#
1010
// ⌄ enclosing_range_start semanticdb maven . . minimized/Bar#value().
1111
double value();
1212
// ^^^^^ definition semanticdb maven . . minimized/Bar#value().
@@ -23,7 +23,7 @@
2323
// display_name BarB
2424
// signature_documentation java @interface BarB
2525
// kind Interface
26-
// relationship is_implementation semanticdb maven jdk . java/lang/annotation/Annotation#
26+
// relationship is_implementation semanticdb maven jdk 11 java/lang/annotation/Annotation#
2727
// ⌄ enclosing_range_start semanticdb maven . . minimized/BarB#value().
2828
boolean value();
2929
// ^^^^^ definition semanticdb maven . . minimized/BarB#value().
@@ -40,10 +40,10 @@
4040
// display_name Nullable
4141
// signature_documentation java @interface Nullable
4242
// kind Interface
43-
// relationship is_implementation semanticdb maven jdk . java/lang/annotation/Annotation#
43+
// relationship is_implementation semanticdb maven jdk 11 java/lang/annotation/Annotation#
4444
// ⌄ enclosing_range_start semanticdb maven . . minimized/Nullable#value().
4545
String value() default "";
46-
// ^^^^^^ reference semanticdb maven jdk . java/lang/String#
46+
// ^^^^^^ reference semanticdb maven jdk 11 java/lang/String#
4747
// ^^^^^ definition semanticdb maven . . minimized/Nullable#value().
4848
// display_name value
4949
// signature_documentation java public abstract String value()
@@ -59,10 +59,10 @@
5959
// display_name BarRef
6060
// signature_documentation java @interface BarRef
6161
// kind Interface
62-
// relationship is_implementation semanticdb maven jdk . java/lang/annotation/Annotation#
62+
// relationship is_implementation semanticdb maven jdk 11 java/lang/annotation/Annotation#
6363
// ⌄ enclosing_range_start semanticdb maven . . minimized/BarRef#value().
6464
SuppressWarnings value();
65-
// ^^^^^^^^^^^^^^^^ reference semanticdb maven jdk . java/lang/SuppressWarnings#
65+
// ^^^^^^^^^^^^^^^^ reference semanticdb maven jdk 11 java/lang/SuppressWarnings#
6666
// ^^^^^ definition semanticdb maven . . minimized/BarRef#value().
6767
// display_name value
6868
// signature_documentation java public abstract SuppressWarnings value()
@@ -91,8 +91,8 @@ interface Foo {
9191
@Bar(~5)
9292
// ^^^ reference semanticdb maven . . minimized/Bar#
9393
@SuppressWarnings(value = "unchecked")
94-
// ^^^^^^^^^^^^^^^^ reference semanticdb maven jdk . java/lang/SuppressWarnings#
95-
// ^^^^^ reference semanticdb maven jdk . java/lang/SuppressWarnings#value().
94+
// ^^^^^^^^^^^^^^^^ reference semanticdb maven jdk 11 java/lang/SuppressWarnings#
95+
// ^^^^^ reference semanticdb maven jdk 11 java/lang/SuppressWarnings#value().
9696
double test2();
9797
// ^^^^^ definition semanticdb maven . . minimized/Foo#test2().
9898
// display_name test2
@@ -142,8 +142,8 @@ interface TestRef {
142142
// ⌄ enclosing_range_start semanticdb maven . . minimized/TestRef#testCase().
143143
@BarRef(@SuppressWarnings(value = "unchecked"))
144144
// ^^^^^^ reference semanticdb maven . . minimized/BarRef#
145-
// ^^^^^^^^^^^^^^^^ reference semanticdb maven jdk . java/lang/SuppressWarnings#
146-
// ^^^^^ reference semanticdb maven jdk . java/lang/SuppressWarnings#value().
145+
// ^^^^^^^^^^^^^^^^ reference semanticdb maven jdk 11 java/lang/SuppressWarnings#
146+
// ^^^^^ reference semanticdb maven jdk 11 java/lang/SuppressWarnings#value().
147147
abstract double testCase();
148148
// ^^^^^^^^ definition semanticdb maven . . minimized/TestRef#testCase().
149149
// display_name testCase

‎tests/snapshots/src/main/generated/tests/minimized/src/main/java/minimized/Annotations.java‎

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -4,63 +4,63 @@
44
// ^^^^ reference semanticdb maven . . java/
55
// ^^^^ reference semanticdb maven . . java/lang/
66
// ^^^^^^^^^^ reference semanticdb maven . . java/lang/annotation/
7-
// ^^^^^^^^^^ reference semanticdb maven jdk . java/lang/annotation/Documented#
7+
// ^^^^^^^^^^ reference semanticdb maven jdk 11 java/lang/annotation/Documented#
88
import java.lang.annotation.Retention;
99
// ^^^^ reference semanticdb maven . . java/
1010
// ^^^^ reference semanticdb maven . . java/lang/
1111
// ^^^^^^^^^^ reference semanticdb maven . . java/lang/annotation/
12-
// ^^^^^^^^^ reference semanticdb maven jdk . java/lang/annotation/Retention#
12+
// ^^^^^^^^^ reference semanticdb maven jdk 11 java/lang/annotation/Retention#
1313
import java.lang.annotation.RetentionPolicy;
1414
// ^^^^ reference semanticdb maven . . java/
1515
// ^^^^ reference semanticdb maven . . java/lang/
1616
// ^^^^^^^^^^ reference semanticdb maven . . java/lang/annotation/
17-
// ^^^^^^^^^^^^^^^ reference semanticdb maven jdk . java/lang/annotation/RetentionPolicy#
17+
// ^^^^^^^^^^^^^^^ reference semanticdb maven jdk 11 java/lang/annotation/RetentionPolicy#
1818
import java.lang.annotation.Target;
1919
// ^^^^ reference semanticdb maven . . java/
2020
// ^^^^ reference semanticdb maven . . java/lang/
2121
// ^^^^^^^^^^ reference semanticdb maven . . java/lang/annotation/
22-
// ^^^^^^ reference semanticdb maven jdk . java/lang/annotation/Target#
22+
// ^^^^^^ reference semanticdb maven jdk 11 java/lang/annotation/Target#
2323

2424
import static java.lang.annotation.ElementType.*;
2525
// ^^^^ reference semanticdb maven . . java/
2626
// ^^^^ reference semanticdb maven . . java/lang/
2727
// ^^^^^^^^^^ reference semanticdb maven . . java/lang/annotation/
28-
// ^^^^^^^^^^^ reference semanticdb maven jdk . java/lang/annotation/ElementType#
28+
// ^^^^^^^^^^^ reference semanticdb maven jdk 11 java/lang/annotation/ElementType#
2929

3030
//⌄ enclosing_range_start semanticdb maven . . minimized/Annotations#
3131
@Documented
32-
// ^^^^^^^^^ reference semanticdb maven jdk . java/lang/annotation/Documented#
32+
// ^^^^^^^^^ reference semanticdb maven jdk 11 java/lang/annotation/Documented#
3333
@Retention(RetentionPolicy.RUNTIME)
34-
// ^^^^^^^^ reference semanticdb maven jdk . java/lang/annotation/Retention#
35-
// ^^^^^^^^^^^^^^^ reference semanticdb maven jdk . java/lang/annotation/RetentionPolicy#
36-
// ^^^^^^^ reference semanticdb maven jdk . java/lang/annotation/RetentionPolicy#RUNTIME.
34+
// ^^^^^^^^ reference semanticdb maven jdk 11 java/lang/annotation/Retention#
35+
// ^^^^^^^^^^^^^^^ reference semanticdb maven jdk 11 java/lang/annotation/RetentionPolicy#
36+
// ^^^^^^^ reference semanticdb maven jdk 11 java/lang/annotation/RetentionPolicy#RUNTIME.
3737
@Target(value = {CONSTRUCTOR,
38-
// ^^^^^ reference semanticdb maven jdk . java/lang/annotation/Target#
39-
// ^^^^^ reference semanticdb maven jdk . java/lang/annotation/Target#value().
40-
// ^^^^^^^^^^^ reference semanticdb maven jdk . java/lang/annotation/ElementType#CONSTRUCTOR.
38+
// ^^^^^ reference semanticdb maven jdk 11 java/lang/annotation/Target#
39+
// ^^^^^ reference semanticdb maven jdk 11 java/lang/annotation/Target#value().
40+
// ^^^^^^^^^^^ reference semanticdb maven jdk 11 java/lang/annotation/ElementType#CONSTRUCTOR.
4141
FIELD,
42-
// ^^^^^ reference semanticdb maven jdk . java/lang/annotation/ElementType#FIELD.
42+
// ^^^^^ reference semanticdb maven jdk 11 java/lang/annotation/ElementType#FIELD.
4343
LOCAL_VARIABLE,
44-
// ^^^^^^^^^^^^^^ reference semanticdb maven jdk . java/lang/annotation/ElementType#LOCAL_VARIABLE.
44+
// ^^^^^^^^^^^^^^ reference semanticdb maven jdk 11 java/lang/annotation/ElementType#LOCAL_VARIABLE.
4545
METHOD,
46-
// ^^^^^^ reference semanticdb maven jdk . java/lang/annotation/ElementType#METHOD.
46+
// ^^^^^^ reference semanticdb maven jdk 11 java/lang/annotation/ElementType#METHOD.
4747
PACKAGE,
48-
// ^^^^^^^ reference semanticdb maven jdk . java/lang/annotation/ElementType#PACKAGE.
48+
// ^^^^^^^ reference semanticdb maven jdk 11 java/lang/annotation/ElementType#PACKAGE.
4949
PARAMETER,
50-
// ^^^^^^^^^ reference semanticdb maven jdk . java/lang/annotation/ElementType#PARAMETER.
50+
// ^^^^^^^^^ reference semanticdb maven jdk 11 java/lang/annotation/ElementType#PARAMETER.
5151
TYPE}
52-
// ^^^^ reference semanticdb maven jdk . java/lang/annotation/ElementType#TYPE.
52+
// ^^^^ reference semanticdb maven jdk 11 java/lang/annotation/ElementType#TYPE.
5353
)
5454
public @interface Annotations {
5555
// ^^^^^^^^^^^ definition semanticdb maven . . minimized/Annotations#
5656
// display_name Annotations
5757
// signature_documentation java @Documented\n@Retention(RetentionPolicy.RUNTIME)\n@Target({CONSTRUCTOR, FIELD, LOCAL_VARIABLE, METHOD, PACKAGE, PARAMETER, TYPE})\npublic @interface Annotations
5858
// kind Interface
59-
// relationship is_implementation semanticdb maven jdk . java/lang/annotation/Annotation#
59+
// relationship is_implementation semanticdb maven jdk 11 java/lang/annotation/Annotation#
6060

6161
// ⌄ enclosing_range_start semanticdb maven . . minimized/Annotations#value().
6262
String value() default "";
63-
// ^^^^^^ reference semanticdb maven jdk . java/lang/String#
63+
// ^^^^^^ reference semanticdb maven jdk 11 java/lang/String#
6464
// ^^^^^ definition semanticdb maven . . minimized/Annotations#value().
6565
// display_name value
6666
// signature_documentation java public abstract String value()
@@ -69,7 +69,7 @@
6969

7070
// ⌄ enclosing_range_start semanticdb maven . . minimized/Annotations#format().
7171
String format() default "";
72-
// ^^^^^^ reference semanticdb maven jdk . java/lang/String#
72+
// ^^^^^^ reference semanticdb maven jdk 11 java/lang/String#
7373
// ^^^^^^ definition semanticdb maven . . minimized/Annotations#format().
7474
// display_name format
7575
// signature_documentation java public abstract String format()

0 commit comments

Comments
 (0)