Skip to content

Commit

Permalink
Implement reflective support for Java Records (#1381)
Browse files Browse the repository at this point in the history
* Standardize around JDK 8

* Update GJF to support newer JDKs

* Fix misc java 8 issues in tests

* Prepare java 16/records checking at runtime

* Implement real RecordJsonAdapter

* Spotless

* Prepare build for JDK 16+

* Fix property name for kapt

* Small cleanup

* Make FallbackEnum java-8 happy

* Remove animalsniffer

* Fix format

* Add opens for ExtendsPlatformClassWithProtectedFields

* Return null every time in shim for main tests

* Use JDK 16 + release 8 to replace animalsniffer

* Simplify accessor accessible handling

* Remove manifest attrs

* Fix typo

* Fix KCT tests + upgrade it

* Cover another

* Try explicit kotlin daemon args for java 17?

* Disable 17-ea for now until kotlin 1.5.30

* Add JsonQualifier and Json(name) tests + fix qualifiers

* Ensure constructor is accessible

* GJF it properly

* GJF 1.11

* Unwrap InvocationTargetException

* Use MethodHandle for constructor

* Use MethodHandle for accessor too

* Revert "Remove manifest attrs"

This reverts commit 3eb768f.

* Proper MR jar

* *actually* fix GJF, which wasn't getting applied before

We can just enable this everywhere now since we require JDK 16 anyway

* Make IDE happy about modules access

* Fixup records tests to play nice with modules

Gotta be public

* Add complex smoke test

* Remove comment

Not a regression test in this case
  • Loading branch information
ZacSweers committed Aug 23, 2021
1 parent 2572c29 commit 95250b0
Show file tree
Hide file tree
Showing 20 changed files with 653 additions and 104 deletions.
11 changes: 2 additions & 9 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,7 @@ jobs:
fail-fast: false
matrix:
java-version:
- 8
- 9
- 10
- 11
- 12
- 13
- 14
- 15
- 16

steps:
- name: Checkout
Expand Down Expand Up @@ -46,7 +39,7 @@ jobs:
run: ./gradlew build check --stacktrace

- name: Publish (default branch only)
if: github.repository == 'square/moshi' && github.ref == 'refs/heads/master' && matrix.java-version == '8'
if: github.repository == 'square/moshi' && github.ref == 'refs/heads/master' && matrix.java-version == '16'
run: ./gradlew uploadArchives
env:
ORG_GRADLE_PROJECT_SONATYPE_NEXUS_USERNAME: ${{ secrets.SONATYPE_NEXUS_USERNAME }}
Expand Down
9 changes: 0 additions & 9 deletions adapters/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,9 @@
* limitations under the License.
*/

import org.jetbrains.kotlin.gradle.tasks.KotlinCompile

plugins {
kotlin("jvm")
id("com.vanniktech.maven.publish")
id("ru.vyarus.animalsniffer")
}

tasks.withType<KotlinCompile>().configureEach {
kotlinOptions {
jvmTarget = "1.6"
}
}

dependencies {
Expand Down
100 changes: 48 additions & 52 deletions build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ plugins {
id("com.vanniktech.maven.publish") version "0.14.2" apply false
id("org.jetbrains.dokka") version "1.4.32" apply false
id("com.diffplug.spotless") version "5.12.4"
id("ru.vyarus.animalsniffer") version "1.5.3" apply false
id("me.champeau.gradle.japicmp") version "0.2.9" apply false
}

Expand All @@ -44,50 +43,47 @@ spotless {
indentWithSpaces(2)
endWithNewline()
}
// GJF not compatible with JDK 15 yet
if (!JavaVersion.current().isCompatibleWith(JavaVersion.VERSION_15)) {
val externalJavaFiles = arrayOf(
"**/ClassFactory.java",
"**/Iso8601Utils.java",
"**/JsonReader.java",
"**/JsonReaderPathTest.java",
"**/JsonReaderTest.java",
"**/JsonScope.java",
"**/JsonUtf8Reader.java",
"**/JsonUtf8ReaderPathTest.java",
"**/JsonUtf8ReaderTest.java",
"**/JsonUtf8ReaderTest.java",
"**/JsonUtf8Writer.java",
"**/JsonUtf8WriterTest.java",
"**/JsonWriter.java",
"**/JsonWriterPathTest.java",
"**/JsonWriterTest.java",
"**/LinkedHashTreeMap.java",
"**/LinkedHashTreeMapTest.java",
"**/PolymorphicJsonAdapterFactory.java",
"**/RecursiveTypesResolveTest.java",
"**/Types.java",
"**/TypesTest.java"
val externalJavaFiles = arrayOf(
"**/ClassFactory.java",
"**/Iso8601Utils.java",
"**/JsonReader.java",
"**/JsonReaderPathTest.java",
"**/JsonReaderTest.java",
"**/JsonScope.java",
"**/JsonUtf8Reader.java",
"**/JsonUtf8ReaderPathTest.java",
"**/JsonUtf8ReaderTest.java",
"**/JsonUtf8ReaderTest.java",
"**/JsonUtf8Writer.java",
"**/JsonUtf8WriterTest.java",
"**/JsonWriter.java",
"**/JsonWriterPathTest.java",
"**/JsonWriterTest.java",
"**/LinkedHashTreeMap.java",
"**/LinkedHashTreeMapTest.java",
"**/PolymorphicJsonAdapterFactory.java",
"**/RecursiveTypesResolveTest.java",
"**/Types.java",
"**/TypesTest.java"
)
val configureCommonJavaFormat: JavaExtension.() -> Unit = {
googleJavaFormat("1.11.0")
}
java {
configureCommonJavaFormat()
target("**/*.java")
targetExclude(
"**/spotless.java",
"**/build/**",
*externalJavaFiles
)
val configureCommonJavaFormat: JavaExtension.() -> Unit = {
googleJavaFormat("1.7")
}
java {
configureCommonJavaFormat()
target("**/*.java")
targetExclude(
"**/spotless.java",
"**/build/**",
*externalJavaFiles
)
licenseHeaderFile("spotless/spotless.java")
}
format("externalJava", JavaExtension::class.java) {
// These don't use our spotless config for header files since we don't want to overwrite the
// existing copyright headers.
configureCommonJavaFormat()
target(*externalJavaFiles)
}
licenseHeaderFile("spotless/spotless.java")
}
format("externalJava", JavaExtension::class.java) {
// These don't use our spotless config for header files since we don't want to overwrite the
// existing copyright headers.
configureCommonJavaFormat()
target(*externalJavaFiles)
}
kotlin {
ktlint(Dependencies.ktlintVersion).userData(mapOf("indent_size" to "2"))
Expand Down Expand Up @@ -129,15 +125,14 @@ subprojects {
// Apply with "java" instead of just "java-library" so kotlin projects get it too
pluginManager.withPlugin("java") {
configure<JavaPluginExtension> {
sourceCompatibility = JavaVersion.VERSION_1_7
targetCompatibility = JavaVersion.VERSION_1_7
toolchain {
languageVersion.set(JavaLanguageVersion.of(16))
}
}
}

pluginManager.withPlugin("ru.vyarus.animalsniffer") {
dependencies {
"compileOnly"(Dependencies.AnimalSniffer.annotations)
"signature"(Dependencies.AnimalSniffer.java7Signature)
if (project.name != "records-tests") {
tasks.withType<JavaCompile>().configureEach {
options.release.set(8)
}
}
}

Expand All @@ -146,6 +141,7 @@ subprojects {
kotlinOptions {
@Suppress("SuspiciousCollectionReassignment")
freeCompilerArgs += listOf("-progressive")
jvmTarget = "1.8"
}
}

Expand Down
7 changes: 1 addition & 6 deletions buildSrc/src/main/kotlin/Dependencies.kt
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,6 @@ object Dependencies {
const val ktlintVersion = "0.41.0"
const val okio = "com.squareup.okio:okio:2.10.0"

object AnimalSniffer {
const val annotations = "org.codehaus.mojo:animal-sniffer-annotations:1.16"
const val java7Signature = "org.codehaus.mojo.signature:java17:1.0@signature"
}

object AutoService {
private const val version = "1.0"
const val annotations = "com.google.auto.service:auto-service-annotations:$version"
Expand Down Expand Up @@ -53,7 +48,7 @@ object Dependencies {

object Testing {
const val assertj = "org.assertj:assertj-core:3.11.1"
const val compileTesting = "com.github.tschuchortdev:kotlin-compile-testing:1.4.0"
const val compileTesting = "com.github.tschuchortdev:kotlin-compile-testing:1.4.3"
const val junit = "junit:junit:4.13.2"
const val truth = "com.google.truth:truth:1.0.1"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,9 @@ public JsonAdapter<?> create(
if (!(annotation instanceof Fallback)) {
return null;
}
Class<Enum> enumType = (Class<Enum>) rawType;
Enum<?> fallback = Enum.valueOf(enumType, ((Fallback) annotation).value());
return new FallbackEnumJsonAdapter<>(enumType, fallback);
//noinspection rawtypes
return new FallbackEnumJsonAdapter<>(
(Class<? extends Enum>) rawType, ((Fallback) annotation).value());
}
};

Expand All @@ -70,9 +70,9 @@ public JsonAdapter<?> create(
final JsonReader.Options options;
final T defaultValue;

FallbackEnumJsonAdapter(Class<T> enumType, T defaultValue) {
FallbackEnumJsonAdapter(Class<T> enumType, String fallbackName) {
this.enumType = enumType;
this.defaultValue = defaultValue;
this.defaultValue = Enum.valueOf(enumType, fallbackName);
try {
constants = enumType.getEnumConstants();
nameStrings = new String[constants.length];
Expand Down
31 changes: 29 additions & 2 deletions gradle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,36 @@
#

# For Dokka https://github.com/Kotlin/dokka/issues/1405
org.gradle.jvmargs=-XX:MaxMetaspaceSize=512m
org.gradle.jvmargs=-Xmx2048m -Dfile.encoding=UTF-8 \
--add-opens=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED \
--add-opens=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED \
--add-opens=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED \
--add-opens=jdk.compiler/com.sun.tools.javac.jvm=ALL-UNNAMED \
--add-opens=jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED \
--add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED \
--add-opens=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED \
--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED \
--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED \
--add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED \
--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED \
--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED

kapt.includeCompileClasspath=false
# TODO move this to DSL in Kotlin 1.5.30 https://youtrack.jetbrains.com/issue/KT-44266
kotlin.daemon.jvmargs=-Dfile.encoding=UTF-8 \
--add-opens=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED \
--add-opens=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED \
--add-opens=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED \
--add-opens=jdk.compiler/com.sun.tools.javac.jvm=ALL-UNNAMED \
--add-opens=jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED \
--add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED \
--add-opens=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED \
--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED \
--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED \
--add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED \
--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED \
--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED

kapt.include.compile.classpath=false

GROUP=com.squareup.moshi
VERSION_NAME=1.13.0-SNAPSHOT
Expand Down
19 changes: 14 additions & 5 deletions kotlin/codegen/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -27,18 +27,27 @@ plugins {

tasks.withType<KotlinCompile>().configureEach {
kotlinOptions {
jvmTarget = "1.8"
@Suppress("SuspiciousCollectionReassignment")
freeCompilerArgs += listOf(
"-Xopt-in=com.squareup.kotlinpoet.metadata.KotlinPoetMetadataPreview"
)
}
}

// To make Gradle happy
java {
sourceCompatibility = JavaVersion.VERSION_1_8
targetCompatibility = JavaVersion.VERSION_1_8
tasks.withType<Test>().configureEach {
// For kapt to work with kotlin-compile-testing
jvmArgs(
"--add-opens=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED",
"--add-opens=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED",
"--add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED",
"--add-opens=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED",
"--add-opens=jdk.compiler/com.sun.tools.javac.jvm=ALL-UNNAMED",
"--add-opens=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED",
"--add-opens=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED",
"--add-opens=jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED",
"--add-opens=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED",
"--add-opens=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED",
)
}

val shade: Configuration = configurations.maybeCreate("compileShaded")
Expand Down
5 changes: 5 additions & 0 deletions kotlin/tests/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ plugins {
kotlin("kapt")
}

tasks.withType<Test>().configureEach {
// ExtendsPlatformClassWithProtectedField tests a case where we set a protected ByteArrayOutputStream.buf field
jvmArgs("--add-opens=java.base/java.io=ALL-UNNAMED")
}

tasks.withType<KotlinCompile>().configureEach {
kotlinOptions {
@Suppress("SuspiciousCollectionReassignment")
Expand Down
43 changes: 40 additions & 3 deletions moshi/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,49 @@ import org.jetbrains.kotlin.gradle.tasks.KotlinCompile
plugins {
kotlin("jvm")
id("com.vanniktech.maven.publish")
id("ru.vyarus.animalsniffer")
}

val mainSourceSet by sourceSets.named("main")
val java16 by sourceSets.creating {
java {
srcDir("src/main/java16")
}
}

tasks.named<JavaCompile>("compileJava16Java") {
javaCompiler.set(
javaToolchains.compilerFor {
languageVersion.set(JavaLanguageVersion.of(16))
}
)
options.release.set(16)
}

// Package our actual RecordJsonAdapter from java16 sources in and denote it as an MRJAR
tasks.named<Jar>("jar") {
from(java16.output) {
into("META-INF/versions/16")
}
manifest {
attributes("Multi-Release" to "true")
}
}

configurations {
"java16Implementation" {
extendsFrom(api.get())
extendsFrom(implementation.get())
}
}

tasks.withType<Test>().configureEach {
// ExtendsPlatformClassWithProtectedField tests a case where we set a protected ByteArrayOutputStream.buf field
jvmArgs("--add-opens=java.base/java.io=ALL-UNNAMED")
}

tasks.withType<KotlinCompile>()
.configureEach {
kotlinOptions {
jvmTarget = "1.6"

if (name.contains("test", true)) {
@Suppress("SuspiciousCollectionReassignment") // It's not suspicious
freeCompilerArgs += listOf("-Xopt-in=kotlin.ExperimentalStdlibApi")
Expand All @@ -35,6 +70,8 @@ tasks.withType<KotlinCompile>()
}

dependencies {
// So the j16 source set can "see" main Moshi sources
"java16Implementation"(mainSourceSet.output)
compileOnly(Dependencies.jsr305)
api(Dependencies.okio)

Expand Down
30 changes: 30 additions & 0 deletions moshi/records-tests/build.gradle.kts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/*
* Copyright (C) 2021 Square, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

plugins {
`java-library`
}

tasks.withType<JavaCompile>().configureEach {
options.release.set(16)
}

dependencies {
testImplementation(project(":moshi"))
testCompileOnly(Dependencies.jsr305)
testImplementation(Dependencies.Testing.junit)
testImplementation(Dependencies.Testing.truth)
}
Loading

0 comments on commit 95250b0

Please sign in to comment.