From: Michael Vorburger <vorburger@redhat.com>
Date: Wed, 25 May 2016 12:58:03 +0000 (+0200)
Subject: New LogMessageExtractorCheck which writes out TXT report of all loggers
X-Git-Tag: release/boron~67
X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=refs%2Fchanges%2F10%2F39410%2F10;hp=4a575b0bf11bfd39cd0169e63983c054b3760bce;p=yangtools.git

New LogMessageExtractorCheck which writes out TXT report of all loggers

Change-Id: I853c742901131293c58c78bea821fe0d12957f07
Signed-off-by: Michael Vorburger <vorburger@redhat.com>
---

diff --git a/common/checkstyle-logging/pom.xml b/common/checkstyle-logging/pom.xml
index 4628cf1216..53467208a9 100644
--- a/common/checkstyle-logging/pom.xml
+++ b/common/checkstyle-logging/pom.xml
@@ -42,6 +42,18 @@
     </dependency>
   </dependencies>
 
+  <build>
+    <plugins>
+      <plugin>
+        <groupId>org.apache.maven.plugins</groupId>
+        <artifactId>maven-checkstyle-plugin</artifactId>
+        <configuration>
+          <propertyExpansion>checkstyle.violationSeverity=error</propertyExpansion>
+        </configuration>
+      </plugin>
+    </plugins>
+  </build>
+
   <!--
       Maven Site Configuration
 
diff --git a/common/checkstyle-logging/src/main/java/org/opendaylight/yangtools/checkstyle/AbstractLogMessageCheck.java b/common/checkstyle-logging/src/main/java/org/opendaylight/yangtools/checkstyle/AbstractLogMessageCheck.java
new file mode 100644
index 0000000000..02ba5b8d78
--- /dev/null
+++ b/common/checkstyle-logging/src/main/java/org/opendaylight/yangtools/checkstyle/AbstractLogMessageCheck.java
@@ -0,0 +1,48 @@
+/*
+ * Copyright (c) 2016 Red Hat, Inc. and others.  All rights reserved.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v1.0 which accompanies this distribution,
+ * and is available at http://www.eclipse.org/legal/epl-v10.html
+ */
+package org.opendaylight.yangtools.checkstyle;
+
+import com.puppycrawl.tools.checkstyle.api.AbstractCheck;
+import com.puppycrawl.tools.checkstyle.api.DetailAST;
+import com.puppycrawl.tools.checkstyle.api.TokenTypes;
+import java.util.Optional;
+
+public abstract class AbstractLogMessageCheck extends AbstractCheck {
+
+    @Override
+    public int[] getDefaultTokens() {
+        return new int[]{TokenTypes.METHOD_CALL};
+    }
+
+    @Override
+    public void visitToken(DetailAST ast) {
+        String methodName = CheckLoggingUtil.getMethodName(ast);
+        if (CheckLoggingUtil.isLogMethod(methodName)) {
+            Optional<String> optLogMessage = getLogMessage(ast);
+            optLogMessage.ifPresent(logMessage -> visitLogMessage(ast, logMessage));
+        }
+    }
+
+    private Optional<String> getLogMessage(DetailAST ast) {
+        ast = ast.findFirstToken(TokenTypes.ELIST);
+        if (ast != null) {
+            ast = ast.getFirstChild();
+            if (ast != null) {
+                ast = ast.getFirstChild();
+                if (ast != null) {
+                    if (ast.getType() == TokenTypes.STRING_LITERAL) {
+                        return Optional.ofNullable(ast.getText());
+                    }
+                }
+            }
+        }
+        return Optional.empty();
+    }
+
+    protected abstract void visitLogMessage(DetailAST ast, String logMessage);
+}
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 e8f86bffb8..e5e079d88b 100644
--- 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
@@ -47,6 +47,13 @@ public final class CheckLoggingUtil {
         return ast.getParent().getType() == TokenTypes.OBJBLOCK;
     }
 
