Collapse extern/source getter/setter maps into a single dedicated obj…

…ect to better support upcoming changes.

The old API, while possibly more efficient, wasn't being leveraged anyway.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=256054249

src/com/google/javascript/jscomp/AbstractCompiler.java

@@ -574,88 +574,16 @@ abstract void updateGlobalVarReferences(Map<Var, ReferenceCollection>
 
   abstract void addComments(String filename, List<Comment> comments);
 
-  /** Indicates whether a property has a getter or a setter, or both. */
-  public enum PropertyAccessKind {
-    // To save space properties without getters or setters won't appear
-    // in the maps at all, but NORMAL will be returned by some methods.
-    NORMAL(0),
-    GETTER_ONLY(1),
-    SETTER_ONLY(2),
-    GETTER_AND_SETTER(3);
-
-    final byte flags;
-
-    PropertyAccessKind(int flags) {
-      this.flags = (byte) flags;
-    }
-
-    boolean hasGetter() {
-      return (flags & 1) != 0;
-    }
-
-    boolean hasSetter() {
-      return (flags & 2) != 0;
-    }
-
-    boolean hasGetterOrSetter() {
-      return (flags & 3) != 0;
-    }
-
-    // used to combine information from externs and from sources
-    PropertyAccessKind unionWith(PropertyAccessKind other) {
-      int combinedFlags = this.flags | other.flags;
-      switch (combinedFlags) {
-        case 0:
-          return NORMAL;
-        case 1:
-          return GETTER_ONLY;
-        case 2:
-          return SETTER_ONLY;
-        case 3:
-          return GETTER_AND_SETTER;
-        default:
-          throw new IllegalStateException("unexpected value: " + combinedFlags);
-      }
-    }
-  }
-
   /**
-   * Returns a map containing an entry for every property name found in the externs files with
-   * a getter and / or setter defined.
+   * Returns a summary an entry for every property name found in the AST with a getter and / or
+   * setter defined.
    *
    * <p>Property names for which there are no getters or setters will not be in the map.
    */
-  abstract ImmutableMap<String, PropertyAccessKind> getExternGetterAndSetterProperties();
+  abstract AccessorSummary getAccessorSummary();
 
-  /** Sets the map of extern properties with getters and setters. */
-  abstract void setExternGetterAndSetterProperties(
-      ImmutableMap<String, PropertyAccessKind> externGetterAndSetterProperties);
-
-  /**
-   * Returns a map containing an entry for every property name found in the source AST with
-   * a getter and / or setter defined.
-   *
-   * <p>Property names for which there are no getters or setters will not be in the map.
-   */
-  abstract ImmutableMap<String, PropertyAccessKind> getSourceGetterAndSetterProperties();
-
-  /** Sets the map of properties with getters and setters defined in the sources AST. */
-  abstract void setSourceGetterAndSetterProperties(
-      ImmutableMap<String, PropertyAccessKind> externGetterAndSetterProperties);
-
-  /**
-   * Returns any property seen in the externs or source with the given name was a getter, setter, or
-   * both.
-   *
-   * <p>This defaults to {@link PropertyAccessKind#NORMAL} for any property not known to have a
-   * getter or setter, even for property names that do not exist in the given program.
-   */
-  final PropertyAccessKind getPropertyAccessKind(String property) {
-    return getExternGetterAndSetterProperties()
-        .getOrDefault(property, PropertyAccessKind.NORMAL)
-        .unionWith(
-            getSourceGetterAndSetterProperties().getOrDefault(property, PropertyAccessKind.NORMAL));
-  }
+  /** Sets the summary of properties with getters and setters. */
+  abstract void setAccessorSummary(AccessorSummary summary);
 
   /**
    * Returns all the comments from the given file.

src/com/google/javascript/jscomp/AccessorSummary.java

@@ -0,0 +1,90 @@
+/*
+ * Copyright 2019 The Closure Compiler Authors.
+ *
+ * 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
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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.
+ */
+
+package com.google.javascript.jscomp;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.errorprone.annotations.Immutable;
+import java.util.Map;
+
+/** A strongly typed view of information about getters and setters collected from the AST. */
+@Immutable
+final class AccessorSummary {
+
+  /** Indicates whether a property has a getter or a setter, or both. */
+  public enum PropertyAccessKind {
+    // To save space properties without getters or setters won't appear
+    // in the maps at all, but NORMAL will be returned by some methods.
+    NORMAL(0),
+    GETTER_ONLY(1),
+    SETTER_ONLY(2),
+    GETTER_AND_SETTER(3);
+
+    final byte flags;
+
+    PropertyAccessKind(int flags) {
+      this.flags = (byte) flags;
+    }
+
+    boolean hasGetter() {
+      return (flags & 1) != 0;
+    }
+
+    boolean hasSetter() {
+      return (flags & 2) != 0;
+    }
+
+    boolean hasGetterOrSetter() {
+      return (flags & 3) != 0;
+    }
+
+    // used to combine information from externs and from sources
+    PropertyAccessKind unionWith(PropertyAccessKind other) {
+      int combinedFlags = this.flags | other.flags;
+      switch (combinedFlags) {
+        case 0:
+          return NORMAL;
+        case 1:
+          return GETTER_ONLY;
+        case 2:
+          return SETTER_ONLY;
+        case 3:
+          return GETTER_AND_SETTER;
+        default:
+          throw new IllegalStateException("unexpected value: " + combinedFlags);
+      }
+    }
+  }
+
+  static AccessorSummary create(Map<String, PropertyAccessKind> accessors) {
+    // TODO(nickreid): Efficiently verify that no entry in `accessor` is `NORMAL`.
+    return new AccessorSummary(ImmutableMap.copyOf(accessors));
+  }
+
+  private final ImmutableMap<String, PropertyAccessKind> accessors;
+
+  private AccessorSummary(ImmutableMap<String, PropertyAccessKind> accessors) {
+    this.accessors = accessors;
+  }
+
+  public ImmutableMap<String, PropertyAccessKind> getAccessors() {
+    return accessors;
+  }
+
+  public PropertyAccessKind getKind(String name) {
+    return accessors.getOrDefault(name, PropertyAccessKind.NORMAL);
+  }
+}

src/com/google/javascript/jscomp/Compiler.java

@@ -264,10 +264,7 @@ public SourceFile loadSource(String filename) {
   private String lastPassName;
 
   private Set<String> externProperties = null;
-  private ImmutableMap<String, PropertyAccessKind> externGetterAndSetterProperties =
-      ImmutableMap.of();
-  private ImmutableMap<String, PropertyAccessKind> sourceGetterAndSetterProperties =
-      ImmutableMap.of();
+  private AccessorSummary accessorSummary = AccessorSummary.create(ImmutableMap.of());
 
   private static final Joiner pathJoiner = Joiner.on(File.separator);
 
@@ -3131,25 +3128,13 @@ Set<String> getExternProperties() {
   }
 
   @Override
-  ImmutableMap<String, PropertyAccessKind> getExternGetterAndSetterProperties() {
-    return externGetterAndSetterProperties;
+  AccessorSummary getAccessorSummary() {
+    return accessorSummary;
   }
 
   @Override
-  void setExternGetterAndSetterProperties(
-      ImmutableMap<String, PropertyAccessKind> externGetterAndSetterProperties) {
-    this.externGetterAndSetterProperties = externGetterAndSetterProperties;
-  }
-
-  @Override
-  ImmutableMap<String, PropertyAccessKind> getSourceGetterAndSetterProperties() {
-    return sourceGetterAndSetterProperties;
-  }
-
-  @Override
-  void setSourceGetterAndSetterProperties(
-      ImmutableMap<String, PropertyAccessKind> sourceGetterAndSetterProperties) {
-    this.sourceGetterAndSetterProperties = sourceGetterAndSetterProperties;
+  void setAccessorSummary(AccessorSummary summary) {
+    this.accessorSummary = summary;
   }
 
   /**

src/com/google/javascript/jscomp/GatherGetterAndSetterProperties.java

@@ -18,7 +18,7 @@
 import static com.google.common.base.Preconditions.checkState;
 
 import com.google.common.collect.ImmutableMap;
-import com.google.javascript.jscomp.AbstractCompiler.PropertyAccessKind;
+import com.google.javascript.jscomp.AccessorSummary.PropertyAccessKind;
 import com.google.javascript.jscomp.NodeTraversal.AbstractPostOrderCallback;
 import com.google.javascript.rhino.Node;
 import java.util.LinkedHashMap;
@@ -49,12 +49,13 @@ static void update(AbstractCompiler compiler, Node externs, Node root) {
 
     // TODO(nickreid): We probably don't need to re-gather from the externs. They don't change so
     // the first collection should be good forever.
-    compiler.setExternGetterAndSetterProperties(gather(compiler, externs));
-    compiler.setSourceGetterAndSetterProperties(gather(compiler, root));
+    // For now we traverse both trees every time because there's no reason we have to treat them
+    // differently.
+    checkState(externs.getParent() == root.getParent());
+    compiler.setAccessorSummary(AccessorSummary.create(gather(compiler, externs.getParent())));
   }
 
   static ImmutableMap<String, PropertyAccessKind> gather(AbstractCompiler compiler, Node root) {
-
     GatherCallback gatherCallback = new GatherCallback();
     NodeTraversal.traverse(compiler, root, gatherCallback);
     return ImmutableMap.copyOf(gatherCallback.properties);

test/com/google/javascript/jscomp/CompilerTestCase.java

@@ -1873,23 +1873,18 @@ private void verifyGetterAndSetterCollection(Compiler compiler, Node externsRoot
           .that(verifyNoNewGettersOrSetters)
           .isFalse();
 
-      assertThat(compiler.getExternGetterAndSetterProperties()).isEmpty();
-      assertThat(compiler.getSourceGetterAndSetterProperties()).isEmpty();
+      assertThat(compiler.getAccessorSummary().getAccessors()).isEmpty();
     } else if (verifyGetterAndSetterUpdates) {
-      assertWithMessage("Pass did not update extern getters / setters")
-          .that(compiler.getExternGetterAndSetterProperties())
-          .containsExactlyEntriesIn(GatherGetterAndSetterProperties.gather(compiler, externsRoot));
-      assertWithMessage("Pass did not update source getters / setters")
-          .that(compiler.getSourceGetterAndSetterProperties())
-          .containsExactlyEntriesIn(GatherGetterAndSetterProperties.gather(compiler, mainRoot));
+      assertWithMessage("Pass did not update getters / setters")
+          .that(compiler.getAccessorSummary().getAccessors())
+          .containsExactlyEntriesIn(
+              GatherGetterAndSetterProperties.gather(compiler, mainRoot.getParent()));
     } else if (verifyNoNewGettersOrSetters) {
       // If the above assertions hold then these two must also hold.
-      assertWithMessage("Pass did not update new extern getters / setters")
-          .that(compiler.getExternGetterAndSetterProperties())
-          .containsAtLeastEntriesIn(GatherGetterAndSetterProperties.gather(compiler, externsRoot));
-      assertWithMessage("Pass did not update new source getters / setters")
-          .that(compiler.getSourceGetterAndSetterProperties())
-          .containsAtLeastEntriesIn(GatherGetterAndSetterProperties.gather(compiler, mainRoot));
+      assertWithMessage("Pass did not update new getters / setters")
+          .that(compiler.getAccessorSummary().getAccessors())
+          .containsAtLeastEntriesIn(
+              GatherGetterAndSetterProperties.gather(compiler, mainRoot.getParent()));
     }
   }