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

Implement reflective support for Java Records #1381

Merged
merged 36 commits into from
Aug 23, 2021
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
fba0e63
Standardize around JDK 8
ZacSweers Aug 20, 2021
8ea1047
Update GJF to support newer JDKs
ZacSweers Aug 20, 2021
2e21d08
Fix misc java 8 issues in tests
ZacSweers Aug 20, 2021
261c263
Prepare java 16/records checking at runtime
ZacSweers Aug 20, 2021
25c4c1c
Implement real RecordJsonAdapter
ZacSweers Aug 20, 2021
2c4f98f
Spotless
ZacSweers Aug 20, 2021
055257d
Prepare build for JDK 16+
ZacSweers Aug 20, 2021
5f30573
Fix property name for kapt
ZacSweers Aug 20, 2021
10e5d0f
Small cleanup
ZacSweers Aug 20, 2021
7d1c602
Make FallbackEnum java-8 happy
ZacSweers Aug 20, 2021
cc6d1b6
Remove animalsniffer
ZacSweers Aug 20, 2021
5f851ea
Fix format
ZacSweers Aug 20, 2021
5841cfc
Add opens for ExtendsPlatformClassWithProtectedFields
ZacSweers Aug 20, 2021
f8391bd
Return null every time in shim for main tests
ZacSweers Aug 20, 2021
bd0a70f
Use JDK 16 + release 8 to replace animalsniffer
ZacSweers Aug 20, 2021
0f94869
Simplify accessor accessible handling
ZacSweers Aug 20, 2021
3eb768f
Remove manifest attrs
ZacSweers Aug 20, 2021
9913e17
Fix typo
ZacSweers Aug 20, 2021
76a6bb8
Fix KCT tests + upgrade it
ZacSweers Aug 20, 2021
6668795
Cover another
ZacSweers Aug 20, 2021
c49b119
Try explicit kotlin daemon args for java 17?
ZacSweers Aug 20, 2021
360179f
Disable 17-ea for now until kotlin 1.5.30
ZacSweers Aug 20, 2021
ef41c63
Add JsonQualifier and Json(name) tests + fix qualifiers
ZacSweers Aug 20, 2021
68fcc68
Ensure constructor is accessible
ZacSweers Aug 20, 2021
d98ee90
GJF it properly
ZacSweers Aug 21, 2021
20d9715
GJF 1.11
ZacSweers Aug 21, 2021
765a642
Unwrap InvocationTargetException
ZacSweers Aug 21, 2021
355422c
Use MethodHandle for constructor
ZacSweers Aug 21, 2021
4700a63
Use MethodHandle for accessor too
ZacSweers Aug 21, 2021
b89f722
Revert "Remove manifest attrs"
ZacSweers Aug 21, 2021
5906d57
Proper MR jar
ZacSweers Aug 21, 2021
d025c15
*actually* fix GJF, which wasn't getting applied before
ZacSweers Aug 21, 2021
3863c9b
Make IDE happy about modules access
ZacSweers Aug 21, 2021
5d74681
Fixup records tests to play nice with modules
ZacSweers Aug 21, 2021
82d22e9
Add complex smoke test
ZacSweers Aug 21, 2021
c68005f
Remove comment
ZacSweers Aug 21, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 3 additions & 9 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,8 @@ jobs:
fail-fast: false
matrix:
java-version:
- 8
- 9
- 10
- 11
- 12
- 13
- 14
- 15
- 16
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not testing on Java 8 is annoying. Something to look into separately from this change.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can make the main tests run with java 8 via Gradle toolchains

- 17-ea

