Merge "Fix logging checkstyle"

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 0747a85e0adb7ca07f3a5d69d181d519f8f9a258..f8673c7f7a81b460074c5a47223021c2ee50bd37 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
@@ -23,7 +23,7 @@ public class CheckLoggingUtil {
     public static final String LOGGER_TYPE_NAME = Logger.class.getSimpleName();
     public static final String LOGGER_TYPE_FULL_NAME = Logger.class.getName();
     public static final String LOGGER_VAR_NAME = "LOG";
-    private static final List<String> LOG_METHODS = Lists.newArrayList("debug", "info", "error", "warn", "trace");
+    private static final List<String> LOG_METHODS = Lists.newArrayList("LOG.debug", "LOG.info", "LOG.error", "LOG.warn", "LOG.trace");
 
     private CheckLoggingUtil() {}
 
@@ -51,7 +51,7 @@ public class CheckLoggingUtil {
 
     public static String getMethodName(final DetailAST aAST) {
         if(aAST.getFirstChild().getLastChild() != null) {
-            return aAST.getFirstChild().getLastChild().getText();
+            return aAST.getFirstChild().getFirstChild().getText() + "." + aAST.getFirstChild().getLastChild().getText();
         }
         return aAST.getFirstChild().getText();
     }

diff --git a/common/checkstyle-logging/src/main/java/org/opendaylight/yangtools/checkstyle/LogMessageConcatenationCheck.java b/common/checkstyle-logging/src/main/java/org/opendaylight/yangtools/checkstyle/LogMessageConcatenationCheck.java
index d725c451c7fbefd6629e8d862c9104ac090525d2..d078ba7e306dec9e1acb2632451f07058dd3dc01 100644 (file)
--- a/common/checkstyle-logging/src/main/java/org/opendaylight/yangtools/checkstyle/LogMessageConcatenationCheck.java
+++ b/common/checkstyle-logging/src/main/java/org/opendaylight/yangtools/checkstyle/LogMessageConcatenationCheck.java
@@ -25,9 +25,14 @@ public class LogMessageConcatenationCheck extends Check {
     public void visitToken(DetailAST aAST) {
         final String methodName = CheckLoggingUtil.getMethodName(aAST);
         if(CheckLoggingUtil.isLogMethod(methodName)) {
-            final String logMessage = aAST.findFirstToken(TokenTypes.ELIST).getFirstChild().getFirstChild().getText();
-            if(logMessage.contains("+")) {
-                log(aAST.getLineNo(), LOG_MESSAGE);
+            DetailAST plus = aAST.findFirstToken(TokenTypes.ELIST).getFirstChild().findFirstToken(TokenTypes.PLUS);
+            if (plus != null) {
+                while (plus.getChildCount(TokenTypes.PLUS) != 0) {
+                    plus = plus.findFirstToken(TokenTypes.PLUS);
+                }
+                if (plus.getChildCount(TokenTypes.STRING_LITERAL) != 2) {
+                    log(aAST.getLineNo(), LOG_MESSAGE);
+                }
             }
         }
     }

diff --git a/common/checkstyle-logging/src/main/java/org/opendaylight/yangtools/checkstyle/LoggerFactoryClassParameterCheck.java b/common/checkstyle-logging/src/main/java/org/opendaylight/yangtools/checkstyle/LoggerFactoryClassParameterCheck.java
index 95915250e46c90222dbc2f9c81497143ba53325b..8007aecdf5bb1775517d8268a0c333b1aa8b2ccb 100644 (file)
--- a/common/checkstyle-logging/src/main/java/org/opendaylight/yangtools/checkstyle/LoggerFactoryClassParameterCheck.java
+++ b/common/checkstyle-logging/src/main/java/org/opendaylight/yangtools/checkstyle/LoggerFactoryClassParameterCheck.java
@@ -15,7 +15,7 @@ import com.puppycrawl.tools.checkstyle.api.TokenTypes;
 public class LoggerFactoryClassParameterCheck extends Check {
 
     private static final String LOG_MESSAGE = "LoggerFactory.getLogger Class argument is incorrect.";
-    private static final String METHOD_NAME = "getLogger";
+    private static final String METHOD_NAME = "LoggerFactory.getLogger";
 
     @Override
     public int[] getDefaultTokens() {

diff --git a/common/checkstyle-logging/src/test/java/org/opendaylight/yangtools/checkstyle/CheckLoggingTestClass.java b/common/checkstyle-logging/src/test/java/org/opendaylight/yangtools/checkstyle/CheckLoggingTestClass.java
index 8b771f952a3b67776a1a1bb53fc733554cd36ab5..f038dda5f5bc7ea74d8fc86a0d06395e77e04df7 100644 (file)
--- a/common/checkstyle-logging/src/test/java/org/opendaylight/yangtools/checkstyle/CheckLoggingTestClass.java
+++ b/common/checkstyle-logging/src/test/java/org/opendaylight/yangtools/checkstyle/CheckLoggingTestClass.java
@@ -33,5 +33,13 @@ public class CheckLoggingTestClass {
         LOG.warn("foo", string);
         LOG.warn("foo {}", string);
         LOG.warn("foo {}", string, e);
+        LOG.warn(
+            "Unable to parse configuration snapshot from {}. Initial config from {} will be IGNORED in this run. " +
+            "Note that subsequent config files may fail due to this problem. " +
+            "Xml markup in this file needs to be fixed, for detailed information see enclosed exception.",
+            string, string, e);
+        LOG.warn("foo" + string);
+        LOG.warn("foo" + "bar");
+        LOG.warn("foo" + "bar" + "bar");
     }
 }

diff --git a/common/checkstyle-logging/src/test/java/org/opendaylight/yangtools/checkstyle/CheckstyleTest.java b/common/checkstyle-logging/src/test/java/org/opendaylight/yangtools/checkstyle/CheckstyleTest.java
index 89f4cf39f60695ddc863a4c2102a47520025baad..24e048ad97a2027a4989bac705dbd5e471f5baab 100644 (file)
--- a/common/checkstyle-logging/src/test/java/org/opendaylight/yangtools/checkstyle/CheckstyleTest.java
+++ b/common/checkstyle-logging/src/test/java/org/opendaylight/yangtools/checkstyle/CheckstyleTest.java
@@ -8,6 +8,7 @@
 
 package org.opendaylight.yangtools.checkstyle;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
 import java.io.ByteArrayOutputStream;
@@ -55,25 +56,30 @@ public class CheckstyleTest {
 
     @Test
     public void testLoggerChecks() throws Exception {
-        verify(CheckLoggingTestClass.class, "15: Logger must be declared as private static final.", "15: Logger name should be LOG.",
+        verify(CheckLoggingTestClass.class, true, "15: Logger must be declared as private static final.", "15: Logger name should be LOG.",
+                "15: LoggerFactory.getLogger Class argument is incorrect.",
                 "17: Logger might be declared only once.", "16: Logger must be slf4j.", "22: Line contains printStacktrace",
-                "23: Line contains console output", "24: Line contains console output",
-                "15: LoggerFactory.getLogger Class argument is incorrect.", "20: Log message contains string concatenation.",
-                "26: Log message placeholders count is incorrect.", "32: Log message placeholders count is incorrect");
+                "23: Line contains console output", "24: Line contains console output", "26: Log message placeholders count is incorrect.",
+                "32: Log message placeholders count is incorrect", "41: Log message contains string concatenation.");
     }
 
     @Test
     public void testCodingChecks() {
-        verify(CheckCodingStyleTestClass.class, "9: Line has Windows line delimiter.", "14: Wrong order for", "24:1: Line contains a tab character.",
+        verify(CheckCodingStyleTestClass.class, false, "9: Line has Windows line delimiter.", "14: Wrong order for", "24:1: Line contains a tab character.",
                 "22: Line has trailing spaces.", "22: ctor def child at indentation level 16 not at correct indentation, 8", "17:8: Unused import",
                 "23: Line has trailing spaces.");
     }
 
-    private void verify(final Class<?> testClass, final String... expectedMessages) {
+    private void verify(final Class<?> testClass, final boolean checkCount, final String... expectedMessages) {
         final String filePath = System.getProperty("user.dir") + File.separator + "src" + File.separator + "test" + File.separator + "java" + File.separator + testClass.getName().replaceAll("\\.", "/") + ".java";
         final File testFile = new File(filePath);
         checker.process(Lists.newArrayList(testFile));
         final String output = baos.toString();
+        System.out.println();
+        if (checkCount) {
+            final int count = output.split("\n").length - 2;
+            assertEquals(expectedMessages.length, count);
+        }
         for(final String message : expectedMessages) {
             assertTrue("Expected message not found: " + message, output.contains(message));
         }