IDEA-147395 (Slf4j placeholder inspection should be able to handle simple concatenation in logStringArgument)

This commit is contained in:
Bas Leijdekkers
2015-11-17 10:44:24 +01:00
parent 95bd36ecc3
commit 75dff6418f
3 changed files with 68 additions and 17 deletions
@@ -1996,8 +1996,8 @@ string.concatenation.argument.to.log.call.display.name=Non-constant string conca
string.concatenation.argument.to.log.call.problem.descriptor=Non-constant string concatenation as argument to <code>#ref()</code> logging call #loc
string.concatenation.argument.to.log.call.quickfix=Replace concatenation with parameterized log message
placeholder.count.matches.argument.count.display.name=Number of placeholders does not match number of arguments in logging call
placeholder.count.matches.argument.count.more.problem.descriptor=More arguments provided ({0}) than placeholders specified ({1}) in ''{2}'' #loc
placeholder.count.matches.argument.count.fewer.problem.descriptor=Fewer arguments provided ({0}) than placeholders specified ({1}) in ''{2}'' #loc
placeholder.count.matches.argument.count.more.problem.descriptor=More arguments provided ({0}) than placeholders specified ({1}) #loc
placeholder.count.matches.argument.count.fewer.problem.descriptor=Fewer arguments provided ({0}) than placeholders specified ({1}) #loc
assignment.to.superclass.field.display.name=Constructor assigns value to field defined in superclass
assignment.to.superclass.field.problem.descriptor=Assignment to field ''{0}'' defined in superclass ''{1}'' #loc
junit.rule.display.name=Malformed @Rule/@ClassRule field
@@ -45,14 +45,13 @@ public class PlaceholderCountMatchesArgumentCountInspection extends BaseInspecti
protected String buildErrorString(Object... infos) {
final Integer argumentCount = (Integer)infos[0];
final Integer placeholderCount = (Integer)infos[1];
final Object value = infos[2];
if (argumentCount.intValue() > placeholderCount.intValue()) {
return InspectionGadgetsBundle.message("placeholder.count.matches.argument.count.more.problem.descriptor",
argumentCount, placeholderCount, value);
argumentCount, placeholderCount);
}
else {
return InspectionGadgetsBundle.message("placeholder.count.matches.argument.count.fewer.problem.descriptor",
argumentCount, placeholderCount, value);
argumentCount, placeholderCount);
}
}
@@ -96,8 +95,7 @@ public class PlaceholderCountMatchesArgumentCountInspection extends BaseInspecti
else {
argumentCount = countArguments(arguments, 1);
}
final Object value = ExpressionUtils.computeConstantExpression(logStringArgument);
final int placeholderCount = countPlaceholders(value);
final int placeholderCount = countPlaceholders(logStringArgument);
if (placeholderCount < 0 || argumentCount < 0 || placeholderCount == argumentCount) {
return;
}
@@ -106,7 +104,7 @@ public class PlaceholderCountMatchesArgumentCountInspection extends BaseInspecti
// the exception, then the stack trace won't be logged.
return;
}
registerError(logStringArgument, Integer.valueOf(argumentCount), Integer.valueOf(placeholderCount), value);
registerError(logStringArgument, Integer.valueOf(argumentCount), Integer.valueOf(placeholderCount));
}
private static boolean hasThrowableType(PsiExpression lastArgument) {
@@ -123,11 +121,53 @@ public class PlaceholderCountMatchesArgumentCountInspection extends BaseInspecti
return InheritanceUtil.isInheritor(type, CommonClassNames.JAVA_LANG_THROWABLE);
}
public static int countPlaceholders(Object value) {
if (!(value instanceof String)) {
return -1;
public static int countPlaceholders(PsiExpression expression) {
final Object value = ExpressionUtils.computeConstantExpression(expression);
if (value == null) {
final StringBuilder builder = new StringBuilder();
return buildString(expression, builder) ? countPlaceholders(builder.toString()) : -1;
}
final String string = (String)value;
return value instanceof String ? countPlaceholders((String)value) : 0;
}
private static boolean buildString(PsiExpression expression, StringBuilder builder) {
if (expression instanceof PsiParenthesizedExpression) {
final PsiParenthesizedExpression parenthesizedExpression = (PsiParenthesizedExpression)expression;
return buildString(parenthesizedExpression.getExpression(), builder);
}
else if (expression instanceof PsiPolyadicExpression) {
if (!ExpressionUtils.hasStringType(expression)) {
return false;
}
final PsiPolyadicExpression polyadicExpression = (PsiPolyadicExpression)expression;
for (PsiExpression operand : polyadicExpression.getOperands()) {
if (!buildString(operand, builder)) {
return false;
}
}
return true;
}
else if (expression instanceof PsiLiteralExpression) {
if (ExpressionUtils.hasStringType(expression)) {
final PsiLiteralExpression literalExpression = (PsiLiteralExpression)expression;
builder.append(literalExpression.getValue());
}
return true;
}
else {
if (!ExpressionUtils.hasStringType(expression)) {
return true;
}
final Object value = ExpressionUtils.computeConstantExpression(expression);
if (value == null) {
return false;
}
builder.append(value);
return true;
}
}
private static int countPlaceholders(String string) {
int count = 0;
int index = string.indexOf("{}");
while (index >= 0) {
@@ -21,7 +21,7 @@ public class PlaceholderCountMatchesArgumentCountInspectionTest extends LightIns
"class X {" +
" void foo() {" +
" RuntimeException e = new RuntimeException();" +
" LoggerFactory.getLogger(X.class).info(/*Fewer arguments provided (0) than placeholders specified (1) in 'this: {}'*/\"this: {}\"/**/, e);" +
" LoggerFactory.getLogger(X.class).info(/*Fewer arguments provided (0) than placeholders specified (1)*/\"this: {}\"/**/, e);" +
" }" +
"}");
}
@@ -41,7 +41,7 @@ public class PlaceholderCountMatchesArgumentCountInspectionTest extends LightIns
"class X {" +
" void foo() {" +
" RuntimeException e = new RuntimeException();" +
" LoggerFactory.getLogger(X.class).info(/*Fewer arguments provided (1) than placeholders specified (3) in '1: {} {} {}'*/\"1: {} {} {}\"/**/, 1, e);" +
" LoggerFactory.getLogger(X.class).info(/*Fewer arguments provided (1) than placeholders specified (3)*/\"1: {} {} {}\"/**/, 1, e);" +
" }" +
"}");
}
@@ -62,7 +62,7 @@ public class PlaceholderCountMatchesArgumentCountInspectionTest extends LightIns
"class X {\n" +
" void foo() {\n" +
" Logger logger = LoggerFactory.getLogger(X.class);\n" +
" logger.info(/*Fewer arguments provided (1) than placeholders specified (2) in 'string {}{}'*/\"string {}{}\"/**/, 1);\n" +
" logger.info(/*Fewer arguments provided (1) than placeholders specified (2)*/\"string {}{}\"/**/, 1);\n" +
" }\n" +
"}"
);
@@ -73,7 +73,7 @@ public class PlaceholderCountMatchesArgumentCountInspectionTest extends LightIns
"class X {\n" +
" void foo() {\n" +
" Logger logger = LoggerFactory.getLogger(X.class);\n" +
" logger.info(/*More arguments provided (1) than placeholders specified (0) in 'string'*/\"string\"/**/, 1);\n" +
" logger.info(/*More arguments provided (1) than placeholders specified (0)*/\"string\"/**/, 1);\n" +
" }\n" +
"}"
);
@@ -144,7 +144,18 @@ public class PlaceholderCountMatchesArgumentCountInspectionTest extends LightIns
" Logger LOG = LoggerFactory.getLogger(X.class);" +
" private static final String message = \"HELLO {}\";" +
" void m() {" +
" LOG.info(/*Fewer arguments provided (0) than placeholders specified (1) in 'HELLO {}'*/message/**/);" +
" LOG.info(/*Fewer arguments provided (0) than placeholders specified (1)*/message/**/);" +
" }" +
"}");
}
public void testNonConstantString() {
doTest("import org.slf4j.*;" +
"class X {" +
" Logger LOG = LoggerFactory.getLogger(X.class);" +
" private static final String S = \"{}\";" +
" void m() {" +
" LOG.info(/*Fewer arguments provided (0) than placeholders specified (2)*/S +\"{}\" + Integer.class/**/);" +
" }" +
"}");
}