From 55bc513149c209b824857aff99a5ab9a9cd0f6c9 Mon Sep 17 00:00:00 2001 From: peter Date: Tue, 22 Feb 2011 16:46:46 +0100 Subject: [PATCH] IDEA-50627 Don't suggest uninitialized instance members in constructor when smart-completing --- .../completion/JavaCompletionUtil.java | 2 +- .../JavaDocCompletionContributor.java | 2 +- .../scope/JavaCompletionProcessor.java | 65 ++++++++++++++++--- .../normalSorting/DispreferDeclared.java | 1 + .../DispreferDeclaredOfExpectedType.java | 1 + .../smartType/FieldsSetAbove-out.java | 9 +++ .../completion/smartType/FieldsSetAbove.java | 9 +++ .../FieldsSetInAnotherConstructor-out.java | 13 ++++ .../FieldsSetInAnotherConstructor.java | 13 ++++ .../NoFieldsInSuperConstructorCall-out.java | 11 ++++ .../NoFieldsInSuperConstructorCall.java | 11 ++++ ...oUninitializedFieldsInConstructor-out.java | 8 +++ .../NoUninitializedFieldsInConstructor.java | 8 +++ .../second/NonInitializedField-out.java | 7 ++ .../smartType/second/NonInitializedField.java | 7 ++ .../SecondSmartTypeCompletionTest.java | 3 +- .../completion/SmartTypeCompletionTest.java | 5 ++ 17 files changed, 162 insertions(+), 13 deletions(-) create mode 100644 java/java-tests/testData/codeInsight/completion/smartType/FieldsSetAbove-out.java create mode 100644 java/java-tests/testData/codeInsight/completion/smartType/FieldsSetAbove.java create mode 100644 java/java-tests/testData/codeInsight/completion/smartType/FieldsSetInAnotherConstructor-out.java create mode 100644 java/java-tests/testData/codeInsight/completion/smartType/FieldsSetInAnotherConstructor.java create mode 100644 java/java-tests/testData/codeInsight/completion/smartType/NoFieldsInSuperConstructorCall-out.java create mode 100644 java/java-tests/testData/codeInsight/completion/smartType/NoFieldsInSuperConstructorCall.java create mode 100644 java/java-tests/testData/codeInsight/completion/smartType/NoUninitializedFieldsInConstructor-out.java create mode 100644 java/java-tests/testData/codeInsight/completion/smartType/NoUninitializedFieldsInConstructor.java create mode 100644 java/java-tests/testData/codeInsight/completion/smartType/second/NonInitializedField-out.java create mode 100644 java/java-tests/testData/codeInsight/completion/smartType/second/NonInitializedField.java diff --git a/java/java-impl/src/com/intellij/codeInsight/completion/JavaCompletionUtil.java b/java/java-impl/src/com/intellij/codeInsight/completion/JavaCompletionUtil.java index 8969ae0704a5..c96d7f3d6535 100644 --- a/java/java-impl/src/com/intellij/codeInsight/completion/JavaCompletionUtil.java +++ b/java/java-impl/src/com/intellij/codeInsight/completion/JavaCompletionUtil.java @@ -573,7 +573,7 @@ public class JavaCompletionUtil { return matcher.prefixMatches(s); } }; - final JavaCompletionProcessor processor = new JavaCompletionProcessor(element, elementFilter, checkAccess, nameCondition); + final JavaCompletionProcessor processor = new JavaCompletionProcessor(element, elementFilter, checkAccess, parameters.getInvocationCount() <= 1, nameCondition); javaReference.processVariants(processor); final Collection plainResults = processor.getResults(); diff --git a/java/java-impl/src/com/intellij/codeInsight/completion/JavaDocCompletionContributor.java b/java/java-impl/src/com/intellij/codeInsight/completion/JavaDocCompletionContributor.java index e57b990ae312..aea4cc222e4c 100644 --- a/java/java-impl/src/com/intellij/codeInsight/completion/JavaDocCompletionContributor.java +++ b/java/java-impl/src/com/intellij/codeInsight/completion/JavaDocCompletionContributor.java @@ -77,7 +77,7 @@ public class JavaDocCompletionContributor extends CompletionContributor { if (ref instanceof PsiJavaReference) { result.stopHere(); - final JavaCompletionProcessor processor = new JavaCompletionProcessor(position, TrueFilter.INSTANCE, false, null); + final JavaCompletionProcessor processor = new JavaCompletionProcessor(position, TrueFilter.INSTANCE, false, false, null); ((PsiJavaReference) ref).processVariants(processor); for (final CompletionElement _item : processor.getResults()) { diff --git a/java/java-impl/src/com/intellij/codeInsight/completion/scope/JavaCompletionProcessor.java b/java/java-impl/src/com/intellij/codeInsight/completion/scope/JavaCompletionProcessor.java index dc79715dc74b..e6a8a065789c 100644 --- a/java/java-impl/src/com/intellij/codeInsight/completion/scope/JavaCompletionProcessor.java +++ b/java/java-impl/src/com/intellij/codeInsight/completion/scope/JavaCompletionProcessor.java @@ -26,12 +26,14 @@ import com.intellij.psi.infos.CandidateInfo; import com.intellij.psi.scope.BaseScopeProcessor; import com.intellij.psi.scope.ElementClassHint; import com.intellij.psi.scope.JavaScopeProcessorEvent; +import com.intellij.psi.util.PsiTreeUtil; import com.intellij.psi.util.PsiUtil; import gnu.trove.THashSet; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import java.util.ArrayList; +import java.util.HashSet; import java.util.List; import java.util.Set; @@ -58,8 +60,9 @@ public class JavaCompletionProcessor extends BaseScopeProcessor implements Eleme private PsiClass myQualifierClass = null; private final Condition myMatcher; private final boolean myCheckAccess; + private final Set myNonInitializedFields = new HashSet(); - public JavaCompletionProcessor(PsiElement element, ElementFilter filter, final boolean checkAccess, @Nullable Condition nameCondition) { + public JavaCompletionProcessor(PsiElement element, ElementFilter filter, final boolean checkAccess, boolean checkInitialized, @Nullable Condition nameCondition) { myCheckAccess = checkAccess; mySettings = CodeInsightSettings.getInstance(); myResults = new ArrayList(); @@ -102,6 +105,45 @@ public class JavaCompletionProcessor extends BaseScopeProcessor implements Eleme } } } + + if (checkInitialized) { + final PsiStatement statement = PsiTreeUtil.getParentOfType(element, PsiStatement.class); + final PsiMethod method = PsiTreeUtil.getParentOfType(element, PsiMethod.class, true, PsiClass.class); + if (statement != null && method != null && method.isConstructor()) { + final PsiClass containingClass = method.getContainingClass(); + assert containingClass != null; + for (PsiField field : containingClass.getFields()) { + if (!field.hasModifierProperty(PsiModifier.STATIC) && field.getInitializer() == null) { + myNonInitializedFields.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 + myNonInitializedFields.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")) { + myNonInitializedFields.clear(); + } + } + super.visitMethodCallExpression(expression); + } + }); + } + } } public void handleEvent(Event event, Object associated){ @@ -113,19 +155,24 @@ public class JavaCompletionProcessor extends BaseScopeProcessor implements Eleme } } - public boolean execute(PsiElement element, ResolveState state){ - if(!(element instanceof PsiClass) && element instanceof PsiModifierListOwner){ + public boolean execute(PsiElement element, ResolveState state) { + //noinspection SuspiciousMethodCalls + if (myNonInitializedFields.contains(element)) { + return true; + } + + if (!(element instanceof PsiClass) && element instanceof PsiModifierListOwner) { PsiModifierListOwner modifierListOwner = (PsiModifierListOwner)element; - if(myStatic){ - if(!modifierListOwner.hasModifierProperty(PsiModifier.STATIC)){ + if (myStatic) { + if (!modifierListOwner.hasModifierProperty(PsiModifier.STATIC)) { // we don't need non static method in static context. return true; } } - else{ - if(!mySettings.SHOW_STATIC_AFTER_INSTANCE - && modifierListOwner.hasModifierProperty(PsiModifier.STATIC) - && !myMembersFlag){ + else { + if (!mySettings.SHOW_STATIC_AFTER_INSTANCE + && modifierListOwner.hasModifierProperty(PsiModifier.STATIC) + && !myMembersFlag) { // according settings we don't need to process such fields/methods return true; } diff --git a/java/java-tests/testData/codeInsight/completion/normalSorting/DispreferDeclared.java b/java/java-tests/testData/codeInsight/completion/normalSorting/DispreferDeclared.java index e846cff17463..d2e0abca1ea5 100644 --- a/java/java-tests/testData/codeInsight/completion/normalSorting/DispreferDeclared.java +++ b/java/java-tests/testData/codeInsight/completion/normalSorting/DispreferDeclared.java @@ -2,6 +2,7 @@ public class Aaaaaaa { private final String aaa; Aaaaaaa(String aabbb) { + aaa = ""; aaa = true ? null : aa } diff --git a/java/java-tests/testData/codeInsight/completion/normalSorting/DispreferDeclaredOfExpectedType.java b/java/java-tests/testData/codeInsight/completion/normalSorting/DispreferDeclaredOfExpectedType.java index 5d79d9702a89..4acc0b1cc375 100644 --- a/java/java-tests/testData/codeInsight/completion/normalSorting/DispreferDeclaredOfExpectedType.java +++ b/java/java-tests/testData/codeInsight/completion/normalSorting/DispreferDeclaredOfExpectedType.java @@ -2,6 +2,7 @@ public class Aaaaaaa { private final String aaa; Aaaaaaa(Object aabbb) { + aaa = ""; aaa = true ? null : aa } diff --git a/java/java-tests/testData/codeInsight/completion/smartType/FieldsSetAbove-out.java b/java/java-tests/testData/codeInsight/completion/smartType/FieldsSetAbove-out.java new file mode 100644 index 000000000000..cc99ed5c463a --- /dev/null +++ b/java/java-tests/testData/codeInsight/completion/smartType/FieldsSetAbove-out.java @@ -0,0 +1,9 @@ +class A { + int doo; + int aaa; + A(int b) { + doo = b; + aaa = doo; + } +} + diff --git a/java/java-tests/testData/codeInsight/completion/smartType/FieldsSetAbove.java b/java/java-tests/testData/codeInsight/completion/smartType/FieldsSetAbove.java new file mode 100644 index 000000000000..bad8dd3a2920 --- /dev/null +++ b/java/java-tests/testData/codeInsight/completion/smartType/FieldsSetAbove.java @@ -0,0 +1,9 @@ +class A { + int doo; + int aaa; + A(int b) { + doo = b; + aaa = d + } +} + diff --git a/java/java-tests/testData/codeInsight/completion/smartType/FieldsSetInAnotherConstructor-out.java b/java/java-tests/testData/codeInsight/completion/smartType/FieldsSetInAnotherConstructor-out.java new file mode 100644 index 000000000000..f78838d00bf1 --- /dev/null +++ b/java/java-tests/testData/codeInsight/completion/smartType/FieldsSetInAnotherConstructor-out.java @@ -0,0 +1,13 @@ +class A { + int doo; + int aaa; + A(int b) { + doo = b; + } + + A(int aac, int aad) { + this(aac); + aaa = doo; + } +} + diff --git a/java/java-tests/testData/codeInsight/completion/smartType/FieldsSetInAnotherConstructor.java b/java/java-tests/testData/codeInsight/completion/smartType/FieldsSetInAnotherConstructor.java new file mode 100644 index 000000000000..51b47e14411a --- /dev/null +++ b/java/java-tests/testData/codeInsight/completion/smartType/FieldsSetInAnotherConstructor.java @@ -0,0 +1,13 @@ +class A { + int doo; + int aaa; + A(int b) { + doo = b; + } + + A(int aac, int aad) { + this(aac); + aaa = d + } +} + diff --git a/java/java-tests/testData/codeInsight/completion/smartType/NoFieldsInSuperConstructorCall-out.java b/java/java-tests/testData/codeInsight/completion/smartType/NoFieldsInSuperConstructorCall-out.java new file mode 100644 index 000000000000..dd4e04b8048a --- /dev/null +++ b/java/java-tests/testData/codeInsight/completion/smartType/NoFieldsInSuperConstructorCall-out.java @@ -0,0 +1,11 @@ +class A { + A(int a) {} +} + +class B extends A { + int aaa; + + B(int aab) { + super(aab); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/completion/smartType/NoFieldsInSuperConstructorCall.java b/java/java-tests/testData/codeInsight/completion/smartType/NoFieldsInSuperConstructorCall.java new file mode 100644 index 000000000000..654cfc42cf98 --- /dev/null +++ b/java/java-tests/testData/codeInsight/completion/smartType/NoFieldsInSuperConstructorCall.java @@ -0,0 +1,11 @@ +class A { + A(int a) {} +} + +class B extends A { + int aaa; + + B(int aab) { + super(a); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/completion/smartType/NoUninitializedFieldsInConstructor-out.java b/java/java-tests/testData/codeInsight/completion/smartType/NoUninitializedFieldsInConstructor-out.java new file mode 100644 index 000000000000..e49b17247f65 --- /dev/null +++ b/java/java-tests/testData/codeInsight/completion/smartType/NoUninitializedFieldsInConstructor-out.java @@ -0,0 +1,8 @@ +class A { + int aaa; + int aab; + A(int aac) { + aaa = aac; + } +} + diff --git a/java/java-tests/testData/codeInsight/completion/smartType/NoUninitializedFieldsInConstructor.java b/java/java-tests/testData/codeInsight/completion/smartType/NoUninitializedFieldsInConstructor.java new file mode 100644 index 000000000000..4d45189923f9 --- /dev/null +++ b/java/java-tests/testData/codeInsight/completion/smartType/NoUninitializedFieldsInConstructor.java @@ -0,0 +1,8 @@ +class A { + int aaa; + int aab; + A(int aac) { + aaa = a + } +} + diff --git a/java/java-tests/testData/codeInsight/completion/smartType/second/NonInitializedField-out.java b/java/java-tests/testData/codeInsight/completion/smartType/second/NonInitializedField-out.java new file mode 100644 index 000000000000..b74ac99da84f --- /dev/null +++ b/java/java-tests/testData/codeInsight/completion/smartType/second/NonInitializedField-out.java @@ -0,0 +1,7 @@ +class Foo { + int aaa; + int bbb; + Foo() { + aaa = bbb; + } +} \ No newline at end of file diff --git a/java/java-tests/testData/codeInsight/completion/smartType/second/NonInitializedField.java b/java/java-tests/testData/codeInsight/completion/smartType/second/NonInitializedField.java new file mode 100644 index 000000000000..e8eee5d953f6 --- /dev/null +++ b/java/java-tests/testData/codeInsight/completion/smartType/second/NonInitializedField.java @@ -0,0 +1,7 @@ +class Foo { + int aaa; + int bbb; + Foo() { + aaa = b + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/codeInsight/completion/SecondSmartTypeCompletionTest.java b/java/java-tests/testSrc/com/intellij/codeInsight/completion/SecondSmartTypeCompletionTest.java index 85c3caba21ec..3415ee36487e 100644 --- a/java/java-tests/testSrc/com/intellij/codeInsight/completion/SecondSmartTypeCompletionTest.java +++ b/java/java-tests/testSrc/com/intellij/codeInsight/completion/SecondSmartTypeCompletionTest.java @@ -7,8 +7,6 @@ import com.intellij.codeInsight.lookup.LookupElementDecorator; import com.intellij.codeInsight.lookup.LookupItem; import com.intellij.codeInsight.lookup.LookupManager; import com.intellij.codeInsight.lookup.impl.LookupImpl; -import com.intellij.openapi.projectRoots.Sdk; -import com.intellij.openapi.projectRoots.impl.JavaSdkImpl; import com.intellij.testFramework.IdeaTestUtil; import org.jetbrains.annotations.NonNls; @@ -67,6 +65,7 @@ public class SecondSmartTypeCompletionTest extends LightCompletionTestCase { checkResultByFile(BASE_PATH + "/" + getTestName(false) + "-out.java"); } + public void testNonInitializedField() throws Throwable { doTest(); } public void testIgnoreToString() throws Throwable { doTest(); } public void testDontIgnoreToStringInsideIt() throws Throwable { doTest(); } public void testDontIgnoreToStringInStringBuilders() throws Throwable { diff --git a/java/java-tests/testSrc/com/intellij/codeInsight/completion/SmartTypeCompletionTest.java b/java/java-tests/testSrc/com/intellij/codeInsight/completion/SmartTypeCompletionTest.java index ad197fd77080..7150b0a3630c 100644 --- a/java/java-tests/testSrc/com/intellij/codeInsight/completion/SmartTypeCompletionTest.java +++ b/java/java-tests/testSrc/com/intellij/codeInsight/completion/SmartTypeCompletionTest.java @@ -431,6 +431,11 @@ public class SmartTypeCompletionTest extends LightFixtureCompletionTestCase { public void testPrivateOverloads() throws Throwable { doTest(); } + public void testNoFieldsInSuperConstructorCall() throws Throwable { doTest(); } + public void testNoUninitializedFieldsInConstructor() throws Throwable { doTest(); } + public void testFieldsSetInAnotherConstructor() throws Throwable { doTest(); } + public void testFieldsSetAbove() throws Throwable { doTest(); } + public void testHonorSelection() throws Throwable { configureByTestName(); select();