diff --git a/java/java-impl/src/com/intellij/codeInspection/magicConstant/MagicConstantInspection.java b/java/java-impl/src/com/intellij/codeInspection/magicConstant/MagicConstantInspection.java index 66489d182f77..4de47ff3af08 100644 --- a/java/java-impl/src/com/intellij/codeInspection/magicConstant/MagicConstantInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/magicConstant/MagicConstantInspection.java @@ -35,6 +35,7 @@ import com.intellij.openapi.roots.JdkOrderEntry; import com.intellij.openapi.roots.OrderEntry; import com.intellij.openapi.roots.ProjectRootManager; import com.intellij.openapi.util.Comparing; +import com.intellij.openapi.util.Key; import com.intellij.openapi.util.io.FileUtil; import com.intellij.openapi.util.text.StringUtil; import com.intellij.openapi.vfs.LocalFileSystem; @@ -50,6 +51,7 @@ import com.intellij.psi.util.*; import com.intellij.slicer.*; import com.intellij.util.Function; import com.intellij.util.Processor; +import com.intellij.util.containers.ConcurrentSoftValueHashMap; import gnu.trove.THashSet; import org.intellij.lang.annotations.MagicConstant; import org.jetbrains.annotations.Nls; @@ -284,6 +286,20 @@ public class MagicConstantInspection extends LocalInspectionTool { result = 31 * result + (canBeOred ? 1 : 0); return result; } + + public boolean isSubsetOf(@NotNull AllowedValues other, @NotNull PsiManager manager) { + for (PsiAnnotationMemberValue value : values) { + boolean found = false; + for (PsiAnnotationMemberValue otherValue : other.values) { + if (same(value, otherValue, manager)) { + found = true; + break; + } + } + if (!found) return false; + } + return true; + } } private static AllowedValues getAllowedValuesFromMagic(@NotNull PsiModifierListOwner element, @@ -480,7 +496,7 @@ public class MagicConstantInspection extends LocalInspectionTool { return value.getText(); } }, ", "); - holder.registerProblem(argument, "Must be one of the: "+ values); + holder.registerProblem(argument, "Must be one of: "+ values); } private static boolean isAllowed(@NotNull final PsiElement scope, @@ -499,9 +515,9 @@ public class MagicConstantInspection extends LocalInspectionTool { } private static boolean isGoodExpression(PsiExpression expression, - AllowedValues allowedValues, - PsiElement scope, - PsiManager manager) { + @NotNull AllowedValues allowedValues, + @NotNull PsiElement scope, + @NotNull PsiManager manager) { expression = PsiUtil.deparenthesizeExpression(expression); if (expression == null) return true; if (expression instanceof PsiConditionalExpression) { @@ -515,18 +531,24 @@ public class MagicConstantInspection extends LocalInspectionTool { if (isOneOf(expression, allowedValues, manager)) return true; if (allowedValues.canBeOred) { - if (expression instanceof PsiPolyadicExpression && - JavaTokenType.OR.equals(((PsiPolyadicExpression)expression).getOperationTokenType())) { - for (PsiExpression operand : ((PsiPolyadicExpression)expression).getOperands()) { - if (!isAllowed(scope, operand, allowedValues, manager)) return false; - } - return true; - } - //todo & ~(CONST | CONST) - PsiExpression zero = JavaPsiFacade.getElementFactory(manager.getProject()).createExpressionFromText("0", expression); + PsiExpression zero = getLiteralExpression(expression, manager, "0"); if (same(expression, zero, manager)) return true; - PsiExpression mOne = JavaPsiFacade.getElementFactory(manager.getProject()).createExpressionFromText("-1", expression); + PsiExpression mOne = getLiteralExpression(expression, manager, "-1"); if (same(expression, mOne, manager)) return true; + if (expression instanceof PsiPolyadicExpression) { + IElementType tokenType = ((PsiPolyadicExpression)expression).getOperationTokenType(); + if (JavaTokenType.OR.equals(tokenType) || JavaTokenType.AND.equals(tokenType)) { + for (PsiExpression operand : ((PsiPolyadicExpression)expression).getOperands()) { + if (!isAllowed(scope, operand, allowedValues, manager)) return false; + } + return true; + } + } + if (expression instanceof PsiPrefixExpression && + JavaTokenType.TILDE.equals(((PsiPrefixExpression)expression).getOperationTokenType())) { + PsiExpression operand = ((PsiPrefixExpression)expression).getOperand(); + return operand == null || isAllowed(scope, operand, allowedValues, manager); + } } PsiElement resolved = null; @@ -540,11 +562,26 @@ public class MagicConstantInspection extends LocalInspectionTool { AllowedValues allowedForRef; if (resolved instanceof PsiModifierListOwner && (allowedForRef = getAllowedValues((PsiModifierListOwner)resolved, getType((PsiModifierListOwner)resolved), null)) != null && - Comparing.equal(allowedValues, allowedForRef)) return true; + allowedForRef.isSubsetOf(allowedValues, manager)) return true; return PsiType.NULL.equals(expression.getType()); } + private static Key> LITERAL_EXPRESSION_CACHE = Key.create("LITERAL_EXPRESSION_CACHE"); + private static PsiExpression getLiteralExpression(PsiExpression context, PsiManager manager, @NotNull String text) { + Map cache = LITERAL_EXPRESSION_CACHE.get(manager); + if (cache == null) { + cache = new ConcurrentSoftValueHashMap(); + cache = manager.putUserDataIfAbsent(LITERAL_EXPRESSION_CACHE, cache); + } + PsiExpression expression = cache.get(text); + if (expression == null) { + expression = JavaPsiFacade.getElementFactory(manager.getProject()).createExpressionFromText(text, context); + cache.put(text, expression); + } + return expression; + } + private static boolean isOneOf(@NotNull PsiExpression expression, @NotNull AllowedValues allowedValues, @NotNull PsiManager manager) { for (PsiAnnotationMemberValue allowedValue : allowedValues.values) { if (same(allowedValue, expression, manager)) return true; diff --git a/java/java-tests/testData/inspection/magic/simple/expected.xml b/java/java-tests/testData/inspection/magic/simple/expected.xml index 394de9a2ab2f..e247be988b90 100644 --- a/java/java-tests/testData/inspection/magic/simple/expected.xml +++ b/java/java-tests/testData/inspection/magic/simple/expected.xml @@ -14,7 +14,7 @@ magic constant - Must be one of the: Const.X, Const.Y, Const.Z + Must be one of: Const.X, Const.Y, Const.Z @@ -26,7 +26,7 @@ magic constant - Must be one of the: Const.X, Const.Y, Const.Z + Must be one of: Const.X, Const.Y, Const.Z @@ -38,7 +38,7 @@ magic constant - Must be one of the: Const.X, Const.Y, Const.Z + Must be one of: Const.X, Const.Y, Const.Z @@ -50,7 +50,7 @@ magic constant - Must be one of the: Const.X, Const.Y, Const.Z + Must be one of: Const.X, Const.Y, Const.Z @@ -62,7 +62,7 @@ magic constant - Must be one of the: Const.X, Const.Y, Const.Z + Must be one of: Const.X, Const.Y, Const.Z @@ -74,7 +74,7 @@ magic constant - Must be one of the: Const.X, Const.Y, Const.Z + Must be one of: Const.X, Const.Y, Const.Z @@ -86,7 +86,7 @@ magic constant - Must be one of the: Const.X, Const.Y, Const.Z + Must be one of: Const.X, Const.Y, Const.Z @@ -98,7 +98,7 @@ magic constant - Must be one of the: Const.X, Const.Y, Const.Z + Must be one of: Const.X, Const.Y, Const.Z @@ -110,7 +110,7 @@ magic constant - Must be one of the: Const.X, Const.Y, Const.Z + Must be one of: Const.X, Const.Y, Const.Z @@ -122,7 +122,7 @@ magic constant - Must be one of the: Const.X, Const.Y, Const.Z + Must be one of: Const.X, Const.Y, Const.Z @@ -134,7 +134,7 @@ magic constant - Must be one of the: Const.X, Const.Y, Const.Z + Must be one of: Const.X, Const.Y, Const.Z @@ -146,7 +146,7 @@ magic constant - Must be one of the: Const.X, Const.Y, Const.Z + Must be one of: Const.X, Const.Y, Const.Z @@ -158,7 +158,7 @@ magic constant - Must be one of the: Const.X, Const.Y, Const.Z + Must be one of: Const.X, Const.Y, Const.Z @@ -170,7 +170,7 @@ magic constant - Must be one of the: Const.X, Const.Y, Const.Z + Must be one of: Const.X, Const.Y, Const.Z @@ -178,7 +178,7 @@ X.java 81 magic constant - Must be one of the: Const.X, Const.Y, Const.Z + Must be one of: Const.X, Const.Y, Const.Z @@ -189,7 +189,7 @@ magic constant - Must be one of the: Const.X, Const.Y, Const.Z + Must be one of: Const.X, Const.Y, Const.Z @@ -201,7 +201,7 @@ magic constant - Must be one of the: Const.X, Const.Y, Const.Z + Must be one of: Const.X, Const.Y, Const.Z @@ -213,7 +213,7 @@ magic constant - Must be one of the: Const.X, Const.Y, Const.Z + Must be one of: Const.X, Const.Y, Const.Z @@ -225,7 +225,7 @@ magic constant - Must be one of the: Const.X, Const.Y, Const.Z + Must be one of: Const.X, Const.Y, Const.Z @@ -237,7 +237,7 @@ magic constant - Must be one of the: Const.X, Const.Y, Const.Z + Must be one of: Const.X, Const.Y, Const.Z @@ -249,7 +249,7 @@ magic constant - Must be one of the: Const.X, Const.Y, Const.Z + Must be one of: Const.X, Const.Y, Const.Z @@ -257,14 +257,14 @@ X.java 118 magic constant - Must be one of the: Const.X, Const.Y, Const.Z + Must be one of: Const.X, Const.Y, Const.Z X.java 119 magic constant - Must be one of the: Const.X, Const.Y, Const.Z + Must be one of: Const.X, Const.Y, Const.Z @@ -276,7 +276,7 @@ magic constant - Must be one of the: Const.X, Const.Y, Const.Z + Must be one of: Const.X, Const.Y, Const.Z @@ -288,7 +288,7 @@ magic constant - Must be one of the: Const.X, Const.Y, Const.Z + Must be one of: Const.X, Const.Y, Const.Z @@ -300,7 +300,7 @@ magic constant - Must be one of the: Const.X, Const.Y, Const.Z + Must be one of: Const.X, Const.Y, Const.Z @@ -312,7 +312,7 @@ magic constant - Must be one of the: Const.X, Const.Y, Const.Z + Must be one of: Const.X, Const.Y, Const.Z @@ -322,7 +322,7 @@ X.java 173 magic constant - Must be one of the: Const.X, Const.Y, Const.Z + Must be one of: Const.X, Const.Y, Const.Z @@ -331,7 +331,7 @@ X.java 174 magic constant - Must be one of the: Const.X, Const.Y, Const.Z + Must be one of: Const.X, Const.Y, Const.Z @@ -340,7 +340,7 @@ X.java 175 magic constant - Must be one of the: Const.X, Const.Y, Const.Z + Must be one of: Const.X, Const.Y, Const.Z @@ -349,7 +349,7 @@ X.java 177 magic constant - Must be one of the: Const.X, Const.Y, Const.Z + Must be one of: Const.X, Const.Y, Const.Z @@ -357,7 +357,7 @@ X.java 178 magic constant - Must be one of the: Const.X, Const.Y, Const.Z + Must be one of: Const.X, Const.Y, Const.Z @@ -366,7 +366,7 @@ X.java 179 magic constant - Must be one of the: Const.X, Const.Y, Const.Z + Must be one of: Const.X, Const.Y, Const.Z @@ -375,7 +375,7 @@ X.java 180 magic constant - Must be one of the: Const.X, Const.Y, Const.Z + Must be one of: Const.X, Const.Y, Const.Z @@ -383,7 +383,7 @@ X.java 193 magic constant - Must be one of the: Const.X, Const.Y + Must be one of: Const.X, Const.Y @@ -391,14 +391,14 @@ X.java 195 magic constant - Must be one of the: Const.X, Const.Y + Must be one of: Const.X, Const.Y X.java 227 magic constant - Must be one of the: Const.X, Const.Y + Must be one of: Const.X, Const.Y @@ -406,7 +406,7 @@ X.java 228 magic constant - Must be one of the: Const.X, Const.Y + Must be one of: Const.X, Const.Y @@ -414,7 +414,7 @@ X.java 229 magic constant - Must be one of the: Const.X, Const.Y + Must be one of: Const.X, Const.Y @@ -422,28 +422,28 @@ X.java 230 magic constant - Must be one of the: Const.X, Const.Y + Must be one of: Const.X, Const.Y X.java 231 magic constant - Must be one of the: Const.X, Const.Y + Must be one of: Const.X, Const.Y X.java 238 Magic Constant - Must be one of the: Calendar.JANUARY, Calendar.FEBRUARY, Calendar.MARCH, Calendar.APRIL, Calendar.MAY, Calendar.JUNE, Calendar.JULY, Calendar.AUGUST, Calendar.SEPTEMBER, Calendar.OCTOBER, Calendar.NOVEMBER, Calendar.DECEMBER + Must be one of: Calendar.JANUARY, Calendar.FEBRUARY, Calendar.MARCH, Calendar.APRIL, Calendar.MAY, Calendar.JUNE, Calendar.JULY, Calendar.AUGUST, Calendar.SEPTEMBER, Calendar.OCTOBER, Calendar.NOVEMBER, Calendar.DECEMBER X.java 239 Magic Constant - Must be one of the: SwingConstants.LEFT, SwingConstants.CENTER, SwingConstants.RIGHT, SwingConstants.LEADING, SwingConstants.TRAILING + Must be one of: SwingConstants.LEFT, SwingConstants.CENTER, SwingConstants.RIGHT, SwingConstants.LEADING, SwingConstants.TRAILING diff --git a/java/java-tests/testData/inspection/magic/simple/src/X.java b/java/java-tests/testData/inspection/magic/simple/src/X.java index b7cee434f219..84c3ea63651f 100644 --- a/java/java-tests/testData/inspection/magic/simple/src/X.java +++ b/java/java-tests/testData/inspection/magic/simple/src/X.java @@ -103,10 +103,10 @@ public class X { if (x == Const.X) { x = Const.Y; assert x != Const.Z; - f |= Const.Y; + f |= Const.Y; f &= Const.X & ~(Const.Z | Const.X); } else { - f |= Const.X; + f |= Const.X; f = f & ~(Const.X | Const.X); } f3(f);