"Variable is assigned to itself" inspection improvements

java/java-impl/src/com/intellij/codeInspection/sillyAssignment/SillyAssignmentInspection.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2000-2009 JetBrains s.r.o.
+ * Copyright 2000-2012 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.
@@ -15,9 +15,8 @@
  */
 package com.intellij.codeInspection.sillyAssignment;
 
-import com.intellij.codeInsight.daemon.JavaErrorMessages;
 import com.intellij.codeInspection.BaseJavaLocalInspectionTool;
-import com.intellij.codeInspection.LocalQuickFix;
+import com.intellij.codeInspection.InspectionsBundle;
 import com.intellij.codeInspection.ProblemHighlightType;
 import com.intellij.codeInspection.ProblemsHolder;
 import com.intellij.openapi.util.Comparing;
@@ -38,7 +37,7 @@ public class SillyAssignmentInspection extends BaseJavaLocalInspectionTool {
 
   @NotNull
   public String getDisplayName() {
-    return JavaErrorMessages.message("assignment.to.itself");
+    return InspectionsBundle.message("inspection.variable.assigned.to.itself.display.name");
   }
 
   @NotNull
@@ -64,15 +63,27 @@ public class SillyAssignmentInspection extends BaseJavaLocalInspectionTool {
       }
 
       @Override public void visitVariable(final PsiVariable variable) {
-        final PsiExpression initializer = variable.getInitializer();
+        final PsiExpression initializer = PsiUtil.deparenthesizeExpression(variable.getInitializer());
         if (initializer instanceof PsiAssignmentExpression) {
-          final PsiExpression lExpr = ((PsiAssignmentExpression)initializer).getLExpression();
-          if (lExpr instanceof PsiReferenceExpression) {
-            final PsiReferenceExpression refExpr = (PsiReferenceExpression)lExpr;
-            if (!refExpr.isQualified() && refExpr.isReferenceTo(variable)) {
-              holder.registerProblem(lExpr, JavaErrorMessages.message("assignment.to.declared.variable", variable.getName()), 
-                                     ProblemHighlightType.LIKE_UNUSED_SYMBOL, (LocalQuickFix[])null);
+          final PsiExpression lExpr = PsiUtil.deparenthesizeExpression(((PsiAssignmentExpression)initializer).getLExpression());
+          checkExpression(variable, lExpr);
         }
+        else {
+          checkExpression(variable, initializer);
+        }
+      }
+
+      private void checkExpression(PsiVariable variable, PsiExpression expression) {
+        if (!(expression instanceof PsiReferenceExpression)) {
+          return;
+        }
+        final PsiReferenceExpression refExpr = (PsiReferenceExpression)expression;
+        final PsiExpression qualifier = refExpr.getQualifierExpression();
+        if (qualifier == null || qualifier instanceof PsiThisExpression || qualifier instanceof PsiSuperExpression ||
+            variable.hasModifierProperty(PsiModifier.STATIC)) {
+          if (refExpr.isReferenceTo(variable)) {
+            holder.registerProblem(expression, InspectionsBundle.message("assignment.to.declared.variable.problem.descriptor",
+                                                                         variable.getName()), ProblemHighlightType.LIKE_UNUSED_SYMBOL);
           }
         }
       }
@@ -91,7 +102,7 @@ public class SillyAssignmentInspection extends BaseJavaLocalInspectionTool {
     if (!(rExpression instanceof PsiReferenceExpression)) {
       if (!(rExpression instanceof PsiAssignmentExpression)) return;
       final PsiAssignmentExpression rAssignmentExpression = (PsiAssignmentExpression)rExpression;
-      final PsiExpression assignee = rAssignmentExpression.getLExpression();
+      final PsiExpression assignee = PsiUtil.deparenthesizeExpression(rAssignmentExpression.getLExpression());
       if (!(assignee instanceof PsiReferenceExpression)) return;
       rRef = (PsiReferenceExpression)assignee;
     } else {
@@ -100,7 +111,10 @@ public class SillyAssignmentInspection extends BaseJavaLocalInspectionTool {
     PsiReferenceExpression lRef = (PsiReferenceExpression)lExpression;
     PsiManager manager = assignment.getManager();
     if (!sameInstanceReferences(lRef, rRef, manager)) return;
-    holder.registerProblem(assignment, JavaErrorMessages.message("assignment.to.itself"), ProblemHighlightType.LIKE_UNUSED_SYMBOL, (LocalQuickFix[])null);
+    final PsiVariable variable = (PsiVariable)lRef.resolve();
+    if (variable == null) return;
+    holder.registerProblem(assignment, InspectionsBundle.message("assignment.to.itself.problem.descriptor", variable.getName()),
+                           ProblemHighlightType.LIKE_UNUSED_SYMBOL);
   }
 
   /**
@@ -110,6 +124,9 @@ public class SillyAssignmentInspection extends BaseJavaLocalInspectionTool {
     PsiElement lResolved = lRef.resolve();
     PsiElement rResolved = rRef.resolve();
     if (!manager.areElementsEquivalent(lResolved, rResolved)) return false;
+    if (!(lResolved instanceof PsiVariable)) return false;
+    final PsiVariable variable = (PsiVariable)lResolved;
+    if (variable.hasModifierProperty(PsiModifier.STATIC)) return true;
 
     PsiExpression lQualifier = lRef.getQualifierExpression();
     PsiExpression rQualifier = rRef.getQualifierExpression();
@@ -117,8 +134,8 @@ public class SillyAssignmentInspection extends BaseJavaLocalInspectionTool {
       return sameInstanceReferences((PsiReferenceExpression)lQualifier, (PsiReferenceExpression)rQualifier, manager);
     }
     if (Comparing.equal(lQualifier, rQualifier)) return true;
-    boolean lThis = lQualifier == null || lQualifier instanceof PsiThisExpression;
-    boolean rThis = rQualifier == null || rQualifier instanceof PsiThisExpression;
+    boolean lThis = lQualifier == null || lQualifier instanceof PsiThisExpression || lQualifier instanceof PsiSuperExpression;
+    boolean rThis = rQualifier == null || rQualifier instanceof PsiThisExpression || rQualifier instanceof PsiSuperExpression;
     return lThis && rThis;
   }
 

java/java-psi-impl/src/messages/JavaErrorMessages.properties

@@ -229,8 +229,6 @@ member.referenced.before.constructor.called=Cannot reference ''{0}'' before supe
 label.without.statement=Label without statement
 duplicate.label=Label ''{0}'' already in use
 unclosed.comment=Unclosed comment
-assignment.to.itself=Variable is assigned to itself
-assignment.to.declared.variable=Variable ''{0}'' is initialized with self assignment
 exception.already.caught=Exception ''{0}'' has already been caught
 exception.must.be.disjoint=Types in multi-catch must be disjoint: ''{0}'' is a subclass of ''{1}''
 statement.must.be.prepended.with.case.label=Statement must be prepended with case label

java/java-tests/testData/codeInsight/daemonCodeAnalyzer/advHighlighting/SillyAssignment.java

@@ -7,20 +7,20 @@ class a {
 
   void f(int i) {
 
-   <warning descr="Variable is assigned to itself">i = i</warning>;
+   <warning descr="Variable 'i' is assigned to itself">i = i</warning>;
   }
 
   void f2() {
-    <warning descr="Variable is assigned to itself">this.f = f</warning>;
-    <warning descr="Variable is assigned to itself">a.this.f = f</warning>;
-    <warning descr="Variable is assigned to itself">f = this.f</warning>;
+    <warning descr="Variable 'f' is assigned to itself">this.f = f</warning>;
+    <warning descr="Variable 'f' is assigned to itself">a.this.f = f</warning>;
+    <warning descr="Variable 'f' is assigned to itself">f = this.f</warning>;
   }
 
   void f3(Object o) {
     int i = 0;
-    <warning descr="Variable is assigned to itself">i = i</warning>;
-    <warning descr="Variable is assigned to itself">i = (int)i</warning>;
-    <warning descr="Variable is assigned to itself">o = ((Object)(o))</warning>;
+    <warning descr="Variable 'i' is assigned to itself">i = i</warning>;
+    <warning descr="Variable 'i' is assigned to itself">i = (int)i</warning>;
+    <warning descr="Variable 'o' is assigned to itself">o = ((Object)(o))</warning>;
   }
   void f4() {
     fpanel.getSize().height = this.fpanel.getSize().height; // not silly. Are you sure you can bet getSize() has no side effects? 

java/java-tests/testData/inspection/sillyAssignment/multiple/expected.xml

@@ -4,7 +4,43 @@
   <problem>
     <file>Test.java</file>
     <line>3</line>
-    <description>Variable is assigned to itself</description>
+    <description>Variable 'args' is assigned to itself</description>
+  </problem>
+
+  <problem>
+    <file>Test.java</file>
+    <line>4</line>
+    <description>Variable 'j' is initialized with self assignment</description>
+  </problem>
+
+  <problem>
+    <file>Test.java</file>
+    <line>5</line>
+    <description>Variable 'args' is assigned to itself</description>
+  </problem>
+
+  <problem>
+    <file>Test.java</file>
+    <line>8</line>
+    <description>Variable 'z' is initialized with self assignment</description>
+  </problem>
+
+  <problem>
+    <file>Test.java</file>
+    <line>13</line>
+    <description>Variable 'y' is assigned to itself</description>
+  </problem>
+
+  <problem>
+    <file>Test.java</file>
+    <line>17</line>
+    <description>Variable 'h' is assigned to itself</description>
+  </problem>
+
+  <problem>
+    <file>Test.java</file>
+    <line>18</line>
+    <description>Variable 'h' is assigned to itself</description>
   </problem>
 
 </problems>

java/java-tests/testData/inspection/sillyAssignment/multiple/src/Test.java

@@ -1,5 +1,24 @@
-public class Test {
+public class Test extends Super {
     public static void main(String[] args) {
         args = args;
+        int j = ((j) = 1);
+        args = (args) = null;
+    }
+
+    private final int z = this.z;
+    public static int y = ABCV.y;
+    public static final int x = ABCV.x;
+
+    static void foo() {
+        y = Test.y;
+    }
+
+    void call() {
+      h = super.h;
+      h = (h) = 1;
     }
 }
+class Super {
+    int h;
+}
+

platform/platform-resources-en/src/messages/InspectionsBundle.properties

@@ -656,3 +656,7 @@ special.annotations.list.annotation.pattern=Add Annotations Pattern
 
 deprecated.defender.syntax.description=Deprecated extension method syntax
 deprecated.defender.syntax.fix=Convert extension method syntax
+
+inspection.variable.assigned.to.itself.display.name=Variable is assigned to itself
+assignment.to.itself.problem.descriptor=Variable ''{0}'' is assigned to itself
+assignment.to.declared.variable.problem.descriptor=Variable ''{0}'' is initialized with self assignment