From 5a218a6d026f1ea1fe4143da34b5f8b873756cbf Mon Sep 17 00:00:00 2001 From: jrm Date: Mon, 7 Dec 2015 14:31:09 -0800 Subject: [PATCH] treat guava preconditions checks as assume instead of exeption throwing assertions Summary: public The case when a resource leaks is reported because the the resource was not closed on the execution branch created by the preconditions checks are not very interesting in practice because the exceptions thrown, either `NullPointerException` or `IllegalStateException` are very rarely caught anyway. So the legimate use of preconditions checks is creating spurious resource leak reports. Reviewed By: sblackshear Differential Revision: D2707227 fb-gh-sync-id: 6aece73 --- .../com/google/common/base/Preconditions.java | 30 +++++-------------- .../java/infer/NullPointerExceptions.java | 2 +- .../java/infer/ResourceLeaks.java | 6 ++++ 3 files changed, 15 insertions(+), 23 deletions(-) diff --git a/infer/models/java/src/com/google/common/base/Preconditions.java b/infer/models/java/src/com/google/common/base/Preconditions.java index 720195402..fc5c5be8a 100644 --- a/infer/models/java/src/com/google/common/base/Preconditions.java +++ b/infer/models/java/src/com/google/common/base/Preconditions.java @@ -10,53 +10,39 @@ package com.google.common.base; import javax.annotation.Nullable; +import static com.facebook.infer.models.InferBuiltins.assume; public final class Preconditions { public static T checkNotNull(T reference) { - if (reference == null) { - throw new NullPointerException(); - } - return reference; + assume(reference != null); + return reference; } public static T checkNotNull(T reference, @Nullable Object errorMessage) { - if (reference == null) { - throw new NullPointerException(); - } - return reference; + return checkNotNull(reference); } public static T checkNotNull(T reference, @Nullable String errorMessageTemplate, @Nullable Object... errorMessageArgs) { - if (reference == null) { - // If either of these parameters is null, the right thing happens anyway - throw new NullPointerException(errorMessageTemplate); - } - return reference; + return checkNotNull(reference); } public static void checkState(boolean expression) { - if (!expression) { - throw new IllegalStateException(); - } + assume(expression); } public static void checkState(boolean expression, @Nullable Object errorMessage) { - if (!expression) { - throw new IllegalStateException(); - } + assume(expression); } public static void checkState(boolean expression, @Nullable String errorMessageTemplate, @Nullable Object... errorMessageArgs) { - if (!expression) { - throw new IllegalStateException(errorMessageTemplate); - } + assume(expression); } } diff --git a/infer/tests/codetoanalyze/java/infer/NullPointerExceptions.java b/infer/tests/codetoanalyze/java/infer/NullPointerExceptions.java index 1abd21869..d8f04f293 100644 --- a/infer/tests/codetoanalyze/java/infer/NullPointerExceptions.java +++ b/infer/tests/codetoanalyze/java/infer/NullPointerExceptions.java @@ -463,7 +463,7 @@ public class NullPointerExceptions { try { FileLock lock = chan.tryLock(); return (lock != null ? lock.toString() : ""); - } catch (IOException _) { + } catch (IOException e) { Object o = null; return o.toString(); // expect NullPointerException as tryLock can throw } diff --git a/infer/tests/codetoanalyze/java/infer/ResourceLeaks.java b/infer/tests/codetoanalyze/java/infer/ResourceLeaks.java index 7aada8017..fb07dcdbc 100644 --- a/infer/tests/codetoanalyze/java/infer/ResourceLeaks.java +++ b/infer/tests/codetoanalyze/java/infer/ResourceLeaks.java @@ -932,4 +932,10 @@ public class ResourceLeaks { decomp.end(); } + void NoResourceLeakWarningAfterCheckState(File f, int x) throws IOException { + InputStream stream = new FileInputStream(f); + Preconditions.checkState(x > 0); + stream.close(); + } + }