Java: Update docs and promote to quality java/jvm-exit

Author: Napalys

java/ql/integration-tests/java/query-suite/java-code-quality-extended.qls.expected

@@ -81,6 +81,7 @@ ql/java/ql/src/Violations of Best Practice/Naming Conventions/SameNameAsSuper.ql
 ql/java/ql/src/Violations of Best Practice/Records/IgnoredSerializationMembersOfRecordClass.ql
 ql/java/ql/src/Violations of Best Practice/SpecialCharactersInLiterals/NonExplicitControlAndWhitespaceCharsInLiterals.ql
 ql/java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToStringToString.ql
+ql/java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToSystemExit.ql
 ql/java/ql/src/Violations of Best Practice/Undesirable Calls/DefaultToString.ql
 ql/java/ql/src/Violations of Best Practice/Undesirable Calls/DoNotCallFinalize.ql
 ql/java/ql/src/Violations of Best Practice/Undesirable Calls/PrintLnArray.ql

java/ql/integration-tests/java/query-suite/java-code-quality.qls.expected

@@ -79,6 +79,7 @@ ql/java/ql/src/Violations of Best Practice/Naming Conventions/SameNameAsSuper.ql
 ql/java/ql/src/Violations of Best Practice/Records/IgnoredSerializationMembersOfRecordClass.ql
 ql/java/ql/src/Violations of Best Practice/SpecialCharactersInLiterals/NonExplicitControlAndWhitespaceCharsInLiterals.ql
 ql/java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToStringToString.ql
+ql/java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToSystemExit.ql
 ql/java/ql/src/Violations of Best Practice/Undesirable Calls/DefaultToString.ql
 ql/java/ql/src/Violations of Best Practice/Undesirable Calls/DoNotCallFinalize.ql
 ql/java/ql/src/Violations of Best Practice/Undesirable Calls/PrintLnArray.ql

java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToSystemExit.java

@@ -4,7 +4,7 @@ boolean write(String[] s) {
         try {
             output.write(s.getBytes());
         } catch (IOException e) {
-            System.exit(1);
+            System.exit(1); // BAD: Should handle or propagate error instead of exiting
         }
         return true;
     }
@@ -16,9 +16,30 @@ public void run() {
         // ...
         // Perform tasks ...
         // ...
-        System.exit(0);
+        System.exit(0); // BAD: Should return status or throw exception
     }
     public static void main(String[] args) {
-        new Action(args).run();
+        new Action().run();
     }
-}
+}
+
+// Good example: Proper error handling
+class BetterAction {
+    public int run() throws Exception {
+        // ...
+        // Perform tasks ...
+        // ...
+        return 0; // Return status instead of calling System.exit
+    }
+    
+    public static void main(String[] args) {
+        try {
+            BetterAction action = new BetterAction();
+            int exitCode = action.run();
+            System.exit(exitCode); // GOOD: Exit from main method
+        } catch (Exception e) {
+            System.err.println("Error: " + e.getMessage());
+            System.exit(1); // GOOD: Exit from main method on error
+        }
+    }
+}

java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToSystemExit.qhelp

@@ -13,17 +13,20 @@ program state from being written to disk consistently.</p>
 
 <p>It is sometimes considered acceptable to call <code>System.exit</code>
 from a program's <code>main</code> method in order to indicate the overall exit status
-of the program. Such calls are an exception to this rule.</p>
+of the program. The <code>main</code> method should be the primary place
+where exit conditions are handled, as it represents the natural termination point
+of the application. Such calls are an exception to this rule.</p>
 
 </overview>
 <recommendation>
 
-<p>It is usually preferable to use a different mechanism for reporting
-failure conditions. Consider returning a special value (perhaps
-<code>null</code>) that users of the current method check for and
-recover from appropriately. Alternatively, throw a suitable exception, which 
-unwinds the stack and allows properly written code to clean up after itself,
-while leaving other threads undisturbed.</p>
+<p>Instead of calling <code>System.exit</code> from non-main methods, prefer to propagate
+errors upward to the <code>main</code> method where they can be handled appropriately.
+Consider returning a special value (perhaps <code>null</code>) that users of the current 
+method check for and recover from appropriately. Alternatively, throw a suitable exception, 
+which unwinds the stack and allows properly written code to clean up after itself,
+while leaving other threads undisturbed. The <code>main</code> method can then catch
+these exceptions and decide whether to exit the program and with what exit code.</p>
 
 </recommendation>
 <example>
@@ -33,17 +36,20 @@ to write some data to disk and terminates the JVM if this fails. This
 leaves the partially-written file on disk without any cleanup
 code running. It would be better to either return <code>false</code> to
 indicate the failure, or let the <code>IOException</code> propagate
-upwards and be handled by a method that knows how to recover.</p>
+upwards to the <code>main</code> method where it can be handled appropriately
+and the program can exit gracefully if necessary.</p>
 
 <p>Problem 2 is more subtle. In this example, there is just one entry point to
 the program (the <code>main</code> method), which constructs an
 <code>Action</code> and performs it. <code>Action.run</code> calls
-<code>System.exit</code> to indicate successful completion. Consider,
-however, how this code might be integrated in an application server that
-constructs <code>Action</code> instances and calls
+<code>System.exit</code> to indicate successful completion. Instead, the 
+<code>run</code> method should return a status code or throw an exception
+on failure, allowing the <code>main</code> method to decide whether to exit
+and with what exit code. Consider how this code might be integrated in an 
+application server that constructs <code>Action</code> instances and calls
 <code>run</code> on them without going through <code>main</code>.
 The fact that <code>run</code> terminates the JVM instead of returning its
-exit code as an integer makes that use-case impossible.</p>
+exit code makes that use-case impossible.</p>
 
 <sample src="CallsToSystemExit.java" />
 

java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToSystemExit.ql

@@ -4,10 +4,11 @@
  *              reuse and prevent important cleanup steps from running.
  * @kind problem
  * @problem.severity warning
- * @precision low
+ * @precision high
  * @id java/jvm-exit
- * @tags reliability
- *       maintainability
+ * @tags quality
+ *       reliability
+ *       correctness
  *       external/cwe/cwe-382
  */
 
@@ -22,18 +23,12 @@ import java
  */
 class ExitOrHaltMethod extends Method {
   ExitOrHaltMethod() {
-    exists(Class system |
-      this.getDeclaringType() = system and
-      (
-        this.hasName("exit") and
-        (
-          system.hasQualifiedName("java.lang", "System") or
-          system.hasQualifiedName("java.lang", "Runtime")
-        )
-        or
-        this.hasName("halt") and
-        system.hasQualifiedName("java.lang", "Runtime")
-      )
+    exists(Class system | this.getDeclaringType() = system |
+      this.hasName("exit") and
+      system.hasQualifiedName("java.lang", ["System", "Runtime"])
+      or
+      this.hasName("halt") and
+      system.hasQualifiedName("java.lang", "Runtime")
     )
   }
 }
@@ -48,7 +43,7 @@ class ExitOrHaltMethodCall extends MethodCall {
 }
 
 /**
- * Represents an intentional `MethodCall` to a system or runtime "exit" method, such as for
+ * An intentional `MethodCall` to a system or runtime "exit" method, such as for
  * functions which exist for the purpose of exiting the program. Assumes that a an exit method
  * call within a method is intentional if the exit code is passed from a parameter of the
  * enclosing method.