From 8ce22dab616988bcffded916066c834ab312eb7f Mon Sep 17 00:00:00 2001 From: Bas Leijdekkers Date: Thu, 30 Nov 2017 15:05:27 +0100 Subject: [PATCH] Let "Invalid property key" inspection warn in more cases --- .../i18n/InvalidPropertyKeyInspection.java | 86 +++++++++++++------ .../invalidPropertyKey/simple/expected.xml | 18 ++++ .../invalidPropertyKey/simple/src/x/Test.java | 18 +++- 3 files changed, 96 insertions(+), 26 deletions(-) diff --git a/plugins/java-i18n/src/com/intellij/codeInspection/i18n/InvalidPropertyKeyInspection.java b/plugins/java-i18n/src/com/intellij/codeInspection/i18n/InvalidPropertyKeyInspection.java index c223d812b934..14971b9b14f3 100644 --- a/plugins/java-i18n/src/com/intellij/codeInspection/i18n/InvalidPropertyKeyInspection.java +++ b/plugins/java-i18n/src/com/intellij/codeInspection/i18n/InvalidPropertyKeyInspection.java @@ -17,7 +17,12 @@ import com.intellij.openapi.roots.ProjectRootManager; import com.intellij.openapi.util.Comparing; import com.intellij.openapi.util.Ref; import com.intellij.psi.*; +import com.intellij.psi.controlFlow.DefUseUtil; +import com.intellij.psi.util.PsiTreeUtil; +import com.intellij.psi.util.PsiUtil; +import com.intellij.util.SmartList; import com.intellij.util.containers.ContainerUtil; +import com.siyeh.ig.psiutils.ExpressionUtils; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -68,7 +73,7 @@ public class InvalidPropertyKeyInspection extends AbstractBaseJavaLocalInspectio @Nullable public ProblemDescriptor[] checkClass(@NotNull PsiClass aClass, @NotNull InspectionManager manager, boolean isOnTheFly) { final PsiClassInitializer[] initializers = aClass.getInitializers(); - List result = new ArrayList<>(); + List result = new SmartList<>(); for (PsiClassInitializer initializer : initializers) { final ProblemDescriptor[] descriptors = checkElement(initializer, manager, isOnTheFly); if (descriptors != null) { @@ -82,7 +87,7 @@ public class InvalidPropertyKeyInspection extends AbstractBaseJavaLocalInspectio @Override @Nullable public ProblemDescriptor[] checkField(@NotNull PsiField field, @NotNull InspectionManager manager, boolean isOnTheFly) { - List result = new ArrayList<>(); + List result = new SmartList<>(); appendProblems(manager, isOnTheFly, result, field.getInitializer()); appendProblems(manager, isOnTheFly, result, field.getModifierList()); if (field instanceof PsiEnumConstant) { @@ -110,7 +115,7 @@ public class InvalidPropertyKeyInspection extends AbstractBaseJavaLocalInspectio private static class UnresolvedPropertyVisitor extends JavaRecursiveElementWalkingVisitor { private final InspectionManager myManager; - private final List myProblems = new ArrayList<>(); + private final List myProblems = new SmartList<>(); private final boolean onTheFly; @@ -142,31 +147,65 @@ public class InvalidPropertyKeyInspection extends AbstractBaseJavaLocalInspectio return; } final PsiElement resolvedExpression = expression.resolve(); - if (!(resolvedExpression instanceof PsiField)) { - return; + if (resolvedExpression instanceof PsiField) { + final PsiField field = (PsiField)resolvedExpression; + if (!field.hasModifierProperty(PsiModifier.FINAL)) { + return; + } + final PsiExpression initializer = field.getInitializer(); + String key = computeStringValue(initializer); + visitPropertyKeyAnnotationParameter(expression, key, + (field.getContainingFile() == expression.getContainingFile()) ? initializer : expression); } - final PsiField field = (PsiField) resolvedExpression; - PsiExpression initializer; - if ((initializer = field.getInitializer()) == null || !(initializer instanceof PsiLiteralExpression)) { - return; + else if (resolvedExpression instanceof PsiLocalVariable) { + checkLocalVariable((PsiLocalVariable)resolvedExpression, expression); } - if (!field.hasModifierProperty(PsiModifier.FINAL)) { - return; - } - final Object initializerValue = ((PsiLiteralExpression)initializer).getValue(); - if (!(initializerValue instanceof String)) { - return; - } - String key = (String)initializerValue; - visitPropertyKeyAnnotationParameter(expression, key); } - private void visitPropertyKeyAnnotationParameter(PsiExpression expression, String key) { + private void checkLocalVariable(PsiLocalVariable variable, PsiReferenceExpression expression) { + PsiCodeBlock block = PsiTreeUtil.getParentOfType(variable, PsiCodeBlock.class); + final PsiElement[] defs = DefUseUtil.getDefs(block, variable, expression); + for (PsiElement def : defs) { + if(def instanceof PsiLocalVariable) { + final PsiExpression initializer = PsiUtil.deparenthesizeExpression(((PsiLocalVariable)def).getInitializer()); + visitPropertyKeyAnnotationParameter(expression, computeStringValue(initializer), initializer); + } + else if (def instanceof PsiReferenceExpression) { + final PsiAssignmentExpression assignment = ExpressionUtils.getAssignment(def.getParent()); + if (assignment != null && assignment.getLExpression() == def) { + final PsiExpression rhs = PsiUtil.deparenthesizeExpression(assignment.getRExpression()); + if (rhs instanceof PsiConditionalExpression) { + final PsiConditionalExpression conditionalExpression = (PsiConditionalExpression)rhs; + final PsiExpression thenExpression = conditionalExpression.getThenExpression(); + final PsiExpression elseExpression = conditionalExpression.getElseExpression(); + visitPropertyKeyAnnotationParameter(expression, computeStringValue(thenExpression), thenExpression); + visitPropertyKeyAnnotationParameter(expression, computeStringValue(elseExpression), elseExpression); + } + else { + visitPropertyKeyAnnotationParameter(expression, computeStringValue(rhs), rhs); + } + } + } + } + } + + private static String computeStringValue(PsiExpression expression) { + if (expression instanceof PsiLiteralExpression) { + final Object value = ((PsiLiteralExpression)expression).getValue(); + if (value instanceof String) { + return (String)value; + } + } + return null; + } + + private void visitPropertyKeyAnnotationParameter(PsiExpression expression, String key, PsiExpression highlightedExpression) { + if (key == null) return; Ref resourceBundleName = new Ref<>(); if (!JavaI18nUtil.isValidPropertyReference(myManager.getProject(), expression, key, resourceBundleName)) { String bundleName = resourceBundleName.get(); if (bundleName != null) { // can be null if we were unable to resolve literal expression, e.g. when JDK was not set - appendPropertyKeyNotFoundProblem(bundleName, key, expression, myManager, myProblems, onTheFly); + appendPropertyKeyNotFoundProblem(bundleName, key, highlightedExpression, myManager, myProblems, onTheFly); } } else if (expression.getParent() instanceof PsiNameValuePair) { @@ -192,7 +231,7 @@ public class InvalidPropertyKeyInspection extends AbstractBaseJavaLocalInspectio annotationParams.put(AnnotationUtil.PROPERTY_KEY_RESOURCE_BUNDLE_PARAMETER, null); if (!JavaI18nUtil.mustBePropertyKey(expression, annotationParams)) return; - final SortedSet paramsCount = JavaI18nUtil.getPropertyValueParamsCount(expression, resourceBundleName.get()); + final SortedSet paramsCount = JavaI18nUtil.getPropertyValueParamsCount(highlightedExpression, resourceBundleName.get()); if (paramsCount.isEmpty() || (paramsCount.size() != 1 && resourceBundleName.get() == null)) { return; } @@ -223,11 +262,8 @@ public class InvalidPropertyKeyInspection extends AbstractBaseJavaLocalInspectio @Override public void visitLiteralExpression(PsiLiteralExpression expression) { - Object value = expression.getValue(); - if (!(value instanceof String)) return; - String key = (String)value; if (isComputedPropertyExpression(expression)) return; - visitPropertyKeyAnnotationParameter(expression, key); + visitPropertyKeyAnnotationParameter(expression, computeStringValue(expression), expression); } private static void appendPropertyKeyNotFoundProblem(@NotNull String bundleName, diff --git a/plugins/java-i18n/testData/inspections/invalidPropertyKey/simple/expected.xml b/plugins/java-i18n/testData/inspections/invalidPropertyKey/simple/expected.xml index 277010b64aac..94ccaffa897e 100644 --- a/plugins/java-i18n/testData/inspections/invalidPropertyKey/simple/expected.xml +++ b/plugins/java-i18n/testData/inspections/invalidPropertyKey/simple/expected.xml @@ -51,4 +51,22 @@ Invalid property key String literal '.params' doesn't appear to be valid property key + + Test.java + 31 + Invalid property key + String literal 'invalid' doesn't appear to be valid property key + + + Test.java + 41 + Invalid property key + String literal 'invalid' doesn't appear to be valid property key + + + Test.java + 42 + Invalid property key + String literal 'invalid' doesn't appear to be valid property key + diff --git a/plugins/java-i18n/testData/inspections/invalidPropertyKey/simple/src/x/Test.java b/plugins/java-i18n/testData/inspections/invalidPropertyKey/simple/src/x/Test.java index 522d04ed85f4..937061c4b9f7 100644 --- a/plugins/java-i18n/testData/inspections/invalidPropertyKey/simple/src/x/Test.java +++ b/plugins/java-i18n/testData/inspections/invalidPropertyKey/simple/src/x/Test.java @@ -20,11 +20,27 @@ class Test { String f1(@PropertyKey(resourceBundle = IBundle.BUNDLE) String s, Object...params) {return "";} String f2(@PropertyKey(resourceBundle = IBundle.BUNDLE) String s) {return "";} - void f3(@PropertyKey(resourceBundle = "invalid") String s) { + void f3(@PropertyKey(resourceBundle = "invalid") String s, int i) { IBundle.message(s + ".params"); IBundle.message(s + CONST); IBundle.message(s == null ? CONST : "invalid"); IBundle.message(((("invalid")))); IBundle.message((((CONST)))); + String pattern; + if (i == 0) { + pattern = "invalid"; + } + else if (i == 1) { + pattern = s + ".params"; + } + else if (i == 2) { + pattern = "defaultKey"; + } + else { + pattern = i > 10 + ? "invalid" + : "invalid"; + } + f2(pattern); } }