Skip to content

Commit

Permalink
Add restrictions for property name generation to match Angular and Po…
Browse files Browse the repository at this point in the history
…lymer frameworks.

Closes #2337

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=149027339
  • Loading branch information
ChadKillingsworth authored and brad4d committed Mar 3, 2017
1 parent 86e3dd0 commit 8e55385
Show file tree
Hide file tree
Showing 9 changed files with 215 additions and 80 deletions.
24 changes: 16 additions & 8 deletions src/com/google/javascript/jscomp/AmbiguateProperties.java
Expand Up @@ -78,7 +78,9 @@ class AmbiguateProperties implements CompilerPass {

private final List<Node> stringNodesToRename = new ArrayList<>();
// Can't use these as property names.
private final char[] reservedCharacters;
private final char[] reservedFirstCharacters;
// Can't use these as property names.
private final char[] reservedNonFirstCharacters;

/** Map from property name to Property object */
private final Map<String, Property> propertyMap = new HashMap<>();
Expand Down Expand Up @@ -125,11 +127,14 @@ public int compare(Property p1, Property p2) {
*/
static final String SKIP_PREFIX = "JSAbstractCompiler";

AmbiguateProperties(AbstractCompiler compiler,
char[] reservedCharacters) {
AmbiguateProperties(
AbstractCompiler compiler,
char[] reservedFirstCharacters,
char[] reservedNonFirstCharacters) {
Preconditions.checkState(compiler.getLifeCycleStage().isNormalized());
this.compiler = compiler;
this.reservedCharacters = reservedCharacters;
this.reservedFirstCharacters = reservedFirstCharacters;
this.reservedNonFirstCharacters = reservedNonFirstCharacters;

JSTypeRegistry r = compiler.getTypeRegistry();
invalidatingTypes = new HashSet<>(ImmutableSet.of(
Expand Down Expand Up @@ -157,9 +162,11 @@ public int compare(Property p1, Property p2) {
}

static AmbiguateProperties makePassForTesting(
AbstractCompiler compiler, char[] reservedCharacters) {
AbstractCompiler compiler,
char[] reservedFirstCharacters,
char[] reservedNonFirstCharacters) {
AmbiguateProperties ap =
new AmbiguateProperties(compiler, reservedCharacters);
new AmbiguateProperties(compiler, reservedFirstCharacters, reservedNonFirstCharacters);
ap.renamingMap = new HashMap<>();
return ap;
}
Expand Down Expand Up @@ -230,8 +237,9 @@ public void process(Node externs, Node root) {
int numNewPropertyNames = coloring.color();

// Generate new names for the properties that will be renamed.
NameGenerator nameGen = new DefaultNameGenerator(
reservedNames.build(), "", reservedCharacters);
NameGenerator nameGen =
new DefaultNameGenerator(
reservedNames.build(), "", reservedFirstCharacters, reservedNonFirstCharacters);
String[] colorMap = new String[numNewPropertyNames];
for (int i = 0; i < numNewPropertyNames; ++i) {
colorMap[i] = nameGen.generateNextName();
Expand Down
36 changes: 36 additions & 0 deletions src/com/google/javascript/jscomp/CompilerOptions.java
Expand Up @@ -27,6 +27,7 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.LinkedHashMultimap;
import com.google.common.collect.Multimap;
import com.google.common.primitives.Chars;
import com.google.javascript.jscomp.deps.ModuleLoader;
import com.google.javascript.jscomp.parsing.Config;
import com.google.javascript.rhino.IR;
Expand All @@ -49,6 +50,11 @@ public class CompilerOptions {
// The number of characters after which we insert a line break in the code
static final int DEFAULT_LINE_LENGTH_THRESHOLD = 500;

static final char[] POLYMER_PROPERTY_RESERVED_FIRST_CHARS =
"ABCDEFGHIJKLMNOPQRSTUVWXYZ$".toCharArray();
static final char[] POLYMER_PROPERTY_RESERVED_NON_FIRST_CHARS = "_$".toCharArray();
static final char[] ANGULAR_PROPERTY_RESERVED_FIRST_CHARS = {'$'};

/**
* A common enum for compiler passes that can run either globally or locally.
*/
Expand Down Expand Up @@ -3208,4 +3214,34 @@ public CompilerOptions setStrictModeInput(boolean isStrictModeInput) {
this.isStrictModeInput = isStrictModeInput;
return this;
}

public char[] getPropertyReservedNamingFirstChars() {
char[] reservedChars = anonymousFunctionNaming.getReservedCharacters();
if (polymerVersion != null && polymerVersion > 1) {
if (reservedChars == null) {
reservedChars = POLYMER_PROPERTY_RESERVED_FIRST_CHARS;
} else {
reservedChars = Chars.concat(reservedChars, POLYMER_PROPERTY_RESERVED_FIRST_CHARS);
}
} else if (angularPass) {
if (reservedChars == null) {
reservedChars = ANGULAR_PROPERTY_RESERVED_FIRST_CHARS;
} else {
reservedChars = Chars.concat(reservedChars, ANGULAR_PROPERTY_RESERVED_FIRST_CHARS);
}
}
return reservedChars;
}

public char[] getPropertyReservedNamingNonFirstChars() {
char[] reservedChars = anonymousFunctionNaming.getReservedCharacters();
if (polymerVersion != null && polymerVersion > 1) {
if (reservedChars == null) {
reservedChars = POLYMER_PROPERTY_RESERVED_NON_FIRST_CHARS;
} else {
reservedChars = Chars.concat(reservedChars, POLYMER_PROPERTY_RESERVED_NON_FIRST_CHARS);
}
}
return reservedChars;
}
}
49 changes: 32 additions & 17 deletions src/com/google/javascript/jscomp/DefaultNameGenerator.java
Expand Up @@ -106,20 +106,30 @@ public DefaultNameGenerator() {
reset(reservedNames, "", null);
}

public DefaultNameGenerator(
Set<String> reservedNames, String prefix, @Nullable char[] reservedCharacters) {
this(reservedNames, prefix, reservedCharacters, reservedCharacters);
}

/**
* Creates a DefaultNameGenerator.
*
* @param reservedNames set of names that are reserved; generated names will
* not include these names. This set is referenced rather than copied,
* so changes to the set will be reflected in how names are generated.
* @param reservedNames set of names that are reserved; generated names will not include these
* names. This set is referenced rather than copied, so changes to the set will be reflected
* in how names are generated.
* @param prefix all generated names begin with this prefix.
* @param reservedCharacters If specified these characters won't be used in
* generated names
* @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
*/
public DefaultNameGenerator(Set<String> reservedNames, String prefix,
@Nullable char[] reservedCharacters) {
public DefaultNameGenerator(
Set<String> reservedNames,
String prefix,
@Nullable char[] reservedFirstCharacters,
@Nullable char[] reservedNonFirstCharacters) {
buildPriorityLookupMap();
reset(reservedNames, prefix, reservedCharacters);
reset(reservedNames, prefix, reservedFirstCharacters, reservedNonFirstCharacters);
}

private DefaultNameGenerator(Set<String> reservedNames, String prefix,
Expand All @@ -134,7 +144,7 @@ private DefaultNameGenerator(Set<String> reservedNames, String prefix,
this.priorityLookupMap.put(entry.getKey(), entry.getValue().clone());
}

reset(reservedNames, prefix, reservedCharacters);
reset(reservedNames, prefix, reservedCharacters, reservedCharacters);
}

private void buildPriorityLookupMap() {
Expand All @@ -146,26 +156,31 @@ private void buildPriorityLookupMap() {
}
}

@Override
public void reset(Set<String> reservedNames, String prefix, @Nullable char[] reservedCharacters) {
reset(reservedNames, prefix, reservedCharacters, reservedCharacters);
}

/**
* Note that the history of what characters are most used in the program
* (set through calls to 'favor') is not deleted. Upon 'reset', that history
* is taken into account for the names that will be generated later: it
* re-calculates how characters are prioritized based on how often the they
* appear in the final output.
* Note that the history of what characters are most used in the program (set through calls to
* 'favor') is not deleted. Upon 'reset', that history is taken into account for the names that
* will be generated later: it re-calculates how characters are prioritized based on how often the
* they appear in the final output.
*/
@Override
public void reset(
Set<String> reservedNames,
String prefix,
@Nullable char[] reservedCharacters) {
@Nullable char[] reservedFirstCharacters,
@Nullable char[] reservedNonFirstCharacters) {

this.reservedNames = reservedNames;
this.prefix = prefix;
this.nameCount = 0;

// build the character arrays to use
this.firstChars = reserveCharacters(FIRST_CHAR, reservedCharacters);
this.nonFirstChars = reserveCharacters(NONFIRST_CHAR, reservedCharacters);
this.firstChars = reserveCharacters(FIRST_CHAR, reservedFirstCharacters);
this.nonFirstChars = reserveCharacters(NONFIRST_CHAR, reservedNonFirstCharacters);
Arrays.sort(firstChars);
Arrays.sort(nonFirstChars);

Expand Down
26 changes: 13 additions & 13 deletions src/com/google/javascript/jscomp/DefaultPassConfig.java
Expand Up @@ -2554,17 +2554,19 @@ protected CompilerPass create(AbstractCompiler compiler) {
};

/**
* Renames properties so that the two properties that never appear on
* the same object get the same name.
* Renames properties so that the two properties that never appear on the same object get the same
* name.
*/
private final PassFactory ambiguateProperties =
new PassFactory("ambiguateProperties", true) {
@Override
protected CompilerPass create(AbstractCompiler compiler) {
return new AmbiguateProperties(
compiler, options.anonymousFunctionNaming.getReservedCharacters());
}
};
@Override
protected CompilerPass create(AbstractCompiler compiler) {
return new AmbiguateProperties(
compiler,
options.getPropertyReservedNamingFirstChars(),
options.getPropertyReservedNamingNonFirstChars());
}
};

/**
* Mark the point at which the normalized AST assumptions no longer hold.
Expand Down Expand Up @@ -2598,9 +2600,7 @@ protected CompilerPass create(AbstractCompiler compiler) {
}
};

/**
* Renames properties.
*/
/** Renames properties. */
private final PassFactory renameProperties =
new PassFactory("renameProperties", true) {
@Override
Expand All @@ -2610,13 +2610,13 @@ protected CompilerPass create(final AbstractCompiler compiler) {
return new CompilerPass() {
@Override
public void process(Node externs, Node root) {
char[] reservedChars = options.anonymousFunctionNaming.getReservedCharacters();
RenameProperties rprop =
new RenameProperties(
compiler,
options.generatePseudoNames,
prevPropertyMap,
reservedChars,
options.getPropertyReservedNamingFirstChars(),
options.getPropertyReservedNamingNonFirstChars(),
options.nameGenerator);
rprop.process(externs, root);
propertyMap = rprop.getPropertyMap();
Expand Down
28 changes: 21 additions & 7 deletions src/com/google/javascript/jscomp/NameGenerator.java
Expand Up @@ -17,27 +17,41 @@
package com.google.javascript.jscomp;

import java.util.Set;

import javax.annotation.Nullable;

/**
* A class that generates unique JavaScript variable/property names.
*/
interface NameGenerator {

/**
* Reconfigures this NameGenerator, and resets it to the initial state.
*
* @param reservedNames set of names that are reserved; generated names will
* not include these names. This set is referenced rather than copied,
* so changes to the set will be reflected in how names are generated.
* @param reservedNames set of names that are reserved; generated names will not include these
* names. This set is referenced rather than copied, so changes to the set will be reflected
* in how names are generated.
* @param prefix all generated names begin with this prefix.
* @param reservedCharacters If specified these characters won't be used in
* generated names
* @param reservedCharacters If specified these characters won't be used in generated names
*/
void reset(Set<String> reservedNames, String prefix, @Nullable char[] reservedCharacters);

/**
* Reconfigures this NameGenerator, and resets it to the initial state.
*
* @param reservedNames set of names that are reserved; generated names will not include these
* names. This set is referenced rather than copied, so changes to the set will be reflected
* in how names are generated.
* @param prefix all generated names begin with this prefix.
* @param reservedFirstCharacters If specified these characters won't be used as the first
* character in generated names
* @param reservedNonFirstCharacters If specified these characters won't be used for characters
* (after the first) in generated names
*/
void reset(
Set<String> reservedNames,
String prefix,
@Nullable char[] reservedCharacters);
@Nullable char[] reservedFirstCharacters,
@Nullable char[] reservedNonFirstCharacters);

/**
* Returns a clone of this NameGenerator, reconfigured and reset.
Expand Down
45 changes: 32 additions & 13 deletions src/com/google/javascript/jscomp/RandomNameGenerator.java
Expand Up @@ -24,14 +24,12 @@
import com.google.common.hash.Hashing;
import com.google.common.primitives.Chars;
import com.google.javascript.rhino.TokenStream;

import java.util.Collections;
import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Random;
import java.util.Set;

import javax.annotation.Nullable;

/**
Expand Down Expand Up @@ -110,39 +108,60 @@ public RandomNameGenerator(Random random) {
reset(new HashSet<String>(), "", null);
}

RandomNameGenerator(
Set<String> reservedNames,
String prefix,
@Nullable char[] reservedCharacters,
Random random) {
this.random = random;
reset(reservedNames, prefix, reservedCharacters);
}

/**
* Creates a RandomNameGenerator.
*
* @param reservedNames set of names that are reserved; generated names will
* not include these names. This set is referenced rather than copied,
* so changes to the set will be reflected in how names are generated
* @param prefix all generated names begin with this prefix (a name
* consisting of only this prefix, with no suffix, will not be generated)
* @param reservedCharacters if specified these characters won't be used in
* generated names
* @param reservedNames set of names that are reserved; generated names will not include these
* names. This set is referenced rather than copied, so changes to the set will be reflected
* in how names are generated
* @param prefix all generated names begin with this prefix (a name consisting of only this
* prefix, with no suffix, will not be generated)
* @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 random source of randomness when generating random names
*/
RandomNameGenerator(
Set<String> reservedNames,
String prefix,
@Nullable char[] reservedCharacters,
@Nullable char[] reservedFirstCharacters,
@Nullable char[] reservedNonFirstCharacters,
Random random) {
this.random = random;
reset(reservedNames, prefix, reservedCharacters);
reset(reservedNames, prefix, reservedFirstCharacters, reservedNonFirstCharacters);
}

@Override
public void reset(
Set<String> reservedNames,
String prefix,
@Nullable char[] reservedCharacters) {
reset(reservedNames, prefix, reservedCharacters, reservedCharacters);
}

@Override
public void reset(
Set<String> reservedNames,
String prefix,
@Nullable char[] reservedFirstCharacters,
@Nullable char[] reservedNonFirstCharacters) {
this.reservedNames = reservedNames;
this.prefix = prefix;
nameCount = 0;

// Build the character arrays to use
this.firstChars = reserveCharacters(FIRST_CHAR, reservedCharacters);
this.nonFirstChars = reserveCharacters(NONFIRST_CHAR, reservedCharacters);
this.firstChars = reserveCharacters(FIRST_CHAR, reservedFirstCharacters);
this.nonFirstChars = reserveCharacters(NONFIRST_CHAR, reservedNonFirstCharacters);

checkPrefix(prefix);
shuffleAlphabets(random);
Expand Down

0 comments on commit 8e55385

Please sign in to comment.