Don't use LinkedTreeMap for String supertypes as key & small test fix

This reverts a previous commit of this pull request, and matches the
original behavior again.

gson/src/main/java/com/google/gson/internal/ConstructorConstructor.java

@@ -399,8 +399,7 @@ private static boolean hasStringKeyType(Type mapType) {
     if (typeArguments.length == 0) {
       return false;
     }
-    // Consider String and supertypes of it
-    return TypeToken.get(typeArguments[0]).getRawType().isAssignableFrom(String.class);
+    return $Gson$Types.getRawType(typeArguments[0]) == String.class;
   }
 
   // Suppress Error Prone false positive: Pattern is reported for overridden methods, see

gson/src/test/java/com/google/gson/functional/MapTest.java

@@ -230,15 +230,13 @@ public void testMapStringKeyDeserialization() {
 
   @Test
   public void testMapStringSupertypeKeyDeserialization() {
-    // Uses `Object` (= supertype of String) as Map key
+    // Should only use Gson's LinkedTreeMap for String as key, but not for supertypes (e.g. Object)
     Type typeOfMap = new TypeToken<Map<Object, Integer>>() {}.getType();
     Map<?, ?> map = gson.fromJson("{\"a\":1}", typeOfMap);
 
-    assertWithMessage(
-            "Map<Object, ...> should use LinkedTreeMap to protect against DoS in older JDK"
-                + " versions")
+    assertWithMessage("Map<Object, ...> should not use Gson Map implementation")
         .that(map)
-        .isInstanceOf(LinkedTreeMap.class);
+        .isNotInstanceOf(LinkedTreeMap.class);
 
     Map<?, ?> expectedMap = Collections.singletonMap("a", 1);
     assertThat(map).isEqualTo(expectedMap);
@@ -249,7 +247,8 @@ public void testMapNonStringKeyDeserialization() {
     Type typeOfMap = new TypeToken<Map<Integer, Integer>>() {}.getType();
     Map<?, ?> map = gson.fromJson("{\"1\":1}", typeOfMap);
 
-    assertThat("non-Map<String, ...> should not use Gson Map implementation")
+    assertWithMessage("Map<Integer, ...> should not use Gson Map implementation")
+        .that(map)
         .isNotInstanceOf(LinkedTreeMap.class);
 
     Map<?, ?> expectedMap = Collections.singletonMap(1, 1);

gson/src/test/java/com/google/gson/internal/ConstructorConstructorTest.java

@@ -159,32 +159,33 @@ public void testCustomCollectionInterfaceCreation() {
 
   @Test
   public void testStringMapCreation() {
-    // When creating raw `Map` should use Gson's `LinkedTreeMap`, assuming keys _could_ be String
+    // When creating raw Map should use Gson's LinkedTreeMap, assuming keys could be String
     Object actual = constructorConstructor.get(TypeToken.get(Map.class)).construct();
     assertThat(actual).isInstanceOf(LinkedTreeMap.class);
 
-    // When creating a `Map<String, ...>` (or where key type is supertype of `String`) should use
-    // Gson's `LinkedTreeMap`
-    Class<?>[] stringTypes = {String.class, CharSequence.class, Object.class};
-    for (Class<?> stringType : stringTypes) {
+    // When creating a `Map<String, ...>` should use Gson's LinkedTreeMap
+    actual = constructorConstructor.get(new TypeToken<Map<String, Integer>>() {}).construct();
+    assertThat(actual).isInstanceOf(LinkedTreeMap.class);
+
+    // But when explicitly requesting a JDK `LinkedHashMap<String, ...>` should use LinkedHashMap
+    actual =
+        constructorConstructor.get(new TypeToken<LinkedHashMap<String, Integer>>() {}).construct();
+    assertThat(actual).isInstanceOf(LinkedHashMap.class);
+
+    // For all Map types with non-String key, should use JDK LinkedHashMap by default
+    // This is also done to avoid ClassCastException later, because Gson's LinkedTreeMap requires
+    // that keys are Comparable
+    Class<?>[] nonStringTypes = {Integer.class, CharSequence.class, Object.class};
+    for (Class<?> keyType : nonStringTypes) {
       actual =
           constructorConstructor
-              .get(TypeToken.getParameterized(Map.class, stringType, Integer.class))
+              .get(TypeToken.getParameterized(Map.class, keyType, Integer.class))
               .construct();
       assertWithMessage(
-              "Failed for key type " + stringType + "; created instance of " + actual.getClass())
+              "Failed for key type " + keyType + "; created instance of " + actual.getClass())
           .that(actual)
-          .isInstanceOf(LinkedTreeMap.class);
+          .isInstanceOf(LinkedHashMap.class);
     }
-
-    // But when explicitly requesting a `LinkedHashMap<String, ...>` should use JDK `LinkedHashMap`
-    actual =
-        constructorConstructor.get(new TypeToken<LinkedHashMap<String, Integer>>() {}).construct();
-    assertThat(actual).isInstanceOf(LinkedHashMap.class);
-
-    // For all Map types with non-String key, should use JDK `LinkedHashMap` by default
-    actual = constructorConstructor.get(new TypeToken<Map<Integer, Integer>>() {}).construct();
-    assertThat(actual).isInstanceOf(LinkedHashMap.class);
   }
 
   private enum MyEnum {}