IDEA-152325 Smart completion should not suggest class fields in its this/super constructor call

2026-09-27 10:03:11 +07:00 · 2016-03-15 19:55:14 +01:00
parent a0a7c689ef
commit 28b7c2501a
10 changed files with 172 additions and 102 deletions
@@ -0,0 +1,132 @@
+/*
+ * Copyright 2000-2016 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.
+ */
+package com.intellij.codeInsight.completion;
+
+import com.intellij.codeInsight.daemon.ImplicitUsageProvider;
+import com.intellij.psi.*;
+import com.intellij.psi.filters.ElementFilter;
+import com.intellij.psi.infos.CandidateInfo;
+import com.intellij.psi.util.PsiTreeUtil;
+import com.siyeh.ig.psiutils.ExpressionUtils;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+
+/**
+ * @author peter
+ */
+class CheckInitialized implements ElementFilter {
+  private final Set<PsiField> myNonInitializedFields;
+  private final boolean myInsideConstructorCall;
+
+  CheckInitialized(@NotNull PsiElement position) {
+    myNonInitializedFields = getNonInitializedFields(position);
+    myInsideConstructorCall = isInsideConstructorCall(position);
+  }
+
+  static boolean isInsideConstructorCall(@NotNull PsiElement position) {
+    return ExpressionUtils.isConstructorInvocation(PsiTreeUtil.getParentOfType(position, PsiMethodCallExpression.class));
+  }
+
+  private static boolean isInitializedImplicitly(PsiField field) {
+    field = CompletionUtil.getOriginalOrSelf(field);
+    for(ImplicitUsageProvider provider: ImplicitUsageProvider.EP_NAME.getExtensions()) {
+      if (provider.isImplicitWrite(field)) {
+        return true;
+      }
+    }
+    return false;
+  }
+
+  static Set<PsiField> getNonInitializedFields(PsiElement element) {
+    final PsiStatement statement = PsiTreeUtil.getParentOfType(element, PsiStatement.class);
+    //noinspection SSBasedInspection
+    final PsiMethod method = PsiTreeUtil.getParentOfType(element, PsiMethod.class, true, PsiClass.class);
+    if (statement == null || method == null || !method.isConstructor()) {
+      return Collections.emptySet();
+    }
+
+    PsiElement parent = element.getParent();
+    while (parent != statement) {
+      PsiElement next = parent.getParent();
+      if (next instanceof PsiAssignmentExpression && parent == ((PsiAssignmentExpression)next).getLExpression()) {
+        return Collections.emptySet();
+      }
+      if (parent instanceof PsiReferenceExpression && next instanceof PsiExpressionStatement) {
+        return Collections.emptySet();
+      }
+      parent = next;
+    }
+
+    final Set<PsiField> fields = new HashSet<>();
+    final PsiClass containingClass = method.getContainingClass();
+    assert containingClass != null;
+    for (PsiField field : containingClass.getFields()) {
+      if (!field.hasModifierProperty(PsiModifier.STATIC) && field.getInitializer() == null && !isInitializedImplicitly(field)) {
+        fields.add(field);
+      }
+    }
+
+    method.accept(new JavaRecursiveElementWalkingVisitor() {
+      @Override
+      public void visitAssignmentExpression(PsiAssignmentExpression expression) {
+        if (expression.getTextRange().getStartOffset() < statement.getTextRange().getStartOffset()) {
+          final PsiExpression lExpression = expression.getLExpression();
+          if (lExpression instanceof PsiReferenceExpression) {
+            //noinspection SuspiciousMethodCalls
+            fields.remove(((PsiReferenceExpression)lExpression).resolve());
+          }
+        }
+        super.visitAssignmentExpression(expression);
+      }
+
+      @Override
+      public void visitMethodCallExpression(PsiMethodCallExpression expression) {
+        if (expression.getTextRange().getStartOffset() < statement.getTextRange().getStartOffset()) {
+          final PsiReferenceExpression methodExpression = expression.getMethodExpression();
+          if (methodExpression.textMatches(PsiKeyword.THIS)) {
+            fields.clear();
+          }
+        }
+        super.visitMethodCallExpression(expression);
+      }
+    });
+    return fields;
+  }
+
+  @Override
+  public boolean isAcceptable(Object element, @Nullable PsiElement context) {
+    if (element instanceof CandidateInfo) {
+      element = ((CandidateInfo)element).getElement();
+    }
+    if (element instanceof PsiField) {
+      return !myNonInitializedFields.contains(element);
+    }
+    if (element instanceof PsiMethod && myInsideConstructorCall) {
+      return ((PsiMethod)element).hasModifierProperty(PsiModifier.STATIC);
+    }
+
+    return true;
+  }
+
+  @Override
+  public boolean isClassAcceptable(Class hintClass) {
+    return true;
+  }
+}
@@ -62,7 +62,10 @@ import com.intellij.util.containers.ContainerUtil;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;

