From f5f922eaf4f6048f08695bd019536b061e764ead Mon Sep 17 00:00:00 2001 From: Wouter Born Date: Wed, 5 May 2021 21:06:04 +0200 Subject: [PATCH] Fix or suppress SAT CompareObjectsWithEquals findings (#10631) * Fix or suppress SAT CompareObjectsWithEquals findings Signed-off-by: Wouter Born --- .../internal/StatefulHandlerCallback.java | 8 ++------ .../bluegiga/BlueGigaBluetoothDevice.java | 12 ++++++------ .../internal/RoamingBluetoothDevice.java | 1 + .../bosesoundtouch/internal/ContentItem.java | 19 +++++-------------- .../internal/AbstractWeatherHandler.java | 2 ++ .../handler/EthernetBridgeHandler.java | 2 +- .../connection/AbstractStateMachine.java | 1 + .../internal/message/PowermaxCommManager.java | 1 + .../internal/rio/StatefulHandlerCallback.java | 8 ++------ .../internal/util/ShellyChannelCache.java | 2 +- .../SmartthingsDiscoveryService.java | 2 +- .../internal/phonebook/PhonebookProfile.java | 2 ++ .../AbstractHomekitAccessoryImpl.java | 15 ++++++++------- .../internal/accessories/HomekitLockImpl.java | 2 +- 14 files changed, 34 insertions(+), 43 deletions(-) diff --git a/bundles/org.openhab.binding.atlona/src/main/java/org/openhab/binding/atlona/internal/StatefulHandlerCallback.java b/bundles/org.openhab.binding.atlona/src/main/java/org/openhab/binding/atlona/internal/StatefulHandlerCallback.java index 04fd2c559..29d882245 100644 --- a/bundles/org.openhab.binding.atlona/src/main/java/org/openhab/binding/atlona/internal/StatefulHandlerCallback.java +++ b/bundles/org.openhab.binding.atlona/src/main/java/org/openhab/binding/atlona/internal/StatefulHandlerCallback.java @@ -13,6 +13,7 @@ package org.openhab.binding.atlona.internal; import java.util.Map; +import java.util.Objects; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantLock; @@ -93,13 +94,8 @@ public class StatefulHandlerCallback implements AtlonaHandlerCallback { final State oldState = this.state.get(channelId); - // If both null OR the same value (enums), nothing changed - if (oldState == state) { - return; - } - // If they are equal - nothing changed - if (oldState != null && oldState.equals(state)) { + if (Objects.equals(oldState, state)) { return; } diff --git a/bundles/org.openhab.binding.bluetooth.bluegiga/src/main/java/org/openhab/binding/bluetooth/bluegiga/BlueGigaBluetoothDevice.java b/bundles/org.openhab.binding.bluetooth.bluegiga/src/main/java/org/openhab/binding/bluetooth/bluegiga/BlueGigaBluetoothDevice.java index 3fce5a0e4..38ea99071 100644 --- a/bundles/org.openhab.binding.bluetooth.bluegiga/src/main/java/org/openhab/binding/bluetooth/bluegiga/BlueGigaBluetoothDevice.java +++ b/bundles/org.openhab.binding.bluetooth.bluegiga/src/main/java/org/openhab/binding/bluetooth/bluegiga/BlueGigaBluetoothDevice.java @@ -185,7 +185,7 @@ public class BlueGigaBluetoothDevice extends BaseBluetoothDevice implements Blue @Override public boolean discoverServices() { - if (currentProcedure != PROCEDURE_NONE) { + if (!PROCEDURE_NONE.equals(currentProcedure)) { return false; } @@ -218,7 +218,7 @@ public class BlueGigaBluetoothDevice extends BaseBluetoothDevice implements Blue new BluetoothException("Unable to find CCC for characteristic [" + characteristic.getUuid() + "]")); } - if (currentProcedure != PROCEDURE_NONE) { + if (!PROCEDURE_NONE.equals(currentProcedure)) { return CompletableFuture.failedFuture(new BluetoothException("Another procedure is already in progress")); } @@ -264,7 +264,7 @@ public class BlueGigaBluetoothDevice extends BaseBluetoothDevice implements Blue new BluetoothException("Unable to find CCC for characteristic [" + characteristic.getUuid() + "]")); } - if (currentProcedure != PROCEDURE_NONE) { + if (!PROCEDURE_NONE.equals(currentProcedure)) { return CompletableFuture.failedFuture(new BluetoothException("Another procedure is already in progress")); } @@ -311,7 +311,7 @@ public class BlueGigaBluetoothDevice extends BaseBluetoothDevice implements Blue return CompletableFuture.failedFuture(new BluetoothException("Not connected")); } - if (currentProcedure != PROCEDURE_NONE) { + if (!PROCEDURE_NONE.equals(currentProcedure)) { return CompletableFuture.failedFuture(new BluetoothException("Another procedure is already in progress")); } @@ -336,7 +336,7 @@ public class BlueGigaBluetoothDevice extends BaseBluetoothDevice implements Blue return CompletableFuture.failedFuture(new BluetoothException("Not connected")); } - if (currentProcedure != PROCEDURE_NONE) { + if (!PROCEDURE_NONE.equals(currentProcedure)) { return CompletableFuture.failedFuture(new BluetoothException("Another procedure is already in progress")); } @@ -564,7 +564,7 @@ public class BlueGigaBluetoothDevice extends BaseBluetoothDevice implements Blue return; } - if (currentProcedure == PROCEDURE_NONE) { + if (!PROCEDURE_NONE.equals(currentProcedure)) { logger.debug("BlueGiga procedure completed but procedure is null with connection {}, address {}", connection, address); return; diff --git a/bundles/org.openhab.binding.bluetooth.roaming/src/main/java/org/openhab/binding/bluetooth/roaming/internal/RoamingBluetoothDevice.java b/bundles/org.openhab.binding.bluetooth.roaming/src/main/java/org/openhab/binding/bluetooth/roaming/internal/RoamingBluetoothDevice.java index 79cbcfd93..4ad7a0b25 100644 --- a/bundles/org.openhab.binding.bluetooth.roaming/src/main/java/org/openhab/binding/bluetooth/roaming/internal/RoamingBluetoothDevice.java +++ b/bundles/org.openhab.binding.bluetooth.roaming/src/main/java/org/openhab/binding/bluetooth/roaming/internal/RoamingBluetoothDevice.java @@ -68,6 +68,7 @@ public class RoamingBluetoothDevice extends DelegateBluetoothDevice { } @Override + @SuppressWarnings("PMD.CompareObjectsWithEquals") protected @Nullable BluetoothDevice getDelegate() { BluetoothDevice newDelegate = null; int newRssi = Integer.MIN_VALUE; diff --git a/bundles/org.openhab.binding.bosesoundtouch/src/main/java/org/openhab/binding/bosesoundtouch/internal/ContentItem.java b/bundles/org.openhab.binding.bosesoundtouch/src/main/java/org/openhab/binding/bosesoundtouch/internal/ContentItem.java index 84c5a40e3..85fee270f 100644 --- a/bundles/org.openhab.binding.bosesoundtouch/src/main/java/org/openhab/binding/bosesoundtouch/internal/ContentItem.java +++ b/bundles/org.openhab.binding.bosesoundtouch/src/main/java/org/openhab/binding/bosesoundtouch/internal/ContentItem.java @@ -14,6 +14,7 @@ package org.openhab.binding.bosesoundtouch.internal; import java.util.HashMap; import java.util.Map; +import java.util.Objects; import org.apache.commons.lang3.StringEscapeUtils; import org.openhab.core.types.StateOption; @@ -90,19 +91,19 @@ public class ContentItem { public boolean equals(Object obj) { if (obj instanceof ContentItem) { ContentItem other = (ContentItem) obj; - if (!isEqual(other.source, this.source)) { + if (!Objects.equals(other.source, this.source)) { return false; } - if (!isEqual(other.sourceAccount, this.sourceAccount)) { + if (!Objects.equals(other.sourceAccount, this.sourceAccount)) { return false; } if (other.presetable != this.presetable) { return false; } - if (!isEqual(other.location, this.location)) { + if (!Objects.equals(other.location, this.location)) { return false; } - if (!isEqual(other.itemName, this.itemName)) { + if (!Objects.equals(other.itemName, this.itemName)) { return false; } return true; @@ -265,14 +266,4 @@ public class ContentItem { // } return itemName; } - - private boolean isEqual(String s1, String s2) { - if (s1 == s2) { - return true; - } - if (s1 == null || s2 == null) { - return false; - } - return s1.equals(s2); - } } diff --git a/bundles/org.openhab.binding.fmiweather/src/main/java/org/openhab/binding/fmiweather/internal/AbstractWeatherHandler.java b/bundles/org.openhab.binding.fmiweather/src/main/java/org/openhab/binding/fmiweather/internal/AbstractWeatherHandler.java index 42f0cfeda..c819a90bc 100644 --- a/bundles/org.openhab.binding.fmiweather/src/main/java/org/openhab/binding/fmiweather/internal/AbstractWeatherHandler.java +++ b/bundles/org.openhab.binding.fmiweather/src/main/java/org/openhab/binding/fmiweather/internal/AbstractWeatherHandler.java @@ -80,6 +80,7 @@ public abstract class AbstractWeatherHandler extends BaseThingHandler { } @Override + @SuppressWarnings("PMD.CompareObjectsWithEquals") public void handleCommand(ChannelUID channelUID, Command command) { if (RefreshType.REFRESH == command) { ScheduledFuture prevFuture = updateChannelsFutureRef.get(); @@ -94,6 +95,7 @@ public abstract class AbstractWeatherHandler extends BaseThingHandler { logger.trace("REFRESH received. Delaying by {} ms to avoid throttle excessive REFRESH", delayRemainingMillis); } + // Compare by reference to check if the future changed if (prevFuture == newFuture) { logger.trace("REFRESH received. Previous refresh ongoing, will wait for it to complete in {} ms", lastRefreshMillis + REFRESH_THROTTLE_MILLIS - System.currentTimeMillis()); diff --git a/bundles/org.openhab.binding.irtrans/src/main/java/org/openhab/binding/irtrans/internal/handler/EthernetBridgeHandler.java b/bundles/org.openhab.binding.irtrans/src/main/java/org/openhab/binding/irtrans/internal/handler/EthernetBridgeHandler.java index 4992ebc34..2ffe31d92 100644 --- a/bundles/org.openhab.binding.irtrans/src/main/java/org/openhab/binding/irtrans/internal/handler/EthernetBridgeHandler.java +++ b/bundles/org.openhab.binding.irtrans/src/main/java/org/openhab/binding/irtrans/internal/handler/EthernetBridgeHandler.java @@ -603,7 +603,7 @@ public class EthernetBridgeHandler extends BaseBridgeHandler implements Transcei SelectionKey selKey = it.next(); it.remove(); if (selKey.isValid()) { - if (selKey.isAcceptable() && selKey == listenerKey) { + if (selKey.isAcceptable() && selKey.equals(listenerKey)) { try { SocketChannel newChannel = listenerChannel.accept(); newChannel.configureBlocking(false); diff --git a/bundles/org.openhab.binding.lcn/src/main/java/org/openhab/binding/lcn/internal/connection/AbstractStateMachine.java b/bundles/org.openhab.binding.lcn/src/main/java/org/openhab/binding/lcn/internal/connection/AbstractStateMachine.java index 3a60c507e..225468a80 100644 --- a/bundles/org.openhab.binding.lcn/src/main/java/org/openhab/binding/lcn/internal/connection/AbstractStateMachine.java +++ b/bundles/org.openhab.binding.lcn/src/main/java/org/openhab/binding/lcn/internal/connection/AbstractStateMachine.java @@ -58,6 +58,7 @@ public abstract class AbstractStateMachine, newState.startWorking(); } + @SuppressWarnings("PMD.CompareObjectsWithEquals") protected boolean isStateActive(AbstractState otherState) { return state == otherState; // compare by identity } diff --git a/bundles/org.openhab.binding.powermax/src/main/java/org/openhab/binding/powermax/internal/message/PowermaxCommManager.java b/bundles/org.openhab.binding.powermax/src/main/java/org/openhab/binding/powermax/internal/message/PowermaxCommManager.java index 36d73935d..64c6aeb6a 100644 --- a/bundles/org.openhab.binding.powermax/src/main/java/org/openhab/binding/powermax/internal/message/PowermaxCommManager.java +++ b/bundles/org.openhab.binding.powermax/src/main/java/org/openhab/binding/powermax/internal/message/PowermaxCommManager.java @@ -621,6 +621,7 @@ public class PowermaxCommManager implements PowermaxMessageEventListener { * * @return true if the message was sent or the sending is delayed; false in other cases */ + @SuppressWarnings("PMD.CompareObjectsWithEquals") private synchronized boolean sendMessage(PowermaxBaseMessage msg, boolean immediate, int waitTime, boolean doNotLog) { if ((waitTime > 0) && (msg != null)) { diff --git a/bundles/org.openhab.binding.russound/src/main/java/org/openhab/binding/russound/internal/rio/StatefulHandlerCallback.java b/bundles/org.openhab.binding.russound/src/main/java/org/openhab/binding/russound/internal/rio/StatefulHandlerCallback.java index e68159458..e6ae4b8cb 100644 --- a/bundles/org.openhab.binding.russound/src/main/java/org/openhab/binding/russound/internal/rio/StatefulHandlerCallback.java +++ b/bundles/org.openhab.binding.russound/src/main/java/org/openhab/binding/russound/internal/rio/StatefulHandlerCallback.java @@ -13,6 +13,7 @@ package org.openhab.binding.russound.internal.rio; import java.util.Map; +import java.util.Objects; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantLock; @@ -94,13 +95,8 @@ public class StatefulHandlerCallback implements RioHandlerCallback { final State oldState = state.get(channelId); - // If both null OR the same value (enums), nothing changed - if (oldState == newState) { - return; - } - // If they are equal - nothing changed - if (oldState != null && oldState.equals(newState)) { + if (Objects.equals(oldState, newState)) { return; } diff --git a/bundles/org.openhab.binding.shelly/src/main/java/org/openhab/binding/shelly/internal/util/ShellyChannelCache.java b/bundles/org.openhab.binding.shelly/src/main/java/org/openhab/binding/shelly/internal/util/ShellyChannelCache.java index 8f1e1d639..d5c0b6240 100644 --- a/bundles/org.openhab.binding.shelly/src/main/java/org/openhab/binding/shelly/internal/util/ShellyChannelCache.java +++ b/bundles/org.openhab.binding.shelly/src/main/java/org/openhab/binding/shelly/internal/util/ShellyChannelCache.java @@ -77,7 +77,7 @@ public class ShellyChannelCache { current = channelData.get(channelId); } if (!enabled || forceUpdate || (current == null) || !current.equals(newValue)) { - if ((current != null) && current.getClass().isEnum() && (current == newValue)) { + if ((current != null) && current.getClass().isEnum() && (current.equals(newValue))) { return false; // special case for OnOffType } // For channels that support multiple types (like brightness) a suffix is added diff --git a/bundles/org.openhab.binding.smartthings/src/main/java/org/openhab/binding/smartthings/internal/discovery/SmartthingsDiscoveryService.java b/bundles/org.openhab.binding.smartthings/src/main/java/org/openhab/binding/smartthings/internal/discovery/SmartthingsDiscoveryService.java index e051b0f55..4fcc572aa 100644 --- a/bundles/org.openhab.binding.smartthings/src/main/java/org/openhab/binding/smartthings/internal/discovery/SmartthingsDiscoveryService.java +++ b/bundles/org.openhab.binding.smartthings/src/main/java/org/openhab/binding/smartthings/internal/discovery/SmartthingsDiscoveryService.java @@ -80,7 +80,7 @@ public class SmartthingsDiscoveryService extends AbstractDiscoveryService implem protected void unsetSmartthingsHubCommand(SmartthingsHubCommand hubCommand) { // Make sure it is this handleFactory that should be unset - if (hubCommand == smartthingsHubCommand) { + if (Objects.equals(hubCommand, smartthingsHubCommand)) { this.smartthingsHubCommand = null; } } diff --git a/bundles/org.openhab.binding.tr064/src/main/java/org/openhab/binding/tr064/internal/phonebook/PhonebookProfile.java b/bundles/org.openhab.binding.tr064/src/main/java/org/openhab/binding/tr064/internal/phonebook/PhonebookProfile.java index 944c87ada..83cc61a2a 100644 --- a/bundles/org.openhab.binding.tr064/src/main/java/org/openhab/binding/tr064/internal/phonebook/PhonebookProfile.java +++ b/bundles/org.openhab.binding.tr064/src/main/java/org/openhab/binding/tr064/internal/phonebook/PhonebookProfile.java @@ -131,6 +131,7 @@ public class PhonebookProfile implements StateProfile { } @Override + @SuppressWarnings("PMD.CompareObjectsWithEquals") public void onStateUpdateFromHandler(State state) { if (state instanceof UnDefType) { // we cannot adjust UNDEF or NULL values, thus we simply apply them without reporting an error or warning @@ -139,6 +140,7 @@ public class PhonebookProfile implements StateProfile { if (state instanceof StringType) { Optional match = resolveNumber(state.toString()); State newState = match.map(name -> (State) new StringType(name)).orElse(state); + // Compare by reference to check if the name is mapped to the same state if (newState == state) { logger.debug("Number '{}' not found in phonebook '{}' from provider '{}'", state, phonebookName, thingUID); diff --git a/bundles/org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/accessories/AbstractHomekitAccessoryImpl.java b/bundles/org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/accessories/AbstractHomekitAccessoryImpl.java index cad68e734..40c401fe2 100644 --- a/bundles/org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/accessories/AbstractHomekitAccessoryImpl.java +++ b/bundles/org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/accessories/AbstractHomekitAccessoryImpl.java @@ -113,6 +113,7 @@ abstract class AbstractHomekitAccessoryImpl implements HomekitAccessory { return accessory; } + @Override public Collection getServices() { return this.services; } @@ -182,7 +183,7 @@ abstract class AbstractHomekitAccessoryImpl implements HomekitAccessory { * return configuration attached to the root accessory, e.g. groupItem. * Note: result will be casted to the type of the default value. * The type for number is BigDecimal. - * + * * @param key configuration key * @param defaultValue default value * @param expected type @@ -197,7 +198,7 @@ abstract class AbstractHomekitAccessoryImpl implements HomekitAccessory { * return configuration of the characteristic item, e.g. currentTemperature. * Note: result will be casted to the type of the default value. * The type for number is BigDecimal. - * + * * @param characteristicType characteristic type * @param key configuration key * @param defaultValue default value @@ -215,7 +216,7 @@ abstract class AbstractHomekitAccessoryImpl implements HomekitAccessory { * update mapping with values from item configuration. * it checks for all keys from the mapping whether there is configuration at item with the same key and if yes, * replace the value. - * + * * @param characteristicType characteristicType to identify item * @param map mapping to update * @param customEnumList list to store custom state enumeration @@ -247,7 +248,7 @@ abstract class AbstractHomekitAccessoryImpl implements HomekitAccessory { /** * takes item state as value and retrieves the key for that value from mapping. * e.g. used to map StringItem value to HomeKit Enum - * + * * @param characteristicType characteristicType to identify item * @param mapping mapping * @param defaultValue default value if nothing found in mapping @@ -283,7 +284,7 @@ abstract class AbstractHomekitAccessoryImpl implements HomekitAccessory { @NonNullByDefault private > double convertAndRound(double value, Unit from, Unit to) { - double rawValue = from == to ? value : from.getConverterTo(to).convert(value); + double rawValue = from.equals(to) ? value : from.getConverterTo(to).convert(value); return new BigDecimal(rawValue).setScale(1, RoundingMode.HALF_UP).doubleValue(); } @@ -302,7 +303,7 @@ abstract class AbstractHomekitAccessoryImpl implements HomekitAccessory { /** * create boolean reader with ON state mapped to trueOnOffValue or trueOpenClosedValue depending of item type - * + * * @param characteristicType characteristic id * @param trueOnOffValue ON value for switch * @param trueOpenClosedValue ON value for contact @@ -320,7 +321,7 @@ abstract class AbstractHomekitAccessoryImpl implements HomekitAccessory { /** * create boolean reader with default ON/OFF mapping considering inverted flag - * + * * @param characteristicType characteristic id * @return boolean reader * @throws IncompleteAccessoryException diff --git a/bundles/org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/accessories/HomekitLockImpl.java b/bundles/org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/accessories/HomekitLockImpl.java index de9379334..468cb9bb2 100644 --- a/bundles/org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/accessories/HomekitLockImpl.java +++ b/bundles/org.openhab.io.homekit/src/main/java/org/openhab/io/homekit/internal/accessories/HomekitLockImpl.java @@ -60,7 +60,7 @@ public class HomekitLockImpl extends AbstractHomekitAccessoryImpl implements Loc if (state instanceof DecimalType) { lockState = LockCurrentStateEnum.fromCode(((DecimalType) state).intValue()); } else if (state instanceof OnOffType) { - lockState = state == securedState ? LockCurrentStateEnum.SECURED : LockCurrentStateEnum.UNSECURED; + lockState = state.equals(securedState) ? LockCurrentStateEnum.SECURED : LockCurrentStateEnum.UNSECURED; } } return CompletableFuture.completedFuture(lockState);