IDEA-163786 Inspection "Local variable '...' is redundant" is not correct; other false-positives fixed in UnnecessaryLocalVariableInspectionBase

plugins/InspectionGadgets/InspectionGadgetsAnalysis/src/com/siyeh/ig/dataflow/UnnecessaryLocalVariableInspectionBase.java

@@ -26,6 +26,7 @@ import com.intellij.psi.tree.IElementType;
 import com.intellij.psi.util.PsiTreeUtil;
 import com.intellij.psi.util.PsiUtil;
 import com.intellij.psi.util.TypeConversionUtil;
+import com.intellij.util.ObjectUtils;
 import com.siyeh.InspectionGadgetsBundle;
 import com.siyeh.ig.BaseInspection;
 import com.siyeh.ig.BaseInspectionVisitor;
@@ -131,8 +132,7 @@ public class UnnecessaryLocalVariableInspectionBase extends BaseInspection {
         return false;
       }
       if (!(referent instanceof PsiLocalVariable || referent instanceof PsiParameter)) {
-        if (!(referent instanceof PsiField) || !((PsiField)referent).hasModifierProperty(PsiModifier.FINAL)
-            || ReferencesSearch.search(variable).findAll().size() != 1) {
+        if (!isFinalChain(reference) || ReferencesSearch.search(variable).findAll().size() != 1) {
           // only warn when variable is referenced once, to avoid warning when a field is cached in local variable
           // as in e.g. gnu.trove.TObjectHash#forEach()
           return false;
@@ -142,13 +142,11 @@ public class UnnecessaryLocalVariableInspectionBase extends BaseInspection {
       if (containingScope == null) {
         return false;
       }
-      if (!variable.hasModifierProperty(PsiModifier.FINAL) &&
-          VariableAccessUtils.variableIsAssigned(variable, containingScope, false)) {
+      if (variableMayChange(containingScope, null, variable)) {
         return false;
       }
       final PsiVariable initialization = (PsiVariable)referent;
-      if (!initialization.hasModifierProperty(PsiModifier.FINAL) &&
-          VariableAccessUtils.variableIsAssigned(initialization, containingScope, false)) {
+      if (variableMayChange(containingScope, PsiUtil.skipParenthesizedExprDown(reference.getQualifierExpression()), initialization)) {
         return false;
       }
 
@@ -158,10 +156,10 @@ public class UnnecessaryLocalVariableInspectionBase extends BaseInspection {
         return false;
       }
 
-      final boolean finalVariableIntroduction = 
+      final boolean finalVariableIntroduction =
         !initialization.hasModifierProperty(PsiModifier.FINAL) && variable.hasModifierProperty(PsiModifier.FINAL) ||
         PsiUtil.isLanguageLevel8OrHigher(initialization) &&
-        !HighlightControlFlowUtil.isEffectivelyFinal(initialization, containingScope, null) && 
+        !HighlightControlFlowUtil.isEffectivelyFinal(initialization, containingScope, null) &&
         HighlightControlFlowUtil.isEffectivelyFinal(variable, containingScope, null);
 
       final PsiType variableType = variable.getType();
@@ -179,12 +177,12 @@ public class UnnecessaryLocalVariableInspectionBase extends BaseInspection {
         if (resolveHelper.resolveReferencedVariable(initializationName, refElement) != initialization) {
           return false;
         }
-        
+
         if (!sameType) {
           final PsiElement parent = refElement.getParent();
           if (parent instanceof PsiReferenceExpression) {
             final PsiElement resolve = ((PsiReferenceExpression)parent).resolve();
-            if (resolve instanceof PsiMember && 
+            if (resolve instanceof PsiMember &&
                 ((PsiMember)resolve).hasModifierProperty(PsiModifier.PRIVATE)) {
               return false;
             }
@@ -195,6 +193,32 @@ public class UnnecessaryLocalVariableInspectionBase extends BaseInspection {
       return !TypeConversionUtil.boxingConversionApplicable(variableType, initializationType);
     }
 
+    private boolean isFinalChain(PsiReferenceExpression reference) {
+      while (true) {
+        PsiElement element = reference.resolve();
+        if (!(element instanceof PsiField)) return true;
+        if (!((PsiField)element).hasModifierProperty(PsiModifier.FINAL)) return false;
+        PsiExpression qualifier = PsiUtil.skipParenthesizedExprDown(reference.getQualifierExpression());
+        if (qualifier == null || qualifier instanceof PsiThisExpression) return true;
+        if (!(qualifier instanceof PsiReferenceExpression)) return false;
+        reference = (PsiReferenceExpression)qualifier;
+      }
+    }
+
+    private boolean variableMayChange(PsiCodeBlock containingScope, PsiExpression qualifier, PsiVariable variable) {
+      while (variable != null) {
+        if (!variable.hasModifierProperty(PsiModifier.FINAL) &&
+            VariableAccessUtils.variableIsAssigned(variable, containingScope, false)) {
+          return true;
+        }
+        if (!(qualifier instanceof PsiReferenceExpression)) break;
+        PsiReferenceExpression qualifierReference = (PsiReferenceExpression)qualifier;
+        qualifier = PsiUtil.skipParenthesizedExprDown(qualifierReference.getQualifierExpression());
+        variable = ObjectUtils.tryCast(qualifierReference.resolve(), PsiVariable.class);
+      }
+      return false;
+    }
+
     private boolean isImmediatelyReturned(PsiVariable variable) {
       final PsiCodeBlock containingScope = PsiTreeUtil.getParentOfType(variable, PsiCodeBlock.class, true, PsiClass.class);
       if (containingScope == null) {

plugins/InspectionGadgets/test/com/siyeh/igtest/dataflow/unnecessary_local_variable/C.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2000-2015 JetBrains s.r.o.
+ * Copyright 2000-2017 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.
@@ -199,4 +199,29 @@ class UsingConstant {
     int yes = YES;
     yes++;
   }
-}
+}
+
+class Test {
+  final int x = (int)(Math.random() * 10);
+
+  int test() {
+    Test t = new Test();
+    int <warning descr="Local variable 'xx' is redundant">xx</warning> = this.x;
+    t = null;
+    return xx;
+  }
+
+  int test2() {
+    Test t = new Test();
+    int xx = t.x;
+    t = null;
+    return xx;
+  }
+
+  int test(Test[] arr) {
+    // copying of the field from array element is conservatively not considered safe to inline as array may change
+    int res = arr[0].x;
+    arr[0] = null;
+    return res;
+  }
+}

plugins/InspectionGadgets/test/com/siyeh/igtest/dataflow/unnecessary_local_variable/Tree.java

@@ -0,0 +1,54 @@
+/*
+ * Copyright 2000-2017 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 java.util.Iterator;
+import java.util.NoSuchElementException;
+
+final class Tree<T> {
+
+    private final T elem;
+    private final Tree<T> parent;
+
+    private Tree(T elem, Tree<T> parent) {
+        this.elem = elem;
+        this.parent = parent;
+    }
+
+    /** Iterator from current tree element to it's root. */
+    public Iterator<T> toRoot() {
+        return new Iterator<T>() {
+            Tree<T> curr = Tree.this;
+
+            @Override
+            public void remove() {}
+
+            @Override
+            public boolean hasNext() {
+                return curr.parent != null;
+            }
+
+            @Override
+            public T next() {
+                if (!hasNext()) {
+                    throw new NoSuchElementException();
+                }
+
+                final T res = curr.elem; // IDEA inspection: Local variable 'res' is redundant (IDEA-163786)
+                curr = curr.parent;
+                return res;
+            }
+        };
+    }
+}

plugins/InspectionGadgets/testsrc/com/siyeh/ig/dataflow/UnnecessaryLocalVariableInspectionTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2000-2015 JetBrains s.r.o.
+ * Copyright 2000-2017 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.
@@ -25,4 +25,6 @@ public class UnnecessaryLocalVariableInspectionTest extends LightInspectionTestC
   }
 
   public void testC() { doTest(); }
+
+  public void testTree() { doTest(); }
 }