Add Guice hints when dealing with Kotlin declaration site variance.

PiperOrigin-RevId: 698431304

Author: java-team-github-bot

core/src/com/google/inject/internal/KotlinSupport.java

@@ -74,5 +74,10 @@ public boolean isLocalClass(Class<?> clazz) {
     public boolean isValueClass(Class<?> clazz) {
       return false;
     }
+
+    @Override
+    public boolean isKotlinClass(Class<?> clazz) {
+      return false;
+    }
   }
 }

core/src/com/google/inject/internal/KotlinSupportInterface.java

@@ -33,4 +33,7 @@ public interface KotlinSupportInterface {
 
   /** Returns whether the {@code clazz} is a Kotlin value class. */
   boolean isValueClass(Class<?> clazz);
+
+  /** Returns whether the {@code clazz} is a Kotlin class. */
+  boolean isKotlinClass(Class<?> clazz);
 }

core/src/com/google/inject/internal/MissingImplementationError.java

@@ -24,7 +24,8 @@ public MissingImplementationError(Key<T> key, Injector injector, List<Object> so
         key,
         // Defer building suggestions until messages are requested, to avoid the work associated
         // with iterating bindings in scenarios where the exceptions are discarded.
-        Suppliers.memoize(() -> MissingImplementationErrorHints.getSuggestions(key, injector)),
+        Suppliers.memoize(
+            () -> MissingImplementationErrorHints.getSuggestions(key, injector, sources)),
         sources);
   }
 

core/src/com/google/inject/internal/MissingImplementationErrorHints.java

@@ -11,15 +11,19 @@
 import com.google.inject.Key;
 import com.google.inject.TypeLiteral;
 import com.google.inject.spi.BindingSourceRestriction;
+import com.google.inject.spi.Dependency;
+import com.google.inject.spi.ElementSource;
 import com.google.inject.spi.UntargettedBinding;
 import java.lang.reflect.GenericArrayType;
+import java.lang.reflect.Member;
 import java.lang.reflect.ParameterizedType;
 import java.lang.reflect.Type;
 import java.lang.reflect.WildcardType;
 import java.util.ArrayList;
 import java.util.Formatter;
 import java.util.List;
 import java.util.Map;
+import java.util.function.BiFunction;
 
 // TODO(b/165344346): Migrate to use suggest hints API.
 /** Helper class to find hints for {@link MissingImplementationError}. */
@@ -33,6 +37,15 @@ private MissingImplementationErrorHints() {}
   /** When a binding is not found, show at most this many bindings that have some similarities */
   private static final int MAX_RELATED_TYPES_REPORTED = 3;
 
