Skip to content

Commit

Permalink
Formatting follow-up (google#2540)
Browse files Browse the repository at this point in the history
* Formatting follow-up

- Adds formatting commits to .git-blame-ignore-revs so that they don't
  distract during Git blame
- Restores hard line breaks in Troubleshooting.md using `\` instead of
  trailing spaces
- Changes formatting of some string literals and comments
- Fixes accidental Javadoc and comment issues introduced by manual changes
  of formatting commit
- Fixes license header in $Gson$Types.java erroneously being a Javadoc
  comment and being reformatted
- Slightly changes `JsonReader` `getPath` and `getPreviousPath` documentation
  to help Javadoc detect first sentence as summary

* Remove `spotless:off` markers

* Add empty line before comment

* Check format for .github YAML files
  • Loading branch information
Marcono1234 authored and tibor-universe committed Aug 17, 2024
1 parent a7d27ed commit d8b9a76
Show file tree
Hide file tree
Showing 33 changed files with 155 additions and 112 deletions.
5 changes: 5 additions & 0 deletions .git-blame-ignore-revs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Ignore commit which reformatted code
2c94c757a6a9426cc2fe47bc1c63f69e7c73b7b4

# Ignore commit which changed line endings consistently to LF
c2a0e4634a2100494159add78db2ee06f5eb9be6
4 changes: 3 additions & 1 deletion .github/pull_request_template.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@
### Checklist
<!-- The following checklist is mainly intended for yourself to verify that you did not miss anything -->

- [ ] New code follows the [Google Java Style Guide](https://google.github.io/styleguide/javaguide.html)
- [ ] New code follows the [Google Java Style Guide](https://google.github.io/styleguide/javaguide.html)\
This is automatically checked by `mvn verify`, but can also be checked on its own using `mvn spotless:check`.\
Style violations can be fixed using `mvn spotless:apply`; this can be done in a separate commit to verify that it did not cause undesired changes.
- [ ] If necessary, new public API validates arguments, for example rejects `null`
- [ ] New public API has Javadoc
- [ ] Javadoc uses `@since $next-version$`
Expand Down
10 changes: 5 additions & 5 deletions Troubleshooting.md
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,8 @@ For example, let's assume you want to deserialize the following JSON data:
}
```

This will fail with an exception similar to this one: `MalformedJsonException: Use JsonReader.setStrictness(Strictness.LENIENT) to accept malformed JSON at line 5 column 4 path $.languages[2]`
The problem here is the trailing comma (`,`) after `"French"`, trailing commas are not allowed by the JSON specification. The location information "line 5 column 4" points to the `]` in the JSON data (with some slight inaccuracies) because Gson expected another value after `,` instead of the closing `]`. The JSONPath `$.languages[2]` in the exception message also points there: `$.` refers to the root object, `languages` refers to its member of that name and `[2]` refers to the (missing) third value in the JSON array value of that member (numbering starts at 0, so it is `[2]` instead of `[3]`).
This will fail with an exception similar to this one: `MalformedJsonException: Use JsonReader.setStrictness(Strictness.LENIENT) to accept malformed JSON at line 5 column 4 path $.languages[2]`\
The problem here is the trailing comma (`,`) after `"French"`, trailing commas are not allowed by the JSON specification. The location information "line 5 column 4" points to the `]` in the JSON data (with some slight inaccuracies) because Gson expected another value after `,` instead of the closing `]`. The JSONPath `$.languages[2]` in the exception message also points there: `$.` refers to the root object, `languages` refers to its member of that name and `[2]` refers to the (missing) third value in the JSON array value of that member (numbering starts at 0, so it is `[2]` instead of `[3]`).\
The proper solution here is to fix the malformed JSON data.

To spot syntax errors in the JSON data easily you can open it in an editor with support for JSON, for example Visual Studio Code. It will highlight within the JSON data the error location and show why the JSON data is considered invalid.
Expand Down Expand Up @@ -178,8 +178,8 @@ And you want to deserialize the following JSON data:
}
```

This will fail with an exception similar to this one: `IllegalStateException: Expected a string but was BEGIN_ARRAY at line 2 column 17 path $.languages`
This means Gson expected a JSON string value but found the beginning of a JSON array (`[`). The location information "line 2 column 17" points to the `[` in the JSON data (with some slight inaccuracies), so does the JSONPath `$.languages` in the exception message. It refers to the `languages` member of the root object (`$.`).
This will fail with an exception similar to this one: `IllegalStateException: Expected a string but was BEGIN_ARRAY at line 2 column 17 path $.languages`\
This means Gson expected a JSON string value but found the beginning of a JSON array (`[`). The location information "line 2 column 17" points to the `[` in the JSON data (with some slight inaccuracies), so does the JSONPath `$.languages` in the exception message. It refers to the `languages` member of the root object (`$.`).\
The solution here is to change in the `WebPage` class the field `String languages` to `List<String> languages`.

## <a id="adapter-not-null-safe"></a> `IllegalStateException`: "Expected ... but was NULL"
Expand Down Expand Up @@ -287,7 +287,7 @@ This will not initialize arbitrary classes, and it will throw a `ClassCastExcept

## <a id="type-token-raw"></a> `IllegalStateException`: 'TypeToken must be created with a type argument' <br> `RuntimeException`: 'Missing type parameter'

**Symptom:** An `IllegalStateException` with the message 'TypeToken must be created with a type argument' is thrown.
**Symptom:** An `IllegalStateException` with the message 'TypeToken must be created with a type argument' is thrown.\
For older Gson versions a `RuntimeException` with message 'Missing type parameter' is thrown.

**Reason:**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ public void testList() {
// Throws NullPointerException without the fix in https://github.com/google/gson/pull/1103
String json = gson.toJson(sandwiches);
assertEquals(
"{\"sandwiches\":[{\"bread\":\"white\",\"cheese\":\"cheddar\"},{\"bread\":\"whole"
+ " wheat\",\"cheese\":\"swiss\"}]}",
"{\"sandwiches\":[{\"bread\":\"white\",\"cheese\":\"cheddar\"},"
+ "{\"bread\":\"whole wheat\",\"cheese\":\"swiss\"}]}",
json);

MultipleSandwiches sandwichesFromJson = gson.fromJson(json, MultipleSandwiches.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,8 @@ void testClassWithoutDefaultConstructor() {
assertThat(c.i).isEqualTo(1);

c = gson.fromJson("{}", ClassWithoutDefaultConstructor.class);
// Class is instantiated with JDK Unsafe, so field keeps its default value instead of assigned
// -1
// Class is instantiated with JDK Unsafe, therefore field keeps its default value instead of
// assigned -1
assertThat(c.i).isEqualTo(0);
}

Expand Down
3 changes: 2 additions & 1 deletion gson/src/main/java/com/google/gson/JsonArray.java
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,8 @@ public boolean remove(JsonElement element) {

/**
* Removes the element at the specified position in this array. Shifts any subsequent elements to
* the left (subtracts one from their indices). Returns the element removed from the array.
* the left (subtracts one from their indices). Returns the element that was removed from the
* array.
*
* @param index index the index of the element to be removed
* @return the element previously at the specified position
Expand Down
20 changes: 11 additions & 9 deletions gson/src/main/java/com/google/gson/internal/$Gson$Types.java
Original file line number Diff line number Diff line change
@@ -1,18 +1,21 @@
/**
/*
* Copyright (C) 2021-2022 Happeo Oy. Copyright (C) 2008 Google Inc.
*
* <p>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
* 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
*
* <p>http://www.apache.org/licenses/LICENSE-2.0
* http://www.apache.org/licenses/LICENSE-2.0
*
* <p>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
* 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.
*
* <p>This file has been modified by Happeo Oy.
*/

package com.google.gson.internal;

import static com.google.gson.internal.$Gson$Preconditions.checkArgument;
Expand Down Expand Up @@ -155,8 +158,7 @@ public static Class<?> getRawType(Type type) {
} else {
String className = type == null ? "null" : type.getClass().getName();
throw new IllegalArgumentException(
"Expected a Class, ParameterizedType, or "
+ "GenericArrayType, but <"
"Expected a Class, ParameterizedType, or GenericArrayType, but <"
+ type
+ "> is of type "
+ className);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -305,17 +305,15 @@ public T construct() {
throw new RuntimeException(
"Failed to invoke constructor '"
+ ReflectionHelper.constructorToString(constructor)
+ "'"
+ " with no args",
+ "' with no args",
e);
} catch (InvocationTargetException e) {
// TODO: don't wrap if cause is unchecked?
// TODO: JsonParseException ?
throw new RuntimeException(
"Failed to invoke constructor '"
+ ReflectionHelper.constructorToString(constructor)
+ "'"
+ " with no args",
+ "' with no args",
e.getCause());
} catch (IllegalAccessException e) {
throw ReflectionHelper.createExceptionForUnexpectedIllegalAccess(e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ static int getMajorJavaVersion(String javaVersion) {
version = extractBeginningInt(javaVersion);
}
if (version == -1) {
return 6; // Choose a minimum supported JDK version as default
return 6; // Choose minimum supported JDK version as default
}
return version;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,8 @@ public <T> T newInstance(Class<T> c) {
throw new UnsupportedOperationException(
"Cannot allocate "
+ c
+ ". Usage of JDK sun.misc.Unsafe is enabled, "
+ "but it could not be used. Make sure your runtime is configured correctly.");
+ ". Usage of JDK sun.misc.Unsafe is enabled, but it could not be used."
+ " Make sure your runtime is configured correctly.");
}
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ private JsonAdapter getAnnotation(Class<?> rawType) {
return rawType.getAnnotation(JsonAdapter.class);
}

// this is not safe; requires that user has specified correct adapter class for @JsonAdapter
// this is not safe; requires that user has specified correct adapter class for @JsonAdapter
@SuppressWarnings("unchecked")
@Override
public <T> TypeAdapter<T> create(Gson gson, TypeToken<T> targetType) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,8 +197,8 @@ void readIntoField(JsonReader reader, Object target)
checkAccessible(target, field);
} else if (isStaticFinalField) {
// Reflection does not permit setting value of `static final` field, even after calling
// `setAccessible` Handle this here to avoid causing IllegalAccessException when calling
// `Field.set`
// `setAccessible`
// Handle this here to avoid causing IllegalAccessException when calling `Field.set`
String fieldDescription = ReflectionHelper.getAccessibleObjectDescription(field, false);
throw new JsonIOException("Cannot set value of 'static final' " + fieldDescription);
}
Expand Down Expand Up @@ -232,8 +232,7 @@ private static IllegalArgumentException createDuplicateFieldException(
+ declaringType.getName()
+ " declares multiple JSON fields named '"
+ duplicateName
+ "'; conflict is caused"
+ " by fields "
+ "'; conflict is caused by fields "
+ ReflectionHelper.fieldToString(field1)
+ " and "
+ ReflectionHelper.fieldToString(field2)
Expand Down Expand Up @@ -266,8 +265,7 @@ private FieldsData getBoundFields(
+ raw
+ " (supertype of "
+ originalRaw
+ "). Register a TypeAdapter for this type"
+ " or adjust the access filter.");
+ "). Register a TypeAdapter for this type or adjust the access filter.");
}
blockInaccessible = filterResult == FilterResult.BLOCK_INACCESSIBLE;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ public void write(JsonWriter out, T value) throws IOException {
// Order of preference for choosing type adapters
// First preference: a type adapter registered for the runtime type
// Second preference: a type adapter registered for the declared type
// Third preference: reflective type adapter for the runtime type (if it is a subclass of the
// declared type)
// Third preference: reflective type adapter for the runtime type
// (if it is a subclass of the declared type)
// Fourth preference: reflective type adapter for the declared type

TypeAdapter<T> chosen = delegate;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -405,8 +405,8 @@ public void write(JsonWriter out, Number value) throws IOException {
out.nullValue();
} else {
// For backward compatibility don't call `JsonWriter.value(float)` because that method
// has
// been newly added and not all custom JsonWriter implementations might override it yet
// has been newly added and not all custom JsonWriter implementations might override
// it yet
Number floatNumber = value instanceof Float ? value : value.floatValue();
out.value(floatNumber);
}
Expand Down Expand Up @@ -956,7 +956,8 @@ private static final class EnumTypeAdapter<T extends Enum<T>> extends TypeAdapte
public EnumTypeAdapter(final Class<T> classOfT) {
try {
// Uses reflection to find enum constants to work around name mismatches for obfuscated
// classes Reflection access might throw SecurityException, therefore run this in privileged
// classes
// Reflection access might throw SecurityException, therefore run this in privileged
// context; should be acceptable because this only retrieves enum constants, but does not
// expose anything else
Field[] constantFields =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,12 +160,14 @@ public static Date parse(String date, ParsePosition pos) throws ParseException {

// extract day
int day = parseInt(date, offset, offset += 2);

// default time value
int hour = 0;
int minutes = 0;
int seconds = 0;
int milliseconds =
0; // always use 0 otherwise returned date will include millis of current time

// always use 0 otherwise returned date will include millis of current time
int milliseconds = 0;

// if the value has no time component (and no time zone), we are done
boolean hasT = checkOffset(date, offset, 'T');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,7 @@ public static void makeAccessible(AccessibleObject ao) {
"Failed making constructor '"
+ constructorToString(constructor)
+ "' accessible; either increase its visibility or write a custom InstanceCreator"
+ " or TypeAdapter for"
// Include the message since it might contain more detailed information
+ " its declaring type: "
+ " or TypeAdapter for its declaring type: "
+ error.getMessage()
+ getInaccessibleTroubleshootingSuffix(error));
} else {
Expand Down
10 changes: 4 additions & 6 deletions gson/src/main/java/com/google/gson/reflect/TypeToken.java
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,8 @@ private Type getTypeTokenTypeArgument() {
else if (superclass == TypeToken.class) {
throw new IllegalStateException(
"TypeToken must be created with a type argument: new TypeToken<...>() {}; When using code"
+ " shrinkers (ProGuard, R8, ...) make sure that generic signatures are preserved.\n"
+ "See "
+ " shrinkers (ProGuard, R8, ...) make sure that generic signatures are preserved."
+ "\nSee "
+ TroubleshootingGuide.createUrl("type-token-raw"));
}

Expand Down Expand Up @@ -420,8 +420,7 @@ public static TypeToken<?> getParameterized(Type rawType, Type... typeArguments)
throw new IllegalArgumentException(
"Raw type "
+ rawClass.getName()
+ " is not supported because"
+ " it requires specifying an owner type");
+ " is not supported because it requires specifying an owner type");
}

for (int i = 0; i < expectedArgsCount; i++) {
Expand All @@ -437,8 +436,7 @@ public static TypeToken<?> getParameterized(Type rawType, Type... typeArguments)
throw new IllegalArgumentException(
"Type argument "
+ typeArgument
+ " does not satisfy bounds"
+ " for type variable "
+ " does not satisfy bounds for type variable "
+ typeVariable
+ " declared by "
+ rawType);
Expand Down
8 changes: 4 additions & 4 deletions gson/src/main/java/com/google/gson/stream/JsonReader.java
Original file line number Diff line number Diff line change
Expand Up @@ -1343,8 +1343,8 @@ public void skipValue() throws IOException {
// Only update when object end is explicitly skipped, otherwise stack is not updated
// anyways
if (count == 0) {
pathNames[stackSize - 1] =
null; // Free the last path name so that it can be garbage collected
// Free the last path name so that it can be garbage collected
pathNames[stackSize - 1] = null;
}
stackSize--;
count--;
Expand Down Expand Up @@ -1620,7 +1620,7 @@ private String getPath(boolean usePreviousPath) {

/**
* Returns a <a href="https://goessner.net/articles/JsonPath/">JSONPath</a> in <i>dot-notation</i>
* to the previous (or current) location in the JSON document:
* to the previous (or current) location in the JSON document. That means:
*
* <ul>
* <li>For JSON arrays the path points to the index of the previous element.<br>
Expand All @@ -1638,7 +1638,7 @@ public String getPreviousPath() {

/**
* Returns a <a href="https://goessner.net/articles/JsonPath/">JSONPath</a> in <i>dot-notation</i>
* to the next (or current) location in the JSON document:
* to the next (or current) location in the JSON document. That means:
*
* <ul>
* <li>For JSON arrays the path points to the index of the next element (even if there are no
Expand Down
6 changes: 3 additions & 3 deletions gson/src/main/java/com/google/gson/stream/JsonWriter.java
Original file line number Diff line number Diff line change
Expand Up @@ -306,9 +306,9 @@ public final FormattingStyle getFormattingStyle() {
* @see #setStrictness(Strictness)
*/
@Deprecated
@SuppressWarnings(
"InlineMeSuggester") // Don't specify @InlineMe, so caller with `setLenient(false)` becomes
// aware of new Strictness.STRICT
// Don't specify @InlineMe, so caller with `setLenient(false)` becomes aware of new
// Strictness.STRICT
@SuppressWarnings("InlineMeSuggester")
public final void setLenient(boolean lenient) {
setStrictness(lenient ? Strictness.LENIENT : Strictness.LEGACY_STRICT);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -648,8 +648,8 @@ public void testJsonElementTypeMismatch() {
assertThat(expected)
.hasMessageThat()
.isEqualTo(
"Expected a com.google.gson.JsonObject but was com.google.gson.JsonPrimitive; at path"
+ " $");
"Expected a com.google.gson.JsonObject but was com.google.gson.JsonPrimitive;"
+ " at path $");
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,14 @@ public void testFormatPretty() {
String expectedJson = buildExpected("\n", " ", true);
assertThat(json).isEqualTo(expectedJson);
// Sanity check to verify that `buildExpected` works correctly
assertThat(json).isEqualTo("{\n" + " \"a\": [\n" + " 1,\n" + " 2\n" + " ]\n" + "}");
assertThat(json)
.isEqualTo(
"{\n" //
+ " \"a\": [\n" //
+ " 1,\n" //
+ " 2\n" //
+ " ]\n" //
+ "}");
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -474,9 +474,8 @@ static class Factory implements TypeAdapterFactory {
@Override
public <T> TypeAdapter<T> create(Gson gson, TypeToken<T> type) {
return new TypeAdapter<T>() {
@SuppressWarnings(
"SameNameButDifferent") // suppress Error Prone warning; should be clear that
// `Factory` refers to enclosing class
// suppress Error Prone warning; should be clear that `Factory` refers to enclosing class
@SuppressWarnings("SameNameButDifferent")
private TypeAdapter<T> delegate() {
return gson.getDelegateAdapter(Factory.this, type);
}
Expand Down
Loading

0 comments on commit d8b9a76

Please sign in to comment.