search

google / closure-compiler

A JavaScript checker and optimizer.
https://developers.google.com/closure/compiler/
Apache License 2.0
7.4k stars 1.15k forks source link

Destructuring an object with a string computed property throws an error #3145

Closed trueadm closed 5 years ago

trueadm commented 5 years ago

Given the current test case:

// ==ClosureCompiler==
// @output_file_name default.js
// @compilation_level ADVANCED_OPTIMIZATIONS
// @language_in ECMASCRIPT6_STRICT
// @language_out ECMASCRIPT6_STRICT
// ==/ClosureCompiler==

var React = require("react");

var {
  ["useState"]: useState
} = React;

function NameComponent() {
  const [name] = useState("Ben");
  return React.createElement("div", null, name);
}

module.exports = NameComponent;

The following error is thrown:

23: java.lang.UnsupportedOperationException: COMPUTED_PROP 4 [length: 22] [source_file: Input_0] is not a string node
    at com.google.javascript.rhino.Node.getString(Node.java:1226)
    at com.google.javascript.jscomp.AnalyzePrototypeProperties$ProcessProperties.visit(AnalyzePrototypeProperties.java:324)
    at com.google.javascript.jscomp.NodeTraversal.traverseBranch(NodeTraversal.java:855)
    at com.google.javascript.jscomp.NodeTraversal.traverseChildren(NodeTraversal.java:967)
    at com.google.javascript.jscomp.NodeTraversal.traverseBranch(NodeTraversal.java:851)
    at com.google.javascript.jscomp.NodeTraversal.traverseChildren(NodeTraversal.java:967)
    at com.google.javascript.jscomp.NodeTraversal.traverseBranch(NodeTraversal.java:851)
    at com.google.javascript.jscomp.NodeTraversal.traverseChildren(NodeTraversal.java:967)
    at com.google.javascript.jscomp.NodeTraversal.handleScript(NodeTraversal.java:805)
    at com.google.javascript.jscomp.NodeTraversal.traverseBranch(NodeTraversal.java:830)
    at com.google.javascript.jscomp.NodeTraversal.traverseChildren(NodeTraversal.java:967)
    at com.google.javascript.jscomp.NodeTraversal.traverseBranch(NodeTraversal.java:851)
    at com.google.javascript.jscomp.NodeTraversal.traverse(NodeTraversal.java:336)
    at com.google.javascript.jscomp.NodeTraversal.traverse(NodeTraversal.java:346)
    at com.google.javascript.jscomp.AnalyzePrototypeProperties.process(AnalyzePrototypeProperties.java:158)
    at com.google.javascript.jscomp.CrossChunkMethodMotion.process(CrossChunkMethodMotion.java:92)
    at com.google.javascript.jscomp.PhaseOptimizer$NamedPass.process(PhaseOptimizer.java:310)
    at com.google.javascript.jscomp.PhaseOptimizer$Loop.process(PhaseOptimizer.java:455)
    at com.google.javascript.jscomp.PhaseOptimizer.process(PhaseOptimizer.java:231)
    at com.google.javascript.jscomp.Compiler.performOptimizations(Compiler.java:2459)
    at com.google.javascript.jscomp.Compiler$3.call(Compiler.java:843)
    at com.google.javascript.jscomp.Compiler$3.call(Compiler.java:839)
    at com.google.javascript.jscomp.CompilerExecutor.runInCompilerThread(CompilerExecutor.java:129)
    at com.google.javascript.jscomp.Compiler.runInCompilerThread(Compiler.java:871)
    at com.google.javascript.jscomp.Compiler.stage2Passes(Compiler.java:838)
    at com.google.javascript.jscomp.Compiler.compile(Compiler.java:734)
    at com.google.javascript.jscomp.webservice.backend.CompilerInvokerImpl.compile(CompilerInvokerImpl.java:46)
    at com.google.javascript.jscomp.webservice.backend.ServerController.executeRequest(ServerController.java:181)
    at com.google.javascript.jscomp.webservice.backend.CompilationRequestHandler.serviceParsedRequest(CompilationRequestHandler.java:180)
    at com.google.javascript.jscomp.webservice.backend.CompilationRequestHandler.service(CompilationRequestHandler.java:162)
    at com.google.javascript.jscomp.webservice.frontend.CompilationServlet.doPost(CompilationServlet.java:85)
    at javax.servlet.http.HttpServlet.service(HttpServlet.java:707)
    at javax.servlet.http.HttpServlet.service(HttpServlet.java:790)
    at org.eclipse.jetty.servlet.ServletHolder.handle(ServletHolder.java:848)
    at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1772)
    at com.google.apphosting.utils.servlet.JdbcMySqlConnectionCleanupFilter.doFilter(JdbcMySqlConnectionCleanupFilter.java:60)
    at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1759)
    at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:582)
    at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:143)
    at org.eclipse.jetty.security.SecurityHandler.handle(SecurityHandler.java:524)
    at org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:226)
    at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:143)
    at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:134)
    at com.google.apphosting.runtime.jetty9.ParseBlobUploadHandler.handle(ParseBlobUploadHandler.java:119)
    at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1182)
    at com.google.apphosting.runtime.jetty9.AppEngineWebAppContext.doHandle(AppEngineWebAppContext.java:171)
    at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:512)
    at org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:185)
    at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1112)
    at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:141)
    at com.google.apphosting.runtime.jetty9.AppVersionHandlerMap.handle(AppVersionHandlerMap.java:296)
    at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:134)
    at org.eclipse.jetty.server.Server.handle(Server.java:539)
    at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:333)
    at com.google.apphosting.runtime.jetty9.RpcConnection.handle(RpcConnection.java:202)
    at com.google.apphosting.runtime.jetty9.RpcConnector.serviceRequest(RpcConnector.java:81)
    at com.google.apphosting.runtime.jetty9.JettyServletEngineAdapter.serviceRequest(JettyServletEngineAdapter.java:123)
    at com.google.apphosting.runtime.JavaRuntime$RequestRunnable.dispatchServletRequest(JavaRuntime.java:699)
    at com.google.apphosting.runtime.JavaRuntime$RequestRunnable.dispatchRequest(JavaRuntime.java:661)
    at com.google.apphosting.runtime.JavaRuntime$RequestRunnable.run(JavaRuntime.java:631)
    at com.google.apphosting.runtime.JavaRuntime$NullSandboxRequestRunnable.run(JavaRuntime.java:825)
    at com.google.apphosting.runtime.ThreadGroupPool$PoolEntry.run(ThreadGroupPool.java:273)
    at java.lang.Thread.run(Thread.java:745)

