Bug 560974 - Fix exception type for null map key

When a Map which does not support null keys receives a null key,
the Map contract specified it should throw a NullPointerException.
This class instead threw IllegalArgumentException. This change
brings this class inline with the Map contract.

Change-Id: Ia227aa18ac024f007dfc8ea1a2beaab12de393c2
Signed-off-by: BJ Hargrave <hargrave@us.ibm.com>

2 files changed, 52 insertions, 56 deletions

diff --git a/bundles/org.eclipse.osgi/supplement/src/org/eclipse/osgi/framework/eventmgr/CopyOnWriteIdentityMap.java b/bundles/org.eclipse.osgi/supplement/src/org/eclipse/osgi/framework/eventmgr/CopyOnWriteIdentityMap.java
index 321aaaca8..4dfa81b22 100644
--- a/bundles/org.eclipse.osgi/supplement/src/org/eclipse/osgi/framework/eventmgr/CopyOnWriteIdentityMap.java
+++ b/bundles/org.eclipse.osgi/supplement/src/org/eclipse/osgi/framework/eventmgr/CopyOnWriteIdentityMap.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2003, 2016 IBM Corporation and others.
+ * Copyright (c) 2003, 2020 IBM Corporation and others.
  *
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
@@ -14,7 +14,16 @@
 
 package org.eclipse.osgi.framework.eventmgr;
 
