I've cleaned this up a bit in git master but I must -1 to the class name Objects and even the package location.
Class Name: I will end up referring to both java.util.Objects and this new org.apache.commons.lang3.function.Objects in the same source file which means that I'll be forced to use the fully qualified class name for one of the classes which is nasty and smelly. Since this is all about NPEs should we consider an NPE specific class (NullChecks?)? Note that we already have a possible home for these kinds of utilities in org.apache.commons.lang3.exception.*ExceptionUtils*, see below. Package Location: A definitie -1 since this is not about lambads. Why is this in org.apache.commons.lang3.function? It seems like org.apache.commons.lang3 is the proper home unless we want to parallel the JRE and put it in org.apache.commons.lang3.util which is OK with me but a bit odd. I'd be remiss to fail to mention that we already have a package called org.apache.commons.lang3.*exception* so it really should go in there. Looking forward to thoughts. Gary On Sat, Feb 6, 2021 at 4:52 PM <joc...@apache.org> wrote: > This is an automated email from the ASF dual-hosted git repository. > > jochen pushed a commit to branch master > in repository https://gitbox.apache.org/repos/asf/commons-lang.git > > > The following commit(s) were added to refs/heads/master by this push: > new 45e8395 Introducing @Nonnull, @Nullable, and the Objects class > as a helper tool. > 45e8395 is described below > > commit 45e8395ec7a63f432ce8df03159ee5465b1380ca > Author: Jochen Wiedmann <jochen.wiedm...@gmail.com> > AuthorDate: Sat Feb 6 22:51:47 2021 +0100 > > Introducing @Nonnull, @Nullable, and the Objects class as a helper > tool. > --- > pom.xml | 7 +- > spotbugs-exclude-filter.xml | 8 + > src/changes/changes.xml | 1 + > .../org/apache/commons/lang3/function/Objects.java | 176 > +++++++++++++++++++++ > .../apache/commons/lang3/function/ObjectsTest.java | 146 +++++++++++++++++ > 5 files changed, 337 insertions(+), 1 deletion(-) > > diff --git a/pom.xml b/pom.xml > index f8524e8..5135e4c 100644 > --- a/pom.xml > +++ b/pom.xml > @@ -566,7 +566,12 @@ > <version>${jmh.version}</version> > <scope>test</scope> > </dependency> > - > + <dependency> > + <groupId>com.google.code.findbugs</groupId> > + <artifactId>jsr305</artifactId> > + <version>3.0.2</version> > + <scope>provided</scope> > + </dependency> > </dependencies> > > <distributionManagement> > diff --git a/spotbugs-exclude-filter.xml b/spotbugs-exclude-filter.xml > index 8636890..be00732 100644 > --- a/spotbugs-exclude-filter.xml > +++ b/spotbugs-exclude-filter.xml > @@ -153,4 +153,12 @@ > <Method name="compare" /> > <Bug pattern="RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE" /> > </Match> > + > + <!-- Reason: requireNonNull is supposed to take a nullable parameter, > + whatever Spotbugs thinks of it. --> > + <Match> > + <Class name="org.apache.commons.lang3.function.Objects" /> > + <Method name="requireNonNull" /> > + <Bug pattern="NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE" /> > + </Match> > </FindBugsFilter> > diff --git a/src/changes/changes.xml b/src/changes/changes.xml > index 1eca750..4908376 100644 > --- a/src/changes/changes.xml > +++ b/src/changes/changes.xml > @@ -105,6 +105,7 @@ The <action> type attribute can be > add,update,fix,remove. > <action type="update" dev="ggregory" due-to="Ali K. > Nouri">Processor.java: check enum equality with == instead of .equals() > method #690.</action> > <action type="update" dev="ggregory" > due-to="Dependabot">Bump junit-pioneer from 1.1.0 to 1.3.0 #702.</action> > <action type="update" dev="ggregory" > due-to="Dependabot">Bump maven-checkstyle-plugin from 3.1.1 to 3.1.2 > #705.</action> > + <action type="add" dev="jochen">Introduce the > use of @Nonnull, and @Nullable, and the Objects class as a helper > tool.</action> > </release> > <release version="3.11" date="2020-07-12" description="New features and > bug fixes."> > <action type="update" dev="chtompki" due-to="Jin > Xu">Refine test output for FastDateParserTest</action> > diff --git a/src/main/java/org/apache/commons/lang3/function/Objects.java > b/src/main/java/org/apache/commons/lang3/function/Objects.java > new file mode 100755 > index 0000000..02f7ddb > --- /dev/null > +++ b/src/main/java/org/apache/commons/lang3/function/Objects.java > @@ -0,0 +1,176 @@ > +/* > + * 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.apache.commons.lang3.function; > + > +import java.util.function.Supplier; > + > +import javax.annotation.Nonnull; > +import javax.annotation.Nullable; > + > +import org.apache.commons.lang3.ObjectUtils; > + > + > +/** > + * This class provides some replacements for the corresponding methods in > + * {@link Objects}. The replacements have the advantage, that they are > properly > + * annotated with {@link Nullable}, and/or {@link Nonnull}, so they let > the > + * compiler know, what their respective results are. > + * > + * The various {@code requireNonNull} methods are particularly handy, when > + * dealing with external code, that a) doesn't support the {@link Nonnull} > + * annotation, or if you know for other reasons, that an object is > non-null. > + * Take for example, a {@link java.util.Map map}, that you have filled > with > + * non-null values. So, in your opinion, the following should be > perfectably > + * valid code: > + * <pre> > + * final Map<String,Object> map = getMapOfNonNullValues(); > + * final @Nonnull Object o = map.get("SomeKey"); > + * </pre> > + * However, if your Java compiler *does* null analysis, it will reject > this > + * example as invalid, because {@link java.util.Map#get(Object)} might > return > + * a null value. As a workaround, you can use this: > + * <pre> > + * import static > org.apache.commons.lang3.function.Objects.requireNonNull; > + * > + * final Map<String,Object> map = getMapOfNonNullValues(); > + * final @Nonnull Object o = requireNonNull(map.get("SomeKey")); > + * </pre> > + * > + * This class is somewhat redundant with regards to {@link ObjectUtils}. > + * For example, {@link #requireNonNull(Object, Object)} is almost > equivalent > + * with {@link ObjectUtils#defaultIfNull(Object, Object)}. However, it > isn't > + * quite the same, because the latter can, in fact, return null. The > former > + * can't, and the Java compiler confirms this.(An alternative to > redundancy > + * would have been to change the {@code ObjectUtils} class. However, that > + * would mean loosing upwards compatibility, and we don't do that.) > + */ > +public class Objects { > + /** > + * Checks, whether the given object is non-null. If so, returns the > non-null > + * object as a result value. Otherwise, a NullPointerException is > thrown. > + * @param <O> The type of parameter {@code value}, also the result > type. > + * @param value The value, which is being checked. > + * @return The given input value, if it was found to be non-null. > + * @throws NullPointerException The input value was null. > + * @see java.util.Objects#requireNonNull(Object) > + */ > + public static <O> @Nonnull O requireNonNull(@Nullable O value) throws > NullPointerException { > + if (value == null) { > + throw new NullPointerException("The value must not be null."); > + } else { > + return value; > + } > + } > + > + /** > + * Checks, whether the given object is non-null. If so, returns the > non-null > + * object as a result value. Otherwise, a NullPointerException is > thrown. > + * @param <O> The type of parameter {@code value}, also the result > type. > + * @param value The value, which is being checked. > + * @param defaultValue The default value, which is being returned, if > the > + * check fails, and the {@code value} is null. > + * @throws NullPointerException The input value, and the default > value are null. > + * @return The given input value, if it was found to be non-null. > + * @see java.util.Objects#requireNonNull(Object) > + */ > + public static <O> @Nonnull O requireNonNull(@Nullable O value, > @Nonnull O defaultValue) { > + if (value == null) { > + if (defaultValue == null) { > + throw new NullPointerException("The default value must > not be null."); > + } else { > + return defaultValue; > + } > + } else { > + return value; > + } > + } > + > + /** > + * Checks, whether the given object is non-null. If so, returns the > non-null > + * object as a result value. Otherwise, a NullPointerException is > thrown. > + * @param <O> The type of parameter {@code value}, also the result > type. > + * @param value The value, which is being checked. > + * @param msg A string, which is being used as the exceptions > message, if the > + * check fails. > + * @return The given input value, if it was found to be non-null. > + * @throws NullPointerException The input value was null. > + * @see java.util.Objects#requireNonNull(Object, String) > + * @see #requireNonNull(Object, Supplier) > + */ > + public static <O> @Nonnull O requireNonNull(@Nullable O value, > @Nonnull String msg) throws NullPointerException { > + if (value == null) { > + throw new NullPointerException(msg); > + } else { > + return value; > + } > + } > + > + /** > + * Checks, whether the given object is non-null. If so, returns the > non-null > + * object as a result value. Otherwise, a NullPointerException is > thrown. > + * @param <O> The type of parameter {@code value}, also the result > type. > + * @param value The value, which is being checked. > + * @param msgSupplier A supplier, which creates the exception > message, if the check fails. > + * This supplier will only be invoked, if necessary. > + * @return The given input value, if it was found to be non-null. > + * @throws NullPointerException The input value was null. > + * @see java.util.Objects#requireNonNull(Object, String) > + * @see #requireNonNull(Object, String) > + */ > + public static <O> @Nonnull O requireNonNull(@Nullable O value, > @Nonnull Supplier<String> msgSupplier) throws NullPointerException { > + if (value == null) { > + throw new NullPointerException(msgSupplier.get()); > + } else { > + return value; > + } > + } > + > + /** Checks, whether the given object is non-null. If so, returns the > non-null > + * object as a result value. Otherwise, invokes the given {@link > Supplier}, > + * and returns the suppliers result value. > + * @param <O> The type of parameter {@code value}, also the result > type of > + * the default value supplier, and of the method itself. > + * @param <E> The type of exception, that the {@code default value > supplier}, > + * may throw. > + * @param value The value, which is being checked. > + * @param defaultValueSupplier The supplier, which returns the > default value. This default > + * value <em>must</em> be non-null. The supplier will only be > invoked, if > + * necessary. (If the {@code value} parameter is null, that is.) > + * @return The given input value, if it was found to be non-null. > Otherwise, > + * the value, that has been returned by the default value supplier. > + * @see #requireNonNull(Object) > + * @see #requireNonNull(Object, String) > + * @see #requireNonNull(Object, Supplier) > + * @throws NullPointerException The default value supplier is null, > or the default > + * value supplier has returned null. > + */ > + public static <O, E extends Throwable> @Nonnull O > requireNonNull(@Nullable O value, @Nonnull FailableSupplier<O, E> > defaultValueSupplier) { > + if (value == null) { > + final FailableSupplier<O, ?> supp = > requireNonNull(defaultValueSupplier, "The supplier must not be null"); > + final O o; > + try { > + o = supp.get(); > + } catch (Throwable t) { > + throw Failable.rethrow(t); > + } > + return requireNonNull(o, "The supplier must not return > null."); > + } else { > + return value; > + } > + } > +} > diff --git > a/src/test/java/org/apache/commons/lang3/function/ObjectsTest.java > b/src/test/java/org/apache/commons/lang3/function/ObjectsTest.java > new file mode 100755 > index 0000000..cea1c8e > --- /dev/null > +++ b/src/test/java/org/apache/commons/lang3/function/ObjectsTest.java > @@ -0,0 +1,146 @@ > +/* > + * 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.apache.commons.lang3.function; > + > +import static org.junit.jupiter.api.Assertions.assertEquals; > +import static org.junit.jupiter.api.Assertions.assertFalse; > +import static org.junit.jupiter.api.Assertions.assertSame; > +import static org.junit.jupiter.api.Assertions.assertTrue; > +import static org.junit.jupiter.api.Assertions.fail; > + > +import java.util.function.Supplier; > + > +import org.junit.jupiter.api.Test; > + > + > +class ObjectsTest { > + @Test > + void testRequireNonNullObject() { > + assertSame("foo", Objects.requireNonNull("foo")); > + try { > + Objects.requireNonNull(null); > + fail("Expected Exception"); > + } catch (NullPointerException e) { > + assertEquals("The value must not be null.", e.getMessage()); > + } > + } > + > + @Test > + void testRequireNonNullObjectString() { > + assertSame("foo", Objects.requireNonNull("foo", "bar")); > + try { > + Objects.requireNonNull(null, "bar"); > + fail("Expected Exception"); > + } catch (NullPointerException e) { > + assertEquals("bar", e.getMessage()); > + } > + } > + > + public static class TestableSupplier<O> implements Supplier<O> { > + private final Supplier<O> supplier; > + private boolean invoked; > + > + TestableSupplier(Supplier<O> pSupplier) { > + this.supplier = pSupplier; > + } > + > + @Override > + public O get() { > + invoked = true; > + return supplier.get(); > + } > + > + public boolean isInvoked() { > + return invoked; > + } > + } > + > + @Test > + void testRequireNonNullObjectSupplierString() { > + final TestableSupplier<String> supplier = new > TestableSupplier<>(() -> "bar"); > + assertSame("foo", Objects.requireNonNull("foo", supplier)); > + assertFalse(supplier.isInvoked()); > + try { > + Objects.requireNonNull(null, supplier); > + fail("Expected Exception"); > + } catch (NullPointerException e) { > + assertEquals("bar", e.getMessage()); > + assertTrue(supplier.isInvoked()); > + } > + } > + > + public static class TestableFailableSupplier<O, E extends Exception> > implements FailableSupplier<O, E> { > + private final FailableSupplier<O, E> supplier; > + private boolean invoked; > + > + TestableFailableSupplier(FailableSupplier<O, E> pSupplier) { > + this.supplier = pSupplier; > + } > + > + @Override > + public O get() throws E { > + invoked = true; > + return supplier.get(); > + } > + > + public boolean isInvoked() { > + return invoked; > + } > + } > + > + @Test > + void testRequireNonNullObjectFailableSupplierString() { > + final TestableFailableSupplier<String, ?> supplier = new > TestableFailableSupplier<>(() -> { > + return null; > + }); > + assertSame("foo", Objects.requireNonNull("foo", supplier)); > + assertFalse(supplier.isInvoked()); > + try { > + Objects.requireNonNull(null, supplier); > + fail("Expected Exception"); > + } catch (NullPointerException e) { > + assertEquals("The supplier must not return null.", > e.getMessage()); > + assertTrue(supplier.isInvoked()); > + } > + final TestableFailableSupplier<String, ?> supplier2 = new > TestableFailableSupplier<>(() -> { > + return null; > + }); > + try { > + Objects.requireNonNull(null, supplier2); > + fail("Expected Exception"); > + } catch (NullPointerException e) { > + assertEquals("The supplier must not return null.", > e.getMessage()); > + assertTrue(supplier2.isInvoked()); > + } > + final TestableFailableSupplier<String, ?> supplier3 = new > TestableFailableSupplier<>(() -> { > + return "bar"; > + }); > + assertSame("bar", Objects.requireNonNull(null, supplier3)); > + final RuntimeException rte = new RuntimeException(); > + final TestableFailableSupplier<String, ?> supplier4 = new > TestableFailableSupplier<>(() -> { > + throw rte; > + }); > + try { > + Objects.requireNonNull(null, supplier4); > + fail("Expected Exception"); > + } catch (RuntimeException e) { > + assertSame(rte, e); > + assertTrue(supplier4.isInvoked()); > + } > + } > +} > >
