replace intention with inspection+quickfix

2026-09-27 10:03:11 +07:00 · 2011-11-08 16:57:37 +04:00
parent d30ad19af2
commit 74e039bab6
14 changed files with 160 additions and 205 deletions
@@ -0,0 +1,5 @@
+<html><body>
+Validates using resource IDs in a switch statement in Android library module.<br>
+Resource IDs are non final in the library projects since <a href="http://tools.android.com/recent/buildchangesinrevision14">SDK tools r14</a>,
+means that the library code cannot treat these IDs as constants.
+</body></html>
@@ -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}
+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
@@ -136,14 +136,15 @@
    <intentionAction>
      <className>org.jetbrains.android.intentions.AndroidAddStringResourceAction</className>
    </intentionAction>
-    <intentionAction>
-      <className>org.jetbrains.android.intentions.AndroidReplaceSwitchWithIfIntention</className>
-    </intentionAction>

    <localInspection language="XML" shortName="AndroidDomInspection" bundle="messages.AndroidBundle" key="android.inspections.dom.name"
                     groupKey="android.inspections.group.name" enabledByDefault="true" level="ERROR"
                     implementationClass="org.jetbrains.android.inspections.AndroidDomInspection"/>

+    <localInspection language="JAVA" shortName="AndroidNonConstantResIdsInSwitch" bundle="messages.AndroidBundle" key="android.inspections.group.name"
+                         groupKey="android.inspections.group.name" enabledByDefault="true" level="ERROR"
+                         implementationClass="org.jetbrains.android.inspections.AndroidNonConstantResIdsInSwitchInspection"/>
+
    <colorSettingsPage implementation="org.jetbrains.android.logcat.AndroidLogcatColorPage"/>
    <findUsagesHandlerFactory implementation="org.jetbrains.android.AndroidFindUsagesHandlerFactory"/>
    <xml.schemaProvider implementation="org.jetbrains.android.AndroidXmlSchemaProvider"/>
@@ -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); 
+    }
+  }
+}
@@ -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;
-  }
-}
@@ -3,7 +3,7 @@ package p1.p2;
 public class Test1 {
  public void f(int n) {
    switch (n) {
-      case R.dr<caret>awable.icon:
+      case <error><warning descr="Resource IDs cannot be used in a switch statement in Android library modules">R.dr<caret>awable.icon</warning></error>:
        System.out.println("Icon");
        break;
    }
@@ -3,7 +3,7 @@ package p1.p2;
 public class Test1 {
  public void f(int n) {
    switch (n) {
-      cas<caret>e R.drawable.icon:
+      case <error>R.dr<caret>awable.icon</error>:
        System.out.println("Icon");
        break;
    }
@@ -2,8 +2,10 @@ package p1.p2;

 public class Test1 {
  public void f(int n) {
+    int abacaba = 13;
+    
    switch (n) {
-      case R<caret>.drawable.icon:
+      case <error>abac<caret>aba</error>:
        System.out.println("Icon");
        break;
    }
@@ -1,10 +0,0 @@
-package p1.p2;
-
-public class Test1 {
-  public void f(int n) {
-      if (n == R.drawable.icon) {
-          System.out.println("Icon");
-
-      }
-  }
-}
@@ -1,11 +0,0 @@
-package p1.p2;
-
-public class Test1 {
-  public void f(int n) {
-    switch (n) {
-      case <caret>R.drawable.icon:
-        System.out.println("Icon");
-        break;
-    }
-  }
-}
@@ -1,10 +0,0 @@
-package p1.p2;
-
-public class Test1 {
-  public void f(int n) {
-      if (n == R.drawable.icon) {
-          System.out.println("Icon");
-
-      }
-  }
-}
@@ -1,11 +0,0 @@
-package p1.p2;
-
-public class Test1 {
-  public void f(int n) {
-    switch (n) {
-      case R.drawable.icon:
-        Syste<caret>m.out.println("Icon");
-        break;
-    }
-  }
-}
@@ -1,11 +0,0 @@
-package p1.p2;
-
-public class Test1 {
-  public void f(int n) {
-    swit<caret>ch (n) {
-      case R.drawable.icon:
-        System.out.println("Icon");
-        break;
-    }
-  }
-}
@@ -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);
    }
  }
 }