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

Nashorn is deprecated in Java 11 #1481

Closed
sormuras opened this issue Jun 28, 2018 · 37 comments

Comments

@sormuras
Copy link
Member

commented Jun 28, 2018

Overview

With "Deprecate the Nashorn JavaScript Engine" http://openjdk.java.net/jeps/335 integrated in JDK 11 we need to find another default value for DisabledIf::engine() default "Nashorn";:

and also EnabledIf::engine() default "Nashorn";:

Another Nashorn lives here:

Marked for removal

"A separate JEP will be filed for the actual removal of the types and modules in a future JDK feature release."

Which feature version will be the first without Nashorn? 12?

Alternatives

As explicitly stated in JEP 335: "This deprecation does not affect, in any way, the javax.script API." finding another "general purpose language" via using the support exposed by the java.scripting module should be possible.

@jbduncan

This comment has been minimized.

Copy link
Contributor

commented Jun 28, 2018

My gut feeling would be to switch the default from Nashorn to any one of Groovy, JRuby or Jython.

Groovy has the upside of being Gradle's and Grails' base language, so at least some people will be familiar with it. A downside AFAIK is it's a bit niche as it's a JVM-only language with not as much traction as, say, Kotlin or Scala. (I'm sure this is arguable, though. 😉)

JRuby has the upside of being a very actively supported project, at least AFAIK. However, I don't know how many Java developers know Ruby compared to those who know Groovy.

Jython has the advantage of being based on Python, a generally well-known language AFAIK. It's major downside though is it only supports Python 2 syntax (last time I checked), and I don't know if Jython is still maintained or in the process of being adjusted to work with Python 3 syntax.

According to http://java-source.net/open-source/scripting-languages/scala, Scala would also work as an option. I don't know how feasible that is, though, or if the link is up-to-date, as it's the first time I've heard of using Scala in a javax.script context.

LuaJava or Clojure may work too, but I think even less Java developers know either Lua or Clojure, so I doubt they'd be good defaults.

@jbduncan

This comment has been minimized.

Copy link
Contributor

commented Jun 28, 2018

@mkobit

This comment has been minimized.

Copy link
Contributor

commented Jun 28, 2018

May be worth thinking about https://github.com/graalvm/graaljs (or something like it)

sormuras added a commit that referenced this issue Jun 29, 2018

[WIP] JShell as ScriptEngine
State: proof-of-concept
Issue: #1481

sormuras added a commit that referenced this issue Jun 29, 2018

[WIP] JShell as ScriptEngine
State: proof-of-concept
Issue: #1481
@sormuras

This comment has been minimized.

Copy link
Member Author

commented Jul 19, 2018

Graal VM offers a compatibility mode with ScriptEngine support: https://medium.com/graalvm/oracle-graalvm-announces-support-for-nashorn-migration-c04810d75c1f

@Frontrider

This comment has been minimized.

Copy link

commented Aug 24, 2018

Groovy would be a prime candidate because it's similar to Java, and Gradle already uses it.

Kotlin would also be a good candidate, but I'm finding it really slow when used like this.

@sormuras

This comment has been minimized.

Copy link
Member Author

commented Sep 14, 2018

Groovy and Kotlin are both too heavy for the purpose of evaluating a simple (but generic) boolean expression. Groovy is already supported via its JSR 223 adapter. See this test:

@EnabledIf(engine = "groovy", value = { "System.properties['jsr'] = '233'", "'233' == System.properties['jsr']" })

Just found out that BeanShell is still alive: https://github.com/beanshell/beanshell/tree/merge-fork-beanshell2 -- maybe we may shade it into the Jupiter Engine artifact. Or have it in provided scope.

@Frontrider

This comment has been minimized.

@sormuras

This comment has been minimized.

Copy link
Member Author

commented Sep 16, 2018

Pulling in an entire JRE in order to evaluate a simple expression still feels heavy to me. Or is it distributed as minimal "standalone" jar?

Btw. now, using JDK 11 to build JUnit 5, this warning is emitted:

> Task :documentation:consoleLauncherTest
Warning: Nashorn engine is planned to be removed from a future JDK release
@Frontrider

This comment has been minimized.

Copy link

commented Sep 17, 2018

The only reason I threw that in, is because that contains a version that might be maintained if there is a use case.

I should probably just extract the script engine from it, then turn it into a library, but I don't have time for that. :/

@leonard84

This comment has been minimized.

Copy link
Collaborator

commented Sep 20, 2018

Maybe spring-expression SpEL?

@sormuras

This comment has been minimized.

Copy link
Member Author

commented Sep 20, 2018

Is it available as self-contained distro with a JSR 223 implementation?

@leonard84

This comment has been minimized.

Copy link
Collaborator

commented Sep 20, 2018

It seems to need some glue-code https://github.com/melin/starflow/blob/master/src/main/java/com/googlecode/starflow/core/script/spel/SpelScriptEngine.java

There is an open issue for that https://jira.spring.io/browse/SPR-7651

<dependency>
    <groupId>org.springframework</groupId>
    <artifactId>spring-expression</artifactId>
    <version>4.3.19.RELEASE</version>
</dependency>

See https://stackoverflow.com/questions/40074932/maven-entry-for-spel-as-a-scriptengine

It has a dependency on spring-core but I don't know if that is a crucial dependency or only some edge case.

sormuras added a commit that referenced this issue Sep 25, 2018

sormuras added a commit that referenced this issue Sep 25, 2018

@sormuras

This comment has been minimized.

Copy link
Member Author

commented Sep 25, 2018

I tend to favor an in-memory javac-based ScriptEngine solution as the default engine.
It embeds a user script as a snippet into a pre-defined class template, compiles it to bytecode and evaluates it on demand.

  • no external library is needed
  • foundation tool javac will remain in the JDK
  • the syntax (Java) is well-known

See #1599 for an implemetation spike.

@Frontrider

This comment has been minimized.

Copy link

commented Sep 25, 2018

So your solution is to use Java itself for scripting. That should work for all future Java versions.

Can you put a space between Java and ScriptEngine? I can't help to read that it uses javascript.

@sormuras

This comment has been minimized.

Copy link
Member Author

commented Sep 25, 2018

So your solution is to use Java itself from scripting.

Aye.

That should work for all future Java versions.

Don't be too sure, here. Thought that Nashorn (or some other Javascript interpreter) was here to stay "for ever". :-)

Can you put a space between Java and ScriptEngine? I can't help to read that it uses javascript.

Names are not final, yet. The "Java[S|s]cript" pun was intended.

@Frontrider

This comment has been minimized.

Copy link

commented Sep 25, 2018

Don't be too sure, here. Thought that Nashorn (or some other Javascript interpreter) was here to stay "forever". :-)

Well, I think nashorn was more of "look we made the JVM can run dynamic languages now! Here's an example implementation.". It didn't catch on as much as they probably wanted to.
If there is no javac then Java won't work.

Can I have a request for that mini project? Can you sandbox it (or is it gonna be sandboxed by default)?

sormuras added a commit that referenced this issue Sep 25, 2018

@sormuras

This comment has been minimized.

Copy link
Member Author

commented Sep 25, 2018

Can you sandbox it (or is it gonna be sandboxed by default)?

Mh, like with a SecurityManager? Don't know how feasible this is. The purpose of the scripted conditional execution extension is to provide a tool that users may use to test non-default stuff in a quick manner. The current solution using Nashorn is not guarded as well.

@leonard84

This comment has been minimized.

Copy link
Collaborator

commented Sep 25, 2018

Haven't looked in deeply but, can we use (parts of) jshell https://docs.oracle.com/javase/9/docs/api/jdk.jshell-summary.html for Java9+

sormuras added a commit that referenced this issue Sep 26, 2018

@sormuras

This comment has been minimized.

Copy link
Member Author

commented Sep 26, 2018

See first alternative in the initial issue description. jshell would be it, if only it was introduced in Java 8... trying hard to get JUnit 5 on 9+, but will fail for years, methinks.

@sormuras

This comment has been minimized.

Copy link
Member Author

commented Sep 26, 2018

@leonard84

This comment has been minimized.

Copy link
Collaborator

commented Sep 26, 2018

We could support Nashorn up to Java 10 and offer fallback/forward to jshell, this should be doable via feature detection. EnabledIf::engine() default "Nashorn"; could be changed to default "Autodetect" as long as the expressions the engines support are similar.

@Frontrider

This comment has been minimized.

Copy link

commented Sep 27, 2018

java and javascript are too different for that. old scripts will definitely break.

@leonard84

This comment has been minimized.

Copy link
Collaborator

commented Sep 27, 2018

We should at least allow to define a default engine for a project, instead of having to repeat it on every annotation if you want to choose groovy for example.

@sormuras

This comment has been minimized.

Copy link
Member Author

commented Sep 27, 2018

Before we invest too much time into this "quick and one-shot-helper" and still experimental feature, I'd rather scrap it from the API. It was and is intended to be used very seldom, a custom ExecutionCondition is just a class away and the pre-defined ones (@EnabledOnOs, @EnabledOnJre, @EnabledIfSystemProperty, @EnabledIfEnvironmentVariable) cover already a lot of use-cases.

Mind the 3 notes at the start of https://junit.org/junit5/docs/current/user-guide/#writing-tests-conditional-execution-scripts

Having said this, my current plan is to do the following:

  1. remove the default value from the annotation, forcing the user to specify an engine
  2. show how to include an external scripting language via the JSR223 API
  3. provide a javac-based script engine via https://github.com/sormuras/java-compiler-script-engine

Added the "team discussion" tag.

@leonard84

This comment has been minimized.

Copy link
Collaborator

commented Sep 28, 2018

One possibility would be to change @EnabledIf/@DisabledIf to have Class<? extends ExecutionCondition> or just Class<? extends Predicate<ExtensionContext>> as value which would be instantiated and then invoked to determine the result.

This would reduce some boilerplate for one-off conditions.

@EnabledIf(MyCustomCondition.class)
@Frontrider

This comment has been minimized.

Copy link

commented Sep 28, 2018

One possibility would be to change @EnabledIf/@DisabledIf to have Class<? extends ExecutionCondition> or just Class<? extends Predicate<ExtensionContext>> as value which would be instantiated and then invoked to determine the result.

This would reduce some boilerplate for one-off conditions.

@EnabledIf(MyCustomCondition.class)

I like the idea of that. Basically the same as implementing a java scripting engine, with actual ide support.

@marcphilipp

This comment has been minimized.

Copy link
Member

commented Sep 29, 2018

Is that really so much different from @ExtendWith(MyCustomCondition.class)?

@sormuras

This comment has been minimized.

Copy link
Member Author

commented Sep 30, 2018

Again, the USP of the current @EnabledIf and @DisabledIf programming model is that it uses a simple user-provided string, treats it as a script, and evaluates it with the in-house JavaScript engine. Not a single external library is needed. Every extra work we require the user to perform must be compared to the work of creating a custom ExecutionCondition -- which is already not that much.

I see only three real options:

  • Scrap the API for good: @ExtendWith(MyCustomCondition.class) is easy enough and offers "ide support" out of the box.
  • Resort to javac as the in-house tool evaluating the user-provided predicate.
  • Bundle (shadow) a 3rd party script engine into Jupiter: tiny footprint, well-known syntax, ideally works via JSR-223
@leonard84

This comment has been minimized.

Copy link
Collaborator

commented Oct 1, 2018

Is that really so much different from @ExtendWith(MyCustomCondition.class)?

I'd argue that @EnabledIf(Is42.class) reads better than @ExtendWith(Is42Condition.class). Ideally we could use a lamda here, but that is not possible.

The idea of @EnabledIf is to have small one-off conditions. So you can use static inner classes for that (pseudo code):

public class MyTest {

 @EnableIf(Is42.class)
 @Test
 public void solved() {}

  static class Is42 implements Condition {
    public boolean test(Context context) { return context.contains("42); }
  }
}
@marcphilipp

This comment has been minimized.

Copy link
Member

commented Oct 3, 2018

The downside of a Condition or Predicate over ExecutionCondition is that it lacks the ability to return a reason.

sormuras added a commit that referenced this issue Oct 9, 2018

sormuras added a commit that referenced this issue Oct 9, 2018

@sormuras

This comment has been minimized.

Copy link
Member Author

commented Oct 9, 2018

See PR #1599 which is now very small and concise. Only thing left open: remove default value Nashorn from the annotation declarations.

@sormuras

This comment has been minimized.

Copy link
Member Author

commented Oct 9, 2018

The upside of the java as script engine approach:

@Test
@EnabledIf(
  engine = "java",
  value = "return org.junit.jupiter.api.extension.ConditionEvaluationResult.enabled(\"Go!\");")
void javaWithCustomMessage() {
}

Reasoning still works.

@sormuras

This comment has been minimized.

Copy link
Member Author

commented Oct 10, 2018

Accessing script bindings via attributes also works:

@Test
@EnabledIf(
  engine = "java",
  value = "return \"\\\\o/\".equals(context.getAttribute(\"junitDisplayName\"));")
@DisplayName("\\o/")
void javaAccessingBindings() {
}

Where art thou, Raw String Literals?

@sormuras

This comment has been minimized.

Copy link
Member Author

commented May 14, 2019

Just found this https://github.com/graalvm/graaljs/blob/master/docs/user/ScriptEngine.md and https://medium.com/graalvm/oracle-graalvm-announces-support-for-nashorn-migration-c04810d75c1f ... and at first glance it seemed that OpenJDK 11 ships graal.js by default. But it doesn't. Does it?

@sbrannen

This comment has been minimized.

Copy link
Member

commented May 14, 2019

at first glance it seemed that OpenJDK 11 ships graal.js by default. But it doesn't. Does it?

It doesn't appear so, but the truth is in the build.

@sormuras

This comment has been minimized.

Copy link
Member Author

commented May 14, 2019

You linked "graal-js" -- we want that "project/module/lib" to be included into OpenJDK, though: https://github.com/openjdk/jdk/search?q=Graal&type=Commits

@sormuras

This comment has been minimized.

Copy link
Member Author

commented Jul 13, 2019

Removed @EnabledIf and @DisabledIf via #1902 -- thus closing this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.