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

Allow trailing comma in lenient mode #1688

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 2 additions & 4 deletions build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,8 @@ import java.net.URL

buildscript {
dependencies {
val kotlinVersion = System.getenv("MOSHI_KOTLIN_VERSION")
?: libs.versions.kotlin.get()
val kspVersion = System.getenv("MOSHI_KSP_VERSION")
?: libs.versions.ksp.get()
val kotlinVersion = System.getenv("MOSHI_KOTLIN_VERSION") ?: libs.versions.kotlin.get()
val kspVersion = System.getenv("MOSHI_KSP_VERSION") ?: libs.versions.ksp.get()
classpath(kotlin("gradle-plugin", version = kotlinVersion))
classpath("com.google.devtools.ksp:symbol-processing-gradle-plugin:$kspVersion")
// https://github.com/melix/japicmp-gradle-plugin/issues/36
Expand Down
28 changes: 16 additions & 12 deletions moshi/src/main/java/com/squareup/moshi/ClassJsonAdapter.kt
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,7 @@ internal class ClassJsonAdapter<T>(
private val options = JsonReader.Options.of(*fieldsMap.keys.toTypedArray())

override fun fromJson(reader: JsonReader): T {
val result: T = try {
classFactory.newInstance()
} catch (e: InstantiationException) {
throw RuntimeException(e)
} catch (e: InvocationTargetException) {
throw e.rethrowCause()
} catch (e: IllegalAccessException) {
throw AssertionError()
}
val instance: T = instanciateTargetClass()

try {
reader.beginObject()
Expand All @@ -74,10 +66,22 @@ internal class ClassJsonAdapter<T>(
reader.skipValue()
continue
}
fieldsArray[index].read(reader, result)
fieldsArray[index].read(reader, instance)
}
reader.endObject()
return result
return instance
} catch (e: IllegalAccessException) {
throw AssertionError()
}
}

private fun instanciateTargetClass(): T {
return try {
classFactory.newInstance()
} catch (e: InstantiationException) {
throw RuntimeException(e)
} catch (e: InvocationTargetException) {
throw e.rethrowCause()
} catch (e: IllegalAccessException) {
throw AssertionError()
}
Expand All @@ -96,7 +100,7 @@ internal class ClassJsonAdapter<T>(
}
}

override fun toString() = "JsonAdapter($classFactory)"
override fun toString() = "ClassJsonAdapter($classFactory)"

internal class FieldBinding<T>(val name: String, val field: Field, val adapter: JsonAdapter<T>) {
fun read(reader: JsonReader, value: Any?) {
Expand Down
3 changes: 2 additions & 1 deletion moshi/src/main/java/com/squareup/moshi/JsonUtf8Reader.kt
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,8 @@ internal class JsonUtf8Reader : JsonReader {
buffer.readByte() // consume the '}'.
PEEKED_END_OBJECT
} else {
throw syntaxError("Expected name")
checkLenient()
PEEKED_END_OBJECT
}
else -> {
checkLenient()
Expand Down
45 changes: 45 additions & 0 deletions moshi/src/test/java/com/squareup/moshi/JsonAdapterTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package com.squareup.moshi;

import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.fail;
import static org.junit.Assume.assumeTrue;

Expand All @@ -26,6 +27,7 @@
import java.util.Locale;
import java.util.Map;
import javax.annotation.Nullable;
import org.intellij.lang.annotations.Language;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
Expand Down Expand Up @@ -338,4 +340,47 @@ public void nonNullDoesntDuplicate() {
JsonAdapter<Boolean> adapter = new Moshi.Builder().build().adapter(Boolean.class).nonNull();
assertThat(adapter.nonNull()).isSameInstanceAs(adapter);
}

@Test
public void givenJsonWithTrailingComma_whenDeserializeInLenientMode_thenDeserializeSuccessfully()
throws IOException {
JsonAdapter<Ticket> adapter = new Moshi.Builder().build().adapter(Ticket.class).lenient();
@Language("JSON")
String json = "{ \"number\" : \"912392\", \"seat\": 1234, \"type\" : \"COMMON\", }";
Ticket ticket = adapter.fromJson(json);
assertNotNull(ticket);
assertThat(ticket.getNumber()).isEqualTo("912392");
assertThat(ticket.getSeat()).isEqualTo(1234);
assertThat(ticket.getType()).isEqualTo("COMMON");
}

static class Ticket {
private String number;
private Integer seat;
private String type;

public String getNumber() {
return number;
}

public void setNumber(String number) {
this.number = number;
}

public Integer getSeat() {
return seat;
}

public void setSeat(Integer seat) {
this.seat = seat;
}

public String getType() {
return type;
}

public void setType(String type) {
this.type = type;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import okio.ForwardingSource;
import okio.Okio;
import okio.Source;
import org.junit.Assert;
import org.junit.Ignore;
import org.junit.Test;

Expand Down Expand Up @@ -1268,9 +1269,10 @@ public void lenientExtraCommasInMaps() throws IOException {
assertThat(reader.nextName()).isEqualTo("a");
assertThat(reader.nextString()).isEqualTo("b");
try {
reader.peek();
JsonReader.Token peek = reader.peek();
Assert.assertEquals(JsonReader.Token.END_OBJECT, peek);
} catch (JsonEncodingException unexpected) {
fail();
} catch (JsonEncodingException expected) {
}
}

Expand Down