From 65fb327b93e6edab4f5dadcb22ffbde2bd0745c4 Mon Sep 17 00:00:00 2001 From: Bas Leijdekkers Date: Thu, 14 Mar 2019 17:57:31 +0100 Subject: [PATCH] fix NPE in Java Extract Method (IDEA-199784) --- .../refactoring/util/VariableData.java | 23 ++++---------- .../extractMethod/ExtractMethodProcessor.java | 24 +++++++-------- .../refactoring/extractMethod/NoNPE1.java | 17 +++++++++++ .../extractMethod/NoNPE1_after.java | 25 ++++++++++++++++ .../refactoring/extractMethod/NoNPE2.java | 25 ++++++++++++++++ .../extractMethod/NoNPE2_after.java | 30 +++++++++++++++++++ .../java/refactoring/ExtractMethodTest.java | 24 ++++++--------- 7 files changed, 123 insertions(+), 45 deletions(-) create mode 100644 java/java-tests/testData/refactoring/extractMethod/NoNPE1.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/NoNPE1_after.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/NoNPE2.java create mode 100644 java/java-tests/testData/refactoring/extractMethod/NoNPE2_after.java diff --git a/java/java-analysis-impl/src/com/intellij/refactoring/util/VariableData.java b/java/java-analysis-impl/src/com/intellij/refactoring/util/VariableData.java index 03b125c4a4fe..4a21c555b80c 100644 --- a/java/java-analysis-impl/src/com/intellij/refactoring/util/VariableData.java +++ b/java/java-analysis-impl/src/com/intellij/refactoring/util/VariableData.java @@ -1,21 +1,8 @@ -/* - * 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. - */ +// Copyright 2000-2019 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. package com.intellij.refactoring.util; import com.intellij.psi.*; +import com.intellij.psi.impl.source.PsiImmediateClassType; import com.intellij.psi.search.GlobalSearchScope; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -48,8 +35,10 @@ public class VariableData extends AbstractVariableData { return this; } // The copied type needs to be valid in a non-physical copy of the original file. - // If the type references a class or a type variable declared in the original file, it might not work in the copy. - PsiType type = JavaPsiFacade.getElementFactory(var.getProject()).createTypeFromText(this.type.getCanonicalText(), var); + // If the type references a type variable declared in the original file, it might not work in the copy. + PsiType type = this.type instanceof PsiImmediateClassType && ((PsiImmediateClassType)this.type).resolve() instanceof PsiTypeParameter + ? JavaPsiFacade.getElementFactory(var.getProject()).createTypeFromText(this.type.getCanonicalText(), var) + : this.type; VariableData data = new VariableData(var, type); data.name = this.name; data.originalName = this.originalName; 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 61213b822fc8..e5085564b17a 100644 --- a/java/java-impl/src/com/intellij/refactoring/extractMethod/ExtractMethodProcessor.java +++ b/java/java-impl/src/com/intellij/refactoring/extractMethod/ExtractMethodProcessor.java @@ -1,4 +1,4 @@ -// Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. +// Copyright 2000-2019 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. package com.intellij.refactoring.extractMethod; import com.intellij.application.options.CodeStyle; @@ -944,7 +944,7 @@ public class ExtractMethodProcessor implements MatchProvider { LOG.assertTrue(myElements[0].isValid()); PsiCodeBlock body = newMethod.getBody(); - myMethodCall = generateMethodCall(null, true); + myMethodCall = generateMethodCall(null, true, myExpression); LOG.assertTrue(myElements[0].isValid()); @@ -1053,14 +1053,12 @@ public class ExtractMethodProcessor implements MatchProvider { final PsiType paramType = psiParameter.getType(); for (PsiReference reference : ReferencesSearch.search(psiParameter, new LocalSearchScope(body))){ final PsiElement element = reference.getElement(); - if (element != null) { - final PsiElement parent = element.getParent(); - if (parent instanceof PsiTypeCastExpression) { - final PsiTypeCastExpression typeCastExpression = (PsiTypeCastExpression)parent; - final PsiTypeElement castType = typeCastExpression.getCastType(); - if (castType != null && Comparing.equal(castType.getType(), paramType)) { - RemoveRedundantCastUtil.removeCast(typeCastExpression); - } + final PsiElement parent = element.getParent(); + if (parent instanceof PsiTypeCastExpression) { + final PsiTypeCastExpression typeCastExpression = (PsiTypeCastExpression)parent; + final PsiTypeElement castType = typeCastExpression.getCastType(); + if (castType != null && Comparing.equal(castType.getType(), paramType)) { + RemoveRedundantCastUtil.removeCast(typeCastExpression); } } } @@ -1340,7 +1338,7 @@ public class ExtractMethodProcessor implements MatchProvider { RefactoringUtil.isInStaticContext(match.getMatchStart(), myExtractedMethod.getContainingClass())) { PsiUtil.setModifierProperty(myExtractedMethod, PsiModifier.STATIC, true); } - final PsiMethodCallExpression methodCallExpression = generateMethodCall(match.getInstanceExpression(), false); + final PsiMethodCallExpression methodCallExpression = generateMethodCall(match.getInstanceExpression(), false, match.getMatchStart()); ArrayList datas = new ArrayList<>(); for (final VariableData variableData : myVariableDatum) { @@ -1779,7 +1777,7 @@ public class ExtractMethodProcessor implements MatchProvider { } @NotNull - protected PsiMethodCallExpression generateMethodCall(PsiExpression instanceQualifier, final boolean generateArgs) throws IncorrectOperationException { + protected PsiMethodCallExpression generateMethodCall(PsiExpression instanceQualifier, final boolean generateArgs, PsiElement context) { @NonNls StringBuilder buffer = new StringBuilder(); final boolean skipInstanceQualifier; @@ -1831,7 +1829,7 @@ public class ExtractMethodProcessor implements MatchProvider { buffer.append(")"); String text = buffer.toString(); - PsiMethodCallExpression expr = (PsiMethodCallExpression)myElementFactory.createExpressionFromText(text, null); + PsiMethodCallExpression expr = (PsiMethodCallExpression)myElementFactory.createExpressionFromText(text, context); expr = (PsiMethodCallExpression)myStyleManager.reformat(expr); if (!skipInstanceQualifier) { PsiExpression qualifierExpression = expr.getMethodExpression().getQualifierExpression(); diff --git a/java/java-tests/testData/refactoring/extractMethod/NoNPE1.java b/java/java-tests/testData/refactoring/extractMethod/NoNPE1.java new file mode 100644 index 000000000000..6150f3d359b4 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/NoNPE1.java @@ -0,0 +1,17 @@ +package extractMethod; + +class Zzz { + + protected void doAction(C c, boolean b) { + if (b) { + c.foo(() -> c.bar()); + } + else { + c.foo(() -> c.bar()); + } + } + private class C { + void foo(Runnable r) {} + void bar() {} + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/NoNPE1_after.java b/java/java-tests/testData/refactoring/extractMethod/NoNPE1_after.java new file mode 100644 index 000000000000..6c7711325c22 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/NoNPE1_after.java @@ -0,0 +1,25 @@ +package extractMethod; + +import org.jetbrains.annotations.NotNull; + +class Zzz { + + protected void doAction(C c, boolean b) { + if (b) { + c.foo(newMethod(c)); + } + else { + c.foo(newMethod(c)); + } + } + + @NotNull + private Runnable newMethod(C c) { + return () -> c.bar(); + } + + private class C { + void foo(Runnable r) {} + void bar() {} + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/NoNPE2.java b/java/java-tests/testData/refactoring/extractMethod/NoNPE2.java new file mode 100644 index 000000000000..beafddbd53e5 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/NoNPE2.java @@ -0,0 +1,25 @@ +import java.util.Iterator; +public class ConcatIterables { + class ConcatenatingIterable implements Iterable { + ImmutableQueue> iterables; + public ConcatenatingIterable(Iterable xs, Iterable ys) { + ((ConcatenatingIterable) ys).iterables.pushFront(xs); + } + @Override + public Iterator iterator() { + return null; + } + } + static class ImmutableQueue implements Iterable { + public static ImmutableQueue empty() { + return new ImmutableQueue<>(); + } + @Override + public Iterator iterator() { + return null; + } + ImmutableQueue pushFront(A a) { + return null; + } + } +} \ No newline at end of file diff --git a/java/java-tests/testData/refactoring/extractMethod/NoNPE2_after.java b/java/java-tests/testData/refactoring/extractMethod/NoNPE2_after.java new file mode 100644 index 000000000000..8b1a72673c76 --- /dev/null +++ b/java/java-tests/testData/refactoring/extractMethod/NoNPE2_after.java @@ -0,0 +1,30 @@ +import java.util.Iterator; +public class ConcatIterables { + class ConcatenatingIterable implements Iterable { + ImmutableQueue> iterables; + public ConcatenatingIterable(Iterable xs, Iterable ys) { + newMethod(xs, (ConcatenatingIterable) ys); + } + + private ImmutableQueue> newMethod(Iterable xs, ConcatenatingIterable ys) { + return ys.iterables.pushFront(xs); + } + + @Override + public Iterator iterator() { + return null; + } + } + static class ImmutableQueue implements Iterable { + public static ImmutableQueue empty() { + return new ImmutableQueue<>(); + } + @Override + public Iterator iterator() { + return null; + } + ImmutableQueue pushFront(A a) { + return null; + } + } +} \ 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 555f2f824d7a..4032688b4ced 100644 --- a/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodTest.java +++ b/java/java-tests/testSrc/com/intellij/java/refactoring/ExtractMethodTest.java @@ -1,18 +1,4 @@ -/* - * Copyright 2000-2017 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. - */ +// Copyright 2000-2019 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. package com.intellij.java.refactoring; import com.intellij.JavaTestUtil; @@ -977,6 +963,14 @@ public class ExtractMethodTest extends LightCodeInsightTestCase { doTest(); } + public void testNoNPE1() throws Exception { + doTest(); + } + + public void testNoNPE2() throws Exception { + doTest(); + } + public void testTheOnlyParenthesisExpressionWhichIsSkippedInControlFlow() throws Exception { doTest(); }