Original Post Data: 
output_format=json&output_info=compiled_code&output_info=warnings&output_info=errors&output_info=statistics&compilation_level=ADVANCED_OPTIMIZATIONS&warning_level=verbose&language_out=ECMASCRIPT6_STRICT&output_file_name=default.js&js_code=var%20React%20%3D%20require(%22react%22)%3B%0A%0Avar%20%7B%0A%20%20%5B%22useState%22%5D%3A%20useState%0A%7D%20%3D%20React%3B%0A%0Afunction%20Component_ComputeFunction()%20%7B%0A%20%20const%20%5Bname%5D%20%3D%20useState(%22Ben%22)%3B%0A%20%20return%20React.createElement(%22div%22%2C%20null%2C%20name)%3B%0A%7D%0A%0Avar%20Component%20%3D%20%2F%2F%20Component%20OPCODES%0A%5B1%20%2F%2F%20COMPONENT_WITH_HOOKS%0A%2C%202%20%2F%2F%20DISPLAY_NAME%0A%2C%20%22Component%22%2C%203%20%2F%2F%20ROOT_PROPS_SHAPE%0A%2C%20null%2C%2020%20%2F%2F%20UNCONDITIONAL_TEMPLATE%0A%2C%20%5B7%20%2F%2F%20OPEN_ELEMENT_DIV%0A%2C%2042%20%2F%2F%20ELEMENT_DYNAMIC_TEXT_CHILD%0A%2C%200%2C%2042%20%2F%2F%20ELEMENT_DYNAMIC_TEXT_CHILD%0A%2C%201%2C%209%20%2F%2F%20CLOSE_ELEMENT%0A%5D%2C%20Component_ComputeFunction%5D%3B%0Amodule%5B%22exports%22%5D%20%3D%20Component%3B
prateekbh commented 5 years ago

i am facing something similar

