SONARJAVA-5444 Handle case of Reader.of for try-with-resources (#5231)

Author: romainbrenguier

java-checks-test-sources/default/src/main/java/checks/TryWithResourcesCheckSample.java

@@ -3,8 +3,37 @@
 import java.io.BufferedReader;
 import java.io.FileReader;
 import java.io.IOException;
+import java.io.Reader;
 
 class TryWithResourcesCheckSample {
+  String readerFactoryNonCompliant(String sequence) throws IOException {
+    char[] buf = new char[1024];
+    Reader reader = null;
+    try { // Noncompliant {{Change this "try" to a try-with-resources.}}
+//  ^^^
+      reader = Reader.of(sequence);
+//             ^^^^^^^^^^^^^^^^^^^<
+      reader.read(buf, 0, buf.length);
+    } catch (IOException e) {
+      throw new RuntimeException(e);
+    } finally {
+      if (reader != null) {
+        reader.close();
+      }
+    }
+    return String.valueOf(buf);
+  }
+
+  String readerFactoryCompliant(String sequence) {
+    char[] buf = new char[1024];
+    try (Reader reader = Reader.of(sequence)) { // Compliant
+      reader.read(buf, 0, buf.length);
+    } catch (IOException e) {
+      throw new RuntimeException(e);
+    }
+    return String.valueOf(buf);
+  }
+
   String foo(String fileName) {
     FileReader fr = null;
     BufferedReader br = null;
@@ -32,7 +61,7 @@ String foo(String fileName) {
     }
     try { // compliant, no finally block so let's rely on unclosed resource rule
       fr = new FileReader(fileName);
-    } catch (Exception e){
+    } catch (Exception e) {
 
     }
     try (
@@ -52,7 +81,7 @@ void newJustBeforeTryStatement() {
     try { // Noncompliant {{Change this "try" to a try-with-resources.}}
 //  ^^^
       a1.doSomething();
-    }  finally {
+    } finally {
       a1.close();
       a2.close();
     }
@@ -67,7 +96,7 @@ void newJustBeforeAndAfterTryStatement() {
       a1 = new Auto();
 //         ^^^^^^^^^^<
       a1.doSomething();
-    }  finally {
+    } finally {
       a1.close();
       a2.close();
     }
@@ -78,30 +107,33 @@ void methodBetweenNewAndTry() {
     a.doSomething();
     try {
       a.doSomething();
-    }  finally {
+    } finally {
       a.close();
     }
   }
 
-  class B {}
+  class B {
+  }
 
   void unknownNewBetweenNewAndTry() {
     Auto a = new Auto();
     B b = new B();
     try {
       a.doSomething();
-    }  finally {
+    } finally {
       a.close();
     }
   }
 
-  Auto passThrough(Auto a) { return a; }
+  Auto passThrough(Auto a) {
+    return a;
+  }
 
   void newInsideMethodInvocation() {
     Auto a = passThrough(new Auto()); // Compliant, we do not know what happens in the method
     try {
       a.doSomething();
-    }  finally {
+    } finally {
       a.close();
     }
   }
@@ -110,7 +142,7 @@ void newJustBeforeTryWithResource() {
     Auto a1 = new Auto();
     try (Auto a2 = new Auto()) {
       a1.doSomething();
-    }  finally {
+    } finally {
       a1.close();
     }
   }
@@ -128,7 +160,7 @@ void enclosedTryWithFinallyStatements() {
       } finally {
         a2.close();
       }
-    }  finally {
+    } finally {
       a1.close();
     }
   }
@@ -144,8 +176,9 @@ void enclosedTryStatements() {
       try {
         a2.doSomething();
         a2.close();
-      } catch (Exception e) {}
-    }  finally {
+      } catch (Exception e) {
+      }
+    } finally {
       a1.close();
     }
   }
@@ -161,10 +194,12 @@ void method_with_while_continue(boolean a) {
   }
 
   static class Auto implements AutoCloseable {
-    public void doSomething() {}
+    public void doSomething() {
+    }
 
     @Override
-    public void close() {}
+    public void close() {
+    }
   }
 
 }

java-checks/src/main/java/org/sonar/java/checks/TryWithResourcesCheck.java

@@ -42,6 +42,11 @@ public class TryWithResourcesCheck extends IssuableSubscriptionVisitor implement
     MethodMatchers.create().ofTypes("java.net.http.HttpClient").names("newHttpClient").addWithoutParametersMatcher().build()
   );
 
+  private static final MethodMatchers AUTOCLOSEABLE_FACTORY_MATCHER =
+    MethodMatchers.create().ofSubTypes("java.io.Reader")
+      .names("of")
+      .addParametersMatcher("java.lang.CharSequence").build();
+
   private final Deque<TryStatementTree> withinTry = new LinkedList<>();
   private final Deque<List<Tree>> toReport = new LinkedList<>();
 
@@ -78,7 +83,8 @@ public void visitNode(Tree tree) {
 
   private static boolean isNewAutocloseableOrBuilder(Tree tree, JavaFileScannerContext context) {
     return (tree instanceof NewClassTree newClass && newClass.symbolType().isSubtypeOf("java.lang.AutoCloseable")) ||
-      (context.getJavaVersion().isJava21Compatible() && tree instanceof MethodInvocationTree mit && AUTOCLOSEABLE_BUILDER_MATCHER.matches(mit));
+      (context.getJavaVersion().isJava21Compatible() && tree instanceof MethodInvocationTree mit && AUTOCLOSEABLE_BUILDER_MATCHER.matches(mit))||
+      (tree instanceof MethodInvocationTree mit2 && AUTOCLOSEABLE_FACTORY_MATCHER.matches(mit2));
   }
 
   private static boolean isFollowedByTryWithFinally(Tree tree) {