-import java.util.*;
+import static java.util.Objects.requireNonNull;
+
+import java.util.AbstractCollection;
+import java.util.AbstractSet;
+import java.util.Collection;
+import java.util.ConcurrentModificationException;
+import java.util.Iterator;
+import java.util.Map;
+import java.util.NoSuchElementException;
+import java.util.Set;
 
 /**
  * A copy-on-write identity map. Write operations result in copying the underlying data so that
@@ -61,22 +70,20 @@ public class CopyOnWriteIdentityMap<K, V> implements Map<K, V> {
 	/* These methods modify the map and are synchronized. */
 
 	/**
-	 * Add a key, value pair to the map.
-	 * If the key object is already in the map, then its value is replaced with the new value.
-	 * Keys are compared using identity.
+	 * Add a key, value pair to the map. If the key object is already in the map,
+	 * then its value is replaced with the new value. Keys are compared using
+	 * identity.
 	 *
-	 * @param key The key object to be added to the list.
-	 * @param value The value object to be associated with the key.
-	 * This may be null.
+	 * @param key   The key object to be added to the list.
+	 * @param value The value object to be associated with the key. This may be
+	 *              null.
 	 * @return <code>null</code> if the specified key was newly added to the map.
-	 * Otherwise the previous value of the key.
-	 * @throws IllegalArgumentException If key is null.
+	 *         Otherwise the previous value of the key.
+	 * @throws NullPointerException If key is null.
 	 */
 	@Override
 	public synchronized V put(K key, V value) {
-		if (key == null) {
-			throw new IllegalArgumentException();
-		}
+		requireNonNull(key);
 
 		int size = entries.length;
 		for (int i = 0; i < size; i++) {
@@ -183,19 +190,17 @@ public class CopyOnWriteIdentityMap<K, V> implements Map<K, V> {
 	}
 
 	/**
-	 * Remove a key from the map and returns the value associated with the key.
-	 * Key objects are compared using identity.
+	 * Remove a key from the map and returns the value associated with the key. Key
+	 * objects are compared using identity.
 	 *
 	 * @param key The key object to be removed from the map.
-	 * @return <code>null</code> if the key was not in the list.
-	 * Otherwise, the value associated with the key.
-	 * @throws IllegalArgumentException If key is null.
+	 * @return <code>null</code> if the key was not in the list. Otherwise, the
+	 *         value associated with the key.
+	 * @throws NullPointerException If key is null.
 	 */
 	@Override
 	public synchronized V remove(Object key) {
-		if (key == null) {
-			throw new IllegalArgumentException();
-		}
+		requireNonNull(key);
 
 		int size = entries.length;
 		for (int i = 0; i < size; i++) {
@@ -278,18 +283,16 @@ public class CopyOnWriteIdentityMap<K, V> implements Map<K, V> {
 	}
 
 	/**
-	 * Return the value object for the specified key.
-	 * Keys are compared using identity.
+	 * Return the value object for the specified key. Keys are compared using
+	 * identity.
 	 *
 	 * @param key The key object.
 	 * @return The value object for the specified key.
-	 * @throws IllegalArgumentException If key is null.
+	 * @throws NullPointerException If key is null.
 	 */
 	@Override
 	public V get(Object key) {
-		if (key == null) {
-			throw new IllegalArgumentException();
-		}
+		requireNonNull(key);
 
 		for (Entry<K, V> entry : entries()) {
 			if (entry.key == key) {
@@ -300,18 +303,16 @@ public class CopyOnWriteIdentityMap<K, V> implements Map<K, V> {
 	}
 
 	/**
-	 * Check if the map contains the specified key.
-	 * Keys are compared using identity.
+	 * Check if the map contains the specified key. Keys are compared using
+	 * identity.
 	 *
 	 * @param key The key object.
 	 * @return <code>true</code> if the specified key is in the map.
-	 * @throws IllegalArgumentException If key is null.
+	 * @throws NullPointerException If key is null.
 	 */
 	@Override
 	public boolean containsKey(Object key) {
-		if (key == null) {
-			throw new IllegalArgumentException();
-		}
+		requireNonNull(key);
 
 		for (Entry<K, V> entry : entries()) {
 			if (entry.key == key) {
@@ -389,11 +390,13 @@ public class CopyOnWriteIdentityMap<K, V> implements Map<K, V> {
 
 		/**
 		 * Constructor for map entry.
-		 * @param key Key object in entry. Used for uniqueness.
+		 * 
+		 * @param key   Key object in entry. Used for uniqueness.
 		 * @param value Value object stored with key object.
+		 * @throws NullPointerException If key is null.
 		 */
 		Entry(final K key, final V value) {
-			this.key = key;
+			this.key = requireNonNull(key);
 			this.value = value;
 		}
 
@@ -494,9 +497,7 @@ public class CopyOnWriteIdentityMap<K, V> implements Map<K, V> {
 
 			@Override
 			public boolean remove(Object o) {
-				if (o == null) {
-					throw new IllegalArgumentException();
-				}
+				requireNonNull(o);
 
 				synchronized (Snapshot.this) {
 					int size = entries.length;
@@ -550,9 +551,7 @@ public class CopyOnWriteIdentityMap<K, V> implements Map<K, V> {
 
 			@Override
 			public boolean remove(Object o) {
-				if (o == null) {
-					throw new IllegalArgumentException();
-				}
+				requireNonNull(o);
 
 				synchronized (Snapshot.this) {
 					int size = entries.length;
@@ -606,9 +605,7 @@ public class CopyOnWriteIdentityMap<K, V> implements Map<K, V> {
 
 			@Override
 			public boolean remove(Object o) {
-				if (o == null) {
-					throw new IllegalArgumentException();
-				}
+				requireNonNull(o);
 
 				synchronized (Snapshot.this) {
 					int size = entries.length;
diff --git a/bundles/org.eclipse.osgi/supplement/src/org/eclipse/osgi/framework/eventmgr/EventListeners.java b/bundles/org.eclipse.osgi/supplement/src/org/eclipse/osgi/framework/eventmgr/EventListeners.java
index 9779cd95b..65d4794e6 100644
--- a/bundles/org.eclipse.osgi/supplement/src/org/eclipse/osgi/framework/eventmgr/EventListeners.java
+++ b/bundles/org.eclipse.osgi/supplement/src/org/eclipse/osgi/framework/eventmgr/EventListeners.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2003, 2017 IBM Corporation and others.
+ * Copyright (c) 2003, 2020 IBM Corporation and others.
  *
  * This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License 2.0
@@ -49,26 +49,25 @@ public class EventListeners<K, V> {
 	}
 
 	/**
-	 * Add a listener to the list.
-	 * If a listener object is already in the list, then it is replaced.
-	 * This method calls the put method.
+	 * Add a listener to the list. If a listener object is already in the list, then
+	 * it is replaced. This method calls the put method.
 	 *
-	 * @param listener This is the listener object to be added to the list.
-	 * @param listenerObject This is an optional listener-specific object.
-	 * This object will be passed to the EventDispatcher along with the listener
-	 * when the listener is to be called. This may be null
-	 * @throws IllegalArgumentException If listener is null.
+	 * @param listener       This is the listener object to be added to the list.
+	 * @param listenerObject This is an optional listener-specific object. This
+	 *                       object will be passed to the EventDispatcher along with
+	 *                       the listener when the listener is to be called. This
+	 *                       may be null
+	 * @throws NullPointerException If listener is null.
 	 */
 	public void addListener(K listener, V listenerObject) {
 		list.put(listener, listenerObject);
 	}
 
 	/**
-	 * Remove a listener from the list.
-	 * This method calls the remove method.
+	 * Remove a listener from the list. This method calls the remove method.
 	 *
 	 * @param listener This is the listener object to be removed from the list.
-	 * @throws IllegalArgumentException If listener is null.
+	 * @throws NullPointerException If listener is null.
 	 */
 	public void removeListener(K listener) {
 		list.remove(listener);