Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Generate a warning if the exception is not added as a last parameter …
…to the logging
- Loading branch information
Tanya D. Georgieva
authored and
Tanya D. Georgieva
committed
Feb 13, 2018
1 parent
b453e9b
commit 55a95ff
Showing
28 changed files
with
1,111 additions
and
0 deletions.
There are no files selected for viewing
273 changes: 273 additions & 0 deletions
273
...tyle/src/main/java/org/openhab/tools/analysis/checkstyle/ExceptionLastParameterCheck.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,273 @@ | ||
/** | ||
* Copyright (c) 2010-2018 by the respective copyright holders. | ||
* | ||
* 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.openhab.tools.analysis.checkstyle; | ||
|
||
import java.text.MessageFormat; | ||
import java.util.LinkedList; | ||
import java.util.Queue; | ||
import java.util.stream.Collectors; | ||
|
||
import org.apache.commons.logging.Log; | ||
import org.apache.commons.logging.LogFactory; | ||
|
||
import com.puppycrawl.tools.checkstyle.api.AbstractCheck; | ||
import com.puppycrawl.tools.checkstyle.api.DetailAST; | ||
import com.puppycrawl.tools.checkstyle.api.TokenTypes; | ||
import com.puppycrawl.tools.checkstyle.utils.CheckUtils; | ||
import com.puppycrawl.tools.checkstyle.utils.CommonUtils; | ||
|
||
/** | ||
* Checks the code and generates a warning if an exception is caught and logged | ||
* and it is not set as a last parameter to the logging. | ||
* | ||
* @author Tanya Georgieva - Initial Contribution | ||
*/ | ||
public class ExceptionLastParameterCheck extends AbstractCheck { | ||
|
||
private static final Class EXCEPTION = Exception.class; | ||
private static final Class RUNTIME_EXCEPTION = RuntimeException.class; | ||
|
||
private static final String EXCEPTION_AS_LAST_PARAMETER_MSG = "Please set exception as last parameter"; | ||
private static final String FULL_STACK_TRACE_MSG = "Log exception full stack trace"; | ||
private static final String FULL_STACK_TRACE_EXCEPTION_AS_LAST_PARAMETER_MSG = EXCEPTION_AS_LAST_PARAMETER_MSG | ||
.concat(" and ").concat(FULL_STACK_TRACE_MSG); | ||
|
||
private static final String EXCEPTION_PACKAGE = "java.lang."; | ||
private static final String LOG_LEVEL_ERROR = "error"; | ||
private static final String LOGGER = "logger"; | ||
|
||
private final Log logger = LogFactory.getLog(ExceptionLastParameterCheck.class); | ||
|
||
private LinkedList<String> imports = new LinkedList<>(); | ||
private LinkedList<DetailAST> exceptions = new LinkedList<>(); | ||
|
||
private String exceptionName = ""; | ||
|
||
@Override | ||
public int[] getDefaultTokens() { | ||
return getAcceptableTokens(); | ||
} | ||
|
||
@Override | ||
public int[] getAcceptableTokens() { | ||
return new int[] { TokenTypes.LITERAL_CATCH, TokenTypes.IMPORT }; | ||
} | ||
|
||
@Override | ||
public int[] getRequiredTokens() { | ||
return CommonUtils.EMPTY_INT_ARRAY; | ||
} | ||
|
||
@Override | ||
public void visitToken(DetailAST ast) { | ||
// imports are always prior to catch blocks | ||
if (ast.getType() == TokenTypes.IMPORT) { | ||
String currentImport = CheckUtils.createFullType(ast).getText(); | ||
this.imports.add(currentImport); | ||
} else { | ||
findLoggedExceptions(ast); | ||
} | ||
} | ||
|
||
private void findLoggedExceptions(DetailAST catchAst) { | ||
extractCaughtExceptions(catchAst); | ||
|
||
LinkedList<DetailAST> loggers = getAllChildrenNodesOfType(catchAst.getLastChild(), TokenTypes.EXPR); | ||
if (loggers.isEmpty()) { | ||
LinkedList<DetailAST> requiredNodes = new LinkedList<>(); | ||
// if the logger is in if-else-statement the list of loggers would be empty. | ||
// the logger nodes we are looking for are nested, so we use recursion to find them | ||
loggers = getAllNodesOfType(requiredNodes, catchAst.getLastChild(), TokenTypes.EXPR); | ||
} | ||
for (DetailAST loggerNode : loggers) { | ||
DetailAST currentLogger = getLogger(loggerNode); | ||
if (currentLogger != null) { | ||
String logLevel = currentLogger.getNextSibling().getText(); | ||
Class<?> currentExceptionClass = getExceptionClass(); | ||
boolean isExceptionUnchecked = RUNTIME_EXCEPTION.isAssignableFrom(currentExceptionClass); | ||
boolean isExceptionChecked = !isExceptionUnchecked && EXCEPTION.isAssignableFrom(currentExceptionClass); | ||
boolean isExceptionValid = isExceptionUnchecked | ||
|| (isExceptionChecked && LOG_LEVEL_ERROR.equals(logLevel)); | ||
if (isExceptionValid) { | ||
// ELIST node contains the logger's parameters | ||
DetailAST elistAst = loggerNode.getFirstChild().findFirstToken(TokenTypes.ELIST); | ||
LinkedList<DetailAST> parameters = getAllChildrenNodesOfType(elistAst, TokenTypes.EXPR); | ||
validateLogger(parameters); | ||
} | ||
} | ||
} | ||
} | ||
|
||
private void validateLogger(LinkedList<DetailAST> parameters) { | ||
int lastParameterPosition = parameters.size(); | ||
int logLineNumber = 0; | ||
// checking if exception parameter is with full stack trace | ||
boolean isFullStackTraceExceptionFound = false; | ||
// checking if at some position in the logger | ||
// there is exception parameter calling one of its methods | ||
boolean isExceptionCallingMethodFound = false; | ||
int fullStackTraceExceptionPosition = 0; | ||
int exceptionCallingMethodPosition = 0; | ||
|
||
for (int i = 0; i < lastParameterPosition; i++) { | ||
DetailAST currentParameter = getFirstNode(parameters.get(i).getFirstChild(), TokenTypes.IDENT); | ||
// The current parameter would be null if it's token type is anything different than IDENT. | ||
// For example the parameter could be of type STRING_LITERAL or NUM_INT. | ||
// In this case: logger.trace("\"{}\" is not a valid integer number.", e, value) | ||
// the current parameters will be "value" and "e" | ||
if (currentParameter != null) { | ||
String currentParameterName = currentParameter.getText(); | ||
logLineNumber = currentParameter.getLineNo(); | ||
|
||
if (this.exceptionName.equals(currentParameterName)) { | ||
// if a parameter is calling a method - e.getMessage() | ||
// the abstract syntax tree will represent the method call as its sibling | ||
// so we are checking the childCount of the parameter's parent | ||
boolean isParameterCallingMethod = currentParameter.getParent().getChildCount() > 1; | ||
int parameterPosition = i + 1; | ||
|
||
if (!isParameterCallingMethod) { | ||
isFullStackTraceExceptionFound = true; | ||
fullStackTraceExceptionPosition = parameterPosition; | ||
} else { | ||
isExceptionCallingMethodFound = true; | ||
exceptionCallingMethodPosition = parameterPosition; | ||
} | ||
} | ||
} | ||
} | ||
// Log message when the exception parameter is not set as last: | ||
// logger.trace("StringExample", e, e.getMessage()); | ||
if (isFullStackTraceExceptionFound && fullStackTraceExceptionPosition != lastParameterPosition) { | ||
log(logLineNumber, EXCEPTION_AS_LAST_PARAMETER_MSG); | ||
} | ||
|
||
if (!isFullStackTraceExceptionFound && isExceptionCallingMethodFound) { | ||
if (exceptionCallingMethodPosition == lastParameterPosition) { | ||
// Log message when the exception parameter is not logged with the full stack trace: | ||
// logger.trace("StringExample", e.getMessage()); | ||
log(logLineNumber, FULL_STACK_TRACE_MSG); | ||
} else { | ||
// Log message when the exception parameter is not logged with the full stack trace | ||
// and is not set as last: logger.trace("StringExample", e.getMessage(), value); | ||
log(logLineNumber, FULL_STACK_TRACE_EXCEPTION_AS_LAST_PARAMETER_MSG); | ||
} | ||
} | ||
} | ||
|
||
private Class<?> getExceptionClass() { | ||
for (DetailAST exceptionNode : this.exceptions) { | ||
String exception = exceptionNode.getText(); | ||
// get the package and the class name for the exception: | ||
// "java.lang.annotation.AnnotationTypeMismatchException" | ||
String fullyQualifiedExceptionName = this.imports.stream().filter(e -> e.endsWith("." + exception)) | ||
.collect(Collectors.joining()); | ||
// if the fullyQualifiedExceptionName is empty | ||
// the caught exception is from package "java.lang" which is not imported | ||
if (fullyQualifiedExceptionName.isEmpty()) { | ||
fullyQualifiedExceptionName = EXCEPTION_PACKAGE + exception; | ||
} | ||
|
||
try { | ||
Class<?> exceptionClass = Class.forName(fullyQualifiedExceptionName); | ||
return exceptionClass; | ||
} catch (ClassNotFoundException e) { | ||
String message = MessageFormat.format("Exception class {0} not found.", fullyQualifiedExceptionName); | ||
logger.info(message, e); | ||
} | ||
} | ||
return null; | ||
} | ||
|
||
private void extractCaughtExceptions(DetailAST catchAst) { | ||
DetailAST catchBlockParameters = catchAst.findFirstToken(TokenTypes.PARAMETER_DEF); | ||
DetailAST parametersType = catchBlockParameters.findFirstToken(TokenTypes.TYPE); | ||
DetailAST currentException = parametersType.getFirstChild(); | ||
|
||
this.exceptionName = catchBlockParameters.getLastChild().getText(); | ||
// if there are more than one caught exceptions: | ||
// catch (ArithmeticException | NumberFormatException e) | ||
// the current exception node will be the binary or operator "|" | ||
if (currentException.getType() != TokenTypes.IDENT) { | ||
this.exceptions = getAllChildrenNodesOfType(currentException, TokenTypes.IDENT); | ||
} else { | ||
this.exceptions = getAllChildrenNodesOfType(parametersType, TokenTypes.IDENT); | ||
} | ||
} | ||
|
||
private DetailAST getLogger(DetailAST ast) { | ||
if (ast == null) { | ||
return null; | ||
} | ||
if (LOGGER.equalsIgnoreCase(ast.getText())) { | ||
return ast; | ||
} | ||
return getLogger(ast.getFirstChild()); | ||
} | ||
|
||
/** | ||
* Find the first (direct or indirect) child node of a given type | ||
* | ||
* @param DetailAST {@link DetailAST} instance | ||
* @param type the token type to match | ||
*/ | ||
private DetailAST getFirstNode(DetailAST ast, int type) { | ||
if (ast == null) { | ||
return null; | ||
} | ||
if (ast.getType() == type) { | ||
return ast; | ||
} | ||
DetailAST nextNode = getFirstNode(ast.getFirstChild(), type); | ||
DetailAST nextSibling = getFirstNode(ast.getNextSibling(), type); | ||
// if the current node does not have a child node return its sibling | ||
return nextNode != null ? nextNode : nextSibling; | ||
} | ||
|
||
/** | ||
* Collect all direct children nodes of a given type | ||
* | ||
* @param DetailAST {@link DetailAST} instance | ||
* @param type the token type to match | ||
*/ | ||
private LinkedList<DetailAST> getAllChildrenNodesOfType(DetailAST ast, int type) { | ||
LinkedList<DetailAST> children = new LinkedList<>(); | ||
Queue<DetailAST> visitedNodes = new LinkedList<>(); | ||
visitedNodes.add(ast.getFirstChild()); | ||
|
||
while (!visitedNodes.isEmpty()) { | ||
DetailAST currentNode = visitedNodes.poll(); | ||
if (currentNode.getType() == type) { | ||
children.add(currentNode); | ||
} | ||
if (currentNode.getNextSibling() != null) { | ||
visitedNodes.add(currentNode.getNextSibling()); | ||
} | ||
} | ||
return children; | ||
} | ||
|
||
/** | ||
* Collect all direct and indirect children nodes of a given type using recursion | ||
* | ||
* @param DetailAST {@link DetailAST} instance | ||
* @param type the token type to match | ||
*/ | ||
private LinkedList<DetailAST> getAllNodesOfType(LinkedList<DetailAST> requiredNodes, DetailAST ast, int type) { | ||
if (ast == null) { | ||
return new LinkedList<>(); | ||
} | ||
if (ast.getType() == type) { | ||
requiredNodes.add(ast); | ||
} | ||
getAllNodesOfType(requiredNodes, ast.getFirstChild(), type); | ||
getAllNodesOfType(requiredNodes, ast.getNextSibling(), type); | ||
return requiredNodes; | ||
} | ||
} |
Oops, something went wrong.