rules_closure icon indicating copy to clipboard operation
rules_closure copied to clipboard

JsChecker crashes when assigning an arrow function to a var in a class

Open mbrukman opened this issue 3 years ago • 8 comments

I'm using latest commit of rules_closure because the latest release (0.10.0) is quite old (Oct 2019), and I've run into similar issues there as well.

Here's a relevant excerpt from my WORKSPACE:

git_repository(
    name = "io_bazel_rules_closure",
    remote = "https://github.com/bazelbuild/rules_closure.git",
    commit = "62746bdd1087c1198a81143e7d8ef3d144a43c0f",
)

I've created closure_js_library() and closure_js_binary() rules to compile Preact Todo MVC, but I'm running into various compiler errors.

Here's a minified example (index.min.js):

class A {
  b = c => {};
}

Here's the failure:

$ bazel-out/host/bin/external/io_bazel_rules_closure/java/io/bazel/rules/closure/ClosureWorker \
      JsChecker --src index.min.js

which results in the following output:

ERROR: Program threw uncaught exception with args: JsChecker --src index.min.js
java.lang.RuntimeException: Exception parsing "index.min.js"
	at com.google.javascript.jscomp.parsing.ParserRunner.parse(ParserRunner.java:155)
	at com.google.javascript.jscomp.JsAst.parse(JsAst.java:152)
	at com.google.javascript.jscomp.JsAst.getAstRoot(JsAst.java:55)
	at com.google.javascript.jscomp.CompilerInput.getAstRoot(CompilerInput.java:133)
	at com.google.javascript.jscomp.Compiler.parseInputs(Compiler.java:1714)
	at com.google.javascript.jscomp.Compiler.parseForCompilationInternal(Compiler.java:937)
	at com.google.javascript.jscomp.Compiler.lambda$parseForCompilation$4(Compiler.java:920)
	at com.google.javascript.jscomp.CompilerExecutor.runInCompilerThread(CompilerExecutor.java:129)
	at com.google.javascript.jscomp.Compiler.runInCompilerThread(Compiler.java:824)
	at com.google.javascript.jscomp.Compiler.parseForCompilation(Compiler.java:918)
	at com.google.javascript.jscomp.Compiler.compile(Compiler.java:674)
	at com.google.javascript.jscomp.JsChecker.run(JsChecker.java:255)
	at com.google.javascript.jscomp.JsChecker.access$300(JsChecker.java:63)
	at com.google.javascript.jscomp.JsChecker$Program.apply(JsChecker.java:354)
	at io.bazel.rules.closure.worker.LegacyAspect.run(LegacyAspect.java:38)
	at io.bazel.rules.closure.ClosureWorker.run(ClosureWorker.java:69)
	at io.bazel.rules.closure.worker.PersistentWorker.runProgram(PersistentWorker.java:109)
	at io.bazel.rules.closure.worker.PersistentWorker.run(PersistentWorker.java:88)
	at io.bazel.rules.closure.ClosureWorker.main(ClosureWorker.java:111)
Caused by: java.lang.ArrayIndexOutOfBoundsException: Index 0 out of bounds for length 0
	at com.google.common.collect.RegularImmutableList.get(RegularImmutableList.java:60)
	at com.google.javascript.jscomp.parsing.IRFactory$TransformDispatcher.getEndOfArgCommentZones(IRFactory.java:1564)
	at com.google.javascript.jscomp.parsing.IRFactory$TransformDispatcher.processFormalParameterList(IRFactory.java:1712)
	at com.google.javascript.jscomp.parsing.IRFactory$TransformDispatcher.process(IRFactory.java:3388)
	at com.google.javascript.jscomp.parsing.IRFactory.transform(IRFactory.java:833)
	at com.google.javascript.jscomp.parsing.IRFactory$TransformDispatcher.processFunction(IRFactory.java:1666)
	at com.google.javascript.jscomp.parsing.IRFactory$TransformDispatcher.process(IRFactory.java:3313)
	at com.google.javascript.jscomp.parsing.IRFactory.transform(IRFactory.java:833)
	at com.google.javascript.jscomp.parsing.IRFactory$TransformDispatcher.processClassDeclaration(IRFactory.java:2671)
	at com.google.javascript.jscomp.parsing.IRFactory$TransformDispatcher.process(IRFactory.java:3391)
	at com.google.javascript.jscomp.parsing.IRFactory.transform(IRFactory.java:833)
	at com.google.javascript.jscomp.parsing.IRFactory$TransformDispatcher.processAstRoot(IRFactory.java:1303)
	at com.google.javascript.jscomp.parsing.IRFactory$TransformDispatcher.process(IRFactory.java:3351)
	at com.google.javascript.jscomp.parsing.IRFactory.transformTree(IRFactory.java:344)
	at com.google.javascript.jscomp.parsing.ParserRunner.parse(ParserRunner.java:144)
	... 18 more

