Skip to content

Commit

Permalink
nullness checks for ArrayUtil, Beans
Browse files Browse the repository at this point in the history
  • Loading branch information
@mattulbrich
mattulbrich committed Jul 6, 2023
1 parent c9b24c5 commit 6280b57
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 18 deletions.
10 changes: 7 additions & 3 deletions key.util/src/main/java/org/key_project/util/bean/Bean.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,22 @@

import org.key_project.util.java.ArrayUtil;

import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.Nullable;

/**
* Implements the basic methods that a Java bean should have and is the default implementation of
* {@link IBean}.
*
* @author Martin Hentschel
* @see IBean
*/
@SuppressWarnings("nullness")
@NullMarked
public class Bean implements IBean {
/**
* The used {@link PropertyChangeSupport}.
*/
@SuppressWarnings("nullness") // TODO Check with Werner Dietl why this is so.

This comment has been minimized.

Sign in to view

Copy link
@mattulbrich

mattulbrich Jul 6, 2023

Author Member

@wmdietl: I get a weird error here. How can I avoid it?

    private final PropertyChangeSupport pcs = new PropertyChangeSupport(this);
                                              ^
  found   : @UnderInitialization(java.beans.PropertyChangeSupport.class) @NonNull PropertyChangeSupport
  required: @Initialized @NonNull PropertyChangeSupport

This comment has been minimized.

Sign in to view

Copy link
@wmdietl

wmdietl Jul 6, 2023

Contributor

The new PropertyChangeSupport object has a reference to this, which is currently being constructed. So the Initialization Checker raises an error about assigning this partially initialized object to the field pcs, which requires a reference to a fully initialized object.
For reference, the PropertyChangeSupport constructor is annotated to accept a partially initialized object: https://github.com/eisop/jdk/blob/193a0c45257322a8c311607819246f3c3a3ba1fe/src/java.desktop/share/classes/java/beans/PropertyChangeSupport.java#L106
This could lead to an NPE if the pcs object is used before the initialization of this is finished.
In most cases, this is a false positive error, but you should add a comment to make sure nobody uses this value too soon.

This comment has been minimized.

Sign in to view

Copy link
@mattulbrich

mattulbrich Jul 6, 2023

Author Member

Thanks. Is there a way to do this right within the type system?
private final PropertyChangeSupport pcs = new PropertyChangeSupport(this);

/**
Expand Down Expand Up @@ -127,8 +131,8 @@ protected void fireIndexedPropertyChange(String propertyName, int index, int old
* @param oldValue The old value.
* @param newValue The new value.
*/
protected void fireIndexedPropertyChange(String propertyName, int index, Object oldValue,
Object newValue) {
protected void fireIndexedPropertyChange(String propertyName, int index,
@Nullable Object oldValue, @Nullable Object newValue) {
pcs.fireIndexedPropertyChange(propertyName, index, oldValue, newValue);
}

Expand Down
3 changes: 3 additions & 0 deletions key.util/src/main/java/org/key_project/util/bean/IBean.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

import java.beans.PropertyChangeListener;

import org.jspecify.annotations.NullMarked;

/**
* <p>
* Defines the methods that a Java bean must have.
Expand All @@ -13,6 +15,7 @@
* @author Martin Hentschel
* @see Bean
*/
@NullMarked
public interface IBean {
/**
* Adds the given listener.
Expand Down
41 changes: 26 additions & 15 deletions key.util/src/main/java/org/key_project/util/java/ArrayUtil.java
View file
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
package org.key_project.util.java;

import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.Nullable;

import java.util.LinkedList;
import java.util.List;
import java.util.Objects;
import java.util.function.Predicate;

import org.jspecify.annotations.NullMarked;
import org.jspecify.annotations.Nullable;

/**
* Provides static methods to work with arrays.
*
Expand Down Expand Up @@ -60,25 +60,26 @@ private ArrayUtil() {
* @throws IllegalArgumentException Both parameters are {@code null}.
*/
@SuppressWarnings("unchecked")
public static <T extends @Nullable Object> T [] addAll(T @Nullable [] array, T @Nullable [] toAdd) {
public static <T extends @Nullable Object> T[] addAll(T @Nullable [] array,
T @Nullable [] toAdd) {
if (array != null) {
if (toAdd != null) {
T[] result =
(T[]) java.lang.reflect.Array.newInstance(array.getClass().getComponentType(),
(T[]) java.lang.reflect.Array.newInstance(getComponentType(array),
array.length + toAdd.length);
System.arraycopy(array, 0, result, 0, array.length);
System.arraycopy(toAdd, 0, result, array.length, toAdd.length);
return result;
} else {
T[] result = (T[]) java.lang.reflect.Array
.newInstance(array.getClass().getComponentType(), array.length);
.newInstance(getComponentType(array), array.length);
System.arraycopy(array, 0, result, 0, array.length);
return result;
}
} else {
if (toAdd != null) {
T[] result = (T[]) java.lang.reflect.Array
.newInstance(toAdd.getClass().getComponentType(), toAdd.length);
.newInstance(getComponentType(toAdd), toAdd.length);
System.arraycopy(toAdd, 0, result, 0, toAdd.length);
return result;
} else {
Expand All @@ -88,6 +89,12 @@ private ArrayUtil() {
}
}

private static <T extends @Nullable Object> Class<? extends T> getComponentType(T[] array) {
Class<? extends Object[]> arrayClass = array.getClass();
assert arrayClass.isArray() : "@AssumeAssertion(nullness): This is always the case";
return (Class<? extends T>) arrayClass.getComponentType();
}

/**
* <p>
* Adds the given elements to the existing array. The result is a new array that contains the
Expand All @@ -105,7 +112,8 @@ private ArrayUtil() {
* @throws IllegalArgumentException Both parameters are {@code null}.
*/
@SuppressWarnings("unchecked")
public static <T extends @Nullable Object> T[] addAll(T @Nullable [] array, T @Nullable [] toAdd, Class<?> newArrayType) {
public static <T extends @Nullable Object> T[] addAll(T @Nullable [] array,
T @Nullable [] toAdd, Class<?> newArrayType) {
if (array != null) {
if (toAdd != null) {
T[] result = (T[]) java.lang.reflect.Array.newInstance(newArrayType,
Expand Down Expand Up @@ -145,10 +153,12 @@ private ArrayUtil() {
* @throws IllegalArgumentException Both parameters are {@code null}.
*/
@SuppressWarnings("unchecked")
public static <T extends @Nullable Object> T[] add(T @Nullable [] array, @Nullable T toAdd) {
public static <T extends @Nullable Object> @Nullable T[] add(T @Nullable [] array,
@Nullable T toAdd) {
if (array != null) {
T[] result = (T[]) java.lang.reflect.Array
.newInstance(array.getClass().getComponentType(), array.length + 1);
@Nullable
T[] result = (@Nullable T[]) java.lang.reflect.Array
.newInstance(getComponentType(array), array.length + 1);
System.arraycopy(array, 0, result, 0, array.length);
result[array.length] = toAdd;
return result;
Expand Down Expand Up @@ -200,7 +210,7 @@ public static int[] add(int[] array, int toAdd) {
public static <T> T[] insert(T[] array, T toInsert, int index) {
if (array != null) {
T[] result = (T[]) java.lang.reflect.Array
.newInstance(array.getClass().getComponentType(), array.length + 1);
.newInstance(getComponentType(array), array.length + 1);
if (index >= 1) {
System.arraycopy(array, 0, result, 0, index);
}
Expand Down Expand Up @@ -263,16 +273,17 @@ public static <T> int indexOf(T[] array, T toSearch) {
* was {@code null}.
*/
@SuppressWarnings("unchecked")
public static <T> T[] remove(T[] array, T toRemove) {
public static <T extends @Nullable Object> T @Nullable [] remove(T @Nullable [] array,
@Nullable T toRemove) {
if (array != null) {
List<T> result = new LinkedList<>();
for (T element : array) {
if (!Objects.equals(element, toRemove)) {
result.add(element);
}
}
return result.toArray((T[]) java.lang.reflect.Array
.newInstance(array.getClass().getComponentType(), result.size()));
return (T[]) result.toArray((T[]) java.lang.reflect.Array
.newInstance(getComponentType(array), result.size()));
} else {
return null;
}
Expand Down

0 comments on commit 6280b57

Please sign in to comment.