Add the ability to limit selecting candidates for the RenameProperties based on a predicate function, supplied via the constructor.

Closure Team · copybara-github · commit c2af98e3bd9b · 2025-02-19T13:27:40.000-08:00
PiperOrigin-RevId: 728793352
diff --git a/src/com/google/javascript/jscomp/DefaultPassConfig.java b/src/com/google/javascript/jscomp/DefaultPassConfig.java
@@ -640,6 +640,7 @@ protected PassListBuilder getFinalizations() {
       passes.maybeAdd(ambiguateProperties);
     }
 
+    passes.maybeAdd(createEmptyPass(PassNames.BEFORE_RENAME_PROPERTIES));
     if (options.propertyRenaming == PropertyRenamingPolicy.ALL_UNQUOTED) {
       passes.maybeAdd(renameProperties);
     } else {
diff --git a/src/com/google/javascript/jscomp/PassNames.java b/src/com/google/javascript/jscomp/PassNames.java
@@ -88,6 +88,7 @@ public final class PassNames {
   public static final String RESOLVE_TYPES = "resolveTypes";
   public static final String REWRITE_FUNCTION_EXPRESSIONS = "rewriteFunctionExpressions";
   public static final String RENAME_PROPERTIES = "renameProperties";
+  public static final String BEFORE_RENAME_PROPERTIES = "beforeRenameProperties";
   public static final String STRIP_SIDE_EFFECT_PROTECTION = "stripSideEffectProtection";
   public static final String WIZ_PASS = "wizPass";
 
diff --git a/src/com/google/javascript/jscomp/RenameProperties.java b/src/com/google/javascript/jscomp/RenameProperties.java
@@ -36,32 +36,32 @@
 import java.util.Map;
 import java.util.Set;
 import java.util.TreeSet;
+import java.util.function.Predicate;
 import org.jspecify.annotations.Nullable;
 
 /**
- * RenameProperties renames properties (including methods) of all JavaScript
- * objects. This includes prototypes, functions, object literals, etc.
+ * RenameProperties renames properties (including methods) of all JavaScript objects. This includes
+ * prototypes, functions, object literals, etc.
  *
- * <p> If provided a VariableMap of previously used names, it tries to reuse
- * those names.
+ * <p>If provided a VariableMap of previously used names, it tries to reuse those names.
  *
- * <p> To prevent a property from getting renamed you may extern it (add it to
- * your externs file) or put it in quotes.
+ * <p>To prevent a property from getting renamed you may extern it (add it to your externs file) or
+ * put it in quotes.
  *
- * <p> To avoid run-time JavaScript errors, use quotes when accessing properties
- * that are defined using quotes.
+ * <p>To avoid run-time JavaScript errors, use quotes when accessing properties that are defined
+ * using quotes.
  *
  * <pre>
  *   var a = {'myprop': 0}, b = a['myprop'];  // correct
  *   var x = {'myprop': 0}, y = x.myprop;     // incorrect
  * </pre>
  *
- * This pass also recognizes and replaces special renaming functions. They supply
- * a property name as the string literal for the first argument.
- * This pass will replace them as though they were JS property
- * references. Here are two examples:
- *    JSCompiler_renameProperty('propertyName') -> 'jYq'
- *    JSCompiler_renameProperty('myProp.nestedProp.innerProp') -> 'e4.sW.C$'
+ * This pass also recognizes and replaces special renaming functions. They supply a property name as
+ * the string literal for the first argument. This pass will replace them as though they were JS
+ * property references. Here are two examples: JSCompiler_renameProperty('propertyName') -> 'jYq'
+ * JSCompiler_renameProperty('myProp.nestedProp.innerProp') -> 'e4.sW.C$'
+ *
+ * <p>This class is not thread-safe.
  */
 class RenameProperties implements CompilerPass {
   private static final Splitter DOT_SPLITTER = Splitter.on('.');
@@ -92,6 +92,13 @@ class RenameProperties implements CompilerPass {
   // Shared name generator
   private final NameGenerator nameGenerator;
 
+  // Filter function for property names. The function takes a node as input and returns true if the
+  // node should be renamed.
+  private final Predicate<Node> propertyRenameEligibilityFilter;
+  // Property names that should not be renamed, based on the propertyNameFilter. Unlike
+  // externedNames, this set is dynamically constructed during the pass.
+  private final Set<String> filteredOutNames = new LinkedHashSet<>();
+
   private static final Comparator<Property> FREQUENCY_COMPARATOR =
       (Property p1, Property p2) -> {
 
@@ -133,12 +140,50 @@ class RenameProperties implements CompilerPass {
       char @Nullable [] reservedFirstCharacters,
       char @Nullable [] reservedNonFirstCharacters,
       NameGenerator nameGenerator) {
+    this(
+        compiler,
+        generatePseudoNames,
+        prevUsedPropertyMap,
+        reservedFirstCharacters,
+        reservedNonFirstCharacters,
+        nameGenerator,
+        (Node n) -> true);
+  }
+
+  /**
+   * Creates an instance.
+   *
+   * @param compiler the JSCompiler
+   * @param generatePseudoNames generate pseudo names. e.g. foo -> $foo$ instead of compact
+   *     obfuscated names. This is used for debugging.
+   * @param prevUsedPropertyMap the property renaming map used in a previous compilation
+   * @param reservedFirstCharacters if specified these characters won't be used in generated names
+   *     for the first character
+   * @param reservedNonFirstCharacters if specified these characters won't be used in generated
+   *     names for characters after the first
+   * @param nameGenerator a shared NameGenerator that this instance can use; the instance may reset
+   *     or reconfigure it, so the caller should not expect any state to be preserved
+   * @param propertyRenameEligibilityFilter limit the nodes that are renamed using a predicate
+   *     function. When this returns true, the property name is renamed if and only if all other
+   *     property references for the same name match the predicate. If this returns false for any
+   *     instance of a property, then no property references anywhere in the AST to that property
+   *     are renamed
+   */
+  RenameProperties(
+      AbstractCompiler compiler,
+      boolean generatePseudoNames,
+      VariableMap prevUsedPropertyMap,
+      char @Nullable [] reservedFirstCharacters,
+      char @Nullable [] reservedNonFirstCharacters,
+      NameGenerator nameGenerator,
+      Predicate<Node> propertyRenameEligibilityFilter) {
     this.compiler = compiler;
     this.generatePseudoNames = generatePseudoNames;
     this.prevUsedPropertyMap = prevUsedPropertyMap;
     this.reservedFirstCharacters = reservedFirstCharacters;
     this.reservedNonFirstCharacters = reservedNonFirstCharacters;
     this.nameGenerator = nameGenerator;
+    this.propertyRenameEligibilityFilter = propertyRenameEligibilityFilter;
     externedNames.addAll(compiler.getExternProperties());
   }
 
@@ -168,6 +213,9 @@ public void process(Node externs, Node root) {
     // Update the string nodes.
     for (Node n : stringNodesToRename) {
       String oldName = n.getString();
+      if (filteredOutNames.contains(oldName)) {
+        continue;
+      }
       Property p = propertyMap.get(oldName);
       if (p != null && p.newName != null) {
         checkState(oldName.equals(p.oldName));
@@ -188,7 +236,11 @@ public void process(Node externs, Node root) {
         String replacement;
         if (p != null && p.newName != null) {
           checkState(oldName.equals(p.oldName));
-          replacement = p.newName;
+          if (filteredOutNames.contains(oldName)) {
+            replacement = oldName;
+          } else {
+            replacement = p.newName;
+          }
         } else {
           replacement = oldName;
         }
@@ -306,16 +358,24 @@ public void visit(NodeTraversal t, Node n, Node parent) {
             quotedNames.add(child.getString());
           }
           break;
-        case CALL: {
-          // We replace property renaming function calls with a string
-          // containing the renamed property.
-          Node fnName = n.getFirstChild();
+        case CALL:
+          {
+            // We replace property renaming function calls with a string
+            // containing the renamed property.
+            Node fnName = n.getFirstChild();
+
             if (compiler.getCodingConvention().isPropertyRenameFunction(fnName)) {
-            callNodeToParentMap.put(n, parent);
-            countCallCandidates(t, n);
+              callNodeToParentMap.put(n, parent);
+              countCallCandidates(t, n);
+              Node stringArgument = n.getSecondChild();
+              if (!propertyRenameEligibilityFilter.test(n)) {
+                for (String name : DOT_SPLITTER.split(stringArgument.getString())) {
+                  filteredOutNames.add(name);
+                }
+              }
+            }
+            break;
           }
-          break;
-        }
         case MEMBER_FUNCTION_DEF:
           checkState(!n.isQuotedStringKey());
           if (NodeUtil.isEs6ConstructorMemberFunctionDef(n)) {
@@ -382,6 +442,9 @@ public void visit(NodeTraversal t, Node n, Node parent) {
     private void maybeMarkCandidate(Node n) {
       String name = n.getString();
       if (!externedNames.contains(name)) {
+        if (!propertyRenameEligibilityFilter.test(n)) {
+          filteredOutNames.add(n.getString());
+        }
         stringNodesToRename.add(n);
         countPropertyOccurrence(name);
       }
diff --git a/test/com/google/javascript/jscomp/RenamePropertiesTest.java b/test/com/google/javascript/jscomp/RenamePropertiesTest.java
@@ -16,9 +16,12 @@
 
 package com.google.javascript.jscomp;
 
+import static com.google.common.base.Strings.nullToEmpty;
 import static com.google.common.truth.Truth.assertThat;
 
 import com.google.common.collect.ImmutableList;
+import com.google.javascript.rhino.Node;
+import java.util.function.Predicate;
 import org.jspecify.annotations.Nullable;
 import org.junit.Before;
 import org.junit.Test;
@@ -35,6 +38,7 @@ public final class RenamePropertiesTest extends CompilerTestCase {
   private RenameProperties renameProperties;
   private boolean generatePseudoNames;
   private @Nullable VariableMap prevUsedPropertyMap;
+  private @Nullable Predicate<Node> propertyNameFilter = (Node name) -> true;
 
   public RenamePropertiesTest() {
     super(EXTERNS);
@@ -787,6 +791,146 @@ public void testObjectMethodProperty() {
         lines("var foo = { ", "  a: 1, ", "  b() {", "    return this.a", "  }", "};", "foo.b();"));
   }
 
+  @Test
+  public void testPrototypePropertiesUsingFilterFunction() {
+    propertyNameFilter =
+        (node) -> {
+          String name = node.getString();
+          name = nullToEmpty(name);
+          return name.startsWith("get");
+        };
+
+    test(
+        "Bar.prototype.getA = function(){}; bar.getA();"
+            + "Bar.prototype.getB = function(){};"
+            + "Bar.prototype.setC = function(){};",
+        "Bar.prototype.a = function(){}; bar.a();"
+            + "Bar.prototype.b = function(){};"
+            + "Bar.prototype.setC = function(){}");
+  }
+
+  @Test
+  public void testPrototypePropertiesUsingFilterOnFilename() {
+    propertyNameFilter =
+        (node) -> {
+          String name = node.getSourceFileName();
+          name = nullToEmpty(name);
+          return name.equals("foo.js");
+        };
+
+    test(
+        srcs(
+            SourceFile.fromCode(
+                "foo.js",
+                """
+                Foo.prototype.getA = function(){};
+                Foo.prototype.getB = function(){};
+                foo.getA();
+                foo.getB();
+                """),
+            SourceFile.fromCode(
+                "bar.js",
+                """
+                Bar.prototype.getA = function(){};
+                bar.getA();
+                """)),
+        expected(
+            SourceFile.fromCode(
+                "foo.js",
+                """
+                Foo.prototype.getA = function(){};
+                Foo.prototype.b = function(){};
+                foo.getA();
+                foo.b();
+                """),
+            SourceFile.fromCode(
+                "bar.js",
+                """
+                Bar.prototype.getA = function(){};
+                bar.getA();
+                """)));
+  }
+
+  @Test
+  public void testPrototypeAndRenameFunctionsFilterOnFilename() {
+    propertyNameFilter =
+        (node) -> {
+          String name = node.getSourceFileName();
+          name = nullToEmpty(name);
+          return name.equals("foo.js");
+        };
+
+    test(
+        srcs(
+            SourceFile.fromCode(
+                "foo.js",
+                """
+                var foo = {myProp: 0, myProp2: 1};
+                f(foo[JSCompiler_renameProperty('myProp')]);
+                f(foo[JSCompiler_renameProperty('myProp2')]);
+                """),
+            SourceFile.fromCode(
+                "bar.js",
+                """
+                var bar = {myProp: 0};
+                f(bar[JSCompiler_renameProperty('myProp')]);
+                """)),
+        expected(
+            SourceFile.fromCode(
+                "foo.js",
+                """
+                var foo = {myProp: 0, b: 1};
+                f(foo['myProp']);
+                f(foo['b']);
+                """),
+            SourceFile.fromCode(
+                "bar.js",
+                """
+                var bar = {myProp: 0};
+                f(bar['myProp']);
+                """)));
+  }
+
+  @Test
+  public void testPrototypeAndRenameFunctionsFilterOnFilename2() {
+    propertyNameFilter =
+        (node) -> {
+          String name = node.getSourceFileName();
+          name = nullToEmpty(name);
+          return name.equals("foo.js");
+        };
+
+    test(
+        srcs(
+            SourceFile.fromCode(
+                "foo.js",
+                """
+                var foo = {myProp: 0, myProp2: 1};
+                f(JSCompiler_renameProperty('otherProp.myProp.someProp'));
+                f(JSCompiler_renameProperty('otherProp.myProp2.someProp'));
+                """),
+            SourceFile.fromCode(
+                "bar.js",
+                """
+                var bar = {myProp: 0};
+                f(JSCompiler_renameProperty('otherProp.myProp.someProp'));
+                """)),
+        expected(
+            SourceFile.fromCode(
+                "foo.js",
+                """
+                var foo = {myProp: 0, d: 1};
+                f('otherProp.myProp.someProp');
+                f('otherProp.d.someProp');
+                """),
+            SourceFile.fromCode(
+                "bar.js",
+                """
+                var bar = {myProp: 0};
+                f('otherProp.myProp.someProp');
+                """)));
+  }
+
   private Compiler compileChunks(String externs, JSChunk[] chunks) {
     SourceFile externsInput = SourceFile.fromCode("externs", externs);
 
@@ -810,6 +954,7 @@ protected CompilerPass getProcessor(Compiler compiler) {
             prevUsedPropertyMap,
             null,
             null,
-            new DefaultNameGenerator());
+            new DefaultNameGenerator(),
+            propertyNameFilter);
   }
 }

Original file line number	Diff line number	Diff line change
`@@ -640,6 +640,7 @@ protected PassListBuilder getFinalizations() {`
`640`	`640`	`passes.maybeAdd(ambiguateProperties);`
`641`	`641`	`}`
`642`	`642`
	`643`	`+ passes.maybeAdd(createEmptyPass(PassNames.BEFORE_RENAME_PROPERTIES));`
`643`	`644`	`if (options.propertyRenaming == PropertyRenamingPolicy.ALL_UNQUOTED) {`
`644`	`645`	`passes.maybeAdd(renameProperties);`
`645`	`646`	`} else {`