[jira] [Commented] (GROOVY-11118) Partial JEP 445 compatibility

[ASF GitHub Bot (Jira)]

    [ 
https://issues.apache.org/jira/browse/GROOVY-11118?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17739791#comment-17739791
 ] 

ASF GitHub Bot commented on GROOVY-11118:
-----------------------------------------

sonatype-lift[bot] commented on code in PR #1910:
URL: https://github.com/apache/groovy/pull/1910#discussion_r1251489336


##########
src/main/java/groovy/lang/GroovyShell.java:
##########
@@ -290,36 +293,62 @@ private Object runScriptOrMainOrTestOrRunnable(Class 
scriptClass, String[] args)
             }
         }
         try {
-            // let's find a main method
-            scriptClass.getMethod(MAIN_METHOD_NAME, String[].class);
-            // if that main method exist, invoke it
-            return InvokerHelper.invokeMethod(scriptClass, MAIN_METHOD_NAME, 
new Object[]{args});
-        } catch (NoSuchMethodException e) {
-            // if it implements Runnable, try to instantiate it
-            if (Runnable.class.isAssignableFrom(scriptClass)) {
-                return runRunnable(scriptClass, args);
+            // let's find a String[] main method
+            Method stringArrayMain = scriptClass.getMethod(MAIN_METHOD_NAME, 
String[].class);
+            // if that main method exists, invoke it
+            if (Modifier.isStatic(stringArrayMain.getModifiers())) {
+                return InvokerHelper.invokeStaticMethod(scriptClass, 
MAIN_METHOD_NAME, new Object[]{args});
+            } else {
+                Object script = 
InvokerHelper.invokeNoArgumentsConstructorOf(scriptClass);
+                return InvokerHelper.invokeMethod(script, MAIN_METHOD_NAME, 
args);
             }
-            GroovyRunnerRegistry runnerRegistry = 
GroovyRunnerRegistry.getInstance();
-            for (GroovyRunner runner : runnerRegistry) {
-                if (runner.canRun(scriptClass, this.loader)) {
-                    return runner.run(scriptClass, this.loader);
-                }
+        } catch (NoSuchMethodException ignore) { }

Review Comment:
   <picture><img alt="25% of developers fix this issue" 
src="https://lift.sonatype.com/api/commentimage/fixrate/25/display.svg";></picture>
   
   
<b>*[EmptyCatch](https://google.github.io/styleguide/javaguide.html#s6.2-caught-exceptions):</b>*
  Caught exceptions should not be ignored
   
   ❗❗ <b>2 similar findings have been found in this PR</b>
   
   <details><summary>🔎 Expand here to view all instances of this 
finding</summary><br/>
     
     
   <div align=\"center\">
   
   
   | **File Path** | **Line Number** |
   | -----------

> Partial JEP 445 compatibility
> -----------------------------
>
>                 Key: GROOVY-11118
>                 URL: https://issues.apache.org/jira/browse/GROOVY-11118
>             Project: Groovy
>          Issue Type: New Feature
>            Reporter: Paul King
>            Assignee: Paul King
>            Priority: Major
>              Labels: GEP, breaking
>             Fix For: 5.x
>
>
> [JEP 445: Unnamed Classes and Instance Main 
> Methods|https://openjdk.org/jeps/445] provides features that improve using 
> Java for scripting purposes. While these features offer a subset of the 
> features offered by Groovy for scripting, it would be nice to offer some 
> interworking for cut-n-paste compatibility and leveraging the updated launch 
> protocol.
> (i) This description will contain the most update-to-date proposal for the 
> design but may undergo change during discussions. If needed, a separate GEP 
> section in the wiki will capture the final design.
> h2. Scripting semantics
> ||Source file characteristics||Semantics determined||
> |Contains class declaration|Compiled as a class|
> |Contains uncontained non-declaration statements
> or a static main method|Compiled as a Groovy script|
> |Contains an instance main method, and
> possibly field and other method declarations|Compiled as a JEP 445 compatible 
> class|
> Groovy scripts are wrapped into an encompassing class that extends 
> {{{}groovy.lang.Script{}}}.
> Such classes have a {{public static void main}} added which (summarising) 
> creates a class instance and calls its {{run}} method.
> Uncontained statements within the script can be thought of as belonging 
> within the {{run}} method.
> Declaration statements within the script are treated as local variable 
> definitions (also within the {{run}} method).
> Field definitions can be obtained by annotating declaration statements with 
> the {{@Field}} annotation.
> JEP 445 scripts do not extend {{Script}} and do not have any methods added.
> Declaration statements within the script are treated as field definitions.
> h2. Script runner
> Groovy's runner mechanism will be extended to follow a similar protocol to 
> Java's revised launch protocol. This should be available for JDK11+ and a 
> JEP-445 compatible class could be invoked using JDK21+ with preview enabled 
> (with Groovy on the classpath unless a POJO class is created).
> h2. Breaking changes
> Previously, if a script had a single instance {{main}} method not taking a 
> {{String[]}} parameter and no loose statements, that was previously accepted 
> as a normal Groovy script (and nothing would execute). Now if the method has 
> an Object parameter, that is treated as a JEP-445 script and its contents 
> executed.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)