Skip to content

Commit

Permalink
Add consideration for SPREAD and REST to code that inspects object li…
Browse files Browse the repository at this point in the history
…terals and object patterns.

This isn't intended to add feature support, only to prevent crashes; it's expected that essentially ignoring them will still be safe in these situations.

The supported node types of these traversals are also made explicit so that any future errors are louder.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=236218251
  • Loading branch information
nreid260 authored and brad4d committed Mar 1, 2019
1 parent d5f9178 commit ec106cd
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 51 deletions.
41 changes: 22 additions & 19 deletions src/com/google/javascript/jscomp/AnalyzePrototypeProperties.java
Expand Up @@ -285,15 +285,29 @@ public void visit(NodeTraversal t, Node n, Node parent) {
return;
}

// var x = {a: 1, b: 2}
// Fall through.
case OBJECT_PATTERN:
// `var x = {a: 1, b: 2}` and `var {a: x, b: y} = obj;`
// should count as a use of property a and b.
for (Node propNameNode = n.getFirstChild();
propNameNode != null;
propNameNode = propNameNode.getNext()) {
// May be STRING, GET, or SET, but NUMBER isn't interesting.
// Also ignore computed properties
if (!propNameNode.isQuotedString() && !propNameNode.isComputedProp()) {
addSymbolUse(propNameNode.getString(), t.getModule(), PROPERTY);
for (Node propNode = n.getFirstChild(); propNode != null; propNode = propNode.getNext()) {
switch (propNode.getToken()) {
case COMPUTED_PROP:
case SPREAD:
break;

case STRING_KEY:
case GETTER_DEF:
case SETTER_DEF:
case MEMBER_FUNCTION_DEF:
if (!propNode.isQuotedString()) {
// May be STRING, GET, or SET, but NUMBER isn't interesting.
addSymbolUse(propNode.getString(), t.getModule(), PROPERTY);
}
break;

default:
throw new IllegalStateException(
"Unexpected child of " + n.getToken() + ": " + propNode.toStringTree());
}
}
break;
Expand All @@ -307,17 +321,6 @@ public void visit(NodeTraversal t, Node n, Node parent) {
}
break;

case OBJECT_PATTERN:
for (Node stringKeyNode = n.getFirstChild();
stringKeyNode != null;
stringKeyNode = stringKeyNode.getNext()) {
if (!stringKeyNode.isComputedProp() && !stringKeyNode.isQuotedString()) {
// skip over const {['foobar']: foo} = ...; and const {'foobar': foo} = ...;
addSymbolUse(stringKeyNode.getString(), t.getModule(), PROPERTY);
}
}
break;

case NAME:
String name = n.getString();

Expand Down
3 changes: 2 additions & 1 deletion src/com/google/javascript/jscomp/MethodCompilerPass.java
Expand Up @@ -212,9 +212,10 @@ public void visit(NodeTraversal t, Node n, Node parent) {
nonMethodProperties.add(key.getString());
break;
case COMPUTED_PROP: // complicated
case SPREAD:
break;
default:
throw new IllegalStateException("unexpected OBJECTLIT key: " + key);
throw new IllegalStateException("Unexpected OBJECTLIT key: " + key);
}
}
break;
Expand Down
16 changes: 12 additions & 4 deletions src/com/google/javascript/jscomp/RemoveUnusedCode.java
Expand Up @@ -1062,10 +1062,18 @@ private void traverseObjectPattern(Node objectPattern, Scope scope) {
for (Node propertyNode = objectPattern.getFirstChild();
propertyNode != null;
propertyNode = propertyNode.getNext()) {
if (propertyNode.isComputedProp()) {
traverseObjectPatternComputedProperty(propertyNode, scope);
} else {
traverseObjectPatternStringKey(propertyNode, scope);
switch (propertyNode.getToken()) {
case COMPUTED_PROP:
traverseObjectPatternComputedProperty(propertyNode, scope);
break;
case STRING_KEY:
traverseObjectPatternStringKey(propertyNode, scope);
break;
case REST:
break;
default:
throw new IllegalStateException(
"Unexpected child of OBJECT_PATTERN: " + propertyNode.toStringTree());
}
}
}
Expand Down
53 changes: 26 additions & 27 deletions src/com/google/javascript/jscomp/RenameProperties.java
Expand Up @@ -346,35 +346,34 @@ public void visit(NodeTraversal t, Node n, Node parent) {
}
break;
case OBJECTLIT:
for (Node key = n.getFirstChild(); key != null; key = key.getNext()) {
if (key.isComputedProp()) {
// We don't want to rename computed properties
continue;
} else if (key.isQuotedString()) {
// Ensure that we never rename some other property in a way
// that could conflict with this quoted key.
quotedNames.add(key.getString());
} else if (compiler.getCodingConvention().blockRenamingForProperty(key.getString())) {
externedNames.add(key.getString());
} else {
maybeMarkCandidate(key);
}
}
break;
case OBJECT_PATTERN:
// Iterate through all the nodes in the object pattern
// Iterate through all the properties.
for (Node key = n.getFirstChild(); key != null; key = key.getNext()) {
if (key.isComputedProp()) {
// We don't want to rename computed properties
continue;
} else if (key.isQuotedString()) {
// Ensure that we never rename some other property in a way
// that could conflict with this quoted key.
quotedNames.add(key.getString());
} else if (compiler.getCodingConvention().blockRenamingForProperty(key.getString())) {
externedNames.add(key.getString());
} else {
maybeMarkCandidate(key);
switch (key.getToken()) {
case COMPUTED_PROP: // We don't want to rename computed properties
case REST:
case SPREAD:
break;

case GETTER_DEF:
case MEMBER_FUNCTION_DEF:
case SETTER_DEF:
case STRING_KEY:
String propName = key.getString();
if (key.isQuotedString()) {
// Ensure that we never rename some other property in a way
// that could conflict with this quoted key.
quotedNames.add(propName);
} else if (compiler.getCodingConvention().blockRenamingForProperty(propName)) {
externedNames.add(propName);
} else {
maybeMarkCandidate(key);
}
break;

default:
throw new IllegalStateException(
"Unexpected child of " + n.getToken() + ": " + key.toStringTree());
}
}
break;
Expand Down

0 comments on commit ec106cd

Please sign in to comment.