Skip to content

Commit

Permalink
Remove unnecessary checks from RewriteClassMembers
Browse files Browse the repository at this point in the history
`RewriteClassMembers` is now running `MakeDeclaredNamesUnique` in its `process()` method.
Therefore, all variable names are unique when it does its actual work.
Therefore, it is a waste of time for it to continue doing checks it previously did
in order to ensure name shadowing doesn't happen.

PiperOrigin-RevId: 561104386
  • Loading branch information
brad4d authored and copybara-github committed Aug 29, 2023
1 parent 43850b5 commit 3b72456
Showing 1 changed file with 0 additions and 53 deletions.
53 changes: 0 additions & 53 deletions src/com/google/javascript/jscomp/RewriteClassMembers.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,8 @@

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkState;
import static com.google.common.collect.ImmutableSet.toImmutableSet;

import com.google.common.collect.ImmutableSet;
import com.google.common.collect.MultimapBuilder;
import com.google.common.collect.SetMultimap;
import com.google.javascript.jscomp.parsing.parser.FeatureSet;
import com.google.javascript.jscomp.parsing.parser.FeatureSet.Feature;
import com.google.javascript.rhino.Node;
Expand Down Expand Up @@ -99,19 +96,6 @@ public boolean shouldTraverse(NodeTraversal t, Node n, Node parent) {
checkState(!classStack.isEmpty());
classStack.peek().recordStaticBlock(n);
break;
case NAME:
for (ClassRecord record : classStack) {
// For now, we are just processing these names as strings, and so we will also give
// CANNOT_CONVERT_YET errors for patterns that technically can be simply inlined, such as:
// class C {
// y = (x) => x;
// constructor(x) {}
// }
// Either using scopes to be more precise or just doing renaming for all conflicting
// constructor declarations would address this issue.
record.potentiallyRecordNameInRhs(n);
}
break;
case COMPUTED_PROP:
checkState(!classStack.isEmpty());
ClassRecord record = classStack.peek();
Expand Down Expand Up @@ -143,10 +127,6 @@ public void visit(NodeTraversal t, Node n, Node parent) {
case CLASS:
visitClass(t);
return;
case COMPUTED_FIELD_DEF:
case MEMBER_FIELD_DEF:
classStack.peek().exitField();
return;
case THIS:
Node rootNode = t.getClosestScopeRootNodeBindingThisOrSuper();
if (rootNode.isStaticMember()
Expand Down Expand Up @@ -303,21 +283,12 @@ private void rewriteInstanceMembers(NodeTraversal t, ClassRecord record) {
Node ctor = NodeUtil.getEs6ClassConstructorMemberFunctionDef(record.classNode);
Node ctorBlock = ctor.getFirstChild().getLastChild();
Node insertionPoint = findInitialInstanceInsertionPoint(ctorBlock);
ImmutableSet<String> ctorDefinedNames = record.getConstructorDefinedNames();

while (!instanceMembers.isEmpty()) {
Node instanceMember = instanceMembers.pop();
checkState(
instanceMember.isMemberFieldDef() || instanceMember.isComputedFieldDef(), instanceMember);

for (Node nameInRhs : record.referencedNamesByMember.get(instanceMember)) {
String name = nameInRhs.getString();
checkState(
!ctorDefinedNames.contains(name),
"Rhs of public field cannot use the same name as a constructor parameter: %s",
name);
}

Node thisNode = astFactory.createThisForEs6ClassMember(instanceMember);
if (instanceMember.isComputedFieldDef()
&& NodeUtil.canBeSideEffected(instanceMember.getFirstChild())) {
Expand Down Expand Up @@ -470,8 +441,6 @@ private Node findInitialInstanceInsertionPoint(Node ctorBlock) {
*/
private static final class ClassRecord {

// During traversal, contains the current member being traversed. After traversal, always null
@Nullable Node currentMember;
boolean cannotConvert;

// Instance fields
Expand All @@ -481,9 +450,6 @@ private static final class ClassRecord {
// Computed fields with side effects after the most recent computed member function
final Deque<Node> computedFieldsWithSideEffectsToMove = new ArrayDeque<>();

// Mapping from MEMBER_FIELD_DEF (& COMPUTED_FIELD_DEF) nodes to all name nodes in that RHS
final SetMultimap<Node, Node> referencedNamesByMember =
MultimapBuilder.linkedHashKeys().hashSetValues().build();
// Set of all the Vars defined in the constructor arguments scope and constructor body scope
ImmutableSet<Var> constructorVars = ImmutableSet.of();

Expand All @@ -510,28 +476,13 @@ void enterField(Node field) {
} else {
instanceMembers.push(field);
}
currentMember = field;
}

void exitField() {
currentMember = null;
}

void recordStaticBlock(Node block) {
checkArgument(NodeUtil.isClassStaticBlock(block));
staticMembers.push(block);
}

void potentiallyRecordNameInRhs(Node nameNode) {
checkArgument(nameNode.isName());
if (currentMember == null) {
return;
}
checkState(
currentMember.isMemberFieldDef() || currentMember.isComputedFieldDef(), currentMember);
referencedNamesByMember.put(currentMember, nameNode);
}

void recordConstructorScope(Scope s) {
checkArgument(s.isFunctionBlockScope(), s);
checkState(constructorVars.isEmpty(), constructorVars);
Expand All @@ -541,9 +492,5 @@ void recordConstructorScope(Scope s) {
builder.addAll(argsScope.getAllSymbols());
constructorVars = builder.build();
}

ImmutableSet<String> getConstructorDefinedNames() {
return constructorVars.stream().map(Var::getName).collect(toImmutableSet());
}
}
}

0 comments on commit 3b72456

Please sign in to comment.