diff --git a/plugins/android/resources/inspectionDescriptions/AndroidNonConstantResIdsInSwitch.html b/plugins/android/resources/inspectionDescriptions/AndroidNonConstantResIdsInSwitch.html new file mode 100644 index 000000000000..29a17ef8736f --- /dev/null +++ b/plugins/android/resources/inspectionDescriptions/AndroidNonConstantResIdsInSwitch.html @@ -0,0 +1,5 @@ + +Validates using resource IDs in a switch statement in Android library module.
+Resource IDs are non final in the library projects since SDK tools r14, +means that the library code cannot treat these IDs as constants. + \ No newline at end of file diff --git a/plugins/android/resources/messages/AndroidBundle.properties b/plugins/android/resources/messages/AndroidBundle.properties index e5877f16cf2c..0efc02a72eb7 100644 --- a/plugins/android/resources/messages/AndroidBundle.properties +++ b/plugins/android/resources/messages/AndroidBundle.properties @@ -298,4 +298,6 @@ deployment.target.settings.wizard.configure.later=&Do not create run configurati deployment.target.settings.wizard.show.dialog=&Show device chooser dialog deployment.target.settings.wizard.usb.device=&USB device deployment.target.settings.wizard.emulator=Emulat&or -android.compilation.error.cannot.create.png.cache.directory=Cannot create PNG cache directory for module {0} \ No newline at end of file +android.compilation.error.cannot.create.png.cache.directory=Cannot create PNG cache directory for module {0} +android.inspections.non.constant.res.ids.in.switch.name=Non-constant resource ID in a switch statement +android.inspections.non.constant.res.ids.in.switch.message=Resource IDs cannot be used in a switch statement in Android library modules \ No newline at end of file diff --git a/plugins/android/src/META-INF/plugin.xml b/plugins/android/src/META-INF/plugin.xml index 1a0e0c3e5edd..13f5c865dd07 100644 --- a/plugins/android/src/META-INF/plugin.xml +++ b/plugins/android/src/META-INF/plugin.xml @@ -136,14 +136,15 @@ org.jetbrains.android.intentions.AndroidAddStringResourceAction - - org.jetbrains.android.intentions.AndroidReplaceSwitchWithIfIntention - + + diff --git a/plugins/android/src/org/jetbrains/android/inspections/AndroidNonConstantResIdsInSwitchInspection.java b/plugins/android/src/org/jetbrains/android/inspections/AndroidNonConstantResIdsInSwitchInspection.java new file mode 100644 index 000000000000..2e2562eaa835 --- /dev/null +++ b/plugins/android/src/org/jetbrains/android/inspections/AndroidNonConstantResIdsInSwitchInspection.java @@ -0,0 +1,119 @@ +package org.jetbrains.android.inspections; + +import com.intellij.codeInspection.LocalInspectionTool; +import com.intellij.codeInspection.LocalQuickFix; +import com.intellij.codeInspection.ProblemDescriptor; +import com.intellij.codeInspection.ProblemsHolder; +import com.intellij.openapi.project.Project; +import com.intellij.psi.*; +import com.intellij.psi.util.PsiTreeUtil; +import com.siyeh.ipp.switchtoif.ReplaceSwitchWithIfIntention; +import org.jetbrains.android.facet.AndroidFacet; +import org.jetbrains.android.util.AndroidBundle; +import org.jetbrains.android.util.AndroidResourceUtil; +import org.jetbrains.annotations.Nls; +import org.jetbrains.annotations.NotNull; + +/** + * @author Eugene.Kudelevsky + */ +public class AndroidNonConstantResIdsInSwitchInspection extends LocalInspectionTool { + private final ReplaceSwitchWithIfIntention myBaseIntention = new ReplaceSwitchWithIfIntention(); + + @Nls + @NotNull + @Override + public String getGroupDisplayName() { + return AndroidBundle.message("android.inspections.group.name"); + } + + @Nls + @NotNull + @Override + public String getDisplayName() { + return AndroidBundle.message("android.inspections.non.constant.res.ids.in.switch.name"); + } + + @NotNull + @Override + public String getShortName() { + return "AndroidNonConstantResIdsInSwitch"; + } + + @NotNull + @Override + public PsiElementVisitor buildVisitor(@NotNull final ProblemsHolder holder, boolean isOnTheFly) { + return new JavaElementVisitor() { + @Override + public void visitSwitchLabelStatement(PsiSwitchLabelStatement statement) { + final AndroidFacet facet = AndroidFacet.getInstance(statement); + if (facet == null || !facet.getConfiguration().LIBRARY_PROJECT) { + return; + } + + final PsiExpression caseValue = statement.getCaseValue(); + if (!(caseValue instanceof PsiReferenceExpression)) { + return; + } + + final PsiSwitchStatement switchStatement = PsiTreeUtil.getParentOfType(statement, PsiSwitchStatement.class); + if (switchStatement == null || !ReplaceSwitchWithIfIntention.canProcess(switchStatement)) { + return; + } + + final PsiElement resolvedElement = ((PsiReferenceExpression)caseValue).resolve(); + if (resolvedElement == null || !(resolvedElement instanceof PsiField)) { + return; + } + + final PsiField resolvedField = (PsiField)resolvedElement; + final PsiFile containingFile = resolvedField.getContainingFile(); + + if (containingFile == null || !AndroidResourceUtil.isRJavaField(containingFile, resolvedField)) { + return; + } + + final PsiModifierList modifierList = resolvedField.getModifierList(); + + if (modifierList == null || !modifierList.hasModifierProperty(PsiModifier.FINAL)) { + holder.registerProblem(caseValue, AndroidBundle.message("android.inspections.non.constant.res.ids.in.switch.message"), + new MyQuickFix()); + } + } + }; + } + + public String getQuickFixName() { + return myBaseIntention.getText(); + } + + private class MyQuickFix implements LocalQuickFix { + + @NotNull + @Override + public String getName() { + return getQuickFixName(); + } + + @NotNull + @Override + public String getFamilyName() { + return getName(); + } + + @Override + public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) { + final PsiElement element = descriptor.getPsiElement(); + if (element == null) { + return; + } + + final PsiSwitchStatement switchStatement = PsiTreeUtil.getParentOfType(element, PsiSwitchStatement.class); + if (switchStatement == null) { + return; + } + + ReplaceSwitchWithIfIntention.doProcessIntention(switchStatement); + } + } +} diff --git a/plugins/android/src/org/jetbrains/android/intentions/AndroidReplaceSwitchWithIfIntention.java b/plugins/android/src/org/jetbrains/android/intentions/AndroidReplaceSwitchWithIfIntention.java deleted file mode 100644 index 8fa8f369e706..000000000000 --- a/plugins/android/src/org/jetbrains/android/intentions/AndroidReplaceSwitchWithIfIntention.java +++ /dev/null @@ -1,102 +0,0 @@ -package org.jetbrains.android.intentions; - -import com.intellij.codeInsight.intention.HighPriorityAction; -import com.intellij.codeInsight.intention.IntentionAction; -import com.intellij.openapi.editor.Editor; -import com.intellij.openapi.project.Project; -import com.intellij.psi.*; -import com.intellij.psi.util.PsiTreeUtil; -import com.intellij.util.IncorrectOperationException; -import com.siyeh.ipp.switchtoif.ReplaceSwitchWithIfIntention; -import org.jetbrains.android.facet.AndroidFacet; -import org.jetbrains.android.util.AndroidBundle; -import org.jetbrains.android.util.AndroidResourceUtil; -import org.jetbrains.annotations.NotNull; - -/** - * @author Eugene.Kudelevsky - */ -public class AndroidReplaceSwitchWithIfIntention implements IntentionAction, HighPriorityAction { - private final ReplaceSwitchWithIfIntention myBaseIntention = new ReplaceSwitchWithIfIntention(); - - @NotNull - @Override - public String getText() { - return myBaseIntention.getText(); - } - - @NotNull - @Override - public String getFamilyName() { - return AndroidBundle.message("intention.family"); - } - - @Override - public boolean isAvailable(@NotNull Project project, Editor editor, PsiFile file) { - if (file == null) { - return false; - } - - final AndroidFacet facet = AndroidFacet.getInstance(file); - if (facet == null || !facet.getConfiguration().LIBRARY_PROJECT) { - return false; - } - - final PsiElement element = file.findElementAt(editor.getCaretModel().getOffset()); - if (element == null) { - return false; - } - - final PsiSwitchLabelStatement switchLabelStatement = PsiTreeUtil.getParentOfType(element, PsiSwitchLabelStatement.class); - if (switchLabelStatement == null) { - return false; - } - - final PsiExpression caseValue = switchLabelStatement.getCaseValue(); - if (!(caseValue instanceof PsiReferenceExpression) || !PsiTreeUtil.isAncestor(caseValue, element, false)) { - return false; - } - - if (myBaseIntention.isAvailable(project, editor, file)) { - // this intention is addition to the base one - return false; - } - - final PsiSwitchStatement switchStatement = PsiTreeUtil.getParentOfType(switchLabelStatement, PsiSwitchStatement.class); - if (switchStatement == null || !ReplaceSwitchWithIfIntention.canProcess(switchStatement)) { - return false; - } - - final PsiElement resolvedElement = ((PsiReferenceExpression)caseValue).resolve(); - if (resolvedElement == null || !(resolvedElement instanceof PsiField)) { - return false; - } - - final PsiField resolvedField = (PsiField)resolvedElement; - final PsiFile containingFile = resolvedField.getContainingFile(); - - if (containingFile == null || !AndroidResourceUtil.isRJavaField(containingFile, resolvedField)) { - return false; - } - - final PsiModifierList modifierList = resolvedField.getModifierList(); - - return modifierList == null || !modifierList.hasModifierProperty(PsiModifier.FINAL); - } - - @Override - public void invoke(@NotNull Project project, Editor editor, PsiFile file) throws IncorrectOperationException { - final PsiElement element = file.findElementAt(editor.getCaretModel().getOffset()); - assert element != null; - - final PsiSwitchStatement switchStatement = PsiTreeUtil.getParentOfType(element, PsiSwitchStatement.class); - assert switchStatement != null; - - ReplaceSwitchWithIfIntention.doProcessIntention(switchStatement); - } - - @Override - public boolean startInWriteAction() { - return true; - } -} diff --git a/plugins/android/testData/intentions/SwitchOnResourceId.java b/plugins/android/testData/intentions/SwitchOnResourceId.java index 3ebc51df96b0..c337a882e147 100644 --- a/plugins/android/testData/intentions/SwitchOnResourceId.java +++ b/plugins/android/testData/intentions/SwitchOnResourceId.java @@ -3,7 +3,7 @@ package p1.p2; public class Test1 { public void f(int n) { switch (n) { - case R.drawable.icon: + case R.drawable.icon: System.out.println("Icon"); break; } diff --git a/plugins/android/testData/intentions/SwitchOnResourceId4.java b/plugins/android/testData/intentions/SwitchOnResourceId1.java similarity index 73% rename from plugins/android/testData/intentions/SwitchOnResourceId4.java rename to plugins/android/testData/intentions/SwitchOnResourceId1.java index 0782e71644bc..52dceb94f564 100644 --- a/plugins/android/testData/intentions/SwitchOnResourceId4.java +++ b/plugins/android/testData/intentions/SwitchOnResourceId1.java @@ -3,7 +3,7 @@ package p1.p2; public class Test1 { public void f(int n) { switch (n) { - case R.drawable.icon: + case R.drawable.icon: System.out.println("Icon"); break; } diff --git a/plugins/android/testData/intentions/SwitchOnResourceId2.java b/plugins/android/testData/intentions/SwitchOnResourceId2.java index 5976821c11bf..ef544426affc 100644 --- a/plugins/android/testData/intentions/SwitchOnResourceId2.java +++ b/plugins/android/testData/intentions/SwitchOnResourceId2.java @@ -2,8 +2,10 @@ package p1.p2; public class Test1 { public void f(int n) { + int abacaba = 13; + switch (n) { - case R.drawable.icon: + case abacaba: System.out.println("Icon"); break; } diff --git a/plugins/android/testData/intentions/SwitchOnResourceId2_after.java b/plugins/android/testData/intentions/SwitchOnResourceId2_after.java deleted file mode 100644 index 1acfaff0ad5c..000000000000 --- a/plugins/android/testData/intentions/SwitchOnResourceId2_after.java +++ /dev/null @@ -1,10 +0,0 @@ -package p1.p2; - -public class Test1 { - public void f(int n) { - if (n == R.drawable.icon) { - System.out.println("Icon"); - - } - } -} \ No newline at end of file diff --git a/plugins/android/testData/intentions/SwitchOnResourceId3.java b/plugins/android/testData/intentions/SwitchOnResourceId3.java deleted file mode 100644 index 9afc5cb99111..000000000000 --- a/plugins/android/testData/intentions/SwitchOnResourceId3.java +++ /dev/null @@ -1,11 +0,0 @@ -package p1.p2; - -public class Test1 { - public void f(int n) { - switch (n) { - case R.drawable.icon: - System.out.println("Icon"); - break; - } - } -} \ No newline at end of file diff --git a/plugins/android/testData/intentions/SwitchOnResourceId3_after.java b/plugins/android/testData/intentions/SwitchOnResourceId3_after.java deleted file mode 100644 index 1acfaff0ad5c..000000000000 --- a/plugins/android/testData/intentions/SwitchOnResourceId3_after.java +++ /dev/null @@ -1,10 +0,0 @@ -package p1.p2; - -public class Test1 { - public void f(int n) { - if (n == R.drawable.icon) { - System.out.println("Icon"); - - } - } -} \ No newline at end of file diff --git a/plugins/android/testData/intentions/SwitchOnResourceId5.java b/plugins/android/testData/intentions/SwitchOnResourceId5.java deleted file mode 100644 index 8e4f7d189fbe..000000000000 --- a/plugins/android/testData/intentions/SwitchOnResourceId5.java +++ /dev/null @@ -1,11 +0,0 @@ -package p1.p2; - -public class Test1 { - public void f(int n) { - switch (n) { - case R.drawable.icon: - System.out.println("Icon"); - break; - } - } -} \ No newline at end of file diff --git a/plugins/android/testData/intentions/SwitchOnResourceId6.java b/plugins/android/testData/intentions/SwitchOnResourceId6.java deleted file mode 100644 index 44c33f5a035e..000000000000 --- a/plugins/android/testData/intentions/SwitchOnResourceId6.java +++ /dev/null @@ -1,11 +0,0 @@ -package p1.p2; - -public class Test1 { - public void f(int n) { - switch (n) { - case R.drawable.icon: - System.out.println("Icon"); - break; - } - } -} \ No newline at end of file diff --git a/plugins/android/testSrc/org/jetbrains/android/intentions/AndroidIntentionsTest.java b/plugins/android/testSrc/org/jetbrains/android/intentions/AndroidIntentionsTest.java index 9eaf94a3b28b..334e7fce70db 100644 --- a/plugins/android/testSrc/org/jetbrains/android/intentions/AndroidIntentionsTest.java +++ b/plugins/android/testSrc/org/jetbrains/android/intentions/AndroidIntentionsTest.java @@ -1,10 +1,10 @@ package org.jetbrains.android.intentions; import com.intellij.codeInsight.intention.IntentionAction; -import com.intellij.openapi.application.ApplicationManager; -import com.intellij.openapi.command.CommandProcessor; +import com.intellij.codeInspection.LocalInspectionTool; import com.intellij.openapi.vfs.VirtualFile; import org.jetbrains.android.AndroidTestCase; +import org.jetbrains.android.inspections.AndroidNonConstantResIdsInSwitchInspection; /** * @author Eugene.Kudelevsky @@ -13,60 +13,41 @@ public class AndroidIntentionsTest extends AndroidTestCase { private static final String BASE_PATH = "intentions/"; public void testSwitchOnResourceId() { - doTestSwitchOnResourceId(true); + myFacet.getConfiguration().LIBRARY_PROJECT = true; + myFixture.copyFileToProject(BASE_PATH + "R.java", "src/p1/p2/R.java"); + final AndroidNonConstantResIdsInSwitchInspection inspection = new AndroidNonConstantResIdsInSwitchInspection(); + doTest(inspection, true, inspection.getQuickFixName()); + } + + public void testSwitchOnResourceId1() { + myFacet.getConfiguration().LIBRARY_PROJECT = false; + myFixture.copyFileToProject(BASE_PATH + "R.java", "src/p1/p2/R.java"); + final AndroidNonConstantResIdsInSwitchInspection inspection = new AndroidNonConstantResIdsInSwitchInspection(); + doTest(inspection, false, inspection.getQuickFixName()); } public void testSwitchOnResourceId2() { - doTestSwitchOnResourceId(true); - } - - public void testSwitchOnResourceId3() { - doTestSwitchOnResourceId(true); - } - - public void testSwitchOnResourceId4() { - doTestSwitchOnResourceId(false); - } - - public void testSwitchOnResourceId5() { - doTestSwitchOnResourceId(false); - } - - public void testSwitchOnResourceId6() { - doTestSwitchOnResourceId(false); - } - - private void doTestSwitchOnResourceId(boolean available) { myFacet.getConfiguration().LIBRARY_PROJECT = true; myFixture.copyFileToProject(BASE_PATH + "R.java", "src/p1/p2/R.java"); - doTest(new AndroidReplaceSwitchWithIfIntention(), available); + final AndroidNonConstantResIdsInSwitchInspection inspection = new AndroidNonConstantResIdsInSwitchInspection(); + doTest(inspection, false, inspection.getQuickFixName()); } - private void doTest(final IntentionAction intention, boolean available) { - VirtualFile javaFile = myFixture.copyFileToProject(BASE_PATH + getTestName(false) + ".java", "src/p1/p2/Class.java"); - myFixture.configureFromExistingVirtualFile(javaFile); + private void doTest(final LocalInspectionTool inspection, boolean available, String quickFixName) { + myFixture.enableInspections(inspection); - final boolean actualAvailable = intention.isAvailable(myFixture.getProject(), myFixture.getEditor(), myFixture.getFile()); + final VirtualFile file = myFixture.copyFileToProject(BASE_PATH + getTestName(false) + ".java", "src/p1/p2/Class.java"); + myFixture.configureFromExistingVirtualFile(file); + myFixture.checkHighlighting(false, false, false); + final IntentionAction quickFix = myFixture.getAvailableIntention(quickFixName); if (available) { - assertTrue(actualAvailable); - - CommandProcessor.getInstance().executeCommand(getProject(), new Runnable() { - @Override - public void run() { - ApplicationManager.getApplication().runWriteAction(new Runnable() { - @Override - public void run() { - intention.invoke(myFixture.getProject(), myFixture.getEditor(), myFixture.getFile()); - } - }); - } - }, "", ""); - + assertNotNull(quickFix); + myFixture.launchAction(quickFix); myFixture.checkResultByFile(BASE_PATH + getTestName(false) + "_after.java"); } else { - assertFalse(actualAvailable); + assertNull(quickFix); } } }