Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
mbrukman opened this issue Jul 26, 2020 · 8 comments
Open

Comments

@mbrukman
Copy link
Contributor

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
@gkdn
Copy link
Collaborator

gkdn commented Jul 27, 2020

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 gkdn closed this as completed Jul 27, 2020
@mbrukman
Copy link
Contributor Author

mbrukman commented Aug 23, 2020

@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?

@gkdn
Copy link
Collaborator

gkdn commented Aug 24, 2020

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.

@mbrukman
Copy link
Contributor Author

@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 (62746bd) includes JsCompiler version v20200614 (as of commit d69cb55), 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 d69cb55):

$ 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.

@gkdn
Copy link
Collaborator

gkdn commented Aug 24, 2020

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.

@mbrukman
Copy link
Contributor Author

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)

@gkdn
Copy link
Collaborator

gkdn commented Aug 24, 2020

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

@gkdn gkdn reopened this Aug 24, 2020
@gkdn
Copy link
Collaborator

gkdn commented Aug 31, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants