fix NPE in Java Extract Method (IDEA-199784)

2026-09-27 10:03:11 +07:00 · 2019-03-14 17:59:35 +01:00
parent 5f578b9250
commit 65fb327b93
7 changed files with 123 additions and 45 deletions
@@ -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;
@@ -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<VariableData> 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();
@@ -0,0 +1,17 @@
+package extractMethod;
+
+class Zzz<R> {
+
+  protected void doAction(C c, boolean b) {
+    if (b) {
+      c.foo(<selection>() -> c.bar()</selection>);
+    }
+    else {
+      c.foo(() -> c.bar());
+    }
+  }
+  private class C {
+    void foo(Runnable r) {}
+    void bar() {}
+  }
+}
@@ -0,0 +1,25 @@
+package extractMethod;
+
+import org.jetbrains.annotations.NotNull;
+
+class Zzz<R> {
+
+  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() {}
+  }
+}
@@ -0,0 +1,25 @@
+import java.util.Iterator;
+public class ConcatIterables {
+  class ConcatenatingIterable<A> implements Iterable<A> {
+    ImmutableQueue<Iterable<A>> iterables;
+    public ConcatenatingIterable(Iterable<A> xs, Iterable<A> ys) {
+      <selection>((ConcatenatingIterable<A>) ys).iterables.pushFront(xs)</selection>;
+    }
+    @Override
+    public Iterator<A> iterator() {
+      return null;
+    }
+  }
+  static class ImmutableQueue<A> implements Iterable<A> {
+    public static <A> ImmutableQueue<A> empty() {
+      return new ImmutableQueue<>();
+    }
+    @Override
+    public Iterator<A> iterator() {
+      return null;
+    }
+    ImmutableQueue<A> pushFront(A a) {
+      return null;
+    }
+  }
+}
@@ -0,0 +1,30 @@
+import java.util.Iterator;
+public class ConcatIterables {
+  class ConcatenatingIterable<A> implements Iterable<A> {
+    ImmutableQueue<Iterable<A>> iterables;
+    public ConcatenatingIterable(Iterable<A> xs, Iterable<A> ys) {
+      newMethod(xs, (ConcatenatingIterable<A>) ys);
+    }
+
+      private ImmutableQueue<Iterable<A>> newMethod(Iterable<A> xs, ConcatenatingIterable<A> ys) {
+          return ys.iterables.pushFront(xs);
+      }
+
+      @Override
+    public Iterator<A> iterator() {
+      return null;
+    }
+  }
+  static class ImmutableQueue<A> implements Iterable<A> {
+    public static <A> ImmutableQueue<A> empty() {
+      return new ImmutableQueue<>();
+    }
+    @Override
+    public Iterator<A> iterator() {
+      return null;
+    }
+    ImmutableQueue<A> pushFront(A a) {
+      return null;
+    }
+  }
+}
@@ -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();
  }