From f060340474f4d42f715a36a2efb899c0f852aeac Mon Sep 17 00:00:00 2001 From: Pavel Dolgov Date: Fri, 16 Jun 2017 17:15:41 +0300 Subject: [PATCH] Java: Infer nullability annotations for extracted method's parameters (IDEA-150243) --- .../extractMethod/ExtractMethodProcessor.java | 117 +++++++++++++++++- .../extractMethod/NotNullArgument0.java | 13 ++ .../extractMethod/NotNullArgument0_after.java | 17 +++ .../extractMethod/NotNullArgument1.java | 18 +++ .../extractMethod/NotNullArgument1_after.java | 22 ++++ .../extractMethod/NotNullArgument2.java | 27 ++++ .../extractMethod/NotNullArgument2_after.java | 31 +++++ .../extractMethod/NotNullArgument3.java | 26 ++++ .../extractMethod/NotNullArgument3_after.java | 30 +++++ .../extractMethod/NotNullArgument4.java | 27 ++++ .../extractMethod/NotNullArgument4_after.java | 31 +++++ .../extractMethod/NotNullArgument5.java | 13 ++ .../extractMethod/NotNullArgument5_after.java | 18 +++ .../extractMethod/NotNullArgument6.java | 13 ++ .../extractMethod/NotNullArgument6_after.java | 17 +++ .../extractMethod/NotNullArgument7.java | 15 +++ .../extractMethod/NotNullArgument7_after.java | 20 +++ .../java/refactoring/ExtractMethodTest.java | 32 +++++ 18 files changed, 483 insertions(+), 4 deletions(-) create mode 100644 java/java-tests/testData/refactoring/extractMethod/NotNullArgument0.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/NotNullArgument0_after.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/NotNullArgument1.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/NotNullArgument1_after.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/NotNullArgument2.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/NotNullArgument2_after.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/NotNullArgument3.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/NotNullArgument3_after.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/NotNullArgument4.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/NotNullArgument4_after.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/NotNullArgument5.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/NotNullArgument5_after.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/NotNullArgument6.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/NotNullArgument6_after.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/NotNullArgument7.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/NotNullArgument7_after.java diff --git a/java/java-impl/src/com/intellij/refactoring/extractMethod/ExtractMethodProcessor.java b/java/java-impl/src/com/intellij/refactoring/extractMethod/ExtractMethodProcessor.java index 5f827d153138..6128e6450e8d 100644 --- a/java/java-impl/src/com/intellij/refactoring/extractMethod/ExtractMethodProcessor.java +++ b/java/java-impl/src/com/intellij/refactoring/extractMethod/ExtractMethodProcessor.java @@ -15,14 +15,12 @@ */ package com.intellij.refactoring.extractMethod; -import com.intellij.codeInsight.ChangeContextUtil; -import com.intellij.codeInsight.CodeInsightUtil; -import com.intellij.codeInsight.ExceptionUtil; -import com.intellij.codeInsight.NullableNotNullManager; +import com.intellij.codeInsight.*; import com.intellij.codeInsight.daemon.impl.analysis.JavaHighlightUtil; import com.intellij.codeInsight.daemon.impl.quickfix.AnonymousTargetClassPreselectionUtil; import com.intellij.codeInsight.generation.GenerateMembersUtil; import com.intellij.codeInsight.highlighting.HighlightManager; +import com.intellij.codeInsight.intention.AddAnnotationPsiFix; import com.intellij.codeInsight.intention.impl.AddNullableNotNullAnnotationFix; import com.intellij.codeInsight.navigation.NavigationUtil; import com.intellij.codeInspection.dataFlow.*; @@ -1442,10 +1440,121 @@ public class ExtractMethodProcessor implements MatchProvider { PsiModifierList parmModifierList = parm.getModifierList(); LOG.assertTrue(parmModifierList != null); GenerateMembersUtil.copyAnnotations(modifierList, parmModifierList, SuppressWarnings.class.getName()); + + final NullableNotNullManager nullabilityManager = NullableNotNullManager.getInstance(myProject); + if (AnnotationUtil.isAnnotated(variable, nullabilityManager.getNullables()) || + AnnotationUtil.isAnnotated(variable, nullabilityManager.getNotNulls()) || + PropertiesComponent.getInstance(myProject).getBoolean(ExtractMethodDialog.EXTRACT_METHOD_GENERATE_ANNOTATIONS, false)) { + final Nullness definitelyNotNull = getDefinitelyNotNull((PsiParameter)variable); + final String toAdd; + final List toKeep; + final List toRemove; + switch (definitelyNotNull) { + case NOT_NULL: + toAdd = nullabilityManager.getDefaultNotNull(); + toKeep = nullabilityManager.getNotNulls(); + toRemove = nullabilityManager.getNullables(); + break; + case NULLABLE: + toAdd = nullabilityManager.getDefaultNullable(); + toKeep = nullabilityManager.getNullables(); + toRemove = nullabilityManager.getNotNulls(); + break; + default: + return; + } + AddAnnotationPsiFix.removePhysicalAnnotations(parm, toRemove.toArray(ArrayUtil.EMPTY_STRING_ARRAY)); + if (!AnnotationUtil.isAnnotated(parm, toKeep)) { + final PsiAnnotation added = AddAnnotationPsiFix.addPhysicalAnnotation(toAdd, PsiNameValuePair.EMPTY_ARRAY, parmModifierList); + JavaCodeStyleManager.getInstance(myProject).shortenClassReferences(added); + } + } } } } + @NotNull + private Nullness getDefinitelyNotNull(@NotNull PsiParameter variable) { + if (variable.getType() instanceof PsiPrimitiveType) { + return Nullness.UNKNOWN; + } + + PsiElement parent = variable.getParent(); + if (parent instanceof PsiParameterList) { + final PsiElement grandParent = parent.getParent(); + String originalMethodText = null; + int extractedCodeRelativeOffset = 0; + + // DFA doesn't work with a part of method body or with lambda body when checking a method/lambda parameter + // we have to copy the whole method or convert the whole lambda to a method + if (grandParent instanceof PsiMethod) { + originalMethodText = grandParent.getText(); + final int methodOffset = grandParent.getTextRange().getStartOffset(); + final int extractOffset = myElements[0].getTextRange().getStartOffset(); + extractedCodeRelativeOffset = extractOffset - methodOffset; + } + else if (grandParent instanceof PsiLambdaExpression) { + final PsiLambdaExpression lambdaExpression = (PsiLambdaExpression)grandParent; + if (lambdaExpression.hasFormalParameterTypes()) { + final PsiElement lambdaBody = lambdaExpression.getBody(); + if (lambdaBody instanceof PsiCodeBlock) { + final PsiMethod interfaceMethod = LambdaUtil.getFunctionalInterfaceMethod(grandParent); + if (interfaceMethod != null) { + PsiType returnType = interfaceMethod.getReturnType(); + if (returnType != null) { + final PsiParameterList parameterList = lambdaExpression.getParameterList(); + final String dummyMethodHeader = returnType.getCanonicalText() + " " + interfaceMethod.getName() + parameterList.getText(); + originalMethodText = dummyMethodHeader + lambdaBody.getText(); + + final int bodyOffset = lambdaBody.getTextRange().getStartOffset(); + final int extractOffset = myElements[0].getTextRange().getStartOffset(); + extractedCodeRelativeOffset = extractOffset - bodyOffset + dummyMethodHeader.length(); + } + } + } + } + } + if (originalMethodText != null) { + // insert a dummy usage of the variable before the extracted fragment, where we're going to check the nullness of the variable + final String dummyMethodText = originalMethodText.substring(0, extractedCodeRelativeOffset) + + "Object _Dummy_ = " + variable.getName() + ";" + + originalMethodText.substring(extractedCodeRelativeOffset); + + final PsiElementFactory factory = JavaPsiFacade.getInstance(myProject).getElementFactory(); + final PsiMethod dummyMethod; + try { + dummyMethod = factory.createMethodFromText(dummyMethodText, grandParent.getParent()); + } + catch (IncorrectOperationException e) { + LOG.debug("Failed to parse dummy method", dummyMethodText); // probably incomplete code + return Nullness.UNKNOWN; + } + PsiElement atOffset = dummyMethod.findElementAt(extractedCodeRelativeOffset); + while (atOffset != null && atOffset.getStartOffsetInParent() == 0) { + atOffset = atOffset.getParent(); + } + if (atOffset instanceof PsiDeclarationStatement) { + final PsiElement[] declaredElements = ((PsiDeclarationStatement)atOffset).getDeclaredElements(); + if (declaredElements.length == 1) { + final PsiElement declaredElement = declaredElements[0]; + if (declaredElement instanceof PsiLocalVariable) { + final PsiExpression initializer = ((PsiLocalVariable)declaredElement).getInitializer(); + if (initializer instanceof PsiReferenceExpression) { + final int parameterIndex = ((PsiParameterList)parent).getParameterIndex(variable); + final PsiParameter dummyParameter = dummyMethod.getParameterList().getParameters()[parameterIndex]; + if (((PsiReferenceExpression)initializer).isReferenceTo(dummyParameter)) { + final Nullness nullness = DfaUtil.checkNullness(dummyParameter, initializer); + return nullness == Nullness.NOT_NULL ? Nullness.NOT_NULL : Nullness.NULLABLE; // 'unknown' counts as 'nullable' + } + } + } + } + } + } + } + return Nullness.UNKNOWN; + } + @NotNull protected PsiMethodCallExpression generateMethodCall(PsiExpression instanceQualifier, final boolean generateArgs) throws IncorrectOperationException { @NonNls StringBuilder buffer = new StringBuilder(); diff --git a/java/java-tests/testData/refactoring/extractMethod/NotNullArgument0.java b/java/java-tests/testData/refactoring/extractMethod/NotNullArgument0.java new file mode 100644 index 000000000000..c140f34fef43 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/NotNullArgument0.java @@ -0,0 +1,13 @@ +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +class C { + void f(@Nullable Object o) { + if (o != null) { + g(o); + } + } + + void g(@NotNull Object o) { + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/NotNullArgument0_after.java b/java/java-tests/testData/refactoring/extractMethod/NotNullArgument0_after.java new file mode 100644 index 000000000000..2998cf4143bb --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/NotNullArgument0_after.java @@ -0,0 +1,17 @@ +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +class C { + void f(@Nullable Object o) { + if (o != null) { + newMethod(o); + } + } + + private void newMethod(@NotNull Object o) { + g(o); + } + + void g(@NotNull Object o) { + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/NotNullArgument1.java b/java/java-tests/testData/refactoring/extractMethod/NotNullArgument1.java new file mode 100644 index 000000000000..d2e6bdb3f777 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/NotNullArgument1.java @@ -0,0 +1,18 @@ +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +class C { + void f(@Nullable Object o) { + if (o != null) { + if (o instanceof String) { + o = 1; + } else { + System.out.println(o); + } + g(o); + } + } + + void g(@NotNull Object o) { + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/NotNullArgument1_after.java b/java/java-tests/testData/refactoring/extractMethod/NotNullArgument1_after.java new file mode 100644 index 000000000000..b404d0396802 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/NotNullArgument1_after.java @@ -0,0 +1,22 @@ +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +class C { + void f(@Nullable Object o) { + if (o != null) { + newMethod(o); + } + } + + private void newMethod(@NotNull Object o) { + if (o instanceof String) { + o = 1; + } else { + System.out.println(o); + } + g(o); + } + + void g(@NotNull Object o) { + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/NotNullArgument2.java b/java/java-tests/testData/refactoring/extractMethod/NotNullArgument2.java new file mode 100644 index 000000000000..4bd157e273c9 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/NotNullArgument2.java @@ -0,0 +1,27 @@ +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +class C { + void f() { + I i = new I() { + @Override + public void m(@Nullable Object o) { + if (o != null) { + if (o instanceof String) { + o = 2; + } else { + System.out.println(o); + } + g(o); + } + } + }; + } + + void g(@NotNull Object o) { + } + + interface I { + void m(Object o); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/NotNullArgument2_after.java b/java/java-tests/testData/refactoring/extractMethod/NotNullArgument2_after.java new file mode 100644 index 000000000000..9d7bd8e22367 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/NotNullArgument2_after.java @@ -0,0 +1,31 @@ +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +class C { + void f() { + I i = new I() { + @Override + public void m(@Nullable Object o) { + if (o != null) { + newMethod(o); + } + } + }; + } + + private void newMethod(@NotNull Object o) { + if (o instanceof String) { + o = 2; + } else { + System.out.println(o); + } + g(o); + } + + void g(@NotNull Object o) { + } + + interface I { + void m(Object o); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/NotNullArgument3.java b/java/java-tests/testData/refactoring/extractMethod/NotNullArgument3.java new file mode 100644 index 000000000000..e7044849c3f7 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/NotNullArgument3.java @@ -0,0 +1,26 @@ +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +class C { + void f() { + I i = (@Nullable Object o) -> { + if (o != null) { + if (o instanceof String) { + o = 3; + } + else { + System.out.println(o); + } + g(o); + } + }; + + } + + void g(@NotNull Object o) { + } + + interface I { + void m(Object o); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/NotNullArgument3_after.java b/java/java-tests/testData/refactoring/extractMethod/NotNullArgument3_after.java new file mode 100644 index 000000000000..cb9d5ee20d8d --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/NotNullArgument3_after.java @@ -0,0 +1,30 @@ +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +class C { + void f() { + I i = (@Nullable Object o) -> { + if (o != null) { + newMethod(o); + } + }; + + } + + private void newMethod(@NotNull Object o) { + if (o instanceof String) { + o = 3; + } + else { + System.out.println(o); + } + g(o); + } + + void g(@NotNull Object o) { + } + + interface I { + void m(Object o); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/NotNullArgument4.java b/java/java-tests/testData/refactoring/extractMethod/NotNullArgument4.java new file mode 100644 index 000000000000..d45ea5fad989 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/NotNullArgument4.java @@ -0,0 +1,27 @@ +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +class C { + void f() { + I i = (@Nullable String o) -> { + if (o != null) { + if (o instanceof String) { + o = "4"; + } + else { + System.out.println(o); + } + g(o); + } + return ""; + }; + + } + + void g(@NotNull Object o) { + } + + interface I { + R m(T t); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/NotNullArgument4_after.java b/java/java-tests/testData/refactoring/extractMethod/NotNullArgument4_after.java new file mode 100644 index 000000000000..4eaffb2c7a93 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/NotNullArgument4_after.java @@ -0,0 +1,31 @@ +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +class C { + void f() { + I i = (@Nullable String o) -> { + if (o != null) { + newMethod(o); + } + return ""; + }; + + } + + private void newMethod(@NotNull String o) { + if (o instanceof String) { + o = "4"; + } + else { + System.out.println(o); + } + g(o); + } + + void g(@NotNull Object o) { + } + + interface I { + R m(T t); + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/NotNullArgument5.java b/java/java-tests/testData/refactoring/extractMethod/NotNullArgument5.java new file mode 100644 index 000000000000..7bb38219dce8 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/NotNullArgument5.java @@ -0,0 +1,13 @@ +import org.jetbrains.annotations.NotNull; + +class C { + void f(@NotNull Object o, boolean b) { + if (b) { + o = null; + } + g(o); + } + + void g(Object o) { + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/NotNullArgument5_after.java b/java/java-tests/testData/refactoring/extractMethod/NotNullArgument5_after.java new file mode 100644 index 000000000000..3db2701c3d42 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/NotNullArgument5_after.java @@ -0,0 +1,18 @@ +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +class C { + void f(@NotNull Object o, boolean b) { + if (b) { + o = null; + } + newMethod(o); + } + + private void newMethod(@Nullable Object o) { + g(o); + } + + void g(Object o) { + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/NotNullArgument6.java b/java/java-tests/testData/refactoring/extractMethod/NotNullArgument6.java new file mode 100644 index 000000000000..246ebaf0c729 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/NotNullArgument6.java @@ -0,0 +1,13 @@ +import org.jetbrains.annotations.Nullable; + +class C { + void f(@Nullable Object o, boolean b) { + if (b) { + o = null; + } + g(o); + } + + void g(Object o) { + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/NotNullArgument6_after.java b/java/java-tests/testData/refactoring/extractMethod/NotNullArgument6_after.java new file mode 100644 index 000000000000..a6673f287084 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/NotNullArgument6_after.java @@ -0,0 +1,17 @@ +import org.jetbrains.annotations.Nullable; + +class C { + void f(@Nullable Object o, boolean b) { + if (b) { + o = null; + } + newMethod(o); + } + + private void newMethod(@Nullable Object o) { + g(o); + } + + void g(Object o) { + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/NotNullArgument7.java b/java/java-tests/testData/refactoring/extractMethod/NotNullArgument7.java new file mode 100644 index 000000000000..9c555c7d4b7d --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/NotNullArgument7.java @@ -0,0 +1,15 @@ +import org.jetbrains.annotations.Nullable; + +class C { + void f(@Nullable Object o, boolean b) { + while (o != null) { + if (b) { + o = 7; + } + g(o); + } + } + + void g(Object o) { + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/NotNullArgument7_after.java b/java/java-tests/testData/refactoring/extractMethod/NotNullArgument7_after.java new file mode 100644 index 000000000000..a1ad2fc1e0d1 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/NotNullArgument7_after.java @@ -0,0 +1,20 @@ +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +class C { + void f(@Nullable Object o, boolean b) { + while (o != null) { + if (b) { + o = 7; + } + newMethod(o); + } + } + + private void newMethod(@NotNull Object o) { + g(o); + } + + void g(Object o) { + } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodTest.java b/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodTest.java index 609292caf8c0..ebfb623c25a2 100644 --- a/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodTest.java +++ b/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodTest.java @@ -922,6 +922,38 @@ public class ExtractMethodTest extends LightCodeInsightTestCase { doTest(); } + public void testNotNullArgument0() throws Exception { + doTest(); + } + + public void testNotNullArgument1() throws Exception { + doTest(); + } + + public void testNotNullArgument2() throws Exception { + doTest(); + } + + public void testNotNullArgument3() throws Exception { + doTest(); + } + + public void testNotNullArgument4() throws Exception { + doTest(); + } + + public void testNotNullArgument5() throws Exception { + doTest(); + } + + public void testNotNullArgument6() throws Exception { + doTest(); + } + + public void testNotNullArgument7() throws Exception { + doTest(); + } + public void testQualifyWhenConflictingNamePresent() throws Exception { final CodeStyleSettings settings = CodeStyleSettingsManager.getSettings(getProject()); settings.ELSE_ON_NEW_LINE = true;