DESTRUCTURING_LHS 23 [length: 20] [source_file: src/polyfills/object-assign.js] is not a string node
  Node(CONST): src/polyfills/object-assign.js:23:0
const {prototype} = Object;
  Parent(SCRIPT): src/polyfills/object-assign.js:17:0
const Object = {

        at com.google.javascript.rhino.Node.getString(Node.java:1113)
        at org.ampproject.AmpPass.maybeReplaceRValueInVar(Unknown Source)
        at org.ampproject.AmpPass.visit(Unknown Source)
        at com.google.javascript.jscomp.NodeTraversal.traverseBranch(NodeTraversal.java:769)
        at com.google.javascript.jscomp.NodeTraversal.traverseChildren(NodeTraversal.java:840)
        at com.google.javascript.jscomp.NodeTraversal.handleScript(NodeTraversal.java:721)
        at com.google.javascript.jscomp.NodeTraversal.traverseBranch(NodeTraversal.java:746)
        at com.google.javascript.jscomp.NodeTraversal.traverseChildren(NodeTraversal.java:840)
        at com.google.javascript.jscomp.NodeTraversal.traverseBranch(NodeTraversal.java:765)
        at com.google.javascript.jscomp.NodeTraversal.traverse(NodeTraversal.java:305)
        at com.google.javascript.jscomp.NodeTraversal.traverseEs6(NodeTraversal.java:680)
        at org.ampproject.AmpPass.hotSwapScript(Unknown Source)
        at org.ampproject.AmpPass.process(Unknown Source)
        at com.google.javascript.jscomp.Compiler.process(Compiler.java:1083)
        at com.google.javascript.jscomp.Compiler.runCustomPasses(Compiler.java:1116)
        at com.google.javascript.jscomp.Compiler.check(Compiler.java:1072)
        at com.google.javascript.jscomp.Compiler.performChecksAndTranspilation(Compiler.java:866)
        at com.google.javascript.jscomp.Compiler.access$000(Compiler.java:102)
        at com.google.javascript.jscomp.Compiler$2.call(Compiler.java:800)
        at com.google.javascript.jscomp.Compiler$2.call(Compiler.java:797)
        at com.google.javascript.jscomp.CompilerExecutor$2.call(CompilerExecutor.java:101)
        at java.util.concurrent.FutureTask.run(FutureTask.java:266)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
        at java.lang.Thread.run(Thread.java:748)
Caused by: java.lang.UnsupportedOperationException: DESTRUCTURING_LHS 23 [length: 20] [source_file: src/polyfills/object-assign.js] is not a string node
        ... 25 more

for code like: const {hasOwnProperty} = Object.prototype; code link: https://github.com/ampproject/amphtml/blob/master/src/polyfills/object-assign.js#L17

brad4d commented 5 years ago

Created internal Google issue b/119638231

brad4d commented 5 years ago

@prateekbh in your case the failure is happening in code that is not part of closure-compiler. AmpPass needs to be updated to understand how to handle destructuring assignments.

If this failure has only recently started happening, that is because we recently moved the transpilation of destructuring assignments later in the compilation process. Probably it previously happened before the exection of AmpPass, so it never saw any destructuring code, but has now been moved after.

brad4d commented 5 years ago

@trueadm it looks like we need to update AnalyzePrototypeProperties to make sure it understands computed properties.

We haven't done that yet, because in the closure-compiler code it is only used within CrossChunkCodeMotion, which is a pass that runs after all language features above ES5 have been removed from the code. Apparently AmpPass also uses it. See my comment above.

brad4d commented 5 years ago

We will be fixing AnalyzePrototypeProperties in the next quarter, because we need to make CrossChunkCodeMotion work for language output > ES5, but we can't do anything about the AmpPass.

brad4d commented 5 years ago

@trueadm correction: I somehow thought I was looking at your stack trace when I was still looking at the one from @prateekbh

I see your failure is in CrossChunkCodeMotion. That pass should have already been updated to handle destructuring. Looks like we failed to notice the issue with AnalyzePrototypeProperties when we enabled CrossChunkCodeMotion for language output levels > ES5.

We'll fix AnalyzePrototypeProperties.

lauraharker commented 5 years ago

@trueadm I've submitted a fix for the crash internally. It will be pushed to GitHub tomorrow.

trueadm commented 5 years ago

@lauraharker Thank you :)