+    /**
+     * Returns the name the method (and the enclosing class) at a given point specified by the
+     * passed-in abstract syntax tree (AST).
+     *
+     * @param ast an abstract syntax tree (AST) pointing to method call
+     * @return the name of the method being called
+     */
     public static String getMethodName(final DetailAST ast) {
         if (ast.getFirstChild().getLastChild() != null) {
             return ast.getFirstChild().getFirstChild().getText() + "." + ast.getFirstChild().getLastChild().getText();
@@ -58,6 +65,13 @@ public final class CheckLoggingUtil {
         return LOG_METHODS.contains(methodName);
     }
 
+    /**
+     * Returns the name of the closest enclosing class of the point by the passed-in abstract syntax
+     * tree (AST).
+     *
+     * @param ast an abstract syntax tree (AST)
+     * @return the name of the closest enclosign class
+     */
     public static String getClassName(final DetailAST ast) {
         DetailAST parent = ast.getParent();
         while (parent.getType() != TokenTypes.CLASS_DEF && parent.getType() != TokenTypes.ENUM_DEF) {
diff --git a/common/checkstyle-logging/src/main/java/org/opendaylight/yangtools/checkstyle/FileNameUtil.java b/common/checkstyle-logging/src/main/java/org/opendaylight/yangtools/checkstyle/FileNameUtil.java
new file mode 100644
index 0000000000..d29122834b
--- /dev/null
+++ b/common/checkstyle-logging/src/main/java/org/opendaylight/yangtools/checkstyle/FileNameUtil.java
@@ -0,0 +1,52 @@
+/*
+ * Copyright (c) 2016 Red Hat, Inc. and others.  All rights reserved.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v1.0 which accompanies this distribution,
+ * and is available at http://www.eclipse.org/legal/epl-v10.html
+ */
+package org.opendaylight.yangtools.checkstyle;
+
+import java.io.File;
+import java.nio.file.Path;
+import java.util.Optional;
+
+/**
+ * Utility to convert absolute file name to path relative to project.
+ *
+ * <p>Current implementation use a sad heuristic based on detecting a pom.xml.
+ * This is of course sub-optimal to say the very least.  Improvements welcome.
+ *
+ * @see <a href="https://groups.google.com/forum/#!topic/checkstyle-devel/Rfwx81YhVQk">checkstyle-devel list thread</a>
+ */
+public class FileNameUtil {
+
+    private FileNameUtil() {
+    }
+
+    static File getPathRelativeToMavenProjectRootIfPossible(File absoluteFile) {
+        return getOptionalPathRelativeToMavenProjectRoot(absoluteFile).orElse(absoluteFile);
+    }
+
+    static Optional<File> getOptionalPathRelativeToMavenProjectRoot(File absoluteFile) {
+        if (!absoluteFile.isAbsolute()) {
+            return Optional.of(absoluteFile);
+        }
+        File projectRoot = absoluteFile;
+        while (!isProjectRootDir(projectRoot) && projectRoot.getParentFile() != null) {
+            projectRoot = projectRoot.getParentFile();
+        }
+        if (isProjectRootDir(projectRoot)) {
+            Path absolutePath = absoluteFile.toPath();
+            Path basePath = projectRoot.toPath();
+            Path relativePath = basePath.relativize(absolutePath);
+            return Optional.of(relativePath.toFile());
+        }
+        return Optional.empty();
+    }
+
+    private static boolean isProjectRootDir(File file) {
+        return new File(file, "pom.xml").exists();
+    }
+
+}
diff --git a/common/checkstyle-logging/src/main/java/org/opendaylight/yangtools/checkstyle/LogMessageExtractorCheck.java b/common/checkstyle-logging/src/main/java/org/opendaylight/yangtools/checkstyle/LogMessageExtractorCheck.java
new file mode 100644
index 0000000000..6041d92748
--- /dev/null
+++ b/common/checkstyle-logging/src/main/java/org/opendaylight/yangtools/checkstyle/LogMessageExtractorCheck.java
@@ -0,0 +1,82 @@
+/*
+ * Copyright (c) 2016 Red Hat, Inc. and others.  All rights reserved.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v1.0 which accompanies this distribution,
+ * and is available at http://www.eclipse.org/legal/epl-v10.html
+ */
+package org.opendaylight.yangtools.checkstyle;
+
+import com.google.common.base.Preconditions;
+import com.google.common.io.Files;
+import com.puppycrawl.tools.checkstyle.api.DetailAST;
+import java.io.File;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Check which extracts the content of Logger messages somewhere (e.g. a file).
+ *
+ * <p>This can be used to create a comprehensive list of all log messages.
+ *
+ * <p>It is a first step towards more formal tracking of all messages
+ * from a system with a unique ID, using e.g. a framework such
+ * as jboss-logging.
+ *
+ * <p>Does not actually Check anything, i.e. never emits any Checkstyle warnings.
+ */
+public class LogMessageExtractorCheck extends AbstractLogMessageCheck {
+
+    private static final Logger LOG = LoggerFactory.getLogger(LogMessageExtractorCheck.class);
+
+    static final File DEFAULT_REPORT_FILE = new File("target/logger-messages.txt");
+
+    private File logMessagesReportFile = DEFAULT_REPORT_FILE;
+
+    public void setLogMessagesReportFileName(String fileName) {
+        logMessagesReportFile = new File(fileName);
+        logMessagesReportFile.getParentFile().mkdirs();
+    }
+
+    public File getLogMessagesReportFile() {
+        return logMessagesReportFile;
+    }
+
+    @Override
+    protected void visitLogMessage(DetailAST ast, String logMessage) {
+        File file = new File(getFileContents().getFileName());
+        String fileName = FileNameUtil.getPathRelativeToMavenProjectRootIfPossible(file).getPath();
+        int lineNumber = ast.getLineNo();
+        LogMessageOccurence log = new LogMessageOccurence(fileName, lineNumber, logMessage);
+        updateMessagesReportFile(log);
+    }
+
+    protected void updateMessagesReportFile(LogMessageOccurence log) {
+        try {
+            Files.append(log.toString() + "\n", getLogMessagesReportFile(), StandardCharsets.UTF_8);
+        } catch (IOException e) {
+            LOG.error("Failed to append to file: {}", logMessagesReportFile.getPath(), e);
+        }
+    }
+
+    public static class LogMessageOccurence {
+
+        // relative to current project root
+        public final String javaSourceFilePath;
+        public final int lineNumber;
+        public final String message;
+
+        public LogMessageOccurence(String javaSourceFilePath, int lineNumber, String message) {
+            this.javaSourceFilePath = Preconditions.checkNotNull(javaSourceFilePath, "javaSourceFilePath");
+            this.lineNumber = lineNumber;
+            this.message = Preconditions.checkNotNull(message, "message");
+        }
+
+        @Override
+        public String toString() {
+            return javaSourceFilePath + ":" + lineNumber + ":" + message;
+        }
+    }
+}
diff --git a/common/checkstyle-logging/src/main/java/org/opendaylight/yangtools/checkstyle/LogMessagePlaceholderCountCheck.java b/common/checkstyle-logging/src/main/java/org/opendaylight/yangtools/checkstyle/LogMessagePlaceholderCountCheck.java
index a9859b26e8..7c0ceec950 100644
--- a/common/checkstyle-logging/src/main/java/org/opendaylight/yangtools/checkstyle/LogMessagePlaceholderCountCheck.java
+++ b/common/checkstyle-logging/src/main/java/org/opendaylight/yangtools/checkstyle/LogMessagePlaceholderCountCheck.java
@@ -8,36 +8,25 @@
 
 package org.opendaylight.yangtools.checkstyle;
 
-import com.puppycrawl.tools.checkstyle.api.AbstractCheck;
 import com.puppycrawl.tools.checkstyle.api.DetailAST;
 import com.puppycrawl.tools.checkstyle.api.TokenTypes;
 
-public class LogMessagePlaceholderCountCheck extends AbstractCheck {
+public class LogMessagePlaceholderCountCheck extends AbstractLogMessageCheck {
 
     private static final String LOG_MESSAGE = "Log message placeholders count is incorrect.";
     private static final String PLACEHOLDER = "{}";
     private static final String EXCEPTION_TYPE = "Exception";
 
     @Override
-    public int[] getDefaultTokens() {
-        return new int[]{TokenTypes.METHOD_CALL};
-    }
-
-    @Override
-    public void visitToken(DetailAST ast) {
-        final String methodName = CheckLoggingUtil.getMethodName(ast);
-        if (CheckLoggingUtil.isLogMethod(methodName)) {
-            final String logMessage = ast.findFirstToken(TokenTypes.ELIST).getFirstChild().getFirstChild().getText();
-            int placeholdersCount = placeholdersCount(logMessage);
-            int argumentsCount = ast.findFirstToken(TokenTypes.ELIST).getChildCount(TokenTypes.EXPR) - 1;
-            final String lastArg = ast.findFirstToken(TokenTypes.ELIST).getLastChild().getFirstChild().getText();
-            if (hasCatchBlockParentWithArgument(lastArg, ast) || hasMethodDefinitionWithExceptionArgument(lastArg,
-                    ast)) {
-                argumentsCount--;
-            }
-            if (placeholdersCount > argumentsCount) {
-                log(ast.getLineNo(), LOG_MESSAGE);
-            }
+    protected void visitLogMessage(DetailAST ast, String logMessage) {
+        int placeholdersCount = placeholdersCount(logMessage);
+        int argumentsCount = ast.findFirstToken(TokenTypes.ELIST).getChildCount(TokenTypes.EXPR) - 1;
+        final String lastArg = ast.findFirstToken(TokenTypes.ELIST).getLastChild().getFirstChild().getText();
+        if (hasCatchBlockParentWithArgument(lastArg, ast) || hasMethodDefinitionWithExceptionArgument(lastArg, ast)) {
+            argumentsCount--;
+        }
+        if (placeholdersCount > argumentsCount) {
+            log(ast.getLineNo(), LOG_MESSAGE);
         }
     }
 
diff --git a/common/checkstyle-logging/src/main/resources/checkstyle-logging.xml b/common/checkstyle-logging/src/main/resources/checkstyle-logging.xml
index c8173b8dd8..818bd230cc 100644
--- a/common/checkstyle-logging/src/main/resources/checkstyle-logging.xml
+++ b/common/checkstyle-logging/src/main/resources/checkstyle-logging.xml
@@ -33,4 +33,8 @@
         <module name="org.opendaylight.yangtools.checkstyle.LoggerDeclarationsCountCheck"/>
     </module>
 
+    <module name="TreeWalker">
+        <module name="org.opendaylight.yangtools.checkstyle.LogMessageExtractorCheck"/>
+    </module>
+
 </module>
\ No newline at end of file
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 ed5a1719cf..ba3a4da067 100644
--- 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
@@ -25,6 +25,9 @@ public class CheckLoggingTestClass {
             System.err.print(e.getMessage());
             logger.debug("foo {}", "bar", e);
             LOG.info("foo {} {}", e.getMessage(), e);
+            // Multi line
+            LOG.info("foo {} "
+                    + "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 df132ab6a9..9c0342355b 100644
--- 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
@@ -11,7 +11,9 @@ package org.opendaylight.yangtools.checkstyle;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
+import com.google.common.base.Charsets;
 import com.google.common.collect.Lists;
+import com.google.common.io.Files;
 import com.puppycrawl.tools.checkstyle.Checker;
 import com.puppycrawl.tools.checkstyle.ConfigurationLoader;
 import com.puppycrawl.tools.checkstyle.DefaultLogger;
@@ -21,6 +23,7 @@ import com.puppycrawl.tools.checkstyle.api.CheckstyleException;
 import com.puppycrawl.tools.checkstyle.api.Configuration;
 import java.io.ByteArrayOutputStream;
 import java.io.File;
+import java.util.List;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -61,10 +64,20 @@ public class CheckstyleTest {
                 "18: Logger might be declared only once",
                 "17: Logger must be slf4j",
                 "27: Log message placeholders count is incorrect",
-                "33: Log message placeholders count is incorrect",
-                "42: Log message contains string concatenation");
+                "36: Log message placeholders count is incorrect",
+                "45: Log message contains string concatenation");
     }
 
+    @Test
+    public void testLogMessageExtractorCheck() throws Exception {
+        File logMessageReport = LogMessageExtractorCheck.DEFAULT_REPORT_FILE;
+        logMessageReport.delete();
+        verify(CheckLoggingTestClass.class, false);
+        List<String> reportLines = Files.readLines(logMessageReport, Charsets.UTF_8);
+        assertEquals(6, reportLines.size());
+        assertEquals("src/test/java/org/opendaylight/yangtools/checkstyle/CheckLoggingTestClass.java:27:\"foo {} {}\"", reportLines.get(0));
+        // TODO assertEquals("src/test/java/org/opendaylight/yangtools/checkstyle/CheckLoggingTestClass.java:28:\"foo {} bar {}\"", reportLines.get(1));
+    }
 
     private void verify(final Class<?> testClass, final boolean checkCount, final String... expectedMessages) throws CheckstyleException {
         final String filePath = System.getProperty("user.dir") + File.separator + "src" + File.separator + "test" + File.separator + "java" + File.separator + testClass.getName().replaceAll("\\.", "/") + ".java";
diff --git a/common/checkstyle-logging/src/test/java/org/opendaylight/yangtools/checkstyle/FileNameUtilTest.java b/common/checkstyle-logging/src/test/java/org/opendaylight/yangtools/checkstyle/FileNameUtilTest.java
new file mode 100644
index 0000000000..ffd5e9cf83
--- /dev/null
+++ b/common/checkstyle-logging/src/test/java/org/opendaylight/yangtools/checkstyle/FileNameUtilTest.java
@@ -0,0 +1,21 @@
+package org.opendaylight.yangtools.checkstyle;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.io.File;
+import org.junit.Test;
+
+public class FileNameUtilTest {
+
+    @Test
+    public void testFileNameUtil() {
+        File relativeFile = new File("src/main/java");
+        assertFalse(relativeFile.isAbsolute());
+        File absoluteFile = relativeFile.getAbsoluteFile();
+        assertTrue(absoluteFile.isAbsolute());
+        assertEquals("src/main/java", FileNameUtil.getOptionalPathRelativeToMavenProjectRoot(absoluteFile).get().getPath());
+    }
+
+}