mbrukman avatar Jul 26 '20 23:07 mbrukman

ES6 Class field initialization is a non-finalized spec (Level 3 as of this writing) and not supported by Closure Compiler yet. If you like you can file a bug there to track.

gkdn avatar Jul 27 '20 21:07 gkdn

@gkdn — I understand this is a not-yet-supported JS feature in JsCompiler, but please note that JsCompiler doesn't crash with the same input:

$ java -jar closure-compiler-v20200719.jar arrow.js
arrow.js:2: ERROR - [JSC_PARSE_ERROR] Parse error. '(' expected
  2|   b = c => {};
         ^

1 error(s), 0 warning(s)

I am not expecting JsChecker to support JS features that even JsCompiler doesn't support yet, but it seems like a bug that JsChecker crashes with an internal error for any user input, even if the input has invalid/unsupported syntax.

Would it be possible to update JsChecker to proxy the error output from JsCompiler, so that developers can debug & fix their code just using bazel build (which implicitly runs JsChecker), without having to manually run JsCompiler every time JsChecker crashes?

mbrukman avatar Aug 23 '20 02:08 mbrukman

JsChecker uses JsCompiler directly so there should be no behavior difference which suggests that you are very likely testing with a different version of JsCompiler than the one used current with rules_closure. rules_closure deps updated periodically so when that happens you will see the same error message here.

gkdn avatar Aug 24 '20 18:08 gkdn

@gkdn wrote:

JsChecker uses JsCompiler directly so there should be no behavior difference which suggests that you are very likely testing with a different version of JsCompiler than the one used current with rules_closure.

The commit of this repo I'm using (https://github.com/bazelbuild/rules_closure/commit/62746bdd1087c1198a81143e7d8ef3d144a43c0f) includes JsCompiler version v20200614 (as of commit https://github.com/bazelbuild/rules_closure/commit/d69cb55fa6e4d2f627c017e9ad3088d70214e938), but the output from JsCompiler is the same at that version as well:

$ cat arrow.js
class A {
  b = c => {};
}
$ java -jar closure-compiler-v20200614.jar --js arrow.js
arrow.js:2: ERROR - [JSC_PARSE_ERROR] Parse error. '(' expected
  2|   b = c => {};
         ^

1 error(s), 0 warning(s)

It's also the same using an even earlier version of Closure Compiler (previously referenced in this repo, and updated in commit https://github.com/bazelbuild/rules_closure/commit/d69cb55fa6e4d2f627c017e9ad3088d70214e938):

$ java -jar closure-compiler-v20200406.jar arrow.js
arrow.js:2: ERROR - [JSC_PARSE_ERROR] Parse error. '(' expected
  b = c => {};
    ^

1 error(s), 0 warning(s)

so this behavior has been the same in Closure Compiler for a while, and it looks like JsChecker has diverged.

mbrukman avatar Aug 24 '20 19:08 mbrukman

Ok the behavior difference is then probably due to selected language from JsChecker: LanguageMode.ECMASCRIPT_2017 while using JsCompiler. It should be using ECMASCRIPT_NEXT or STABLE_IN to not fall behind.

You can probably repro the issue with JsCompiler using --language_in ECMASCRIPT_2017

Thanks for digging this.

gkdn avatar Aug 24 '20 19:08 gkdn

JsCompiler doesn't crash when using ECMASCRIPT_2017:

$ java -jar closure-compiler-v20200406.jar --language_in ECMASCRIPT_2017 arrow.js
arrow.js:2: ERROR - [JSC_PARSE_ERROR] Parse error. '(' expected
  b = c => {};
    ^

1 error(s), 0 warning(s)

Or the more recent one:

$ java -jar closure-compiler-v20200614.jar --language_in ECMASCRIPT_2017 arrow.js
arrow.js:2: ERROR - [JSC_PARSE_ERROR] Parse error. '(' expected
  2|   b = c => {};
         ^

1 error(s), 0 warning(s)

mbrukman avatar Aug 24 '20 21:08 mbrukman

Got it; need to dig where the exception escapes. Re-opening the issue.

gkdn avatar Aug 24 '20 22:08 gkdn

I reproduced the issue with Closure compiler; you need to pass --continue_after_errors; that is the parsing mode that is used for checking but not enabled by default on command line runner.

gkdn avatar Aug 31 '20 18:08 gkdn