lambda: effectively final parameters: copy in final variable if needed - never make implicitly final

2026-09-13 09:19:13 +07:00 · 2012-08-21 14:39:41 +04:00
parent c85e90f7c1
commit a0d858fa5b
13 changed files with 126 additions and 27 deletions
@@ -687,6 +687,13 @@ public class HighlightControlFlowUtil {
    if (variable.hasModifierProperty(PsiModifier.FINAL)) return null;
    final PsiClass innerClass = getInnerClassVariableReferencedFrom(variable, context);
    if (innerClass != null) {
+      if (variable instanceof PsiParameter) {
+        final PsiElement parent = variable.getParent();
+        if (parent instanceof PsiParameterList && parent.getParent() instanceof PsiLambdaExpression && 
+            notAccessedForWriting(variable, new LocalSearchScope(((PsiParameter)variable).getDeclarationScope()))) {
+          return null;
+        }
+      }
      String description = JavaErrorMessages.message("variable.must.be.final", context.getText());

      final HighlightInfo highlightInfo = HighlightInfo.createHighlightInfo(HighlightInfoType.ERROR, context, description);
@@ -701,7 +708,7 @@ public class HighlightControlFlowUtil {
          if (parent instanceof PsiParameterList && parent.getParent() == lambdaExpression) {
            return null;
          }
-          effectivelyFinal = isAccessedForWriting(variable, new LocalSearchScope(((PsiParameter)variable).getDeclarationScope()));
+          effectivelyFinal = notAccessedForWriting(variable, new LocalSearchScope(((PsiParameter)variable).getDeclarationScope()));
        } else {
          final ControlFlow controlFlow;
          try {
@@ -715,7 +722,7 @@ public class HighlightControlFlowUtil {
            final Collection<ControlFlowUtil.VariableInfo> initializedTwice = ControlFlowUtil.getInitializedTwice(controlFlow);
            effectivelyFinal = !initializedTwice.contains(new ControlFlowUtil.VariableInfo(variable, null));
            if (effectivelyFinal) {
-              effectivelyFinal = isAccessedForWriting(variable, new LocalSearchScope(lambdaExpression));
+              effectivelyFinal = notAccessedForWriting(variable, new LocalSearchScope(lambdaExpression));
            }
          } else {
            effectivelyFinal = false;
@@ -729,7 +736,7 @@ public class HighlightControlFlowUtil {
    return null;
  }

-  private static boolean isAccessedForWriting(PsiVariable variable, final LocalSearchScope searchScope) {
+  private static boolean notAccessedForWriting(PsiVariable variable, final LocalSearchScope searchScope) {
    for (PsiReference reference : ReferencesSearch.search(variable, searchScope)) {
      final PsiElement element = reference.getElement();
      if (element instanceof PsiExpression && PsiUtil.isAccessedForWriting((PsiExpression)element)) {
@@ -316,6 +316,7 @@ public class VariableAccessFromInnerClassFix implements IntentionAction {
      if (highlightInfo != null) return false;
      highlightInfo = HighlightControlFlowUtil.checkFinalVariableMightAlreadyHaveBeenAssignedTo(variable, expression, finalVarProblems);
      if (highlightInfo != null) return false;
+      if (variable instanceof PsiParameter && PsiUtil.isAccessedForWriting(expression)) return false;
    }
    return true;
  }
@@ -107,18 +107,8 @@ public class AnonymousCanBeLambdaInspection extends BaseJavaLocalInspectionTool
        LOG.assertTrue(method != null);

        final String lambdaWithTypesDeclared = composeLambdaText(method, true);
-        boolean mustBeFinal = false;
-        for (PsiParameter parameter : method.getParameterList().getParameters()) {
-          for (PsiReference reference : ReferencesSearch.search(parameter)) {
-            if (HighlightControlFlowUtil.getInnerClassVariableReferencedFrom(parameter, reference.getElement()) != null) {
-              mustBeFinal = true;
-              break;
-            }
-          }
-          if (mustBeFinal) break;
-        }
        PsiLambdaExpression lambdaExpression =
-          (PsiLambdaExpression)JavaPsiFacade.getElementFactory(project).createExpressionFromText(mustBeFinal ? lambdaWithTypesDeclared : composeLambdaText(method, false), anonymousClass);
+          (PsiLambdaExpression)JavaPsiFacade.getElementFactory(project).createExpressionFromText(composeLambdaText(method, false), anonymousClass);
        final PsiNewExpression newExpression = (PsiNewExpression)anonymousClass.getParent();
        lambdaExpression = (PsiLambdaExpression)newExpression.replace(lambdaExpression);
        PsiType interfaceType = lambdaExpression.getFunctionalInterfaceType();
@@ -133,17 +123,24 @@ public class AnonymousCanBeLambdaInspection extends BaseJavaLocalInspectionTool
      if (appendType) {
        buf.append(method.getParameterList().getText());
      } else {
-        buf.append("(").append(StringUtil.join(method.getParameterList().getParameters(),
-                                               new Function<PsiParameter, String>() {
-                                                 @Override
-                                                 public String fun(PsiParameter parameter) {
-                                                   String parameterName = parameter.getName();
-                                                   if (parameterName == null) {
-                                                     parameterName = "";
-                                                   }
-                                                   return parameterName;
-                                                 }
-                                               }, ",")).append(")");
+        final PsiParameter[] parameters = method.getParameterList().getParameters();
+        if (parameters.length != 1) {
+          buf.append("(");
+        }
+        buf.append(StringUtil.join(parameters,
+                                   new Function<PsiParameter, String>() {
+                                     @Override
+                                     public String fun(PsiParameter parameter) {
+                                       String parameterName = parameter.getName();
+                                       if (parameterName == null) {
+                                         parameterName = "";
+                                       }
+                                       return parameterName;
+                                     }
+                                   }, ","));
+        if (parameters.length != 1) {
+          buf.append(")");
+        }
      }
      buf.append("->");
      final PsiCodeBlock body = method.getBody();
@@ -259,6 +259,8 @@ public final class PsiUtil extends PsiUtilCore {
      }
      else if (declarationScope instanceof PsiMethod) {
        codeBlock = ((PsiMethod)declarationScope).getBody();
+      } else if (declarationScope instanceof PsiLambdaExpression) {
+        codeBlock = ((PsiLambdaExpression)declarationScope).getBody();
      }
    }
    else if (variable instanceof PsiResourceVariable) {
@@ -90,4 +90,28 @@ class Sample {
                runnable.run();         // prints 111
                runnable2.run();        // prints 444
        }
+}
+
+class ParameterIsEffectivelyFinal {
+  {
+    Comparable<String> c = o->{
+      new Runnable() {
+        @Override
+        public void run() {
+          System.out.println(o);
+        }
+      }.run();
+      return 0;
+    };
+    Comparable<String> c1 = o->{
+      o = "";
+      new Runnable() {
+        @Override
+        public void run() {
+          System.out.println(<error descr="Variable 'o' is accessed from within inner class. Needs to be declared final.">o</error>);
+        }
+      }.run();
+      return 0;
+    };
+  }
 }
@@ -1,7 +1,7 @@
 // "Replace with lambda" "true"
 class Test {
  {
-    Comparable<String> c = (o)->{
+    Comparable<String> c = o->{
        return 0;
    }; 
  }
@@ -1,7 +1,7 @@
 // "Replace with lambda" "true"
 class Test {
  {
-    Comparable<String> c = (final String o)->{
+    Comparable<String> c = o->{
        Runnable r = new Runnable() {
            @Override
            public void run() {
@@ -0,0 +1,17 @@
+// "Copy 'o' to temp final variable" "true"
+class Test1 {
+    void foo(){}
+    {
+        Comparable<String> a = o->{
+            o = "";
+            final String finalO = o;
+            new Runnable() {
+                @Override
+                public void run() {
+                    System.out.println(finalO);
+                }
+            }.run();
+            return 0;
+        };
+    }
+}
@@ -0,0 +1,16 @@
+// "Copy 'o' to temp final variable" "true"
+class Test1 {
+    void foo(){}
+    {
+        Comparable<String> a = o->{
+            o = "";
+            new Runnable() {
+                @Override
+                public void run() {
+                    System.out.println(<caret>o);
+                }
+            }.run();
+            return 0;
+        };
+    }
+}
@@ -15,13 +15,16 @@
 */
 package com.siyeh.ipp.types;

+import com.intellij.codeInsight.daemon.impl.analysis.HighlightControlFlowUtil;
 import com.intellij.codeInsight.generation.GenerateMembersUtil;
 import com.intellij.codeInsight.generation.OverrideImplementUtil;
 import com.intellij.codeInsight.generation.PsiGenerationInfo;
 import com.intellij.openapi.diagnostic.Logger;
 import com.intellij.openapi.editor.Editor;
 import com.intellij.psi.*;
+import com.intellij.psi.search.searches.ReferencesSearch;
 import com.intellij.psi.util.PsiTreeUtil;
+import com.intellij.psi.util.PsiUtil;
 import com.intellij.util.IncorrectOperationException;
 import com.siyeh.ipp.base.Intention;
 import com.siyeh.ipp.base.PsiElementPredicate;
@@ -66,6 +69,20 @@ public class ReplaceLambdaWithAnonymousIntention extends Intention {
      final PsiCodeBlock codeBlock = member.getBody();
      LOG.assertTrue(codeBlock != null);
      codeBlock.replace(psiElementFactory.createCodeBlockFromText(blockText, null));
+      for (PsiParameter parameter : member.getParameterList().getParameters()) {
+        if (parameter.hasModifierProperty(PsiModifier.FINAL)) continue;
+        boolean declareFinal = false;
+        for (PsiReference reference : ReferencesSearch.search(parameter)) {
+          final PsiClass innerClass = HighlightControlFlowUtil.getInnerClassVariableReferencedFrom(parameter, reference.getElement());
+          if (innerClass != null) {
+            declareFinal = true;
+            break;
+          }
+        }
+        if (declareFinal) {
+          PsiUtil.setModifierProperty(parameter, PsiModifier.FINAL, true);
+        }
+      }
      GenerateMembersUtil.positionCaret(editor, member, true);
    }
  }
@@ -0,0 +1,14 @@
+class Test1 {
+    void foo(){}
+    {
+        Comparable<String> a = <caret>o->{
+            new Runnable() {
+                @Override
+                public void run() {
+                    System.out.println(o);
+                }
+            }.run();
+            return 0;
+        };
+    }
+}
@@ -27,6 +27,10 @@ public class ReplaceLambdaWithAnonymousIntentionTest extends IPPTestCase {
    doTest();
  }

+  public void testInsertFinal() {
+    doTest();
+  }
+
  public void testCyclicInference() {
    assertIntentionNotAvailable();
  }