+  private static final String WILDCARD_EXTENDS = "? extends ";
+  private static final String WILDCARD_SUPER = "? super ";
+
+  private static final String WILDCARDS_WARNING =
+      "\nYou might be running into a @JvmSuppressWildcards or @JvmWildcards issue.";
+
+  private static final String WILDCARDS_POSSIBLE_FIXES =
+      "\nConsider these options instead (these are guesses but use your best judgment):";
+
   /**
    * If the key is unknown and it is one of these types, it generally means there is a missing
    * annotation.
@@ -52,13 +65,23 @@ private MissingImplementationErrorHints() {}
    * generic arguments (e.g. Optional<String> won't be similar to Optional<Integer>).
    */
   static boolean areSimilarLookingTypes(Type a, Type b) {
+    return areSimilarTypes(
+        a, b, (aClass, bClass) -> aClass.getSimpleName().equals(bClass.getSimpleName()));
+  }
+
+  private static boolean areOnlyDifferencesInVariance(Type a, Type b) {
+    return areSimilarTypes(a, b, Object::equals);
+  }
+
+  private static boolean areSimilarTypes(
+      Type a, Type b, BiFunction<Class<?>, Class<?>, Boolean> classSimilarityChecker) {
     if (a instanceof Class && b instanceof Class) {
-      return ((Class) a).getSimpleName().equals(((Class) b).getSimpleName());
+      return classSimilarityChecker.apply((Class<?>) a, (Class<?>) b);
     }
     if (a instanceof ParameterizedType && b instanceof ParameterizedType) {
       ParameterizedType aType = (ParameterizedType) a;
       ParameterizedType bType = (ParameterizedType) b;
-      if (!areSimilarLookingTypes(aType.getRawType(), bType.getRawType())) {
+      if (!areSimilarTypes(aType.getRawType(), bType.getRawType(), classSimilarityChecker)) {
         return false;
       }
       Type[] aArgs = aType.getActualTypeArguments();
@@ -67,7 +90,7 @@ static boolean areSimilarLookingTypes(Type a, Type b) {
         return false;
       }
       for (int i = 0; i < aArgs.length; i++) {
-        if (!areSimilarLookingTypes(aArgs[i], bArgs[i])) {
+        if (!areSimilarTypes(aArgs[i], bArgs[i], classSimilarityChecker)) {
           return false;
         }
       }
@@ -76,8 +99,8 @@ static boolean areSimilarLookingTypes(Type a, Type b) {
     if (a instanceof GenericArrayType && b instanceof GenericArrayType) {
       GenericArrayType aType = (GenericArrayType) a;
       GenericArrayType bType = (GenericArrayType) b;
-      return areSimilarLookingTypes(
-          aType.getGenericComponentType(), bType.getGenericComponentType());
+      return areSimilarTypes(
+          aType.getGenericComponentType(), bType.getGenericComponentType(), classSimilarityChecker);
     }
     if (a instanceof WildcardType && b instanceof WildcardType) {
       WildcardType aType = (WildcardType) a;
@@ -88,7 +111,7 @@ static boolean areSimilarLookingTypes(Type a, Type b) {
         return false;
       }
       for (int i = 0; i < aLowerBounds.length; i++) {
-        if (!areSimilarLookingTypes(aLowerBounds[i], bLowerBounds[i])) {
+        if (!areSimilarTypes(aLowerBounds[i], bLowerBounds[i], classSimilarityChecker)) {
           return false;
         }
       }
@@ -98,7 +121,7 @@ static boolean areSimilarLookingTypes(Type a, Type b) {
         return false;
       }
       for (int i = 0; i < aUpperBounds.length; i++) {
-        if (!areSimilarLookingTypes(aUpperBounds[i], bUpperBounds[i])) {
+        if (!areSimilarTypes(aUpperBounds[i], bUpperBounds[i], classSimilarityChecker)) {
           return false;
         }
       }
@@ -113,53 +136,165 @@ static boolean areSimilarLookingTypes(Type a, Type b) {
       Type[] lowerBounds = wildcardType.getLowerBounds();
       if (upperBounds.length == 1 && lowerBounds.length == 0) {
         // This is the '? extends Foo' case
-        return areSimilarLookingTypes(upperBounds[0], otherType);
+        return areSimilarTypes(upperBounds[0], otherType, classSimilarityChecker);
       }
       if (lowerBounds.length == 1
           && upperBounds.length == 1
           && upperBounds[0].equals(Object.class)) {
         // this is the '? super Foo' case
-        return areSimilarLookingTypes(lowerBounds[0], otherType);
+        return areSimilarTypes(lowerBounds[0], otherType, classSimilarityChecker);
       }
     }
     return false;
   }
 
-  static <T> ImmutableList<String> getSuggestions(Key<T> key, Injector injector) {
+  /**
+   * Conceptually, this method converts aType to be like bType by adding
+   * appropriate @JvmSuppressWildcards or @JvmWildcards annotations. This assumes that the two types
+   * only differ in variance (e.g. `Foo` vs `? extends Foo`) and that aType is implemented in
+   * Kotlin. For example, if this method is called with the (List&lt;? super String&gt;,
+   * List&lt;String&gt;), it will return "List&lt;@JvmSuppressWildcards String&gt;".
+   */
+  static String convertToLatterViaJvmAnnotations(TypeLiteral<?> aType, TypeLiteral<?> bType) {
+    String a = aType.toString();
+    String b = bType.toString();
+    int j = 0;
+    StringBuilder conversion = new StringBuilder();
+    for (int i = 0; i < a.length() && j < b.length(); i++, j++) {
+      if (a.charAt(i) != b.charAt(j)) {
+        if (a.startsWith(WILDCARD_EXTENDS, i)) {
+          conversion.append("@JvmSuppressWildcards ");
+          i += WILDCARD_EXTENDS.length(); // Skip over the "? extends " part
+        } else if (a.startsWith(WILDCARD_SUPER, i)) {
+          conversion.append("@JvmSuppressWildcards ");
+          i += WILDCARD_SUPER.length(); // Skip over the "? super " part
+        } else if (b.startsWith(WILDCARD_EXTENDS, j)) {
+          conversion.append("@JvmWildcards ");
+          j += WILDCARD_EXTENDS.length(); // Skip over the "? extends " part
+        } else if (b.startsWith(WILDCARD_SUPER, j)) {
+          conversion.append("@JvmWildcards ");
+          j += WILDCARD_SUPER.length(); // Skip over the "? super " part
+        }
+      }
+      conversion.append(a.charAt(i));
+    }
+
+    // convert any remaining wildcard types to the Kotlin-equivalent.
+    return conversion.toString().replaceAll("\\? extends ", "out ").replaceAll("\\? super ", "in ");
+  }
+
+  private static boolean wasBoundInKotlin(Binding<?> binding) {
+    if (binding.getSource() instanceof ElementSource) {
+      ElementSource elementSource = (ElementSource) binding.getSource();
+      Object declaringSource = elementSource.getDeclaringSource();
+      if (declaringSource instanceof Member) {
+        Member member = (Member) declaringSource;
+        if (KotlinSupport.getInstance().isKotlinClass(member.getDeclaringClass())) {
+          return true;
+        }
+      }
+      if (declaringSource instanceof StackTraceElement) {
+        StackTraceElement stackTraceElement = (StackTraceElement) declaringSource;
+        if (stackTraceElement.getFileName() != null
+            && stackTraceElement.getFileName().endsWith(".kt")) {
+          return true;
+        }
+      }
+    }
+    return false;
+  }
+
+  private static boolean isInjectionFromKotlin(List<Object> sources) {
+    for (Object source : sources) {
+      if (!(source instanceof Dependency)) {
+        continue;
+      }
+      Dependency<?> dependency = (Dependency<?>) source;
+      if (KotlinSupport.getInstance()
+          .isKotlinClass(dependency.getInjectionPoint().getMember().getDeclaringClass())) {
+        return true;
+      }
+    }
+    return false;
+  }
+
+  static <T> ImmutableList<String> getSuggestions(
+      Key<T> key, Injector injector, List<Object> sources) {
     ImmutableList.Builder<String> suggestions = ImmutableList.builder();
     TypeLiteral<T> type = key.getTypeLiteral();
 
     BindingSourceRestriction.getMissingImplementationSuggestion(GuiceInternal.GUICE_INTERNAL, key)
         .ifPresent(suggestions::add);
 
-    // Keys which have similar strings as the desired key
-    List<String> possibleMatches = new ArrayList<>();
-    ImmutableList<Binding<?>> similarTypes =
+    boolean injectionFromKotlin = isInjectionFromKotlin(sources);
+
+    // Keys which have a similar appearance as the desired key
+    ImmutableList<Binding<?>> similarBindings =
         injector.getAllBindings().values().stream()
             .filter(b -> !(b instanceof UntargettedBinding)) // These aren't valid matches
             .filter(
                 b -> areSimilarLookingTypes(b.getKey().getTypeLiteral().getType(), type.getType()))
             .collect(toImmutableList());
-    if (!similarTypes.isEmpty()) {
+
+    // Bindings which differ only in variance.
+    ImmutableList<Binding<?>> wildcardSuggestionBindings =
+        similarBindings.stream()
+            .filter(
+                b ->
+                    (injectionFromKotlin || wasBoundInKotlin(b))
+                        && areOnlyDifferencesInVariance(
+                            b.getKey().getTypeLiteral().getType(), type.getType()))
+            .collect(toImmutableList());
+
+    if (!wildcardSuggestionBindings.isEmpty()) {
+      suggestions.add(WILDCARDS_WARNING);
+      suggestions.add(WILDCARDS_POSSIBLE_FIXES);
+      for (Binding<?> wildcardSuggestionBinding : wildcardSuggestionBindings) {
+        TypeLiteral<?> similarType = wildcardSuggestionBinding.getKey().getTypeLiteral();
+
+        if (injectionFromKotlin) {
+          suggestions.add(
+              "\n    * Inject this: " + convertToLatterViaJvmAnnotations(type, similarType));
+        }
+        if (wasBoundInKotlin(wildcardSuggestionBinding)) {
+          suggestions
+              .add("\n    * ")
+              .add(injectionFromKotlin ? "Or bind this: " : "Bind this: ")
+              .add(
+                  Messages.format(
+                      "%s %s",
+                      convertToLatterViaJvmAnnotations(similarType, type),
+                      formatVarianceSuggestion(wildcardSuggestionBinding)));
+        }
+      }
+    }
+
+    similarBindings =
+        similarBindings.stream()
+            .filter(b -> !wildcardSuggestionBindings.contains(b))
+            .collect(toImmutableList());
+
+    List<String> possibleMatches = new ArrayList<>();
+    if (!similarBindings.isEmpty()) {
       suggestions.add("\nDid you mean?");
-      int howMany = min(similarTypes.size(), MAX_MATCHING_TYPES_REPORTED);
+      int howMany = min(similarBindings.size(), MAX_MATCHING_TYPES_REPORTED);
       for (int i = 0; i < howMany; ++i) {
-        Key<?> bindingKey = similarTypes.get(i).getKey();
+        Key<?> bindingKey = similarBindings.get(i).getKey();
         // TODO: Look into a better way to prioritize suggestions. For example, possibly
         // use levenshtein distance of the given annotation vs actual annotation.
         suggestions.add(
             Messages.format(
                 "\n    * %s",
                 formatSuggestion(bindingKey, injector.getExistingBinding(bindingKey))));
       }
-      int remaining = similarTypes.size() - MAX_MATCHING_TYPES_REPORTED;
+      int remaining = similarBindings.size() - MAX_MATCHING_TYPES_REPORTED;
       if (remaining > 0) {
         String plural = (remaining == 1) ? "" : "s";
         suggestions.add(
             Messages.format(
                 "\n    * %d more binding%s with other annotations.", remaining, plural));
       }
-    } else {
+    } else if (wildcardSuggestionBindings.isEmpty()) {
       // For now, do a simple substring search for possibilities. This can help spot
       // issues when there are generics being used (such as a wrapper class) and the
       // user has forgotten they need to bind based on the wrapper, not the underlying
@@ -196,8 +331,9 @@ static <T> ImmutableList<String> getSuggestions(Key<T> key, Injector injector) {
 
     // If where are no possibilities to suggest, then handle the case of missing
     // annotations on simple types. This is usually a bad idea.
-    if (similarTypes.isEmpty()
+    if (similarBindings.isEmpty()
         && possibleMatches.isEmpty()
+        && wildcardSuggestionBindings.isEmpty()
         && key.getAnnotationType() == null
         && COMMON_AMBIGUOUS_TYPES.contains(key.getTypeLiteral().getRawType())) {
       // We don't recommend using such simple types without annotations.
@@ -213,4 +349,10 @@ private static String formatSuggestion(Key<?> key, Binding<?> binding) {
     new SourceFormatter(binding.getSource(), fmt, /* omitPreposition= */ false).format();
     return fmt.toString();
   }
+
+  private static String formatVarianceSuggestion(Binding<?> binding) {
+    Formatter fmt = new Formatter();
+    new SourceFormatter(binding.getSource(), fmt, /* omitPreposition= */ false).format();
+    return fmt.toString();
+  }
 }

core/test/com/google/inject/errors/ErrorMessageTestUtils.java

@@ -47,6 +47,7 @@ private static String getExpectedError(String fileName) throws IOException {
           ErrorMessageTestUtils.class.getResource(
               "/core/test/com/google/inject/errors/testdata/" + fileName);
     }
-    return Resources.toString(resource, UTF_8);
+    String expectedError = Resources.toString(resource, UTF_8);
+    return expectedError;
   }
 }