From bed291f661355e6adca2bce194db2f8e34a63df8 Mon Sep 17 00:00:00 2001 From: Tagir Valeev Date: Wed, 27 May 2020 13:31:34 +0700 Subject: [PATCH] Simpler and more correct fix for IDEA-240933 Review ID: IDEA-CR-63039 GitOrigin-RevId: 378ecb574a8228992f4cdfa398cc0cacb95e52dd --- .../introduceVariable/VariableExtractor.java | 49 +++++-------------- .../NullabilityAnnotationConflict.after.java | 18 +++++++ .../NullabilityAnnotationConflict.java | 18 +++++++ ...NullabilityAnnotationNoConflict.after.java | 18 +++++++ .../NullabilityAnnotationNoConflict.java | 18 +++++++ .../refactoring/IntroduceVariableTest.java | 8 +++ 6 files changed, 92 insertions(+), 37 deletions(-) create mode 100644 java/java-tests/testData/refactoring/introduceVariable/NullabilityAnnotationConflict.after.java create mode 100644 java/java-tests/testData/refactoring/introduceVariable/NullabilityAnnotationConflict.java create mode 100644 java/java-tests/testData/refactoring/introduceVariable/NullabilityAnnotationNoConflict.after.java create mode 100644 java/java-tests/testData/refactoring/introduceVariable/NullabilityAnnotationNoConflict.java diff --git a/java/java-impl/src/com/intellij/refactoring/introduceVariable/VariableExtractor.java b/java/java-impl/src/com/intellij/refactoring/introduceVariable/VariableExtractor.java index 1fa902254200..58d7ea50e0c1 100644 --- a/java/java-impl/src/com/intellij/refactoring/introduceVariable/VariableExtractor.java +++ b/java/java-impl/src/com/intellij/refactoring/introduceVariable/VariableExtractor.java @@ -2,10 +2,10 @@ package com.intellij.refactoring.introduceVariable; import com.intellij.codeInsight.BlockUtils; -import com.intellij.codeInsight.Nullability; import com.intellij.codeInsight.NullabilityAnnotationInfo; import com.intellij.codeInsight.NullableNotNullManager; import com.intellij.codeInsight.daemon.impl.analysis.HighlightingFeature; +import com.intellij.codeInspection.dataFlow.DfaPsiUtil; import com.intellij.openapi.application.ApplicationManager; import com.intellij.openapi.diagnostic.Attachment; import com.intellij.openapi.diagnostic.Logger; @@ -23,17 +23,17 @@ import com.intellij.psi.util.PsiUtil; import com.intellij.refactoring.introduceField.ElementToWorkOn; import com.intellij.refactoring.util.FieldConflictsResolver; import com.intellij.refactoring.util.RefactoringUtil; -import com.intellij.util.ArrayUtilRt; -import com.intellij.util.IncorrectOperationException; -import com.intellij.util.ObjectUtils; -import com.intellij.util.ThreeState; +import com.intellij.util.*; import com.intellij.util.containers.ContainerUtil; import com.siyeh.ig.psiutils.*; import one.util.streamex.StreamEx; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import java.util.*; +import java.util.Collections; +import java.util.Comparator; +import java.util.Objects; +import java.util.Set; /** * Performs actual write action (see {@link #extractVariable()}) which introduces new variable and replaces all occurrences. @@ -252,37 +252,12 @@ class VariableExtractor { Project project = expression.getProject(); NullabilityAnnotationInfo nullabilityAnnotationInfo = NullableNotNullManager.getInstance(project).findExplicitNullability((PsiLocalVariable)probe.getDeclaredElements()[0]); - - final PsiAnnotation[] annotations = type.getAnnotations(); - return type.annotate(new TypeAnnotationProvider() { - @Override - public PsiAnnotation @NotNull [] getAnnotations() { - final NullableNotNullManager manager = NullableNotNullManager.getInstance(project); - final Set nullables = new HashSet<>(); - Nullability nullability = nullabilityAnnotationInfo != null ? nullabilityAnnotationInfo.getNullability() : Nullability.UNKNOWN; - if (nullability == Nullability.UNKNOWN) { - nullables.addAll(manager.getNotNulls()); - nullables.addAll(manager.getNullables()); - } - else if (nullability == Nullability.NOT_NULL) { - nullables.addAll(manager.getNotNulls()); - } - else if (nullability == Nullability.NULLABLE){ - nullables.addAll(manager.getNullables()); - } - return Arrays.stream(annotations) - .filter(annotation -> { - String qualifiedName = annotation.getQualifiedName(); - if (!manager.getNotNulls().contains(qualifiedName) && - !manager.getNullables().contains(qualifiedName)) { - return false; - } - - return !nullables.contains(qualifiedName); - }) - .toArray(PsiAnnotation[]::new); - } - }); + NullabilityAnnotationInfo info = DfaPsiUtil.getTypeNullabilityInfo(type); + if (info != null && nullabilityAnnotationInfo != null && info.getNullability() != nullabilityAnnotationInfo.getNullability() && + ArrayUtil.contains(info.getAnnotation(), type.getAnnotations())) { + return type.annotate(TypeAnnotationProvider.Static.create(new PsiAnnotation[]{info.getAnnotation()})); + } + return type.annotate(TypeAnnotationProvider.EMPTY); } @NotNull diff --git a/java/java-tests/testData/refactoring/introduceVariable/NullabilityAnnotationConflict.after.java b/java/java-tests/testData/refactoring/introduceVariable/NullabilityAnnotationConflict.after.java new file mode 100644 index 000000000000..b7ef460bc486 --- /dev/null +++ b/java/java-tests/testData/refactoring/introduceVariable/NullabilityAnnotationConflict.after.java @@ -0,0 +1,18 @@ +package org.eclipse.jdt.annotation; + +import java.lang.annotation.*; + +@interface NonNullByDefault {} +@Target(ElementType.TYPE_USE) +@interface Nullable {} + +@NonNullByDefault +class X { + void test() { + @Nullable String x = Y.getFoo(); + } +} + +class Y { + static @Nullable String getFoo() { return null; } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/introduceVariable/NullabilityAnnotationConflict.java b/java/java-tests/testData/refactoring/introduceVariable/NullabilityAnnotationConflict.java new file mode 100644 index 000000000000..509603f03e1d --- /dev/null +++ b/java/java-tests/testData/refactoring/introduceVariable/NullabilityAnnotationConflict.java @@ -0,0 +1,18 @@ +package org.eclipse.jdt.annotation; + +import java.lang.annotation.*; + +@interface NonNullByDefault {} +@Target(ElementType.TYPE_USE) +@interface Nullable {} + +@NonNullByDefault +class X { + void test() { + Y.getFoo(); + } +} + +class Y { + static @Nullable String getFoo() { return null; } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/introduceVariable/NullabilityAnnotationNoConflict.after.java b/java/java-tests/testData/refactoring/introduceVariable/NullabilityAnnotationNoConflict.after.java new file mode 100644 index 000000000000..88660cb07b66 --- /dev/null +++ b/java/java-tests/testData/refactoring/introduceVariable/NullabilityAnnotationNoConflict.after.java @@ -0,0 +1,18 @@ +package org.eclipse.jdt.annotation; + +import java.lang.annotation.*; + +@interface NonNullByDefault {} +@Target(ElementType.TYPE_USE) +@interface NonNull {} + +@NonNullByDefault +class X { + void test() { + String x = Y.getFoo(); + } +} + +class Y { + static @NonNull String getFoo() { return null; } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/introduceVariable/NullabilityAnnotationNoConflict.java b/java/java-tests/testData/refactoring/introduceVariable/NullabilityAnnotationNoConflict.java new file mode 100644 index 000000000000..fe1b388afd30 --- /dev/null +++ b/java/java-tests/testData/refactoring/introduceVariable/NullabilityAnnotationNoConflict.java @@ -0,0 +1,18 @@ +package org.eclipse.jdt.annotation; + +import java.lang.annotation.*; + +@interface NonNullByDefault {} +@Target(ElementType.TYPE_USE) +@interface NonNull {} + +@NonNullByDefault +class X { + void test() { + Y.getFoo(); + } +} + +class Y { + static @NonNull String getFoo() { return null; } +} \ No newline at end of file diff --git a/java/java-tests/testSrc/com/intellij/java/refactoring/IntroduceVariableTest.java b/java/java-tests/testSrc/com/intellij/java/refactoring/IntroduceVariableTest.java index 81c0c3906d26..1f4386022a41 100644 --- a/java/java-tests/testSrc/com/intellij/java/refactoring/IntroduceVariableTest.java +++ b/java/java-tests/testSrc/com/intellij/java/refactoring/IntroduceVariableTest.java @@ -397,6 +397,14 @@ public class IntroduceVariableTest extends LightJavaCodeInsightTestCase { public void testChooseTypeExpressionWhenNotDenotable() { doTest("m", false, false, false, "Foo"); } public void testChooseTypeExpressionWhenNotDenotable1() { doTest("m", false, false, false, "Foo"); } + public void testNullabilityAnnotationConflict() { + doTest("x", true, false, false, "java.lang.@org.eclipse.jdt.annotation.Nullable String"); + } + + public void testNullabilityAnnotationNoConflict() { + doTest("x", true, false, false, "java.lang.@org.eclipse.jdt.annotation.NonNull String"); + } + private void doTestWithVarType(IntroduceVariableBase testMe) { Boolean asVarType = JavaRefactoringSettings.getInstance().INTRODUCE_LOCAL_CREATE_VAR_TYPE; try {