Fix typedef + indirect params typedef in SoyJS and SoyIDOM by omittin…

…g params of type NAMED/INDEXED from indirect types. Make it a compiler error to declare an INDEXED type on a non-existent field. PiperOrigin-RevId: 693715255
google · Nov 6, 2024 · e232d97 · e232d97
1 parent 108afcb
commit e232d97
Show file tree

Hide file tree

Showing 13 changed files with 129 additions and 34 deletions.
diff --git a/java/src/com/google/template/soy/javagencode/SoyFileNodeTransformer.java b/java/src/com/google/template/soy/javagencode/SoyFileNodeTransformer.java
@@ -25,7 +25,6 @@
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Streams;
 import com.google.template.soy.base.SourceFilePath;
 import com.google.template.soy.base.SourceLocation;
 import com.google.template.soy.base.SourceLogicalPath;
@@ -402,7 +401,7 @@ private void updateParamStatuses(Map<String, ParamInfo> params) {
         continue;
       }
 
-      if (Streams.stream(SoyTypes.getTypeTraverser(param.type(), null))
+      if (SoyTypes.allTypesWithDeps(param.type(), null)
           .map(
               t -> {
                 if (t instanceof SoyProtoType) {

diff --git a/java/src/com/google/template/soy/javagencode/javatypes/JavaTypeUtils.java b/java/src/com/google/template/soy/javagencode/javatypes/JavaTypeUtils.java
@@ -22,7 +22,6 @@
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
-import com.google.common.collect.Streams;
 import com.google.template.soy.types.AbstractIterableType;
 import com.google.template.soy.types.AbstractMapType;
 import com.google.template.soy.types.FloatType;
@@ -195,7 +194,7 @@ private static ImmutableList<JavaType> trySimpleRecordType(
     }
 
     // No records of records.
-    if (Streams.stream(SoyTypes.getTypeTraverser(recordType, null))
+    if (SoyTypes.allTypes(recordType, null)
         .anyMatch(t -> t.getKind() == Kind.RECORD && t != recordType)) {
       return ImmutableList.of();
     }

diff --git a/java/src/com/google/template/soy/jssrc/internal/AliasUtils.java b/java/src/com/google/template/soy/jssrc/internal/AliasUtils.java
@@ -39,7 +39,7 @@ private static final class Aliases implements TemplateAliases {
     }
 
     @Override
-    public String get(String fullyQualifiedName) {
+    public String getNullable(String fullyQualifiedName) {
       String alias = aliasMapping.get(fullyQualifiedName);
       if (alias != null) {
         return alias;
@@ -48,7 +48,11 @@ public String get(String fullyQualifiedName) {
       checkState(lastDotPosition != -1);
       String namespace = fullyQualifiedName.substring(0, lastDotPosition);
       String relativeName = fullyQualifiedName.substring(lastDotPosition + 1);
-      return getNamespaceAlias(namespace) + '.' + relativeName;
+      String namespaceAlias = getNamespaceAlias(namespace);
+      if (namespaceAlias == null) {
+        return null;
+      }
+      return namespaceAlias + '.' + relativeName;
     }
 
     @Override
@@ -66,6 +70,11 @@ public String get(String fullyQualifiedName) {
           return fullyQualifiedName;
         }
 
+        @Override
+        public String getNullable(String fullyQualifiedName) {
+          return null;
+        }
+
         @Override
         public String getNamespaceAlias(String namespace) {
           throw new UnsupportedOperationException();

diff --git a/java/src/com/google/template/soy/jssrc/internal/GenJsCodeVisitor.java b/java/src/com/google/template/soy/jssrc/internal/GenJsCodeVisitor.java
@@ -55,6 +55,7 @@
 import com.google.template.soy.base.SourceLocation.ByteSpan;
 import com.google.template.soy.base.SourceLogicalPath;
 import com.google.template.soy.base.internal.SanitizedContentKind;
+import com.google.template.soy.base.internal.SoyFileKind;
 import com.google.template.soy.error.ErrorReporter;
 import com.google.template.soy.error.SoyErrorKind;
 import com.google.template.soy.exprtree.ExprNode;
@@ -507,7 +508,7 @@ private static ByteSpan getNamespaceByteSpan(SoyFileNode soyFile) {
    * @param soyNamespace The namespace as declared by the user.
    */
   protected String getGoogModuleNamespace(String soyNamespace) {
-    return soyNamespace;
+    return Preconditions.checkNotNull(soyNamespace);
   }
 
   /**
@@ -1641,6 +1642,23 @@ private String genParamsRecordType(TemplateNode node) {
         // template with the param. This matches what do for the Java builders.
         continue;
       }
+      // Skip named types from indirect deps.
+      if (SoyTypes.allTypes(combinedType, typeRegistry)
+          .anyMatch(
+              t -> {
+                if (t instanceof NamedType) {
+                  NamedType namedType = (NamedType) t;
+                  SourceLogicalPath path =
+                      fileSetMetadata.getPathForNamespace(namedType.getNamespace());
+                  if (path == null) {
+                    return true;
+                  }
+                  return fileSetMetadata.getFile(path).getSoyFileKind() == SoyFileKind.INDIRECT_DEP;
+                }
+                return false;
+              })) {
+        continue;
+      }
       // TODO: detect cases where nullable is not needed (requires flow
       // analysis to determine if the template is always called.)
       SoyType indirectParamType = SoyTypes.makeNullable(combinedType);
@@ -1814,7 +1832,10 @@ public JsType get(SoyType type) {
       }
       if (type instanceof NamedType) {
         NamedType namedType = (NamedType) type;
-        return JsType.localTypedef(templateAliases.get(namedType.getFqn()));
+        String alias = templateAliases.getNullable(namedType.getFqn());
+        if (alias != null) {
+          return JsType.localTypedef(alias);
+        }
       }
       return delegate.get(type, this);
     }

diff --git a/java/src/com/google/template/soy/jssrc/internal/TemplateAliases.java b/java/src/com/google/template/soy/jssrc/internal/TemplateAliases.java
@@ -16,6 +16,9 @@
 
 package com.google.template.soy.jssrc.internal;
 
+import com.google.common.base.Preconditions;
+import javax.annotation.Nullable;
+
 /**
  * Provides a way to look up local variable aliases for the JavaScript function that corresponds to
  * a given Soy template.
@@ -25,8 +28,14 @@ public interface TemplateAliases {
    * @param fullyQualifiedName The full name, including the namespace, of a Soy template.
    * @return The variable that should be used when referring to the template.
    */
-  String get(String fullyQualifiedName);
+  default String get(String fullyQualifiedName) {
+    return Preconditions.checkNotNull(getNullable(fullyQualifiedName));
+  }
+
+  @Nullable
+  String getNullable(String fullyQualifiedName);
 
   /** Returns the symbol that should be used as an alias for the soy template namespace. */
+  @Nullable
   String getNamespaceAlias(String namespace);
 }
diff --git a/java/src/com/google/template/soy/passes/MoreCallValidationsPass.java b/java/src/com/google/template/soy/passes/MoreCallValidationsPass.java
@@ -18,7 +18,6 @@
 
 import static com.google.common.collect.ImmutableList.toImmutableList;
 import static com.google.common.collect.ImmutableMap.toImmutableMap;
-import static com.google.common.collect.Streams.stream;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
@@ -187,7 +186,7 @@ private void checkTemplateLiteralsUsedInExpr(SoyFileNode file) {
                   return;
                 }
               }
-              stream(SoyTypes.getTypeTraverser(templateNode.getType(), null))
+              SoyTypes.allTypes(templateNode.getType(), null)
                   .filter(t -> t.getKind() == SoyType.Kind.TEMPLATE)
                   .map(TemplateType.class::cast)
                   .filter(

diff --git a/java/src/com/google/template/soy/passes/SoyElementCompositionPass.java b/java/src/com/google/template/soy/passes/SoyElementCompositionPass.java
@@ -237,7 +237,10 @@ private void process(TemplateNode template, HtmlTagNode tagNode, IdGenerator nod
       HtmlTagNode closeTag = tagNode.getTaggedPairs().get(0);
       List<String> params =
           templateType.getParameters().stream()
-              .filter(p -> SoyTypes.transitivelyContainsKind(p.getType(), SoyType.Kind.HTML))
+              .filter(
+                  p ->
+                      SoyTypes.transitivelyContainsKind(
+                          p.getType().getEffectiveType(), SoyType.Kind.HTML))
               .map(Parameter::getName)
               .collect(toCollection(ArrayList::new));
       StandaloneNode next = (StandaloneNode) SoyTreeUtils.nextSibling(tagNode);

diff --git a/java/src/com/google/template/soy/shared/internal/gencode/JavaGenerationUtils.java b/java/src/com/google/template/soy/shared/internal/gencode/JavaGenerationUtils.java
@@ -16,7 +16,6 @@
 package com.google.template.soy.shared.internal.gencode;
 
 import static com.google.common.collect.ImmutableSet.toImmutableSet;
-import static com.google.common.collect.Streams.stream;
 
 import com.google.common.base.Ascii;
 import com.google.common.base.Joiner;
@@ -43,7 +42,6 @@
 import com.google.template.soy.types.SoyTypeRegistry;
 import com.google.template.soy.types.SoyTypes;
 import java.util.Collection;
-import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -402,7 +400,7 @@ public static ImmutableSet<GenericDescriptor> getProtoTypes(
   /** Recursively search for protocol buffer types within the given type. */
   private static Stream<GenericDescriptor> findProtoTypes(
       SoyType root, SoyTypeRegistry typeRegistry) {
-    return stream(typeIterator(root, typeRegistry))
+    return SoyTypes.allTypesWithDeps(root, typeRegistry)
         .map(
             type -> {
               switch (type.getKind()) {
@@ -417,11 +415,6 @@ private static Stream<GenericDescriptor> findProtoTypes(
         .filter(Objects::nonNull);
   }
 
-  private static Iterator<? extends SoyType> typeIterator(
-      SoyType root, SoyTypeRegistry typeRegistry) {
-    return SoyTypes.getTypeTraverser(root, typeRegistry);
-  }
-
   /** Returns whether the given symbol is a keyword reserved by the Java language. */
   public static boolean isReservedKeyword(String symbol) {
     return RESERVED_JAVA_KEYWORDS.contains(symbol);

diff --git a/java/src/com/google/template/soy/soytree/FileSetMetadata.java b/java/src/com/google/template/soy/soytree/FileSetMetadata.java
@@ -16,7 +16,9 @@
 
 package com.google.template.soy.soytree;
 
+
 import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableMap;
 import com.google.template.soy.base.SourceLogicalPath;
 import com.google.template.soy.shared.internal.DelTemplateSelector;
 import com.google.template.soy.types.TemplateType.TemplateKind;
@@ -73,4 +75,10 @@ default TemplateMetadata getBasicTemplateOrElement(String templateFqn) {
     TemplateKind kind = metadata.getTemplateType().getTemplateKind();
     return kind == TemplateKind.BASIC || kind == TemplateKind.ELEMENT ? metadata : null;
   }
+
+  ImmutableMap<String, SourceLogicalPath> getNamespaceIndex();
+
+  default SourceLogicalPath getPathForNamespace(String namespace) {
+    return getNamespaceIndex().get(namespace);
+  }
 }
diff --git a/java/src/com/google/template/soy/soytree/Metadata.java b/java/src/com/google/template/soy/soytree/Metadata.java
@@ -245,6 +245,14 @@ public ImmutableCollection<TemplateMetadata> getAllTemplates() {
           .flatMap(f -> f.getTemplates().stream())
           .collect(toImmutableList());
     }
+
+    @Override
+    @Memoized
+    public ImmutableMap<String, SourceLogicalPath> getNamespaceIndex() {
+      return getAllFiles().stream()
+          .collect(
+              toImmutableMap(PartialFileMetadata::getNamespace, f -> f.getPath().asLogicalPath()));
+    }
   }
 
   /** PartialFileSetMetadata for AST under compilation. */
@@ -285,6 +293,7 @@ private static class AstFileSetMetadata implements FileSetMetadata {
     @LazyInit private ImmutableMap<String, TemplateMetadata> lazyFullTemplateIndex;
     @LazyInit private ImmutableList<TemplateMetadata> lazyAllTemplatesWithCollisions;
     @LazyInit private DelTemplateSelector<TemplateMetadata> delTemplateSelector;
+    @LazyInit private ImmutableMap<String, SourceLogicalPath> namespaceIndex;
 
     /** ASTs are mutable so we need to copy all data in the constructor. */
     public AstFileSetMetadata(FileSetMetadata deps, List<SoyFileNode> ast, ParseContext context) {
@@ -368,6 +377,20 @@ public ImmutableCollection<TemplateMetadata> getAllTemplates() {
       templateIndex(); // init lazyAllTemplatesWithCollisions
       return lazyAllTemplatesWithCollisions;
     }
+
+    @Override
+    public ImmutableMap<String, SourceLogicalPath> getNamespaceIndex() {
+      ImmutableMap<String, SourceLogicalPath> tmp = namespaceIndex;
+      if (tmp == null) {
+        tmp =
+            getAllFiles().stream()
+                .collect(
+                    toImmutableMap(
+                        PartialFileMetadata::getNamespace, f -> f.getPath().asLogicalPath()));
+        namespaceIndex = tmp;
+      }
+      return tmp;
+    }
   }
 
   abstract static class AbstractFileMetadata implements FileMetadata {
@@ -964,7 +987,7 @@ private static final class NamedTypeResolving extends DelegatingSoyTypeRegistry
 
     @Override
     public SoyType getOrCreateNamedType(String name, String namespace) {
-      String key = namespace + "." + name;
+      String key = Preconditions.checkNotNull(namespace) + "." + name;
       NamedType cachedVal = cache.get(key);
       if (cachedVal == null) {
         TypeDefP typeDef = namedTypes.get(key);

diff --git a/java/src/com/google/template/soy/types/SoyTypes.java b/java/src/com/google/template/soy/types/SoyTypes.java
@@ -23,7 +23,6 @@
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
-import com.google.common.collect.Streams;
 import com.google.common.html.types.SafeHtmlProto;
 import com.google.common.html.types.SafeScriptProto;
 import com.google.common.html.types.SafeStyleProto;
@@ -34,14 +33,14 @@
 import com.google.template.soy.types.SoyType.Kind;
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.Iterator;
 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.Predicate;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 import javax.annotation.Nullable;
 
 /** Utility methods for operating on {@link SoyType} instances. */
@@ -574,13 +573,20 @@ public SoyType resolve(SoyType left, SoyType right) {
   }
 
   /**
-   * Returns an iterator that traverses a soy type graph starting at {@code root} and following any
-   * union, list, map, record, or other composite type. The optional type registry parameter is used
-   * for resolving VE types.
+   * Returns an stream that traverses a soy type graph starting at {@code root} and following any
+   * union, list, map, record, or other composite type.
+   *
+   * <p>The optional type registry parameter is used for resolving VE types.
    */
-  public static Iterator<? extends SoyType> getTypeTraverser(
+  public static Stream<? extends SoyType> allTypes(
+      SoyType root, @Nullable SoyTypeRegistry registry) {
+    return TreeStreams.breadthFirst(root, new SoyTypeSuccessorsFunction(registry));
+  }
+
+  /** Like {@link #allTypes} but navigates to the effective types of INDEXED and NAMED. */
+  public static Stream<? extends SoyType> allTypesWithDeps(
       SoyType root, @Nullable SoyTypeRegistry registry) {
-    return TreeStreams.breadthFirst(root, new SoyTypeSuccessorsFunction(registry)).iterator();
+    return TreeStreams.breadthFirst(root, new SoyTypeDepsSuccessorsFunction(registry));
   }
 
   /**
@@ -656,8 +662,11 @@ public Iterable<? extends SoyType> apply(SoyType type) {
         case INTERSECTION:
           return ((IntersectionType) type).getMembers();
         case INDEXED:
+          return ImmutableList.of(((IndexedType) type).getType());
         case NAMED:
-          return ImmutableList.of(type.getEffectiveType());
+          // Use SoyTypeDepsSuccessorsFunction below if you need to navigate to the effective record
+          // type.
+          return ImmutableList.of();
 
         case ITERABLE:
         case LIST:
@@ -692,8 +701,27 @@ public Iterable<? extends SoyType> apply(SoyType type) {
     }
   }
 
+  /** Implementation of SuccessorsFunction that traverses a graph rooted at a SoyType. */
+  private static class SoyTypeDepsSuccessorsFunction extends SoyTypeSuccessorsFunction {
+
+    public SoyTypeDepsSuccessorsFunction(@Nullable SoyTypeRegistry typeRegistry) {
+      super(typeRegistry);
+    }
+
+    @Override
+    public Iterable<? extends SoyType> apply(SoyType type) {
+      switch (type.getKind()) {
+        case INDEXED:
+        case NAMED:
+          return ImmutableList.of(type.getEffectiveType());
+        default:
+          return super.apply(type);
+      }
+    }
+  }
+
   public static boolean hasProtoDep(SoyType type) {
-    return Streams.stream(SoyTypes.getTypeTraverser(type, null))
+    return SoyTypes.allTypes(type, null)
         .anyMatch(t -> t.getKind() == Kind.PROTO || t.getKind() == Kind.PROTO_ENUM);
   }