From ec106cd5576cf2022137492585bc521740229c3b Mon Sep 17 00:00:00 2001 From: nickreid Date: Thu, 28 Feb 2019 16:21:30 -0800 Subject: [PATCH] Add consideration for SPREAD and REST to code that inspects object literals 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 --- .../jscomp/AnalyzePrototypeProperties.java | 41 +++++++------- .../javascript/jscomp/MethodCompilerPass.java | 3 +- .../javascript/jscomp/RemoveUnusedCode.java | 16 ++++-- .../javascript/jscomp/RenameProperties.java | 53 +++++++++---------- 4 files changed, 62 insertions(+), 51 deletions(-) diff --git a/src/com/google/javascript/jscomp/AnalyzePrototypeProperties.java b/src/com/google/javascript/jscomp/AnalyzePrototypeProperties.java index edf23cb2cde..1593a1545cc 100644 --- a/src/com/google/javascript/jscomp/AnalyzePrototypeProperties.java +++ b/src/com/google/javascript/jscomp/AnalyzePrototypeProperties.java @@ -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; @@ -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(); diff --git a/src/com/google/javascript/jscomp/MethodCompilerPass.java b/src/com/google/javascript/jscomp/MethodCompilerPass.java index 6e949c49638..7054c3706e4 100644 --- a/src/com/google/javascript/jscomp/MethodCompilerPass.java +++ b/src/com/google/javascript/jscomp/MethodCompilerPass.java @@ -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; diff --git a/src/com/google/javascript/jscomp/RemoveUnusedCode.java b/src/com/google/javascript/jscomp/RemoveUnusedCode.java index ef314e6b142..c69519dfeee 100644 --- a/src/com/google/javascript/jscomp/RemoveUnusedCode.java +++ b/src/com/google/javascript/jscomp/RemoveUnusedCode.java @@ -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()); } } } diff --git a/src/com/google/javascript/jscomp/RenameProperties.java b/src/com/google/javascript/jscomp/RenameProperties.java index 657c11dde9d..4a93fee9671 100644 --- a/src/com/google/javascript/jscomp/RenameProperties.java +++ b/src/com/google/javascript/jscomp/RenameProperties.java @@ -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;