Skip to content

Commit

Permalink
Fix or suppress SAT CompareObjectsWithEquals findings (#10631)
Browse files Browse the repository at this point in the history
* Fix or suppress SAT CompareObjectsWithEquals findings

Signed-off-by: Wouter Born <github@maindrain.net>
  • Loading branch information
wborn committed May 5, 2021
1 parent 582ef28 commit f5f922e
Show file tree
Hide file tree
Showing 14 changed files with 34 additions and 43 deletions.
Expand Up @@ -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;
Expand Down Expand Up @@ -93,13 +94,8 @@ public void stateChanged(String channelId, State state) {

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;
}

Expand Down
Expand Up @@ -185,7 +185,7 @@ public boolean disconnect() {

@Override
public boolean discoverServices() {
if (currentProcedure != PROCEDURE_NONE) {
if (!PROCEDURE_NONE.equals(currentProcedure)) {
return false;
}

Expand Down Expand Up @@ -218,7 +218,7 @@ public boolean discoverServices() {
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"));
}

Expand Down Expand Up @@ -264,7 +264,7 @@ public boolean discoverServices() {
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"));
}

Expand Down Expand Up @@ -311,7 +311,7 @@ public CompletableFuture<byte[]> readCharacteristic(BluetoothCharacteristic char
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"));
}

Expand All @@ -336,7 +336,7 @@ public CompletableFuture<byte[]> readCharacteristic(BluetoothCharacteristic char
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"));
}

Expand Down Expand Up @@ -564,7 +564,7 @@ private void handleProcedureCompletedEvent(BlueGigaProcedureCompletedEvent event
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;
Expand Down
Expand Up @@ -68,6 +68,7 @@ protected Collection<BluetoothDeviceListener> getListeners() {
}

@Override
@SuppressWarnings("PMD.CompareObjectsWithEquals")
protected @Nullable BluetoothDevice getDelegate() {
BluetoothDevice newDelegate = null;
int newRssi = Integer.MIN_VALUE;
Expand Down
Expand Up @@ -14,6 +14,7 @@

import java.util.HashMap;
import java.util.Map;
import java.util.Objects;

import org.apache.commons.lang3.StringEscapeUtils;
import org.openhab.core.types.StateOption;
Expand Down Expand Up @@ -90,19 +91,19 @@ public boolean isValid() {
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;
Expand Down Expand Up @@ -265,14 +266,4 @@ public String toString() {
// }
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);
}
}
Expand Up @@ -80,6 +80,7 @@ public AbstractWeatherHandler(Thing thing) {
}

@Override
@SuppressWarnings("PMD.CompareObjectsWithEquals")
public void handleCommand(ChannelUID channelUID, Command command) {
if (RefreshType.REFRESH == command) {
ScheduledFuture<?> prevFuture = updateChannelsFutureRef.get();
Expand All @@ -94,6 +95,7 @@ public void handleCommand(ChannelUID channelUID, Command command) {
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());
Expand Down
Expand Up @@ -603,7 +603,7 @@ protected void onAcceptable() {
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);
Expand Down
Expand Up @@ -58,6 +58,7 @@ protected synchronized void setState(Function<T, U> newStateFactory) {
newState.startWorking();
}

@SuppressWarnings("PMD.CompareObjectsWithEquals")
protected boolean isStateActive(AbstractState<?, ?> otherState) {
return state == otherState; // compare by identity
}
Expand Down
Expand Up @@ -621,6 +621,7 @@ private synchronized boolean sendMessage(PowermaxBaseMessage msg, boolean immedi
*
* @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)) {
Expand Down
Expand Up @@ -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;
Expand Down Expand Up @@ -94,13 +95,8 @@ public void stateChanged(String channelId, State newState) {

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;
}

Expand Down
Expand Up @@ -77,7 +77,7 @@ public boolean updateChannel(String channelId, State newValue, Boolean forceUpda
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
Expand Down
Expand Up @@ -80,7 +80,7 @@ protected void setSmartthingsHubCommand(SmartthingsHubCommand hubCommand) {

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;
}
}
Expand Down
Expand Up @@ -131,6 +131,7 @@ public void onCommandFromHandler(Command command) {
}

@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
Expand All @@ -139,6 +140,7 @@ public void onStateUpdateFromHandler(State state) {
if (state instanceof StringType) {
Optional<String> 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);
Expand Down
Expand Up @@ -113,6 +113,7 @@ public HomekitTaggedItem getRootAccessory() {
return accessory;
}

@Override
public Collection<Service> getServices() {
return this.services;
}
Expand Down Expand Up @@ -182,7 +183,7 @@ protected <T extends Item> Optional<T> getItem(HomekitCharacteristicType charact
* 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 <T> expected type
Expand All @@ -197,7 +198,7 @@ protected <T> T getAccessoryConfiguration(String key, T defaultValue) {
* 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
Expand All @@ -215,7 +216,7 @@ protected <T> T getAccessoryConfiguration(HomekitCharacteristicType characterist
* 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
Expand Down Expand Up @@ -247,7 +248,7 @@ protected <T> void updateMapping(HomekitCharacteristicType characteristicType, M
/**
* 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
Expand Down Expand Up @@ -283,7 +284,7 @@ protected void addCharacteristic(HomekitTaggedItem characteristic) {

@NonNullByDefault
private <T extends Quantity<T>> double convertAndRound(double value, Unit<T> from, Unit<T> 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();
}

Expand All @@ -302,7 +303,7 @@ protected double convertFromCelsius(double degrees) {

/**
* 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
Expand All @@ -320,7 +321,7 @@ protected BooleanItemReader createBooleanReader(HomekitCharacteristicType charac

/**
* create boolean reader with default ON/OFF mapping considering inverted flag
*
*
* @param characteristicType characteristic id
* @return boolean reader
* @throws IncompleteAccessoryException
Expand Down
Expand Up @@ -60,7 +60,7 @@ public CompletableFuture<LockCurrentStateEnum> getLockCurrentState() {
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);
Expand Down

1 comment on commit f5f922e

@openhab-bot
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/revival-of-official-bluetooth-binding/92759/182

Please sign in to comment.