steps:
- name: Checkout
Expand Down Expand Up @@ -46,7 +40,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
8 changes: 0 additions & 8 deletions adapters/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -14,20 +14,12 @@
* 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 {
compileOnly(Dependencies.jsr305)
api(project(":moshi"))
Expand Down
14 changes: 9 additions & 5 deletions build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ spotless {
"**/TypesTest.java"
)
val configureCommonJavaFormat: JavaExtension.() -> Unit = {
googleJavaFormat("1.7")
googleJavaFormat("1.10")
ZacSweers marked this conversation as resolved.
Show resolved Hide resolved
}
java {
configureCommonJavaFormat()
Expand Down Expand Up @@ -127,10 +127,13 @@ 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
if (project.name != "records-tests") {
pluginManager.withPlugin("java") {
configure<JavaPluginExtension> {
toolchain {
languageVersion.set(JavaLanguageVersion.of(8))
}
}
}
}

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,8 @@ 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 +69,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
16 changes: 14 additions & 2 deletions gradle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,21 @@
#

# 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
kapt.include.compile.classpath=false

GROUP=com.squareup.moshi
VERSION_NAME=1.13.0-SNAPSHOT
Expand Down
7 changes: 0 additions & 7 deletions kotlin/codegen/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -27,20 +27,13 @@ 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
}

val shade: Configuration = configurations.maybeCreate("compileShaded")
configurations.getByName("compileOnly").extendsFrom(shade)
dependencies {
Expand Down
38 changes: 36 additions & 2 deletions moshi/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,43 @@ plugins {
id("ru.vyarus.animalsniffer")
}

val mainSourceSet by sourceSets.named("main")
val java16 by sourceSets.creating {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

neat

java {
srcDir("src/main/java16")
}
}

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

// We need to include our actual RecordJsonAdapter from java16 sources and replace the shime
tasks.named<Jar>("jar") {
from(java16.output) {
duplicatesStrategy = DuplicatesStrategy.INCLUDE
}
manifest {
// Indicate that this is a multi release jar
ZacSweers marked this conversation as resolved.
Show resolved Hide resolved
attributes("Multi-Release" to "true")
}
}

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

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 +67,8 @@ tasks.withType<KotlinCompile>()
}

dependencies {
// So the j16 source set can "see" main Moshi sources
"java16Implementation"(mainSourceSet.output)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

off topic: this syntax offends me so much

compileOnly(Dependencies.jsr305)
api(Dependencies.okio)

Expand Down
36 changes: 36 additions & 0 deletions moshi/records-tests/build.gradle.kts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
* 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`
}

java {
toolchain {
languageVersion.set(JavaLanguageVersion.of(16))
}
}

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

dependencies {
testImplementation(project(":moshi"))
testCompileOnly(Dependencies.jsr305)
testImplementation(Dependencies.Testing.junit)
testImplementation(Dependencies.Testing.truth)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* 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.
*/
package com.squareup.moshi;

import java.io.IOException;
import org.junit.Test;

import static com.google.common.truth.Truth.assertThat;

public final class RecordsTest {

private final Moshi moshi = new Moshi.Builder().build();

@Test public void genericRecord() throws IOException {
var adapter =
moshi.<GenericRecord<String>>adapter(Types.newParameterizedType(GenericRecord.class,
String.class));
assertThat(adapter.fromJson("{\"value\":\"Okay!\"}")).isEqualTo(new GenericRecord<>("Okay!"));
}

@Test public void genericBoundedRecord() throws IOException {
var adapter = moshi.<GenericBoundedRecord<Integer>>adapter(Types.newParameterizedType(
GenericBoundedRecord.class,
Integer.class));
assertThat(adapter.fromJson("{\"value\":4}")).isEqualTo(new GenericBoundedRecord<>(4));
}
}

record GenericBoundedRecord<T extends Number>(T value) {}

record GenericRecord<T>(T value) {}
3 changes: 3 additions & 0 deletions moshi/src/main/java/com/squareup/moshi/Moshi.java
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ public final class Moshi {
BUILT_IN_FACTORIES.add(CollectionJsonAdapter.FACTORY);
BUILT_IN_FACTORIES.add(MapJsonAdapter.FACTORY);
BUILT_IN_FACTORIES.add(ArrayJsonAdapter.FACTORY);
if (Util.recordsAvailable()) {
BUILT_IN_FACTORIES.add(RecordJsonAdapter.FACTORY);
}
BUILT_IN_FACTORIES.add(ClassJsonAdapter.FACTORY);
}

Expand Down
27 changes: 27 additions & 0 deletions moshi/src/main/java/com/squareup/moshi/RecordJsonAdapter.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package com.squareup.moshi;

import java.io.IOException;
import java.lang.annotation.Annotation;
import java.lang.reflect.Type;
import java.util.Set;
import org.jetbrains.annotations.Nullable;

// This is just a simple shim for linking in StandardJsonAdapters and excluded from the final jar.
final class RecordJsonAdapter<T> extends JsonAdapter<T> {

static final JsonAdapter.Factory FACTORY = new JsonAdapter.Factory() {

@Nullable @Override
public JsonAdapter<?> create(Type type, Set<? extends Annotation> annotations, Moshi moshi) {
throw new AssertionError();
}
};

@Nullable @Override public T fromJson(JsonReader reader) throws IOException {
throw new AssertionError();
}

@Override public void toJson(JsonWriter writer, @Nullable T value) throws IOException {
throw new AssertionError();
}
}
13 changes: 13 additions & 0 deletions moshi/src/main/java/com/squareup/moshi/internal/Util.java
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import java.util.Map;
import java.util.NoSuchElementException;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
import javax.annotation.Nullable;

public final class Util {
Expand All @@ -55,6 +56,8 @@ public final class Util {
/** A map from primitive types to their corresponding wrapper types. */
private static final Map<Class<?>, Class<?>> PRIMITIVE_TO_WRAPPER_TYPE;

private static final AtomicBoolean RECORDS_AVAILABLE = new AtomicBoolean(false);
ZacSweers marked this conversation as resolved.
Show resolved Hide resolved

static {
Class<? extends Annotation> metadata = null;
try {
Expand Down Expand Up @@ -86,6 +89,12 @@ public final class Util {
primToWrap.put(void.class, Void.class);

PRIMITIVE_TO_WRAPPER_TYPE = Collections.unmodifiableMap(primToWrap);

try {
Class.forName("java.lang.reflect.RecordComponent");
RECORDS_AVAILABLE.set(true);
} catch (ClassNotFoundException ignored) {
}
}

// Extracted as a method with a keep rule to prevent R8 from keeping Kotlin Metada
Expand Down Expand Up @@ -670,4 +679,8 @@ public static <T> Class<T> boxIfPrimitive(Class<T> type) {
Class<T> wrapped = (Class<T>) PRIMITIVE_TO_WRAPPER_TYPE.get(type);
return (wrapped == null) ? type : wrapped;
}

public static boolean recordsAvailable() {
return RECORDS_AVAILABLE.get();
}
}
Loading