Simplify naming for TypeVariables.

PiperOrigin-RevId: 233117394

transpiler/java/com/google/j2cl/ast/AstUtils.java

@@ -1020,7 +1020,8 @@ private static Method devirtualizeMethod(
     checkArgument(
         !method.getDescriptor().isJsPropertyGetter()
             && !method.getDescriptor().isJsPropertySetter(),
-        "JsPropery getter and setters should never be devirtualized " + method);
+        "JsPropery getter and setters should never be devirtualized %s",
+        method.getReadableDescription());
     checkArgument(method.getDescriptor().isPolymorphic());
     checkArgument(!method.getDescriptor().isInit(), "Do not devirtualize init().");
 

transpiler/java/com/google/j2cl/ast/AstUtilConstants.java → transpiler/java/com/google/j2cl/ast/HasName.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2016 Google Inc.
+ * Copyright 2019 Google 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
@@ -15,12 +15,8 @@
  */
 package com.google.j2cl.ast;
 
-
-/** Constants supporting manipulation of J2CL AST. */
-public class AstUtilConstants {
-  public static final String OVERLAY_IMPLEMENTATION_CLASS_SUFFIX = "Overlay";
-  public static final String FUNCTIONAL_INTERFACE_ADAPTOR_CLASS_NAME = "LambdaAdaptor";
-  public static final String FUNCTIONAL_INTERFACE_JSFUNCTION_CLASS_NAME = "JsFunction";
-  public static final String TYPE_VARIABLE_IN_METHOD_PREFIX = "M_";
-  public static final String TYPE_VARIABLE_IN_TYPE_PREFIX = "C_";
+/** Implemented by nodes that have names. */
+public interface HasName {
+  /** Returns its name. */
+  String getName();
 }

transpiler/java/com/google/j2cl/ast/LambdaTypeDescriptors.java

@@ -31,6 +31,8 @@
 /** Utility TypeDescriptors methods related to lambda synthesis. */
 // TODO(b/63118697): Simplify this code once TD refactoring makes it easier to implement.
 public class LambdaTypeDescriptors {
+  private static final String FUNCTIONAL_INTERFACE_JSFUNCTION_CLASS_NAME = "JsFunction";
+  private static final String FUNCTIONAL_INTERFACE_ADAPTOR_CLASS_NAME = "LambdaAdaptor";
 
   /** Returns the TypeDescriptor for a LambdaAdaptor class. */
   public static DeclaredTypeDescriptor createLambdaAdaptorTypeDescriptor(
@@ -114,7 +116,7 @@ private static TypeDeclaration createLambdaAdaptorTypeDeclaration(
     List<String> classComponents =
         AstUtils.synthesizeInnerClassComponents(
             enclosingTypeDescriptor,
-            AstUtilConstants.FUNCTIONAL_INTERFACE_ADAPTOR_CLASS_NAME,
+            FUNCTIONAL_INTERFACE_ADAPTOR_CLASS_NAME,
             uniqueId.orElse(null));
 
     ImmutableList<TypeVariable> typeParameterDescriptors =
@@ -256,7 +258,7 @@ private static TypeDeclaration createJsFunctionTypeDeclaration(
 
     List<String> classComponents =
         AstUtils.synthesizeInnerClassComponents(
-            functionalTypeDescriptor, AstUtilConstants.FUNCTIONAL_INTERFACE_JSFUNCTION_CLASS_NAME);
+            functionalTypeDescriptor, FUNCTIONAL_INTERFACE_JSFUNCTION_CLASS_NAME);
 
     return TypeDeclaration.newBuilder()
         .setEnclosingTypeDeclaration(functionalTypeDescriptor.getTypeDeclaration())

transpiler/java/com/google/j2cl/ast/MethodDescriptor.java

@@ -730,7 +730,7 @@ public MethodDescriptor build() {
                   == methodDescriptor.getParameterTypeDescriptors().size(),
               "Method parameters (%s) for method %s don't match method declaration (%s)",
               methodDescriptor.getParameterTypeDescriptors(),
-              methodDescriptor.getEnclosingTypeDescriptor().getSimpleSourceName()
+              methodDescriptor.getEnclosingTypeDescriptor().getReadableDescription()
                   + "."
                   + methodDescriptor.getName(),
               methodDeclarationParameterTypeDescriptors);

transpiler/java/com/google/j2cl/ast/TypeDescriptors.java

@@ -32,6 +32,8 @@
 
 /** Utility class holding type descriptors that need to be referenced directly. */
 public class TypeDescriptors {
+  private static final String OVERLAY_IMPLEMENTATION_CLASS_SUFFIX = "Overlay";
+
   public DeclaredTypeDescriptor javaLangBoolean;
   public DeclaredTypeDescriptor javaLangByte;
   public DeclaredTypeDescriptor javaLangCharacter;
@@ -271,7 +273,7 @@ static TypeDeclaration createOverlayImplementationTypeDeclaration(
 
     List<String> classComponents =
         AstUtils.synthesizeInnerClassComponents(
-            unparameterizedTypeDescriptor, AstUtilConstants.OVERLAY_IMPLEMENTATION_CLASS_SUFFIX);
+            unparameterizedTypeDescriptor, OVERLAY_IMPLEMENTATION_CLASS_SUFFIX);
 
     return TypeDeclaration.newBuilder()
         .setEnclosingTypeDeclaration(unparameterizedTypeDescriptor.getTypeDeclaration())

transpiler/java/com/google/j2cl/ast/TypeVariable.java

@@ -17,15 +17,11 @@
 
 import com.google.auto.value.AutoValue;
 import com.google.auto.value.extension.memoized.Memoized;
-import com.google.common.base.Joiner;
-import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Iterables;
 import com.google.j2cl.ast.annotations.Visitable;
 import com.google.j2cl.common.ThreadLocalInterner;
-import java.util.ArrayList;
-import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
 import java.util.function.Function;
 import java.util.function.Supplier;
@@ -42,14 +38,9 @@
  */
 @AutoValue
 @Visitable
-public abstract class TypeVariable extends TypeDescriptor {
-  public abstract ImmutableList<String> getNameComponents();
+public abstract class TypeVariable extends TypeDescriptor implements HasName {
 
-  public DeclaredTypeDescriptor getEnclosingTypeDescriptor() {
-    return getEnclosingTypeDescriptorSupplier().get();
-  }
-
-  public abstract Supplier<DeclaredTypeDescriptor> getEnclosingTypeDescriptorSupplier();
+  public abstract String getName();
 
   @Memoized
   public TypeDescriptor getBoundTypeDescriptor() {
@@ -59,29 +50,6 @@ public TypeDescriptor getBoundTypeDescriptor() {
 
   public abstract Supplier<TypeDescriptor> getBoundTypeDescriptorSupplier();
 
-  public String getJsName() {
-    // Template variable like "C_T".
-
-    // TODO(b/68715725): Clean up naming for type variables so that no special handling is needed
-    // here.
-
-    // skip the top level class component for better output readability.
-    List<String> classComponents = getNameComponents();
-    List<String> nameComponents =
-        new ArrayList<>(classComponents.subList(1, classComponents.size()));
-
-    // move the prefix in the simple name to the class name to avoid collisions between method-
-    // level and class-level type variable and avoid variable name starts with a number.
-    // concat class components to avoid collisions between type variables in inner/outer class.
-    // use '_' instead of '$' because '$' is not allowed in template variable name in closure.
-    String simpleName = getSourceName();
-    nameComponents.set(
-        nameComponents.size() - 1, simpleName.substring(simpleName.indexOf('_') + 1));
-    String prefix = simpleName.substring(0, simpleName.indexOf('_') + 1);
-
-    return prefix + Joiner.on('_').join(nameComponents);
-  }
-
   @Nullable
   abstract String getUniqueKey();
 
@@ -163,15 +131,11 @@ public TypeDescriptor specializeTypeVariables(
 
   @Override
   public String getReadableDescription() {
-    // TODO(b/114074816): Remove this hack when modeling of type variables is improved and the name
-    // is actually the source name of the variable and does not encode extra information.
-    int lastUnderscore = getSourceName().lastIndexOf("_");
-    return getSourceName().substring(lastUnderscore + 1);
+    return getName();
   }
 
-  @Memoized
   public String getSourceName() {
-    return Iterables.getLast(getNameComponents());
+    return getName();
   }
 
   @Override
@@ -191,29 +155,48 @@ public static Builder newBuilder() {
     return new AutoValue_TypeVariable.Builder().setWildcardOrCapture(false);
   }
 
+  /** Creates a wildcard type variable with a specific bound. */
+  public static TypeVariable createWildcardWithBound(TypeDescriptor bound) {
+    return TypeVariable.newBuilder()
+        .setWildcardOrCapture(true)
+        .setBoundTypeDescriptorSupplier(() -> bound)
+        // Create an unique key that does not conflict with the keys used for other types nor for
+        // type variables coming from JDT, which follow "<declaring_type>:<name>...".
+        // {@see org.eclipse.jdt.core.BindingKey}.
+        .setUniqueKey("<??>" + bound.getUniqueId())
+        .setName("?")
+        .build();
+  }
+
   /** Builder for a TypeVariableDeclaration. */
   @AutoValue.Builder
   public abstract static class Builder {
 
-    public abstract Builder setEnclosingTypeDescriptorSupplier(
-        Supplier<DeclaredTypeDescriptor> enclosingTypeDescriptorSupplier);
-
     public abstract Builder setBoundTypeDescriptorSupplier(
         Supplier<TypeDescriptor> boundTypeDescriptorFactory);
 
     public abstract Builder setUniqueKey(String uniqueKey);
 
-    public abstract Builder setNameComponents(Iterable<String> name);
+    public abstract Builder setName(String name);
 
     public abstract Builder setWildcardOrCapture(boolean isWildcardOrCapture);
 
+    // Internal builder accessors to compute default values.
+    abstract boolean isWildcardOrCapture();
+
+    abstract Optional<String> getUniqueKey();
+
+    abstract Supplier<TypeDescriptor> getBoundTypeDescriptorSupplier();
+
+    abstract Optional<String> getName();
+
     private static final ThreadLocalInterner<TypeVariable> interner = new ThreadLocalInterner<>();
 
     abstract TypeVariable autoBuild();
 
     public TypeVariable build() {
-      TypeVariable typeDeclaration = autoBuild();
-      return interner.intern(typeDeclaration);
+      TypeVariable typeVariable = autoBuild();
+      return interner.intern(typeVariable);
     }
 
     public static Builder from(TypeVariable typeVariable) {

transpiler/java/com/google/j2cl/ast/Variable.java

@@ -24,7 +24,8 @@
 
 /** Class for local variable and parameter. */
 @Visitable
-public class Variable extends Node implements Cloneable<Variable>, HasUnusableByJsSuppression {
+public class Variable extends Node
+    implements Cloneable<Variable>, HasUnusableByJsSuppression, HasName {
   private final String name;
   @Visitable TypeDescriptor typeDescriptor;
   private final boolean isFinal;

transpiler/java/com/google/j2cl/ast/visitors/NormalizeFunctionExpressions.java

@@ -28,7 +28,6 @@
 import com.google.j2cl.ast.Variable;
 import com.google.j2cl.ast.VariableDeclarationExpression;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.List;
 
 /**
@@ -75,7 +74,8 @@ private FunctionExpression rewriteParameters(FunctionExpression functionExpressi
         // Create a new parameter variable of the type "? extends DeclaredType", this takes
         // care of having the right parameter type without the problem of introducing type
         // variables that are not in the current context. (this is a hack).
-        TypeVariable targetType = createWildcardTypeVariable(declaredParameterTypeDescriptor);
+        TypeVariable targetType =
+            TypeVariable.createWildcardWithBound(declaredParameterTypeDescriptor);
 
         Variable newParameter =
             Variable.Builder.from(parameter).setTypeDescriptor(targetType).build();
@@ -106,17 +106,6 @@ private FunctionExpression rewriteParameters(FunctionExpression functionExpressi
         .build();
   }
 
-  /** Synthesizes a wildcard type variable with a specific bound. */
-  private static TypeVariable createWildcardTypeVariable(TypeDescriptor bound) {
-    return TypeVariable.newBuilder()
-        .setWildcardOrCapture(true)
-        .setBoundTypeDescriptorSupplier(() -> bound)
-        .setNameComponents(Arrays.asList("?"))
-        .setUniqueKey("<??>" + bound.getUniqueId())
-        .setEnclosingTypeDescriptorSupplier(() -> null)
-        .build();
-  }
-
   /**
    * Rewrites return statements in {@code functionExpression} to be of the declared return type
    * instead of the type inferred.

transpiler/java/com/google/j2cl/frontend/JdtUtils.java

@@ -25,7 +25,6 @@
 import com.google.common.collect.Iterables;
 import com.google.j2cl.ast.ArrayLength;
 import com.google.j2cl.ast.ArrayTypeDescriptor;
-import com.google.j2cl.ast.AstUtilConstants;
 import com.google.j2cl.ast.BinaryOperator;
 import com.google.j2cl.ast.DeclaredTypeDescriptor;
 import com.google.j2cl.ast.Expression;
@@ -396,11 +395,9 @@ private static TypeDescriptor createTypeVariable(ITypeBinding typeBinding) {
 
     return TypeVariable.newBuilder()
         .setBoundTypeDescriptorSupplier(boundTypeDescriptorFactory)
-        .setEnclosingTypeDescriptorSupplier(
-            () -> (DeclaredTypeDescriptor) createTypeDescriptor(typeBinding.getDeclaringClass()))
         .setWildcardOrCapture(typeBinding.isWildcardType() || typeBinding.isCapture())
         .setUniqueKey(typeBinding.getKey())
-        .setNameComponents(getClassComponentsForTypeVariable(typeBinding))
+        .setName(typeBinding.getName())
         .build();
   }
 
@@ -476,45 +473,11 @@ private static boolean isIntersectionType(ITypeBinding binding) {
         && !binding.isWildcardType();
   }
 
-  private static ImmutableList<String> getClassComponentsForTypeVariable(ITypeBinding typeBinding) {
-    if (typeBinding.isWildcardType() || typeBinding.isCapture()) {
-      return ImmutableList.of("?");
-    }
-    checkArgument(typeBinding.isTypeVariable());
-    if (typeBinding.getDeclaringClass() != null) {
-      // This is a class-level type variable. Use its name prefixed with "C_" as its simple
-      // name component and gather enclosing components from the enclosing class hierarchy.
-      return ImmutableList.<String>builder()
-          .addAll(getClassComponents(typeBinding.getDeclaringClass()))
-          .add(AstUtilConstants.TYPE_VARIABLE_IN_TYPE_PREFIX + typeBinding.getName())
-          .build();
-    } else {
-      // This is a method-level type variable. Use its simple name prefixed with "M_") as its
-      // simple name component, replace the immediate enclosing component with
-      // <EnclosingClass>_<EnclosingComponent> and then continue normally through the enclosing
-      // class hierarchy.
-      return ImmutableList.<String>builder()
-          .addAll(
-              getClassComponents(
-                  typeBinding.getDeclaringMethod().getDeclaringClass().getDeclaringClass()))
-          .add(
-              typeBinding.getDeclaringMethod().getDeclaringClass().getName()
-                  + "_"
-                  + typeBinding.getDeclaringMethod().getName())
-          .add(AstUtilConstants.TYPE_VARIABLE_IN_METHOD_PREFIX + typeBinding.getName())
-          .build();
-    }
-  }
-
   private static List<String> getClassComponents(ITypeBinding typeBinding) {
     List<String> classComponents = new ArrayList<>();
     ITypeBinding currentType = typeBinding;
     while (currentType != null) {
-      checkArgument(
-          !currentType.isTypeVariable()
-              && !currentType.isWildcardType()
-              && !currentType.isCapture());
-      String simpleName;
+      checkArgument(currentType.getTypeDeclaration() != null);
       if (currentType.isLocal()) {
         // JDT binary name for local class is like package.components.EnclosingClass$1SimpleName
         // Extract the generated name by taking the part after the binary name of the declaring
@@ -523,11 +486,10 @@ private static List<String> getClassComponents(ITypeBinding typeBinding) {
         String declaringClassPrefix =
             getBinaryNameFromTypeBinding(currentType.getDeclaringClass()) + "$";
         checkState(binaryName.startsWith(declaringClassPrefix));
-        simpleName = binaryName.substring(declaringClassPrefix.length());
+        classComponents.add(0, binaryName.substring(declaringClassPrefix.length()));
       } else {
-        simpleName = currentType.getErasure().getName();
+        classComponents.add(0, currentType.getName());
       }
-      classComponents.add(0, simpleName);
       currentType = currentType.getDeclaringClass();
     }
     return classComponents;
@@ -704,6 +666,10 @@ private static boolean isLambdaBinding(IBinding binding) {
 
   /** Create a MethodDescriptor directly based on the given JDT method binding. */
   public static MethodDescriptor createMethodDescriptor(IMethodBinding methodBinding) {
+    if (methodBinding == null) {
+      return null;
+    }
+
     DeclaredTypeDescriptor enclosingTypeDescriptor =
         createDeclaredTypeDescriptor(methodBinding.getDeclaringClass());
 

transpiler/java/com/google/j2cl/generator/ClosureTypesGenerator.java

@@ -160,12 +160,12 @@ private ClosureType getClosureTypeForPrimitive(PrimitiveTypeDescriptor typeDescr
   }
 
   /** Returns the template variable name for a type variable for use in JsDoc annotations. */
-  private static ClosureType getClosureTypeForTypeVariable(TypeVariable typeVariable) {
+  private ClosureType getClosureTypeForTypeVariable(TypeVariable typeVariable) {
     if (typeVariable.isWildcardOrCapture()) {
       return UNKNOWN;
     }
 
-    return new ClosureNamedType(typeVariable.getJsName());
+    return new ClosureNamedType(environment.getUniqueNameForVariable(typeVariable));
   }
 
   /** Returns the Closure type for an array type descriptor. */

transpiler/java/com/google/j2cl/generator/ExpressionTranspiler.java

@@ -465,14 +465,14 @@ public Void transformVariable(Variable variable) {
         sourceBuilder.emitWithMapping(
             // Only map variables if they are named.
             AstUtils.emptySourcePositionIfNotNamed(variable.getSourcePosition()),
-            () -> sourceBuilder.append(environment.aliasForVariable(variable)));
+            () -> sourceBuilder.append(environment.getUniqueNameForVariable(variable)));
 
         return null;
       }
 
       @Override
       public Void transformVariableReference(VariableReference expression) {
-        sourceBuilder.append(environment.aliasForVariable(expression.getTarget()));
+        sourceBuilder.append(environment.getUniqueNameForVariable(expression.getTarget()));
         return null;
       }