GROOVY-8251: Implement withCloseable on AutoCloseable

src/main/org/codehaus/groovy/runtime/DefaultGroovyMethodsSupport.java

@@ -83,16 +83,33 @@ protected static int normaliseIndex(int i, int size) {
     /**
      * Close the Closeable. Logging a warning if any problems occur.
      *
-     * @param c the thing to close
+     * @param closeable the thing to close
      */
-    public static void closeWithWarning(Closeable c) {
-        if (c != null) {
+    public static void closeWithWarning(Closeable closeable) {
+        tryClose(closeable, true); // ignore result
+    }
+
+    /**
+     * Attempts to close the closeable returning rather than throwing
+     * any Exception that may occur.
+     *
+     * @param closeable the thing to close
+     * @param logWarning if true will log a warning if an exception occurs
+     * @return throwable Exception from the close method, else null
+     */
+    static Throwable tryClose(AutoCloseable closeable, boolean logWarning) {
+        Throwable thrown = null;
+        if (closeable != null) {
             try {
-                c.close();
-            } catch (IOException e) {
-                LOG.warning("Caught exception during close(): " + e);
+                closeable.close();
+            } catch (Exception e) {
+                thrown = e;
+                if (logWarning) {
+                    LOG.warning("Caught exception during close(): " + e);
+                }
             }
         }
+        return thrown;
     }
 
     /**

src/main/org/codehaus/groovy/runtime/IOGroovyMethods.java

@@ -1589,6 +1589,10 @@ public static void filterLine(InputStream self, Writer writer, String charset, @
     /**
      * Allows this closeable to be used within the closure, ensuring that it
      * is closed once the closure has been executed and before this method returns.
+     * <p>
+     * As with the try-with-resources statement, if multiple exceptions are thrown
+     * the exception from the closure will be returned and the exception from closing
+     * will be added as a suppressed exception.
      *
      * @param self the Closeable
      * @param action the closure taking the Closeable as parameter
@@ -1597,16 +1601,54 @@ public static void filterLine(InputStream self, Writer writer, String charset, @
      * @since 2.4.0
      */
     public static <T, U extends Closeable> T withCloseable(U self, @ClosureParams(value=FirstParam.class) Closure<T> action) throws IOException {
+        Throwable thrown = null;
         try {
-            T result = action.call(self);
-
-            Closeable temp = self;
-            self = null;
-            temp.close();
+            return action.call(self);
+        } catch (Throwable e) {
+            thrown = e;
+            throw e;
+        } finally {
+            if (thrown != null) {
+                Throwable suppressed = tryClose(self, true);
+                if (suppressed != null) {
+                    thrown.addSuppressed(suppressed);
+                }
+            } else {
+                self.close();
+            }
+        }
+    }
 
-            return result;
+    /**
+     * Allows this AutoCloseable to be used within the closure, ensuring that it
+     * is closed once the closure has been executed and before this method returns.
+     * <p>
+     * As with the try-with-resources statement, if multiple exceptions are thrown
+     * the exception from the closure will be returned and the exception from closing
+     * will be added as a suppressed exception.
+     *
+     * @param self the AutoCloseable
+     * @param action the closure taking the AutoCloseable as parameter
+     * @return the value returned by the closure
+     * @throws Exception if an Exception occurs.
+     * @since 2.5.0
+     */
+    public static <T, U extends AutoCloseable> T withCloseable(U self, @ClosureParams(value=FirstParam.class) Closure<T> action) throws Exception {
+        Throwable thrown = null;
+        try {
+            return action.call(self);
+        } catch (Throwable e) {
+            thrown = e;
+            throw e;
         } finally {
-            DefaultGroovyMethodsSupport.closeWithWarning(self);
+            if (thrown != null) {
+                Throwable suppressed = tryClose(self, true);
+                if (suppressed != null) {
+                    thrown.addSuppressed(suppressed);
+                }
+            } else {
+                self.close();
+            }
         }
     }
 

src/test/org/codehaus/groovy/runtime/IOGroovyMethodsTest.groovy

@@ -0,0 +1,130 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package org.codehaus.groovy.runtime
+
+import groovy.test.GroovyAssert
+
+class IOGroovyMethodsTest extends GroovyTestCase {
+
+    void testWithAutoCloseable() {
+        def closeable = new DummyAutoCloseable()
+        def closeableParam = null
+        def result = closeable.withCloseable {
+            closeableParam = it
+            123
+        }
+        assert closeableParam == closeable
+        assert result == 123
+        assert closeable.closed
+    }
+
+    void testWithAutoCloseableDoesNotSuppressException() {
+        def closeable = new DummyAutoCloseable(new Exception('close exception'))
+        def throwable = GroovyAssert.shouldFail(UnsupportedOperationException) {
+            closeable.withCloseable {
+                throw new UnsupportedOperationException('not a close exception')
+            }
+        }
+        assert closeable.closed
+        assert throwable.message == 'not a close exception'
+        assert throwable.suppressed.find { it.message == 'close exception' }
+    }
+
+    void testWithAutoCloseableAndException() {
+        def closeable = new DummyAutoCloseable(new Exception('close exception'))
+        def result = null
+        def message = shouldFail(Exception) {
+            closeable.withCloseable {
+                result = 123
+            }
+        }
+        assert result == 123
+        assert message == 'close exception'
+    }
+
+    void testWithCloseable() {
+        def closeable = new DummyCloseable()
+        def closeableParam = null
+        def result = closeable.withCloseable {
+            closeableParam = it
+            123
+        }
+        assert closeableParam == closeable
+        assert result == 123
+        assert closeable.closed
+    }
+
+    void testWithCloseableDoesNotSuppressException() {
+        def closeable = new DummyCloseable(new IOException('close ioexception'))
+        def throwable = GroovyAssert.shouldFail(Exception) {
+            closeable.withCloseable {
+                throw new Exception('not a close ioexception')
+            }
+        }
+        assert closeable.closed
+        assert throwable.message == 'not a close ioexception'
+        assert throwable.suppressed.find { it.message == 'close ioexception' }
+    }
+
+    void testWithCloseableAndException() {
+        def closeable = new DummyCloseable(new IOException('close ioexception'))
+        def result = null
+        def message = shouldFail(IOException) {
+            closeable.withCloseable {
+                result = 123
+            }
+        }
+        assert result == 123
+        assert message == 'close ioexception'
+    }
+
+    // --------------------------------------------------------------------
+    // Helper Classes
+
+    static class DummyAutoCloseable implements AutoCloseable {
+        Exception throwOnClose
+        boolean closed
+        DummyAutoCloseable(Exception throwOnClose=null) {
+            this.throwOnClose = throwOnClose
+        }
+        @Override
+        void close() throws Exception {
+            closed = true
+            if (throwOnClose) {
+                throw throwOnClose
+            }
+        }
+    }
+
+    static class DummyCloseable implements Closeable {
+        Exception throwOnClose
+        boolean closed
+        DummyCloseable(Exception throwOnClose=null) {
+            this.throwOnClose = throwOnClose
+        }
+        @Override
+        void close() throws IOException {
+            closed = true
+            if (throwOnClose) {
+                throw throwOnClose
+            }
+        }
+    }
+
+}

subprojects/groovy-nio/src/main/java/org/codehaus/groovy/runtime/NioGroovyMethods.java

@@ -50,7 +50,6 @@
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
-import java.util.logging.Logger;
 import java.util.regex.Pattern;
 
 import groovy.io.FileType;
@@ -60,7 +59,6 @@
 import groovy.lang.MetaClass;
 import groovy.lang.Writable;
 import groovy.transform.stc.ClosureParams;
-import groovy.transform.stc.FirstParam;
 import groovy.transform.stc.FromString;
 import groovy.transform.stc.PickFirstResolver;
 import groovy.transform.stc.SimpleType;
@@ -113,8 +111,6 @@
 
 public class NioGroovyMethods extends DefaultGroovyMethodsSupport {
 
-    private static final Logger LOG = Logger.getLogger(NioGroovyMethods.class.getName());
-
     /**
      * Provide the standard Groovy <code>size()</code> method for <code>Path</code>.
      *
@@ -1949,44 +1945,4 @@ public static <T> T withCloseable(Closeable self, @ClosureParams(value = SimpleT
         return IOGroovyMethods.withCloseable(self, action);
     }
 
-    /**
-     * Allows this autocloseable to be used within the closure, ensuring that it
-     * is closed once the closure has been executed and before this method returns.
-     *
-     * @param self the AutoCloseable
-     * @param action the closure taking the AutoCloseable as parameter
-     * @return the value returned by the closure
-     * @throws Exception if an Exception occurs.
-     * @since 2.5.0
-     */
-    public static <T, U extends AutoCloseable> T withAutoCloseable(U self, @ClosureParams(value=FirstParam.class) Closure<T> action) throws Exception {
-        try {
-            T result = action.call(self);
-
-            AutoCloseable temp = self;
-            self = null;
-            temp.close();
-
-            return result;
-        } finally {
-            closeWithWarning(self);
-        }
-    }
-
-    /**
-     * Close the AutoCloseable. Logging a warning if any problems occur.
-     *
-     * @param c the thing to close
-     */
-    public static void closeWithWarning(AutoCloseable c) {
-        if (c != null) {
-            try {
-                c.close();
-            } catch (Exception e) {
-                LOG.warning("Caught exception during close(): " + e);
-            }
-        }
-    }
-
-
 }

subprojects/groovy-nio/src/test/groovy/org/codehaus/groovy/runtime/NioGroovyMethodsTest.groovy

@@ -543,73 +543,4 @@ class NioGroovyMethodsTest extends Specification {
         assert NioGroovyMethods.getBytes(path) == [-2, -1, 0, 72, 0, 101, 0, 108, 0, 108, 0, 111, 0, 32, 0, 119, 0, 111, 0, 114, 0, 108, 0, 100, 0, 33] as byte[]
     }
 
-    def testWithCloseable() {
-        setup:
-        def closeable = new DummyCloseable()
-
-        when:
-        def closeableParam = null
-        def result = closeable.withCloseable {
-            closeableParam = it
-            123
-        }
-
-        then:
-        closeableParam == closeable
-        result == 123
-        closeable.closed
-    }
-
-    def testWithAutoCloseable() {
-        setup:
-            def closeable = new DummyAutoCloseable()
-
-        when:
-            def closeableParam = null
-            def result = closeable.withAutoCloseable {
-                closeableParam = it
-                123
-            }
-
-        then:
-            closeableParam == closeable
-            result == 123
-            closeable.closed
-    }
-
-    def testWithCloseableAndException() {
-        setup:
-        def closeable = new ExceptionDummyCloseable()
-
-        when:
-        closeable.close()
-
-        then:
-        thrown IOException
-    }
-}
-
-class DummyCloseable implements Closeable {
-    boolean closed = false
-
-    @Override
-    void close() throws IOException {
-        closed = true
-    }
-}
-
-class DummyAutoCloseable implements AutoCloseable {
-    boolean closed = false
-
-    @Override
-    void close() throws IOException {
-        closed = true
-    }
-}
-
-class ExceptionDummyCloseable implements Closeable {
-    @Override
-    void close() throws IOException {
-        throw new IOException('boom badaboom')
-    }
 }