-import java.util.*;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;

 import static com.intellij.patterns.PsiJavaPatterns.*;
 import static com.intellij.util.ObjectUtils.assertNotNull;
@@ -102,6 +105,7 @@ public class JavaCompletionContributor extends CompletionContributor {
  private static final ElementPattern<PsiElement> CATCH_OR_FINALLY = psiElement().afterLeaf(
    psiElement().withText("}").withParent(
      psiElement(PsiCodeBlock.class).afterLeaf(PsiKeyword.TRY)));
+  private static final ElementPattern<PsiElement> INSIDE_CONSTRUCTOR = psiElement().inside(psiMethod().constructor(true));

  @Nullable
  public static ElementFilter getReferenceFilter(PsiElement position) {
@@ -347,8 +351,12 @@ public class JavaCompletionContributor extends CompletionContributor {
      @Override
      public void consume(final PsiReference reference, final CompletionResultSet result) {
        if (reference instanceof PsiJavaReference) {
-          final ElementFilter filter = getReferenceFilter(position);
+          ElementFilter filter = getReferenceFilter(position);
          if (filter != null) {
+            if (INSIDE_CONSTRUCTOR.accepts(position) &&
+                (parameters.getInvocationCount() <= 1 || CheckInitialized.isInsideConstructorCall(position))) {
+              filter = new AndFilter(filter, new CheckInitialized(position));
+            }
            final PsiFile originalFile = parameters.getOriginalFile();
            JavaCompletionProcessor.Options options =
              JavaCompletionProcessor.Options.DEFAULT_OPTIONS
@@ -291,10 +291,7 @@ public class JavaCompletionUtil {
    final Set<LookupElement> set = new LinkedHashSet<>();
    final Condition<String> nameCondition = matcher::prefixMatches;

-    PsiMethodCallExpression call = PsiTreeUtil.getParentOfType(element, PsiMethodCallExpression.class);
-    boolean checkInitialized = parameters.getInvocationCount() <= 1 && call != null && PsiKeyword.SUPER.equals(call.getMethodExpression().getText());
-
-    final JavaCompletionProcessor processor = new JavaCompletionProcessor(element, elementFilter, options.withInitialized(checkInitialized), nameCondition);
+    final JavaCompletionProcessor processor = new JavaCompletionProcessor(element, elementFilter, options, nameCondition);
    final PsiType plainQualifier = processor.getQualifierType();
    PsiType qualifierType = plainQualifier;

@@ -17,7 +17,6 @@ package com.intellij.codeInsight.completion;

 import com.intellij.codeInsight.AnnotationTargetUtil;
 import com.intellij.codeInsight.ExceptionUtil;
-import com.intellij.codeInsight.completion.scope.JavaCompletionProcessor;
 import com.intellij.codeInsight.lookup.LookupElement;
 import com.intellij.codeInsight.lookup.LookupElementWeigher;
 import com.intellij.openapi.util.Condition;
@@ -76,7 +75,7 @@ public class PreferByKindWeigher extends LookupElementWeigher {
    super("kind");
    myCompletionType = completionType;
    myPosition = position;
-    myNonInitializedFields = JavaCompletionProcessor.getNonInitializedFields(position);
+    myNonInitializedFields = CheckInitialized.getNonInitializedFields(position);
    myRequiredSuper = createSuitabilityCondition(position);
  }

@@ -98,7 +98,11 @@ public class ReferenceExpressionCompletionContributor {
    final int offset = parameters.getParameters().getOffset();
    final PsiJavaCodeReferenceElement reference = PsiTreeUtil.findElementOfClassAtOffset(element.getContainingFile(), offset, PsiJavaCodeReferenceElement.class, false);
    if (reference != null) {
-      final ElementFilter filter = getReferenceFilter(element, false);
+      ElementFilter filter = getReferenceFilter(element, false);
+      if (CheckInitialized.isInsideConstructorCall(element)) {
+        filter = new AndFilter(filter, new CheckInitialized(element));
+      }
+
      for (final LookupElement item : completeFinalReference(element, reference, filter, parameters)) {
        result.consume(item);
      }
@@ -15,8 +15,6 @@
 */
 package com.intellij.codeInsight.completion.scope;

-import com.intellij.codeInsight.completion.CompletionUtil;
-import com.intellij.codeInsight.daemon.ImplicitUsageProvider;
 import com.intellij.codeInspection.SuppressManager;
 import com.intellij.codeInspection.accessStaticViaInstance.AccessStaticViaInstanceBase;
 import com.intellij.openapi.util.Condition;
@@ -39,8 +37,6 @@ import com.intellij.util.containers.hash.LinkedHashMap;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;

-import java.util.Collections;
-import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;

@@ -70,7 +66,6 @@ public class JavaCompletionProcessor extends BaseScopeProcessor implements Eleme
  private PsiClass myQualifierClass = null;
  private final Condition<String> myMatcher;
  private final Options myOptions;
-  private final Set<PsiField> myNonInitializedFields = new HashSet<>();
  private final boolean myAllowStaticWithInstanceQualifier;

  public JavaCompletionProcessor(@NotNull PsiElement element, ElementFilter filter, Options options, @NotNull Condition<String> nameCondition) {
@@ -115,82 +110,12 @@ public class JavaCompletionProcessor extends BaseScopeProcessor implements Eleme
      myQualifierType = JavaPsiFacade.getElementFactory(element.getProject()).createType(myQualifierClass);
    }

-    if (myOptions.checkInitialized) {
-      myNonInitializedFields.addAll(getNonInitializedFields(element));
-    }
-
    myAllowStaticWithInstanceQualifier = !options.filterStaticAfterInstance ||
                                         SuppressManager.getInstance()
                                           .isSuppressedFor(element, AccessStaticViaInstanceBase.ACCESS_STATIC_VIA_INSTANCE);

  }

-  private static boolean isInitializedImplicitly(PsiField field) {
-    field = CompletionUtil.getOriginalOrSelf(field);
-    for(ImplicitUsageProvider provider: ImplicitUsageProvider.EP_NAME.getExtensions()) {
-      if (provider.isImplicitWrite(field)) {
-        return true;
-      }
-    }
-    return false;
-  }
-
-  public static Set<PsiField> getNonInitializedFields(PsiElement element) {
-    final PsiStatement statement = PsiTreeUtil.getParentOfType(element, PsiStatement.class);
-    //noinspection SSBasedInspection
-    final PsiMethod method = PsiTreeUtil.getParentOfType(element, PsiMethod.class, true, PsiClass.class);
-    if (statement == null || method == null || !method.isConstructor()) {
-      return Collections.emptySet();
-    }
-
-    PsiElement parent = element.getParent();
-    while (parent != statement) {
-      PsiElement next = parent.getParent();
-      if (next instanceof PsiAssignmentExpression && parent == ((PsiAssignmentExpression)next).getLExpression()) {
-        return Collections.emptySet();
-      }
-      if (parent instanceof PsiReferenceExpression && next instanceof PsiExpressionStatement) {
-        return Collections.emptySet();
-      }
-      parent = next;
-    }
-
-    final Set<PsiField> fields = new HashSet<>();
-    final PsiClass containingClass = method.getContainingClass();
-    assert containingClass != null;
-    for (PsiField field : containingClass.getFields()) {
-      if (!field.hasModifierProperty(PsiModifier.STATIC) && field.getInitializer() == null && !isInitializedImplicitly(field)) {
-        fields.add(field);
-      }
-    }
-
-    method.accept(new JavaRecursiveElementWalkingVisitor() {
-      @Override
-      public void visitAssignmentExpression(PsiAssignmentExpression expression) {
-        if (expression.getTextRange().getStartOffset() < statement.getTextRange().getStartOffset()) {
-          final PsiExpression lExpression = expression.getLExpression();
-          if (lExpression instanceof PsiReferenceExpression) {
-            //noinspection SuspiciousMethodCalls
-            fields.remove(((PsiReferenceExpression)lExpression).resolve());
-          }
-        }
-        super.visitAssignmentExpression(expression);
-      }
-
-      @Override
-      public void visitMethodCallExpression(PsiMethodCallExpression expression) {
-        if (expression.getTextRange().getStartOffset() < statement.getTextRange().getStartOffset()) {
-          final PsiReferenceExpression methodExpression = expression.getMethodExpression();
-          if (methodExpression.textMatches("this")) {
-            fields.clear();
-          }
-        }
-        super.visitMethodCallExpression(expression);
-      }
-    });
-    return fields;
-  }
-
  @Override
  public void handleEvent(@NotNull Event event, Object associated){
    if(event == JavaScopeProcessorEvent.START_STATIC){
@@ -208,11 +133,6 @@ public class JavaCompletionProcessor extends BaseScopeProcessor implements Eleme

  @Override
  public boolean execute(@NotNull PsiElement element, @NotNull ResolveState state) {
-    //noinspection SuspiciousMethodCalls
-    if (myNonInitializedFields.contains(element)) {
-      return true;
-    }
-
    if (element instanceof PsiPackage && !isQualifiedContext()) {
      if (myScope instanceof PsiClass) {
        return true;
@@ -402,31 +322,26 @@ public class JavaCompletionProcessor extends BaseScopeProcessor implements Eleme
  }

  public static class Options {
-    public static final Options DEFAULT_OPTIONS = new Options(true, false, true, false);
-    public static final Options CHECK_NOTHING = new Options(false, false, false, false);
+    public static final Options DEFAULT_OPTIONS = new Options(true, true, false);
+    public static final Options CHECK_NOTHING = new Options(false, false, false);
    final boolean checkAccess;
-    final boolean checkInitialized;
    final boolean filterStaticAfterInstance;
    final boolean showInstanceInStaticContext;

-    private Options(boolean checkAccess, boolean checkInitialized, boolean filterStaticAfterInstance, boolean showInstanceInStaticContext) {
+    private Options(boolean checkAccess, boolean filterStaticAfterInstance, boolean showInstanceInStaticContext) {
      this.checkAccess = checkAccess;
-      this.checkInitialized = checkInitialized;
      this.filterStaticAfterInstance = filterStaticAfterInstance;
      this.showInstanceInStaticContext = showInstanceInStaticContext;
    }

-    public Options withInitialized(boolean checkInitialized) {
-      return new Options(checkAccess, checkInitialized, filterStaticAfterInstance, showInstanceInStaticContext);
-    }
    public Options withCheckAccess(boolean checkAccess) {
-      return new Options(checkAccess, checkInitialized, filterStaticAfterInstance, showInstanceInStaticContext);
+      return new Options(checkAccess, filterStaticAfterInstance, showInstanceInStaticContext);
    }
    public Options withFilterStaticAfterInstance(boolean filterStaticAfterInstance) {
-      return new Options(checkAccess, checkInitialized, filterStaticAfterInstance, showInstanceInStaticContext);
+      return new Options(checkAccess, filterStaticAfterInstance, showInstanceInStaticContext);
    }
    public Options withShowInstanceInStaticContext(boolean showInstanceInStaticContext) {
-      return new Options(checkAccess, checkInitialized, filterStaticAfterInstance, showInstanceInStaticContext);
+      return new Options(checkAccess, filterStaticAfterInstance, showInstanceInStaticContext);
    }
  }
  
@@ -1,7 +1,7 @@
 class SearchParameters {

  public SearchParameters(PsiElement element) {
-    this(e<caret>);
+    this(e<caret>x);
  }

  public SearchParameters(final PsiElement element, final boolean checkDeep) {
@@ -0,0 +1,10 @@
+class A {
+  final int field;
+  A(int field) {
+    this.field = field;
+  }
+
+  A(A delegate, int x) {
+    this(<caret>)
+  }
+}
@@ -50,7 +50,7 @@ public class NormalCompletionOrderingTest extends CompletionSortingTestCase {
  }

  public void testDelegatingConstructorCall() {
-    checkPreferredItems 0, 'element', 'equals'
+    checkPreferredItems 0, 'element'
  }

  public void testPreferAnnotationMethods() throws Throwable {
@@ -230,4 +230,9 @@ public class SecondSmartTypeCompletionTest extends LightFixtureCompletionTestCas
    LookupManager.getInstance(getProject()).hideActiveLookup();
    super.tearDown();
  }
+
+  public void testNoThisFieldsInDelegatingConstructorCall() {
+    configure();
+    assertOrderedEquals(myFixture.getLookupElementStrings(), "delegate.field", "x");
+  }
 }