Fixed pmd issues found is smarthome - AvoidCatchingNPE and AvoidThrow…

…ingRawExceptionTypes Fixed SimplifyBooleanExpressions issues found by PMD Signed-off-by: VelinYordanov <[email protected]>
eclipse-archived · Mar 30, 2018 · 4a28914 · 4a28914
1 parent 6852de6
commit 4a28914
Show file tree

Hide file tree

Showing 24 changed files with 60 additions and 47 deletions.
diff --git a/...ipse/smarthome/automation/module/script/internal/handler/AbstractScriptModuleHandler.java b/...ipse/smarthome/automation/module/script/internal/handler/AbstractScriptModuleHandler.java
@@ -90,23 +90,25 @@ private Optional<ScriptEngine> createScriptEngine() {
     private void loadConfig() {
         Object type = module.getConfiguration().get(SCRIPT_TYPE);
         Object script = module.getConfiguration().get(SCRIPT);
-        if (type == null || !(type instanceof String) || ((String) type).trim().isEmpty()) {
-            throw new RuntimeException(
-                    String.format("Type is missing in the configuration of module '%s'.", module.getId()));
-        } else if (script == null || !(script instanceof String) || ((String) script).trim().isEmpty()) {
-            throw new RuntimeException(
-                    String.format("Script is missing in the configuration of module '%s'.", module.getId()));
+        if (!isValid(type)) {
+            throw new IllegalStateException(String.format("Type is missing in the configuration of module '%s'.", module.getId()));
+        } else if (!isValid(script)) {
+            throw new IllegalStateException(String.format("Script is missing in the configuration of module '%s'.", module.getId()));
         } else {
             this.type = (String) type;
             this.script = (String) script;
         }
     }
+
+    private boolean isValid(Object parameter) {
+        return parameter != null && parameter instanceof String && !((String) parameter).trim().isEmpty();
+    }
 
     /**
      * Adds the passed context variables of the rule engine to the context scope of the ScriptEngine, this should be
      * updated each time the module is executed
      *
-     * @param engine the scriptengine that is used
+     * @param engine the script engine that is used
      * @param context the variables and types to put into the execution context
      */
     protected void setExecutionContext(ScriptEngine engine, Map<String, ?> context) {

diff --git a/...config.xml/src/main/java/org/eclipse/smarthome/config/xml/ConfigDescriptionConverter.java b/...config.xml/src/main/java/org/eclipse/smarthome/config/xml/ConfigDescriptionConverter.java
@@ -82,7 +82,7 @@ public Object unmarshal(HierarchicalStreamReader reader, UnmarshallingContext co
 
         // iterate through the nodes, putting the different types into their
         // respective arrays
-        while (nodeIterator.hasNext() == true) {
+        while (nodeIterator.hasNext()) {
             Object node = nodeIterator.next();
             if (node instanceof ConfigDescriptionParameter) {
                 configDescriptionParams.add((ConfigDescriptionParameter) node);

diff --git a/...l/src/main/java/org/eclipse/smarthome/core/binding/xml/internal/BindingInfoConverter.java b/...l/src/main/java/org/eclipse/smarthome/core/binding/xml/internal/BindingInfoConverter.java
@@ -55,7 +55,7 @@ private URI readConfigDescriptionURI(NodeIterator nodeIterator) throws Conversio
         if (uriText != null) {
             try {
                 return new URI(uriText);
-            } catch (NullPointerException | URISyntaxException ex) {
+            } catch (URISyntaxException ex) {
                 throw new ConversionException(
                         "The URI '" + uriText + "' in node " + "'config-description-ref' is invalid!", ex);
             }

diff --git a/...home.core.thing/src/main/java/org/eclipse/smarthome/core/thing/internal/ThingManager.java b/...home.core.thing/src/main/java/org/eclipse/smarthome/core/thing/internal/ThingManager.java
@@ -293,7 +293,7 @@ public void migrateThingType(final Thing thing, final ThingTypeUID thingTypeUID,
             final Configuration configuration) {
         final ThingType thingType = thingTypeRegistry.getThingType(thingTypeUID);
         if (thingType == null) {
-            throw new RuntimeException(
+            throw new IllegalStateException(
                     MessageFormat.format("No thing type {0} registered, cannot change thing type for thing {1}",
                             thingTypeUID.getAsString(), thing.getUID().getAsString()));
         }
@@ -360,7 +360,7 @@ private void waitUntilHandlerUnregistered(final Thing thing, int timeout) {
                         "Thing type migration failed for {0}. The handler deregistration did not complete within {1}ms.",
                         thing.getUID().getAsString(), timeout);
                 logger.error(message);
-                throw new RuntimeException(message);
+                throw new IllegalStateException(message);
             }
 
             private void waitForRunningHandlerRegistrations(ThingUID thingUID) {
@@ -379,7 +379,7 @@ private void waitForRunningHandlerRegistrations(ThingUID thingUID) {
                         "Thing type migration failed for {0}. Could not obtain lock for hander registration.",
                         thingUID.getAsString());
                 logger.error(message);
-                throw new RuntimeException(message);
+                throw new IllegalStateException(message);
             }
         }, 0, TimeUnit.MILLISECONDS);
     }

diff --git a/...e.core.voice/src/main/java/org/eclipse/smarthome/core/voice/internal/DialogProcessor.java b/...e.core.voice/src/main/java/org/eclipse/smarthome/core/voice/internal/DialogProcessor.java
@@ -150,7 +150,7 @@ public void ksEventReceived(KSEvent ksEvent) {
     @Override
     public synchronized void sttEventReceived(STTEvent sttEvent) {
         if (sttEvent instanceof SpeechRecognitionEvent) {
-            if (false == this.isSTTServerAborting) {
+            if (!this.isSTTServerAborting) {
                 this.sttServiceHandle.abort();
                 this.isSTTServerAborting = true;
                 SpeechRecognitionEvent sre = (SpeechRecognitionEvent) sttEvent;
@@ -168,7 +168,7 @@ public synchronized void sttEventReceived(STTEvent sttEvent) {
         } else if (sttEvent instanceof RecognitionStopEvent) {
             toggleProcessing(false);
         } else if (sttEvent instanceof SpeechRecognitionErrorEvent) {
-            if (false == this.isSTTServerAborting) {
+            if (!this.isSTTServerAborting) {
                 this.sttServiceHandle.abort();
                 this.isSTTServerAborting = true;
                 toggleProcessing(false);

diff --git a/...me.core.voice/src/main/java/org/eclipse/smarthome/core/voice/text/ExpressionSequence.java b/...me.core.voice/src/main/java/org/eclipse/smarthome/core/voice/text/ExpressionSequence.java
@@ -71,10 +71,12 @@ List<Expression> getChildExpressions() {
     boolean collectFirsts(ResourceBundle language, HashSet<String> firsts) {
         boolean blocking = false;
         for (Expression e : subExpressions) {
-            if ((blocking = e.collectFirsts(language, firsts)) == true) {
+            blocking = e.collectFirsts(language, firsts);
+            if (blocking) {
                 break;
             }
         }
+
         return blocking;
     }
 

diff --git a/...rg.eclipse.smarthome.core/src/main/java/org/eclipse/smarthome/core/items/GenericItem.java b/...rg.eclipse.smarthome.core/src/main/java/org/eclipse/smarthome/core/items/GenericItem.java
@@ -409,11 +409,10 @@ public void setCategory(@Nullable String category) {
      * @return true if state is an acceptedDataType or subclass thereof
      */
     public boolean isAcceptedState(List<Class<? extends State>> acceptedDataTypes, State state) {
-        if (acceptedDataTypes.stream().map(clazz -> clazz.isAssignableFrom(state.getClass()))
-                .filter(found -> found == true).findAny().isPresent()) {
-            return true;
-        }
-        return false;
+        return acceptedDataTypes.stream()
+                .map(clazz -> clazz.isAssignableFrom(state.getClass()))
+                .filter(found -> found)
+                .findAny().isPresent();
     }
 
     protected void logSetTypeError(State state) {

diff --git a/...e.smarthome.core/src/main/java/org/eclipse/smarthome/core/library/types/DateTimeType.java b/...e.smarthome.core/src/main/java/org/eclipse/smarthome/core/library/types/DateTimeType.java
@@ -121,11 +121,11 @@ public static DateTimeType valueOf(String value) {
 
     @Override
     public String format(String pattern) {
-        try {
-            return String.format(pattern, zonedDateTime);
-        } catch (NullPointerException npe) {
+        if (pattern == null) {
             return DateTimeFormatter.ofPattern(DATE_PATTERN).format(zonedDateTime);
         }
+
+        return String.format(pattern, zonedDateTime);
     }
 
     public String format(Locale locale, String pattern) {

diff --git a/...clipse.smarthome.core/src/main/java/org/eclipse/smarthome/core/library/types/HSBType.java b/...clipse.smarthome.core/src/main/java/org/eclipse/smarthome/core/library/types/HSBType.java
@@ -298,7 +298,7 @@ public PercentType[] toRGB() {
                 blue = b;
                 break;
             default:
-                throw new RuntimeException();
+                throw new IllegalArgumentException("Could not convert to RGB.");
         }
         return new PercentType[] { red, green, blue };
     }

diff --git a/...sitemap/src/main/java/org/eclipse/smarthome/io/rest/sitemap/internal/SitemapResource.java b/...sitemap/src/main/java/org/eclipse/smarthome/io/rest/sitemap/internal/SitemapResource.java
@@ -397,7 +397,7 @@ private PageDTO createPageBean(String sitemapName, String title, String icon, St
     private WidgetDTO createWidgetBean(String sitemapName, Widget widget, boolean drillDown, URI uri, String widgetId,
             Locale locale) {
         // Test visibility
-        if (itemUIRegistry.getVisiblity(widget) == false) {
+        if (!itemUIRegistry.getVisiblity(widget)) {
             return null;
         }
 

diff --git a/...g.eclipse.smarthome.io.rest/src/main/java/org/eclipse/smarthome/io/rest/JSONResponse.java b/...g.eclipse.smarthome.io.rest/src/main/java/org/eclipse/smarthome/io/rest/JSONResponse.java
@@ -164,7 +164,7 @@ private Response createResponse(Status status, Object entity) {
             PipedInputStream in = new PipedInputStream(out);
             rp.entity(in);
         } catch (IOException e) {
-            throw new RuntimeException(e);
+            throw new IllegalStateException(e);
         }
 
         Thread writerThread = new Thread(() -> {

diff --git a/...rg.eclipse.smarthome.test/src/main/java/org/eclipse/smarthome/test/java/JavaOSGiTest.java b/...rg.eclipse.smarthome.test/src/main/java/org/eclipse/smarthome/test/java/JavaOSGiTest.java
@@ -135,7 +135,7 @@ private <T> ServiceReference<T>[] getServices(final Class<T> clazz) {
                     .getServiceReferences(clazz.getName(), null);
             return serviceReferences;
         } catch (InvalidSyntaxException e) {
-            throw new Error("Invalid exception for a null filter");
+            throw new IllegalArgumentException("Invalid exception for a null filter");
         }
     }
 

diff --git a/...st/org.eclipse.smarthome.test/src/main/java/org/eclipse/smarthome/test/java/JavaTest.java b/...st/org.eclipse.smarthome.test/src/main/java/org/eclipse/smarthome/test/java/JavaTest.java
@@ -181,7 +181,7 @@ private void internalSleep(long millis) {
         try {
             Thread.sleep(millis);
         } catch (InterruptedException e) {
-            throw new Error("We shouldn't be interrupted while testing");
+            throw new IllegalStateException("We shouldn't be interrupted while testing");
         }
     }
 

diff --git a/...marthome.ui/src/main/java/org/eclipse/smarthome/ui/internal/items/ItemUIRegistryImpl.java b/...marthome.ui/src/main/java/org/eclipse/smarthome/ui/internal/items/ItemUIRegistryImpl.java
@@ -1065,7 +1065,7 @@ private String processColorDefinition(State state, List<ColorArray> colorList) {
                     value = color.getState();
                 }
 
-                if (matchStateToValue(cmpState, value, color.getCondition()) == true) {
+                if (matchStateToValue(cmpState, value, color.getCondition())) {
                     // We have the color for this value - break!
                     colorString = color.getArg();
                     break;
@@ -1141,7 +1141,7 @@ public boolean getVisiblity(Widget w) {
                 value = rule.getState();
             }
 
-            if (matchStateToValue(state, value, rule.getCondition()) == true) {
+            if (matchStateToValue(state, value, rule.getCondition())) {
                 // We have the name for this value!
                 return true;
             }

diff --git a/....astro/src/main/java/org/eclipse/smarthome/binding/astro/internal/util/PropertyUtils.java b/....astro/src/main/java/org/eclipse/smarthome/binding/astro/internal/util/PropertyUtils.java
@@ -64,7 +64,7 @@ public static State getState(ChannelUID channelUID, AstroChannelConfig config, O
         } else if (value instanceof String || value instanceof Enum) {
             return new StringType(value.toString());
         } else {
-            throw new RuntimeException("Unsupported value type " + value.getClass().getSimpleName());
+            throw new IllegalStateException("Unsupported value type " + value.getClass().getSimpleName());
         }
     }
 

diff --git a/...va/org/eclipse/smarthome/binding/fsinternetradio/test/FSInternetRadioHandlerJavaTest.java b/...va/org/eclipse/smarthome/binding/fsinternetradio/test/FSInternetRadioHandlerJavaTest.java
@@ -197,7 +197,7 @@ public void offlineIfWrongPIN() {
                 String.valueOf(DEFAULT_CONFIG_PROPERTY_PORT), DEFAULT_CONFIG_PROPERTY_REFRESH);
         initializeRadioThing(config);
         waitForAssert(() -> {
-            String exceptionMessage = "java.lang.RuntimeException: Radio does not allow connection, maybe wrong pin?";
+            String exceptionMessage = "Radio does not allow connection, maybe wrong pin?";
             verifyCommunicationError(exceptionMessage);
         });
     }

diff --git a/...ipse/smarthome/binding/fsinternetradio/internal/radio/FrontierSiliconRadioConnection.java b/...ipse/smarthome/binding/fsinternetradio/internal/radio/FrontierSiliconRadioConnection.java
@@ -93,7 +93,7 @@ public boolean doLogin() throws IOException {
                 String reason = response.getReason();
                 logger.debug("Communication with radio failed: {} {}", statusCode, reason);
                 if (statusCode == HttpStatus.FORBIDDEN_403) {
-                    throw new RuntimeException("Radio does not allow connection, maybe wrong pin?");
+                    throw new IllegalStateException("Radio does not allow connection, maybe wrong pin?");
                 }
                 throw new IOException("Communication with radio failed, return code: " + statusCode);
             }

diff --git a/...binding.hue/src/main/java/org/eclipse/smarthome/binding/hue/handler/HueBridgeHandler.java b/...binding.hue/src/main/java/org/eclipse/smarthome/binding/hue/handler/HueBridgeHandler.java
@@ -515,16 +515,19 @@ private boolean isEqual(State state1, State state2) {
 
         boolean colorModeIsEqual = true;
         boolean effectIsEqual = true;
-        try {
-            colorModeIsEqual = state1.getColorMode().equals(state2.getColorMode());
-        } catch (NullPointerException npe) {
+
+        if (state1.getColorMode() == null) {
             logger.trace("Light does not support color mode.");
+        } else {
+            colorModeIsEqual = state1.getColorMode().equals(state2.getColorMode());
         }
-        try {
-            effectIsEqual = state1.getEffect().equals(state2.getEffect());
-        } catch (NullPointerException npe) {
+
+        if (state1.getEffect() == null) {
             logger.trace("Light does not support effect.");
+        } else {
+            effectIsEqual = state1.getEffect().equals(state2.getEffect());
         }
+
         return colorModeIsEqual && effectIsEqual;
     }
 

diff --git a/...thome.binding.hue/src/main/java/org/eclipse/smarthome/binding/hue/internal/HueBridge.java b/...thome.binding.hue/src/main/java/org/eclipse/smarthome/binding/hue/internal/HueBridge.java
@@ -873,6 +873,10 @@ private void handleErrors(Result result) throws IOException, ApiException {
                 }
 
                 for (ErrorResponse error : errors) {
+                    if (error == null) {
+                        continue;
+                    }
+
                     switch (error.getType()) {
                         case 1:
                             username = null;
@@ -893,8 +897,6 @@ private void handleErrors(Result result) throws IOException, ApiException {
                 }
             } catch (JsonParseException e) {
                 // Not an error
-            } catch (NullPointerException e) {
-                // Object that looks like error
             }
         }
     }

diff --git a/...nding.wemo/src/main/java/org/eclipse/smarthome/binding/wemo/handler/WemoLightHandler.java b/...nding.wemo/src/main/java/org/eclipse/smarthome/binding/wemo/handler/WemoLightHandler.java
@@ -280,7 +280,7 @@ public void handleCommand(ChannelUID channelUID, Command command) {
                     }
                 }
             } catch (Exception e) {
-                throw new RuntimeException("Could not send command to WeMo Bridge", e);
+                throw new IllegalStateException("Could not send command to WeMo Bridge", e);
             }
         }
     }
@@ -339,7 +339,7 @@ public void getDeviceState() {
                 }
             }
         } catch (Exception e) {
-            throw new RuntimeException("Could not retrieve new Wemo light state", e);
+            throw new IllegalStateException("Could not retrieve new Wemo light state", e);
         }
     }
 

diff --git a/...ing.wemo/src/main/java/org/eclipse/smarthome/binding/wemo/internal/http/WemoHttpCall.java b/...ing.wemo/src/main/java/org/eclipse/smarthome/binding/wemo/internal/http/WemoHttpCall.java
@@ -40,7 +40,7 @@ public static String executeCall(String wemoURL, String soapHeader, String conte
             String wemoCallResponse = HttpUtil.executeUrl("POST", wemoURL, wemoHeaders, wemoContent, null, 2000);
             return wemoCallResponse;
         } catch (Exception e) {
-            throw new RuntimeException("Could not call WeMo", e);
+            throw new IllegalStateException("Could not call WeMo", e);
         }
     }
 

diff --git a/....classic/src/main/java/org/eclipse/smarthome/ui/classic/internal/render/PageRenderer.java b/....classic/src/main/java/org/eclipse/smarthome/ui/classic/internal/render/PageRenderer.java
@@ -162,7 +162,7 @@ private void processChildren(StringBuilder sb_pre, StringBuilder sb_post, EList<
     @Override
     public EList<Widget> renderWidget(Widget w, StringBuilder sb) throws RenderException {
         // Check if this widget is visible
-        if (itemUIRegistry.getVisiblity(w) == false) {
+        if (!itemUIRegistry.getVisiblity(w)) {
             return null;
         }
 

diff --git a/...lassic/src/main/java/org/eclipse/smarthome/ui/classic/internal/servlet/WebAppServlet.java b/...lassic/src/main/java/org/eclipse/smarthome/ui/classic/internal/servlet/WebAppServlet.java
@@ -141,7 +141,7 @@ public void service(ServletRequest req, ServletResponse res) throws ServletExcep
                 // we are at the homepage, so we render the children of the sitemap root node
                 String label = sitemap.getLabel() != null ? sitemap.getLabel() : sitemapName;
                 EList<Widget> children = renderer.getItemUIRegistry().getChildren(sitemap);
-                if (poll && waitForChanges(children) == false) {
+                if (!(poll && waitForChanges(children))) {
                     // we have reached the timeout, so we do not return any content as nothing has changed
                     res.getWriter().append(getTimeoutResponse()).close();
                     return;
@@ -159,7 +159,7 @@ public void service(ServletRequest req, ServletResponse res) throws ServletExcep
                     EList<Widget> parentAndChildren = new BasicEList<Widget>();
                     parentAndChildren.add(lw);
                     parentAndChildren.addAll(children);
-                    if (poll && waitForChanges(parentAndChildren) == false) {
+                    if (!(poll && waitForChanges(parentAndChildren))) {
                         // we have reached the timeout, so we do not return any content as nothing has changed
                         res.getWriter().append(getTimeoutResponse()).close();
                         return;

diff --git a/tools/static-code-analysis/pmd/suppressions.properties b/tools/static-code-analysis/pmd/suppressions.properties
@@ -3,3 +3,8 @@ org.eclipse.smarthome.io.console.karaf.internal.OSGiConsole=SystemPrintln
 org.eclipse.smarthome.io.console.rfc147.internal.CommandWrapper=SystemPrintln
 org.eclipse.smarthome.io.console.rfc147.internal.OSGiConsole=SystemPrintln
 org.eclipse.smarthome.core.internal.events.ThreadedEventHandler=EmptyIfStmt
+org.eclipse.smarthome.config.xml.ConfigDescriptionConverter=AvoidCatchingNPE
+org.eclipse.smarthome.io.rest.core.internal.thing.ThingResource=AvoidCatchingNPE
+org.eclipse.smarthome.test.java.JavaTest=AvoidCatchingNPE
+org.eclipse.smarthome.model.core.internal.ModelRepositoryImpl=AvoidCatchingNPE
+org.eclipse.smarthome.io.net.http.internal.SecureHttpClientFactory=AvoidThrowingRawExceptionTypes