BUG-4109: Correct checkstyle dependency and avoid a couple of NPEs

We need to ensure that the version of checkstyle used to build the
plugin extension is the same as that used when running the
maven-checkstyle-plugin. (Otherwise, ABI changes break the checks,
notably changes in type values.) To do this, depend on the
maven-checkstyle-plugin; it's not particularly elegant but it gets the
job done.

Also fix a couple of places where a type or variable name is retrieved
before further checks which could be done beforehand. By doing the
risk-free checks before calculating the type or variable name, we
avoid NPEs.

Clean up an "if () { return true; } else { return false; }".

Change-Id: Iec696a7486e43f52db69befe12c6a053903c879d
Signed-off-by: Stephen Kitt <skitt@redhat.com>
Signed-off-by: Robert Varga <rovarga@cisco.com>

diff --git a/common/checkstyle-logging/pom.xml b/common/checkstyle-logging/pom.xml
index 238e78c7ea509ba0ea52587cdd163fd9a7f1e7c5..7ddcee038bac4e1dced8ee2341381c7d80e3b66b 100644 (file)
--- a/common/checkstyle-logging/pom.xml
+++ b/common/checkstyle-logging/pom.xml
@@ -18,10 +18,11 @@
   <artifactId>checkstyle-logging</artifactId>
 
   <dependencies>
+    <!-- This pulls in the appropriate version of checkstyle -->
     <dependency>
-      <groupId>com.puppycrawl.tools</groupId>
-      <artifactId>checkstyle</artifactId>
-      <version>6.1.1</version>
+      <groupId>org.apache.maven.plugins</groupId>
+      <artifactId>maven-checkstyle-plugin</artifactId>
+      <version>${checkstyle.version}</version>
     </dependency>
     <dependency>
         <groupId>org.slf4j</groupId>

diff --git a/common/checkstyle-logging/src/main/java/org/opendaylight/yangtools/checkstyle/CheckLoggingUtil.java b/common/checkstyle-logging/src/main/java/org/opendaylight/yangtools/checkstyle/CheckLoggingUtil.java
index f8673c7f7a81b460074c5a47223021c2ee50bd37..d19555b52abb3d49cc997c57f437e73a6ce7b456 100644 (file)
--- a/common/checkstyle-logging/src/main/java/org/opendaylight/yangtools/checkstyle/CheckLoggingUtil.java
+++ b/common/checkstyle-logging/src/main/java/org/opendaylight/yangtools/checkstyle/CheckLoggingUtil.java
@@ -34,10 +34,7 @@ public class CheckLoggingUtil {
 
     public static boolean isLoggerType(final DetailAST aAST) {
         final String typeName = getTypeName(aAST);
-        if(typeName.equals(LOGGER_TYPE_FULL_NAME) || typeName.equals(LOGGER_TYPE_NAME)) {
-            return true;
-        }
-        return false;
+        return typeName.equals(LOGGER_TYPE_FULL_NAME) || typeName.equals(LOGGER_TYPE_NAME);
     }
 
     public static String getVariableName(final DetailAST aAST) {

diff --git a/common/checkstyle-logging/src/main/java/org/opendaylight/yangtools/checkstyle/LoggerMustBeSlf4jCheck.java b/common/checkstyle-logging/src/main/java/org/opendaylight/yangtools/checkstyle/LoggerMustBeSlf4jCheck.java
index a78eec8528968514caf388528845c2986e77753e..1f814d659e4a29724fb346d47c8b0eda33661929 100644 (file)
--- a/common/checkstyle-logging/src/main/java/org/opendaylight/yangtools/checkstyle/LoggerMustBeSlf4jCheck.java
+++ b/common/checkstyle-logging/src/main/java/org/opendaylight/yangtools/checkstyle/LoggerMustBeSlf4jCheck.java
@@ -28,19 +28,19 @@ public class LoggerMustBeSlf4jCheck extends Check {
     @Override
     public void visitToken(DetailAST aAST) {
         if(aAST.getType() == TokenTypes.VARIABLE_DEF) {
-            final String typeName = CheckLoggingUtil.getTypeName(aAST);
-            if (CheckLoggingUtil.isAFieldVariable(aAST) && typeName.contains("." + LOGGER_TYPE_NAME)) {
-                if(!typeName.equals(LOGGER_TYPE_FULL_NAME)) {
+            if (CheckLoggingUtil.isAFieldVariable(aAST)) {
+                final String typeName = CheckLoggingUtil.getTypeName(aAST);
+                if (typeName.contains("." + LOGGER_TYPE_NAME) && !typeName.equals(LOGGER_TYPE_FULL_NAME)) {
                     log(aAST.getLineNo(), LOG_MESSAGE);
                 }
             }
         } else if(aAST.getType() == TokenTypes.IMPORT) {
             final String importType = aAST.getFirstChild().findFirstToken(TokenTypes.IDENT).getText();
             if(importType.equals(CheckLoggingUtil.LOGGER_TYPE_NAME)) {
-               final String importIdent = aAST.getFirstChild().getFirstChild().getLastChild().getText();
+                final String importIdent = aAST.getFirstChild().getFirstChild().getLastChild().getText();
                 if(!importIdent.equals(SLF4J)) {
-                   log(aAST.getLineNo(), LOG_MESSAGE);
-               }
+                    log(aAST.getLineNo(), LOG_MESSAGE);
+                }
             }
         }
     }

diff --git a/common/checkstyle-logging/src/main/java/org/opendaylight/yangtools/checkstyle/LoggerVariableNameCheck.java b/common/checkstyle-logging/src/main/java/org/opendaylight/yangtools/checkstyle/LoggerVariableNameCheck.java
index 615f9148c03843cc50fe4df3718085bec0723d3f..6e69da05880c0762b7ce52a185db85bca624571b 100644 (file)
--- a/common/checkstyle-logging/src/main/java/org/opendaylight/yangtools/checkstyle/LoggerVariableNameCheck.java
+++ b/common/checkstyle-logging/src/main/java/org/opendaylight/yangtools/checkstyle/LoggerVariableNameCheck.java
@@ -25,8 +25,8 @@ public class LoggerVariableNameCheck extends Check {
 
     @Override
     public void visitToken(DetailAST aAST) {
-        final String variableName = CheckLoggingUtil.getVariableName(aAST);
-        if (CheckLoggingUtil.isAFieldVariable(aAST) && CheckLoggingUtil.isLoggerType(aAST) && !variableName.equals(LOGGER_VAR_NAME)) {
+        if (CheckLoggingUtil.isAFieldVariable(aAST) && CheckLoggingUtil.isLoggerType(aAST)
+                && !LOGGER_VAR_NAME.equals(CheckLoggingUtil.getVariableName(aAST))) {
             log(aAST.getLineNo(), LOG_MESSAGE);
         }
     }