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 15c64e2d345f..0a666ff45051 100644 --- a/java/java-impl/src/com/intellij/codeInspection/magicConstant/MagicConstantInspection.java +++ b/java/java-impl/src/com/intellij/codeInspection/magicConstant/MagicConstantInspection.java @@ -46,6 +46,7 @@ import com.intellij.psi.tree.IElementType; import com.intellij.psi.util.*; import com.intellij.slicer.*; import com.intellij.util.ArrayUtil; +import com.intellij.util.IncorrectOperationException; import com.intellij.util.Processor; import com.intellij.util.containers.ContainerUtil; import com.intellij.util.indexing.FileBasedIndex; @@ -331,29 +332,37 @@ public class MagicConstantInspection extends BaseJavaLocalInspectionTool { if (TypeConversionUtil.getTypeRank(type) <= TypeConversionUtil.LONG_RANK) { PsiAnnotationMemberValue intValues = magic.findAttributeValue("intValues"); if (intValues instanceof PsiArrayInitializerMemberValue) { - allowedValues = ((PsiArrayInitializerMemberValue)intValues).getInitializers(); - values = true; + final PsiAnnotationMemberValue[] initializers = ((PsiArrayInitializerMemberValue)intValues).getInitializers(); + if (initializers.length != 0) { + allowedValues = initializers; + values = true; + } } - else { + if (!values) { PsiAnnotationMemberValue orValue = magic.findAttributeValue("flags"); if (orValue instanceof PsiArrayInitializerMemberValue) { - allowedValues = ((PsiArrayInitializerMemberValue)orValue).getInitializers(); - flags = true; + final PsiAnnotationMemberValue[] initializers = ((PsiArrayInitializerMemberValue)orValue).getInitializers(); + if (initializers.length != 0) { + allowedValues = initializers; + flags = true; + } } } } else if (type.equals(PsiType.getJavaLangString(manager, GlobalSearchScope.allScope(manager.getProject())))) { PsiAnnotationMemberValue strValuesAttr = magic.findAttributeValue("stringValues"); if (strValuesAttr instanceof PsiArrayInitializerMemberValue) { - allowedValues = ((PsiArrayInitializerMemberValue)strValuesAttr).getInitializers(); - values = true; + final PsiAnnotationMemberValue[] initializers = ((PsiArrayInitializerMemberValue)strValuesAttr).getInitializers(); + if (initializers.length != 0) { + allowedValues = initializers; + values = true; + } } } else { return null; //other types not supported } - // Also there're could be valuesFromClass of flagsFromClass PsiAnnotationMemberValue[] valuesFromClass = readFromClass("valuesFromClass", magic, type, manager); if (valuesFromClass != null) { allowedValues = ArrayUtil.mergeArrays(allowedValues, valuesFromClass, PsiAnnotationMemberValue.ARRAY_FACTORY); @@ -368,7 +377,8 @@ public class MagicConstantInspection extends BaseJavaLocalInspectionTool { return null; } if (values && flags) { - // Combination of 'flags' and 'values', that's weird TODO: Log? + throw new IncorrectOperationException( + "Misconfiguration of @MagicConstant annotation: 'flags' and 'values' shouldn't be used at the same time"); } return new AllowedValues(allowedValues, flags); } diff --git a/java/java-tests/testData/inspection/magic/manyConstantSources/expected.xml b/java/java-tests/testData/inspection/magic/manyConstantSources/expected.xml new file mode 100644 index 000000000000..acae8fed90c2 --- /dev/null +++ b/java/java-tests/testData/inspection/magic/manyConstantSources/expected.xml @@ -0,0 +1,79 @@ + + + + X.java + 31 + magic constant + Should be one of: Const1.X, Const2.I + + + X.java + 32 + magic constant + Should be one of: Const1.X, Const2.I + + + X.java + 33 + magic constant + Should be one of: Const1.X, Const2.I + + + + X.java + 44 + magic constant + Should be one of: Const2.I, Const1.X + + + X.java + 45 + magic constant + Should be one of: Const2.I, Const1.X + + + X.java + 46 + magic constant + Should be one of: Const2.I, Const1.X + + + + X.java + 57 + magic constant + Should be one of: Const1.X, Const2.I or their combination + + + X.java + 58 + magic constant + Should be one of: Const1.X, Const2.I or their combination + + + X.java + 59 + magic constant + Should be one of: Const1.X, Const2.I + + + X.java + 61 + magic constant + Should be one of: Const1.X, Const2.I or their combination + + + + X.java + 72 + magic constant + Should be one of: Const2.I, Const1.X or their combination + + + X.java + 73 + magic constant + Should be one of: Const2.I, Const1.X or their combination + + + diff --git a/java/java-tests/testData/inspection/magic/manyConstantSources/src/X.java b/java/java-tests/testData/inspection/magic/manyConstantSources/src/X.java new file mode 100644 index 000000000000..950683a5e24d --- /dev/null +++ b/java/java-tests/testData/inspection/magic/manyConstantSources/src/X.java @@ -0,0 +1,82 @@ +/* + * Copyright 2000-2011 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import org.intellij.lang.annotations.MagicConstant; + +class Const1 { + public static final int X = 1; +} + +class Const2 { + public static final int I = 4; +} + +public class X { + + void f(@MagicConstant(intValues = {Const1.X, Const2.I}) int x) { + /////////// BAD + f(0); + f(1); + f(Const1.X | Const2.I); + + ////////////// GOOD + f(Const1.X); + f(Const2.I); + + f2(x); + } + + void f2(@MagicConstant(valuesFromClass = Const1.class, intValues = {Const2.I}) int x) { + /////////// BAD + f2(0); + f2(1); + f2(Const1.X | Const2.I); + + ////////////// GOOD + f2(Const1.X); + f2(Const2.I); + + f(x); + } + + void f3(@MagicConstant(flags = {Const1.X, Const2.I}) int x) { + /////////// BAD + f3(2); + f3(1); + f(Const1.X | Const2.I); + int i = Const1.X | 4; + f3(i); + + ////////////// GOOD + f3(Const1.X); + f3(Const2.I); + + f4(x); + } + + void f4(@MagicConstant(flagsFromClass = Const1.class, flags = {Const2.I}) int x) { + /////////// BAD + f4(-3); + f4(1); + + ////////////// GOOD + f4(Const1.X); + f4(Const2.I); + f4(Const1.X | Const2.I); + + f3(x); + } +} diff --git a/java/java-tests/testData/inspection/magic/simple/expected.xml b/java/java-tests/testData/inspection/magic/simple/expected.xml index b54c2f926aab..062b3aa7245c 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 - Should be one of: Const2.I, Const.X, Const.Y, Const.Z + Should be one of: Const.X, Const.Y, Const.Z @@ -26,7 +26,7 @@ magic constant - Should be one of: Const2.I, Const.X, Const.Y, Const.Z + Should be one of: Const.X, Const.Y, Const.Z @@ -38,7 +38,7 @@ magic constant - Should be one of: Const2.I, Const.X, Const.Y, Const.Z + Should be one of: Const.X, Const.Y, Const.Z @@ -50,7 +50,7 @@ magic constant - Should be one of: Const2.I, Const.X, Const.Y, Const.Z + Should be one of: Const.X, Const.Y, Const.Z @@ -62,7 +62,7 @@ magic constant - Should be one of: Const2.I, Const.X, Const.Y, Const.Z + Should be one of: Const.X, Const.Y, Const.Z @@ -74,7 +74,7 @@ magic constant - Should be one of: Const2.I, Const.X, Const.Y, Const.Z + Should be one of: Const.X, Const.Y, Const.Z @@ -86,7 +86,7 @@ magic constant - Should be one of: Const2.I, Const.X, Const.Y, Const.Z + Should be one of: Const.X, Const.Y, Const.Z @@ -98,7 +98,7 @@ magic constant - Should be one of: Const2.I, Const.X, Const.Y, Const.Z + Should be one of: Const.X, Const.Y, Const.Z @@ -110,7 +110,7 @@ magic constant - Should be one of: Const2.I, Const.X, Const.Y, Const.Z + Should be one of: Const.X, Const.Y, Const.Z @@ -122,7 +122,7 @@ magic constant - Should be one of: Const2.I, Const.X, Const.Y, Const.Z + Should be one of: Const.X, Const.Y, Const.Z @@ -134,7 +134,7 @@ magic constant - Should be one of: Const2.I, Const.X, Const.Y, Const.Z + Should be one of: Const.X, Const.Y, Const.Z @@ -146,7 +146,7 @@ magic constant - Should be one of: Const2.I, Const.X, Const.Y, Const.Z + Should be one of: Const.X, Const.Y, Const.Z @@ -158,7 +158,7 @@ magic constant - Should be one of: Const2.I, Const.X, Const.Y, Const.Z + Should be one of: Const.X, Const.Y, Const.Z @@ -170,7 +170,7 @@ magic constant - Should be one of: Const2.I, Const.X, Const.Y, Const.Z + Should be one of: Const.X, Const.Y, Const.Z @@ -178,7 +178,7 @@ X.java 81 magic constant - Should be one of: Const2.I, Const.X, Const.Y, Const.Z or their combination + Should be one of: Const.X, Const.Y, Const.Z or their combination @@ -189,7 +189,7 @@ magic constant - Should be one of: Const2.I, Const.X, Const.Y, Const.Z or their combination + Should be one of: Const.X, Const.Y, Const.Z or their combination @@ -201,7 +201,7 @@ magic constant - Should be one of: Const2.I, Const.X, Const.Y, Const.Z + Should be one of: Const.X, Const.Y, Const.Z @@ -213,7 +213,7 @@ magic constant - Should be one of: Const2.I, Const.X, Const.Y, Const.Z or their combination + Should be one of: Const.X, Const.Y, Const.Z or their combination @@ -225,7 +225,7 @@ magic constant - Should be one of: Const2.I, Const.X, Const.Y, Const.Z or their combination + Should be one of: Const.X, Const.Y, Const.Z or their combination @@ -237,7 +237,7 @@ magic constant - Should be one of: Const2.I, Const.X, Const.Y, Const.Z or their combination + Should be one of: Const.X, Const.Y, Const.Z or their combination @@ -249,7 +249,7 @@ magic constant - Should be one of: Const2.I, Const.X, Const.Y, Const.Z or their combination + Should be one of: Const.X, Const.Y, Const.Z or their combination @@ -257,14 +257,14 @@ X.java 118 magic constant - Should be one of: Const2.I, Const.X, Const.Y, Const.Z or their combination + Should be one of: Const.X, Const.Y, Const.Z or their combination X.java 119 magic constant - Should be one of: Const2.I, Const.X, Const.Y, Const.Z or their combination + Should be one of: Const.X, Const.Y, Const.Z or their combination @@ -276,7 +276,7 @@ magic constant - Should be one of: Const2.I, Const.X, Const.Y, Const.Z or their combination + Should be one of: Const.X, Const.Y, Const.Z or their combination @@ -288,7 +288,7 @@ magic constant - Should be one of: Const2.I, Const.X, Const.Y, Const.Z or their combination + Should be one of: Const.X, Const.Y, Const.Z or their combination @@ -300,7 +300,7 @@ magic constant - Should be one of: Const2.I, Const.X, Const.Y, Const.Z or their combination + Should be one of: Const.X, Const.Y, Const.Z or their combination @@ -312,7 +312,7 @@ magic constant - Should be one of: Const2.I, Const.X, Const.Y, Const.Z or their combination + Should be one of: Const.X, Const.Y, Const.Z or their combination @@ -322,7 +322,7 @@ X.java 173 magic constant - Should be one of: Const2.I, Const.X, Const.Y, Const.Z + Should be one of: Const.X, Const.Y, Const.Z @@ -331,7 +331,7 @@ X.java 174 magic constant - Should be one of: Const2.I, Const.X, Const.Y, Const.Z + Should be one of: Const.X, Const.Y, Const.Z @@ -340,7 +340,7 @@ X.java 175 magic constant - Should be one of: Const2.I, Const.X, Const.Y, Const.Z + Should be one of: Const.X, Const.Y, Const.Z @@ -349,7 +349,7 @@ X.java 177 magic constant - Should be one of: Const2.I, Const.X, Const.Y, Const.Z + Should be one of: Const.X, Const.Y, Const.Z @@ -357,7 +357,7 @@ X.java 178 magic constant - Should be one of: Const2.I, Const.X, Const.Y, Const.Z + Should be one of: Const.X, Const.Y, Const.Z @@ -366,7 +366,7 @@ X.java 179 magic constant - Should be one of: Const2.I, Const.X, Const.Y, Const.Z + Should be one of: Const.X, Const.Y, Const.Z @@ -375,7 +375,7 @@ X.java 180 magic constant - Should be one of: Const2.I, Const.X, Const.Y, Const.Z + Should be one of: Const.X, Const.Y, Const.Z 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 21a95e66a51d..043a4037e241 100644 --- a/java/java-tests/testData/inspection/magic/simple/src/X.java +++ b/java/java-tests/testData/inspection/magic/simple/src/X.java @@ -24,7 +24,7 @@ class Const { } public class X { - void f(@MagicConstant(intValues={Const.X, Const.Y, Const.Z, Const2.I}) int x) { + void f(@MagicConstant(intValues={Const.X, Const.Y, Const.Z}) int x) { /////////// BAD f(0); f(1); @@ -50,7 +50,7 @@ public class X { f2(x); } - void f2(@MagicConstant(valuesFromClass =Const.class, intValues={Const2.I}) int x) { + void f2(@MagicConstant(valuesFromClass =Const.class) int x) { /////////// BAD f2(0); f2(1); @@ -61,11 +61,11 @@ public class X { x = 2; assert x != 1; } + ////////////// GOOD f2(Const.X); f2(Const.Y); - f2(Const.Z); - f2(Const2.I); + f2(Const.Z); int i2 = this == null ? Const.X : Const.Y; f2(i2); if (x == Const.X) { @@ -76,7 +76,7 @@ public class X { f(x); } - void f3(@MagicConstant(flags ={Const.X, Const.Y, Const.Z, Const2.I}) int x) { + void f3(@MagicConstant(flags ={Const.X, Const.Y, Const.Z}) int x) { /////////// BAD f3(2); f3(1); @@ -113,7 +113,7 @@ public class X { f4(x); } - void f4(@MagicConstant(flagsFromClass =Const.class, flags={Const2.I}) int x) { + void f4(@MagicConstant(flagsFromClass =Const.class) int x) { /////////// BAD f4(-3); f4(1); @@ -128,8 +128,8 @@ public class X { ////////////// GOOD f4(Const.X); f4(Const.Y); - f4(Const.Z); - f4(Const2.I); + f4(Const.Z); + int i2 = this == null ? Const.X : Const.Y; f4(i2); int ix = Const.X | Const.Y; @@ -152,7 +152,7 @@ public class X { class Alias { - @MagicConstant(intValues={Const.X, Const.Y, Const.Z, Const2.I}) + @MagicConstant(intValues={Const.X, Const.Y, Const.Z}) @interface IntEnum{} void f(@IntEnum int x) { @@ -268,6 +268,3 @@ public class X { font(0); } } -class Const2 { - public static final int I = 0x10; -} diff --git a/java/java-tests/testSrc/com/intellij/codeInspection/MagicConstantInspectionTest.java b/java/java-tests/testSrc/com/intellij/codeInspection/MagicConstantInspectionTest.java index 3c28371a74c2..fcf45f01e189 100644 --- a/java/java-tests/testSrc/com/intellij/codeInspection/MagicConstantInspectionTest.java +++ b/java/java-tests/testSrc/com/intellij/codeInspection/MagicConstantInspectionTest.java @@ -92,6 +92,8 @@ public class MagicConstantInspectionTest extends InspectionTestCase { } public void testSimple() throws Exception { doTest(); } + + public void testManyConstantSources() throws Exception { doTest(); } // test that the optimisation for not loading AST works public void testWithLibrary() throws Exception { doTest(); } }